Merge pull request #13131 from LabNConsulting/chopps/no-startup-file

mgmtd: remove startup config feature for now

lib/vty.c

@@ -2400,7 +2400,7 @@ static void vty_timeout(struct event *thread)
 }
 
 /* Read up configuration file from file_name. */
-static void vty_read_file(struct nb_config *config, FILE *confp)
+void vty_read_file(struct nb_config *config, FILE *confp)
 {
 	int ret;
 	struct vty *vty;

lib/vty.h

@@ -369,6 +369,7 @@ extern void vty_pass_fd(struct vty *vty, int fd);
 
 extern bool vty_read_config(struct nb_config *config, const char *config_file,
 			    char *config_default_dir);
+extern void vty_read_file(struct nb_config *config, FILE *confp);
 extern void vty_time_print(struct vty *, int);
 extern void vty_serv_sock(const char *, unsigned short, const char *);
 extern void vty_close(struct vty *);

mgmtd/mgmt.h

@@ -62,6 +62,8 @@ struct mgmt_master {
 };
 
 extern struct mgmt_master *mm;
+extern char const *const mgmt_daemons[];
+extern uint mgmt_daemons_count;
 
 /* Inline functions */
 static inline unsigned long timeval_elapsed(struct timeval a, struct timeval b)

mgmtd/mgmt_ds.c

@@ -87,7 +87,6 @@ static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src,
 					   struct mgmt_ds_ctx *dst)
 {
 	struct lyd_node *dst_dnode, *src_dnode;
-	struct ly_out *out;
 
 	if (!src || !dst)
 		return -1;
@@ -117,13 +116,6 @@ static int mgmt_ds_replace_dst_with_src_ds(struct mgmt_ds_ctx *src,
 		nb_config_diff_del_changes(&src->root.cfg_root->cfg_chgs);
 	}
 
-	if (dst->ds_id == MGMTD_DS_RUNNING) {
-		if (ly_out_new_filepath(MGMTD_STARTUP_DS_FILE_PATH, &out)
-		    == LY_SUCCESS)
-			mgmt_ds_dump_in_memory(dst, "", LYD_JSON, out);
-		ly_out_free(out, NULL, 0);
-	}
-
 	/* TODO: Update the versions if nb_config present */
 
 	return 0;
@@ -134,7 +126,6 @@ static int mgmt_ds_merge_src_with_dst_ds(struct mgmt_ds_ctx *src,
 {
 	int ret;
 	struct lyd_node **dst_dnode, *src_dnode;
-	struct ly_out *out;
 
 	if (!src || !dst)
 		return -1;
@@ -159,13 +150,6 @@ static int mgmt_ds_merge_src_with_dst_ds(struct mgmt_ds_ctx *src,
 		nb_config_diff_del_changes(&src->root.cfg_root->cfg_chgs);
 	}
 
-	if (dst->ds_id == MGMTD_DS_RUNNING) {
-		if (ly_out_new_filepath(MGMTD_STARTUP_DS_FILE_PATH, &out)
-		    == LY_SUCCESS)
-			mgmt_ds_dump_in_memory(dst, "", LYD_JSON, out);
-		ly_out_free(out, NULL, 0);
-	}
-
 	return 0;
 }
 
@@ -200,8 +184,6 @@ void mgmt_ds_reset_candidate(void)
 
 int mgmt_ds_init(struct mgmt_master *mm)
 {
-	struct lyd_node *root;
-
 	if (mgmt_ds_mm || mm->running_ds || mm->candidate_ds || mm->oper_ds)
 		assert(!"MGMTD: Call ds_init only once!");
 
@@ -209,12 +191,6 @@ int mgmt_ds_init(struct mgmt_master *mm)
 	if (!running_config)
 		assert(!"MGMTD: Call ds_init after frr_init only!");
 
-	if (mgmt_ds_load_cfg_from_file(MGMTD_STARTUP_DS_FILE_PATH, &root)
-	    == 0) {
-		nb_config_free(running_config);
-		running_config = nb_config_new(root);
-	}
-
 	running.root.cfg_root = running_config;
 	running.config_ds = true;
 	running.ds_id = MGMTD_DS_RUNNING;

mgmtd/mgmt_ds.h

@@ -24,8 +24,6 @@
 #define MGMTD_DS_NAME_CANDIDATE "candidate"
 #define MGMTD_DS_NAME_OPERATIONAL "operational"
 
-#define MGMTD_STARTUP_DS_FILE_PATH DAEMON_DB_DIR "/frr_startup.json"
-
 #define FOREACH_MGMTD_DS_ID(id)                                                \
 	for ((id) = MGMTD_DS_NONE; (id) < MGMTD_DS_MAX_ID; (id)++)
 

mgmtd/mgmt_main.c

@@ -17,6 +17,13 @@
 #include "routing_nb.h"
 
 
+char const *const mgmt_daemons[] = {
+#ifdef HAVE_STATICD
+	"staticd",
+#endif
+};
+uint mgmt_daemons_count = array_size(mgmt_daemons);
+
 /* mgmt options, we use GNU getopt library. */
 static const struct option longopts[] = {
 	{"skip_runas", no_argument, NULL, 'S'},

mgmtd/mgmt_vty.c

@@ -10,6 +10,8 @@
 
 #include "command.h"
 #include "json.h"
+#include "northbound_cli.h"
+
 #include "mgmtd/mgmt.h"
 #include "mgmtd/mgmt_be_server.h"
 #include "mgmtd/mgmt_be_adapter.h"
@@ -455,6 +457,33 @@ DEFPY(debug_mgmt, debug_mgmt_cmd,
 	return CMD_SUCCESS;
 }
 
+/*
+ * Analog of `frr_config_read_in()`, instead of our config file though we loop
+ * over all daemons that have transitioned to mgmtd, loading their configs
+ */
+static int mgmt_config_pre_hook(struct event_loop *loop)
+{
+	FILE *confp;
+	char *p;
+
+	for (uint i = 0; i < mgmt_daemons_count; i++) {
+		p = asprintfrr(MTYPE_TMP, "%s/%s.conf", frr_sysconfdir,
+			       mgmt_daemons[i]);
+		confp = fopen(p, "r");
+		if (confp == NULL) {
+			if (errno != ENOENT)
+				zlog_err("%s: couldn't read config file %s: %s",
+					 __func__, p, safe_strerror(errno));
+		} else {
+			zlog_info("mgmtd: reading daemon config from %s", p);
+			vty_read_file(vty_shared_candidate_config, confp);
+			fclose(confp);
+		}
+		XFREE(MTYPE_TMP, p);
+	}
+	return 0;
+}
+
 void mgmt_vty_init(void)
 {
 	/*
@@ -468,6 +497,8 @@ void mgmt_vty_init(void)
 	static_vty_init();
 #endif
 
+	hook_register(frr_config_pre, mgmt_config_pre_hook);
+
 	install_node(&debug_node);
 
 	install_element(VIEW_NODE, &show_mgmt_be_adapter_cmd);

staticd/static_main.c

@@ -161,6 +161,10 @@ static const struct frr_yang_module_info *const staticd_yang_modules[] = {
 
 #define STATIC_VTY_PORT 2616
 
+/*
+ * NOTE: .flags == FRR_NO_SPLIT_CONFIG to avoid reading split config, mgmtd will
+ * do this for us now
+ */
 FRR_DAEMON_INFO(staticd, STATIC, .vty_port = STATIC_VTY_PORT,
 
 		.proghelp = "Implementation of STATIC.",
@@ -170,7 +174,8 @@ FRR_DAEMON_INFO(staticd, STATIC, .vty_port = STATIC_VTY_PORT,
 
 		.privs = &static_privs, .yang_modules = staticd_yang_modules,
 		.n_yang_modules = array_size(staticd_yang_modules),
-);
+
+		.flags = FRR_NO_SPLIT_CONFIG);
 
 int main(int argc, char **argv, char **envp)
 {
@@ -210,9 +215,12 @@ int main(int argc, char **argv, char **envp)
 
 	routing_control_plane_protocols_register_vrf_dependency();
 
-	snprintf(backup_config_file, sizeof(backup_config_file),
+	/*
-		 "%s/zebra.conf", frr_sysconfdir);
+	 * We set FRR_NO_SPLIT_CONFIG flag to avoid reading our config, but we
-	staticd_di.backup_config_file = backup_config_file;
+	 * still need to write one if vtysh tells us to. Setting the host
+	 * config filename does this.
+	 */
+	host_config_set(config_default);
 
 	frr_config_fork();
 	frr_run(master);

tests/topotests/mgmt_tests/test_yang_mgmt.py

@@ -217,7 +217,9 @@ def test_mgmt_commit_check(request):
             ]
         }
     }
-    result = verify_rib(tgen, "ipv4", dut, input_dict_4, protocol=protocol)
+    result = verify_rib(
+        tgen, "ipv4", dut, input_dict_4, protocol=protocol, expected=False
+    )
     assert (
         result is not True
     ), "Testcase {} : Failed" "Error: Routes is missing in RIB".format(tc_name)
@@ -319,7 +321,9 @@ def test_mgmt_commit_abort(request):
             ]
         }
     }
-    result = verify_rib(tgen, "ipv4", dut, input_dict_4, protocol=protocol)
+    result = verify_rib(
+        tgen, "ipv4", dut, input_dict_4, protocol=protocol, expected=False
+    )
     assert (
         result is not True
     ), "Testcase {} : Failed" "Error: Routes is missing in RIB".format(tc_name)
@@ -372,7 +376,7 @@ def test_mgmt_delete_config(request):
     assert (
         result is True
     ), "Testcase {} : Failed" "Error: Routes is missing in RIB".format(tc_name)
-    
+
     step("Mgmt delete config")
     raw_config = {
         "r1": {
@@ -387,7 +391,9 @@ def test_mgmt_delete_config(request):
     assert result is True, "Testcase {} : Failed Error: {}".format(tc_name, result)
 
     step("Verify that the route is deleted from RIB")
-    result = verify_rib(tgen, "ipv4", dut, input_dict_4, protocol=protocol)
+    result = verify_rib(
+        tgen, "ipv4", dut, input_dict_4, protocol=protocol, expected=False
+    )
     assert (
         result is not True
     ), "Testcase {} : Failed" "Error: Routes is still present in RIB".format(tc_name)
@@ -461,7 +467,9 @@ def test_mgmt_chaos_stop_start_frr(request):
     dut = "r1"
     protocol = "static"
     input_dict_4 = {"r1": {"static_routes": [{"network": "192.1.11.200/32"}]}}
-    result = verify_rib(tgen, "ipv4", dut, input_dict_4, protocol=protocol)
+    result = verify_rib(
+        tgen, "ipv4", dut, input_dict_4, protocol=protocol, expected=False
+    )
     assert (
         result is not True
     ), "Testcase {} : Failed" "Error: Routes still present in RIB".format(tc_name)