Merge pull request #13658 from louis-6wind/fix-flex-algo2

isisd: fix wrongly disabled flex-algorithm

2 files changed, 130 insertions, 68 deletions

diff --git a/isisd/isis_spf.c b/isisd/isis_spf.c
index de467c8262..e114467e07 100644
--- a/isisd/isis_spf.c
+++ b/isisd/isis_spf.c
@@ -1843,6 +1843,9 @@ void isis_run_spf(struct isis_spftree *spftree)
 	struct timeval time_end;
 	struct isis_mt_router_info *mt_router_info;
 	uint16_t mtid = 0;
+#ifndef FABRICD
+	bool flex_algo_enabled;
+#endif /* ifndef FABRICD */
 
 	/* Get time that can't roll backwards. */
 	monotime(&time_start);
@@ -1885,16 +1888,27 @@ void isis_run_spf(struct isis_spftree *spftree)
 	 * not supported by the node, it MUST stop participating in such
 	 * Flexible-Algorithm.
 	 */
-	if (flex_algo_id_valid(spftree->algorithm) &&
-	    !flex_algo_get_state(spftree->area->flex_algos,
-				 spftree->algorithm)) {
-		if (!CHECK_FLAG(spftree->flags, F_SPFTREE_DISABLED)) {
-			isis_spftree_clear(spftree);
-			SET_FLAG(spftree->flags, F_SPFTREE_DISABLED);
+	if (flex_algo_id_valid(spftree->algorithm)) {
+		flex_algo_enabled = isis_flex_algo_elected_supported(
+			spftree->algorithm, spftree->area);
+		if (flex_algo_enabled !=
+		    flex_algo_get_state(spftree->area->flex_algos,
+					spftree->algorithm)) {
+			/* actual state is inconsistent with local LSP */
 			lsp_regenerate_schedule(spftree->area,
 						spftree->area->is_type, 0);
+			goto out;
+		}
+		if (!flex_algo_enabled) {
+			if (!CHECK_FLAG(spftree->flags, F_SPFTREE_DISABLED)) {
+				isis_spftree_clear(spftree);
+				SET_FLAG(spftree->flags, F_SPFTREE_DISABLED);
+				lsp_regenerate_schedule(spftree->area,
+							spftree->area->is_type,
+							0);
+			}
+			goto out;
 		}
-		goto out;
 	}
 #endif /* ifndef FABRICD */
 
diff --git a/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py b/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py
index 85600beb0e..c81f63942b 100755
--- a/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py
+++ b/tests/topotests/isis_sr_flex_algo_topo1/test_isis_sr_flex_algo_topo1.py
@@ -38,6 +38,7 @@ import sys
 import pytest
 import json
 import tempfile
+from copy import deepcopy
 from functools import partial
 
 # Save the Current Working Directory to find configuration files.
@@ -111,8 +112,12 @@ def setup_module(mod):
 
     # For all registered routers, load the zebra configuration file
     for rname, router in router_list.items():
-        router.load_config( TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)))
-        router.load_config( TopoRouter.RD_ISIS, os.path.join(CWD, "{}/isisd.conf".format(rname)))
+        router.load_config(
+            TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname))
+        )
+        router.load_config(
+            TopoRouter.RD_ISIS, os.path.join(CWD, "{}/isisd.conf".format(rname))
+        )
     tgen.start_router()
 
 
@@ -130,6 +135,30 @@ def setup_testcase(msg):
     return tgen
 
 
+def router_json_cmp_exact_filter(router, cmd, expected):
+    output = router.vtysh_cmd(cmd)
+    logger.info("{}: {}\n{}".format(router.name, cmd, output))
+
+    json_output = json.loads(output)
+    router_output = deepcopy(json_output)
+
+    # filter out dynamic data from "show mpls table"
+    for label, data in json_output.items():
+        if "1500" in label:
+            # filter out SR local labels
+            router_output.pop(label)
+            continue
+        nexthops = data.get("nexthops", [])
+        for i in range(len(nexthops)):
+            if "fe80::" in nexthops[i].get("nexthop"):
+                router_output.get(label).get("nexthops")[i].pop("nexthop")
+            elif "." in nexthops[i].get("nexthop"):
+                # IPv4, just checking the nexthop
+                router_output.get(label).get("nexthops")[i].pop("interface")
+
+    return topotest.json_cmp(router_output, expected, exact=True)
+
+
 def router_compare_json_output(rname, command, reference):
     "Compare router JSON output"
 
@@ -139,7 +168,9 @@ def router_compare_json_output(rname, command, reference):
     expected = json.loads(reference)
 
     # Run test function until we get an result. Wait at most 60 seconds.
-    test_func = partial(topotest.router_json_cmp, tgen.gears[rname], command, expected)
+    test_func = partial(
+        router_json_cmp_exact_filter, tgen.gears[rname], command, expected
+    )
     _, diff = topotest.run_and_expect(test_func, None, count=120, wait=0.5)
     assertmsg = '"{}" JSON output mismatches the expected result'.format(rname)
     assert diff is None, assertmsg
@@ -153,9 +184,13 @@ def router_compare_output(rname, command, reference):
     tgen = get_topogen()
 
     # Run test function until we get an result. Wait at most 60 seconds.
-    test_func = partial(topotest.router_output_cmp, tgen.gears[rname], command, reference)
+    test_func = partial(
+        topotest.router_output_cmp, tgen.gears[rname], command, reference
+    )
     result, diff = topotest.run_and_expect(test_func, "", count=120, wait=0.5)
-    assertmsg = '{} command "{}" output mismatches the expected result:\n{}'.format(rname, command, diff)
+    assertmsg = '{} command "{}" output mismatches the expected result:\n{}'.format(
+        rname, command, diff
+    )
     assert result, assertmsg
 
 
@@ -176,11 +211,11 @@ def test_step1_mpls_lfib():
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][1]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][1]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][1]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][1]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -207,17 +242,18 @@ def test_step2_mpls_lfib():
          router isis 1
           flex-algo 203
            no advertise-definition
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][2]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][2]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][2]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][2]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -244,17 +280,18 @@ def test_step3_mpls_lfib():
          router isis 1
           flex-algo 203
            no advertise-definition
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][3]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][3]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][3]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][3]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -281,17 +318,18 @@ def test_step4_mpls_lfib():
          router isis 1
           flex-algo 203
            advertise-definition
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][4]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][4]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][4]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][4]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -319,17 +357,18 @@ def test_step5_mpls_lfib():
          router isis 1
           flex-algo 203
            advertise-definition
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][5]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][5]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][5]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][5]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -360,17 +399,18 @@ def test_step6_mpls_lfib():
              router isis 1
               flex-algo 203
                no dataplane sr-mpls
-            """)
+            """
+        )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][6]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][6]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][6]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][6]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -400,17 +440,19 @@ def test_step7_mpls_lfib():
             configure terminal
              router isis 1
               no flex-algo 203
-            """)
+            """
+        )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][7]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][7]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][7]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][7]["show_mpls_table.ref"]
+        )
+
 
 #
 # Step 8
@@ -440,7 +482,8 @@ def test_step8_mpls_lfib():
            advertise-definition
            affinity exclude-any green
            dataplane sr-mpls
-        """)
+        """
+    )
 
     tgen.gears["rt2"].vtysh_cmd(
         """
@@ -450,7 +493,8 @@ def test_step8_mpls_lfib():
            advertise-definition
            affinity exclude-any green
            dataplane sr-mpls
-        """)
+        """
+    )
 
     tgen.gears["rt3"].vtysh_cmd(
         """
@@ -458,17 +502,18 @@ def test_step8_mpls_lfib():
          router isis 1
           flex-algo 203
           dataplane sr-mpls
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][8]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][8]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][8]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][8]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -494,17 +539,18 @@ def test_step9_mpls_lfib():
          router isis 1
           no segment-routing prefix 1.1.1.1/32 algorithm 203 index 301
           no segment-routing prefix 2001:db8:1000::1/128 algorithm 203 index 1301
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][9]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][9]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][9]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][9]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -530,17 +576,18 @@ def test_step10_mpls_lfib():
          router isis 1
           segment-routing prefix 1.1.1.1/32 algorithm 203 index 301
           segment-routing prefix 2001:db8:1000::1/128 algorithm 203 index 1301
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][10]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][10]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][10]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][10]["show_mpls_table.ref"]
+        )
 
 
 #
@@ -565,17 +612,18 @@ def test_step11_mpls_lfib():
          router isis 1
           segment-routing prefix 1.1.1.1/32 algorithm 203 index 311
           segment-routing prefix 2001:db8:1000::1/128 algorithm 203 index 1311
-        """)
+        """
+    )
 
     # For Developers
     # tgen.mininet_cli()
     for rname in ["rt1", "rt2", "rt3"]:
         router_compare_output(
-            rname, "show isis flex-algo",
-            outputs[rname][11]["show_isis_flex_algo.ref"])
+            rname, "show isis flex-algo", outputs[rname][11]["show_isis_flex_algo.ref"]
+        )
         router_compare_json_output(
-            rname, "show mpls table json",
-            outputs[rname][11]["show_mpls_table.ref"])
+            rname, "show mpls table json", outputs[rname][11]["show_mpls_table.ref"]
+        )
 
 
 if __name__ == "__main__":