Merge pull request #3442 from opensourcerouting/confirmed-commits

lib: add support for confirmed commits

lib/libfrr.c

@@ -653,7 +653,7 @@ struct thread_master *frr_init(void)
 	lib_error_init();
 
 	yang_init();
-	nb_init(di->yang_modules, di->n_yang_modules);
+	nb_init(master, di->yang_modules, di->n_yang_modules);
 
 	return master;
 }

lib/northbound.c

@@ -1539,7 +1539,8 @@ static void nb_load_callbacks(const struct frr_yang_module_info *module)
 	}
 }
 
-void nb_init(const struct frr_yang_module_info *modules[], size_t nmodules)
+void nb_init(struct thread_master *tm,
+	     const struct frr_yang_module_info *modules[], size_t nmodules)
 {
 	unsigned int errors = 0;
 
@@ -1574,7 +1575,7 @@ void nb_init(const struct frr_yang_module_info *modules[], size_t nmodules)
 	running_config = nb_config_new(NULL);
 
 	/* Initialize the northbound CLI. */
-	nb_cli_init();
+	nb_cli_init(tm);
 }
 
 void nb_terminate(void)

lib/northbound.h

@@ -20,6 +20,7 @@
 #ifndef _FRR_NORTHBOUND_H_
 #define _FRR_NORTHBOUND_H_
 
+#include "thread.h"
 #include "hook.h"
 #include "linklist.h"
 #include "openbsd-tree.h"
@@ -825,7 +826,7 @@ extern const char *nb_client_name(enum nb_client client);
  * nmodules
  *    Size of the modules array.
  */
-extern void nb_init(const struct frr_yang_module_info *modules[],
+extern void nb_init(struct thread_master *tm, const struct frr_yang_module_info *modules[],
 		    size_t nmodules);
 
 /*

lib/northbound_cli.c

@@ -36,6 +36,7 @@
 
 int debug_northbound;
 struct nb_config *vty_shared_candidate_config;
+static struct thread_master *master;
 
 static void vty_show_libyang_errors(struct vty *vty, struct ly_ctx *ly_ctx)
 {
@@ -213,16 +214,80 @@ int nb_cli_rpc(const char *xpath, struct list *input, struct list *output)
 	}
 }
 
-static int nb_cli_commit(struct vty *vty, bool force, char *comment)
+void nb_cli_confirmed_commit_clean(struct vty *vty)
+{
+	THREAD_TIMER_OFF(vty->t_confirmed_commit_timeout);
+	nb_config_free(vty->confirmed_commit_rollback);
+	vty->confirmed_commit_rollback = NULL;
+}
+
+int nb_cli_confirmed_commit_rollback(struct vty *vty)
 {
 	uint32_t transaction_id;
 	int ret;
 
+	/* Perform the rollback. */
+	ret = nb_candidate_commit(
+		vty->confirmed_commit_rollback, NB_CLIENT_CLI, true,
+		"Rollback to previous configuration - confirmed commit has timed out",
+		&transaction_id);
+	if (ret == NB_OK)
+		vty_out(vty,
+			"Rollback performed successfully (Transaction ID #%u).\n",
+			transaction_id);
+	else
+		vty_out(vty, "Failed to rollback to previous configuration.\n");
+
+	return ret;
+}
+
+static int nb_cli_confirmed_commit_timeout(struct thread *thread)
+{
+	struct vty *vty = THREAD_ARG(thread);
+
+	/* XXX: broadcast this message to all logged-in users? */
+	vty_out(vty,
+		"\nConfirmed commit has timed out, rolling back to previous configuration.\n\n");
+
+	nb_cli_confirmed_commit_rollback(vty);
+	nb_cli_confirmed_commit_clean(vty);
+
+	return 0;
+}
+
+static int nb_cli_commit(struct vty *vty, bool force,
+			 unsigned int confirmed_timeout, char *comment)
+{
+	uint32_t transaction_id;
+	int ret;
+
+	/* Check if there's a pending confirmed commit. */
+	if (vty->t_confirmed_commit_timeout) {
+		if (confirmed_timeout) {
+			/* Reset timeout if "commit confirmed" is used again. */
+			vty_out(vty,
+				"%% Resetting confirmed-commit timeout to %u minute(s)\n\n",
+				confirmed_timeout);
+
+			THREAD_TIMER_OFF(vty->t_confirmed_commit_timeout);
+			thread_add_timer(master,
+					 nb_cli_confirmed_commit_timeout, vty,
+					 confirmed_timeout * 60,
+					 &vty->t_confirmed_commit_timeout);
+		} else {
+			/* Accept commit confirmation. */
+			vty_out(vty, "%% Commit complete.\n\n");
+			nb_cli_confirmed_commit_clean(vty);
+		}
+		return CMD_SUCCESS;
+	}
+
 	if (vty_exclusive_lock != NULL && vty_exclusive_lock != vty) {
 		vty_out(vty, "%% Configuration is locked by another VTY.\n\n");
 		return CMD_WARNING;
 	}
 
+	/* "force" parameter. */
 	if (!force && nb_candidate_needs_update(vty->candidate_config)) {
 		vty_out(vty,
 			"%% Candidate configuration needs to be updated before commit.\n\n");
@@ -231,6 +296,16 @@ static int nb_cli_commit(struct vty *vty, bool force, char *comment)
 		return CMD_WARNING;
 	}
 
+	/* "confirm" parameter. */
+	if (confirmed_timeout) {
+		vty->confirmed_commit_rollback = nb_config_dup(running_config);
+
+		vty->t_confirmed_commit_timeout = NULL;
+		thread_add_timer(master, nb_cli_confirmed_commit_timeout, vty,
+				 confirmed_timeout * 60,
+				 &vty->t_confirmed_commit_timeout);
+	}
+
 	ret = nb_candidate_commit(vty->candidate_config, NB_CLIENT_CLI, true,
 				  comment, &transaction_id);
 
@@ -534,18 +609,22 @@ DEFUN (config_private,
 
 DEFPY (config_commit,
        config_commit_cmd,
-       "commit [force$force]",
+       "commit [{force$force|confirmed (1-60)}]",
        "Commit changes into the running configuration\n"
-       "Force commit even if the candidate is outdated\n")
+       "Force commit even if the candidate is outdated\n"
+       "Rollback this commit unless there is a confirming commit\n"
+       "Timeout in minutes for the commit to be confirmed\n")
 {
-	return nb_cli_commit(vty, !!force, NULL);
+	return nb_cli_commit(vty, !!force, confirmed, NULL);
 }
 
 DEFPY (config_commit_comment,
        config_commit_comment_cmd,
-       "commit [force$force] comment LINE...",
+       "commit [{force$force|confirmed (1-60)}] comment LINE...",
        "Commit changes into the running configuration\n"
        "Force commit even if the candidate is outdated\n"
+       "Rollback this commit unless there is a confirming commit\n"
+       "Timeout in minutes for the commit to be confirmed\n"
        "Assign a comment to this commit\n"
        "Comment for this commit (Max 80 characters)\n")
 {
@@ -555,7 +634,7 @@ DEFPY (config_commit_comment,
 
 	argv_find(argv, argc, "LINE", &idx);
 	comment = argv_concat(argv, argc, idx);
-	ret = nb_cli_commit(vty, !!force, comment);
+	ret = nb_cli_commit(vty, !!force, confirmed, comment);
 	XFREE(MTYPE_TMP, comment);
 
 	return ret;
@@ -1543,8 +1622,10 @@ static const struct cmd_variable_handler yang_var_handlers[] = {
 	 .completions = yang_translator_autocomplete},
 	{.completions = NULL}};
 
-void nb_cli_init(void)
+void nb_cli_init(struct thread_master *tm)
 {
+	master = tm;
+
 	/* Initialize the shared candidate configuration. */
 	vty_shared_candidate_config = nb_config_new(NULL);
 

lib/northbound_cli.h

@@ -105,8 +105,10 @@ extern void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *dnode,
 				   bool show_defaults);
 
 /* Prototypes of internal functions. */
+extern void nb_cli_confirmed_commit_clean(struct vty *vty);
+extern int nb_cli_confirmed_commit_rollback(struct vty *vty);
 extern void nb_cli_install_default(int node);
-extern void nb_cli_init(void);
+extern void nb_cli_init(struct thread_master *tm);
 extern void nb_cli_terminate(void);
 
 #endif /* _FRR_NORTHBOUND_CLI_H_ */

lib/vty.c

@@ -2714,6 +2714,14 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
 
 void vty_config_exit(struct vty *vty)
 {
+	/* Check if there's a pending confirmed commit. */
+	if (vty->t_confirmed_commit_timeout) {
+		vty_out(vty,
+			"WARNING: exiting with a pending confirmed commit. Rolling back to previous configuration.\n\n");
+		nb_cli_confirmed_commit_rollback(vty);
+		nb_cli_confirmed_commit_clean(vty);
+	}
+
 	vty_config_exclusive_unlock(vty);
 
 	if (vty->candidate_config) {

lib/vty.h

@@ -126,6 +126,10 @@ struct vty {
 	/* Base candidate configuration. */
 	struct nb_config *candidate_config_base;
 
+	/* Confirmed-commit timeout and rollback configuration. */
+	struct thread *t_confirmed_commit_timeout;
+	struct nb_config *confirmed_commit_rollback;
+
 	/* qobj object ID (replacement for "index") */
 	uint64_t qobj_index;
 

tests/bgpd/test_peer_attr.c

@@ -1384,10 +1384,10 @@ static void bgp_startup(void)
 		 LOG_DAEMON);
 	zprivs_preinit(&bgpd_privs);
 	zprivs_init(&bgpd_privs);
-	yang_init();
-	nb_init(NULL, 0);
 
 	master = thread_master_create(NULL);
+	yang_init();
+	nb_init(master, NULL, 0);
 	bgp_master_init(master);
 	bgp_option_set(BGP_OPT_NO_LISTEN);
 	vrf_init(NULL, NULL, NULL, NULL, NULL);

tests/helpers/c/main.c

@@ -156,7 +156,7 @@ int main(int argc, char **argv)
 	vty_init(master);
 	memory_init();
 	yang_init();
-	nb_init(NULL, 0);
+	nb_init(master, NULL, 0);
 
 	/* OSPF vty inits. */
 	test_vty_init();

tests/lib/cli/common_cli.c

@@ -84,7 +84,7 @@ int main(int argc, char **argv)
 	vty_init(master);
 	memory_init();
 	yang_init();
-	nb_init(NULL, 0);
+	nb_init(master, NULL, 0);
 
 	test_init(argc, argv);
 

tests/lib/cli/test_commands.c

@@ -143,7 +143,7 @@ static void test_init(void)
 
 	cmd_init(1);
 	yang_init();
-	nb_init(NULL, 0);
+	nb_init(master, NULL, 0);
 
 	install_node(&bgp_node, NULL);
 	install_node(&rip_node, NULL);

tests/lib/northbound/test_oper_data.c

@@ -449,7 +449,7 @@ int main(int argc, char **argv)
 	vty_init(master);
 	memory_init();
 	yang_init();
-	nb_init(modules, array_size(modules));
+	nb_init(master, modules, array_size(modules));
 
 	/* Create artificial data. */
 	create_data(num_vrfs, num_interfaces, num_routes);