fix(junos): trying to clean route-map still being used

Before this commit, we were always deleting autogenerated route-map for all SAFIs. The purpose of the deletion is to start from a clean slate when recreating a route-map (to avoid extra terms/options in the route-map). The goal is not to remove extra policies, which must be done in a dedicated step. The issue is that we push routing_policy update before BGP. So we should never remove a policy which is still in use (this is why the cleaning must be done in a dedicated step). So this commit aims to only delete policy which will be recreated.
criteo · Sep 17, 2024 · ee68f78 · ee68f78
1 parent 685b6ec
commit ee68f78
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 26 deletions.
diff --git a/_states/openconfig_routing_policy.py b/_states/openconfig_routing_policy.py
@@ -313,10 +313,11 @@ def _generate_statement_config(route_map_name, afisafis, statement, prefixes_set
     )
 
 
-def _remove_route_policy_config(route_map_name, saltenv):
+def _remove_route_policy_config(route_map_name, afisafis, saltenv):
     """Remove route policy configuration."""
     context = {
         "route_map_name": route_map_name,
+        "afisafis": afisafis,
         "convert_route_map_name": __utils__["jinja_filters.format_route_policy_name"],
     }
 
@@ -335,9 +336,13 @@ def _generate_policies_config(
     config = []
 
     for policies in openconfig["policy-definition"]:
+        policy_name = policies["config"]["name"]
+
         # remove the route-map to ensure no extra config nor sequence
         if _get_os() != "sonic" or policies["config"]["name"] in existing_route_maps:
-            config.append(_remove_route_policy_config(policies["config"]["name"], saltenv))
+            config.append(
+                _remove_route_policy_config(policies["config"]["name"], rp_afisafis_mapping.get(policy_name), saltenv)
+            )
 
         for statement in policies["statements"]["statement"]:
             prefix_set_name = _safeget(
@@ -354,7 +359,6 @@ def _generate_policies_config(
             ):
                 raise ValueError("Declared community does not exist: {}".format(community_set_name))
 
-            policy_name = policies["config"]["name"]
             policy_config = _generate_statement_config(
                 policy_name,
                 rp_afisafis_mapping.get(policy_name),
@@ -402,10 +406,7 @@ def _get_route_policy_afi_safis_usage(route_policies, bgp):
 
 
 def _generate_routing_policy_config(openconfig_routing_policy, openconfig_bgp, _, saltenv):
-    # TODO: handle when no data
-    # TODO: add safeguards
-    # TODO: generalize this to all OS to be able to remove extra objects)
-    #   Removing extras is only support for SONiC for now
+    #   Removing extras is only supported for SONiC
     #   extract items from the config (prefix list, community list etc...)
     #   it will be used in templates to clean objects needing changes
     existing_assets = {}

diff --git a/states/afk/templates/routing_policy/junos/remove_policy.j2 b/states/afk/templates/routing_policy/junos/remove_policy.j2
@@ -1,13 +1,5 @@
-{% set AFISAFIS = [
-  "",
-  "IPV4_UNICAST",
-  "IPV6_UNICAST",
-  "L2VPN_EVPN"
-]
-%}
-
-{% for afisafi in AFISAFIS %}
-  {% set auto_gen_rm_name = convert_route_map_name(route_map_name, afisafi) %}
 {# Explicitly remove all route policies on the AFI/SAFIS we use #}
+{% for afisafi in afisafis %}
+  {% set auto_gen_rm_name = convert_route_map_name(route_map_name, afisafi) %}
 delete policy-options policy-statement {{ auto_gen_rm_name }}
 {% endfor %}
diff --git a/tests/states/openconfig_routing_policy/data/integration_tests/expected_result_junos.txt b/tests/states/openconfig_routing_policy/data/integration_tests/expected_result_junos.txt
@@ -20,8 +20,6 @@ set policy-options route-filter-list PF-LOOPBACK_IPV6 2001:db8:1::/64 exact
 set policy-options route-filter-list PF-LOOPBACK_IPV6 2001:db8:1::/128 exact
 delete policy-options policy-statement RM-TEST
 delete policy-options policy-statement AUTOGENERATED::RM-TEST::IPV4_UNICAST
-delete policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST
-delete policy-options policy-statement AUTOGENERATED::RM-TEST::L2VPN_EVPN
 set policy-options policy-statement RM-TEST term 10 from route-filter-list PF-LOOPBACK_IPV4
 set policy-options policy-statement RM-TEST term 10 from protocol direct
 set policy-options policy-statement RM-TEST term 10 from local-preference 1234
@@ -51,8 +49,6 @@ set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV4_UNICAST term 10
 set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV4_UNICAST then reject
 delete policy-options policy-statement RM-TEST-OUT
 delete policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::IPV4_UNICAST
-delete policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::IPV6_UNICAST
-delete policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::L2VPN_EVPN
 set policy-options policy-statement RM-TEST-OUT term 10 then reject
 set policy-options policy-statement RM-TEST-OUT then reject
 set policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::IPV4_UNICAST term 10 from family inet

diff --git a/...nfig_routing_policy/integration_tests/test_openconfig_routing_policy_junos_integration.py b/...nfig_routing_policy/integration_tests/test_openconfig_routing_policy_junos_integration.py
@@ -154,8 +154,6 @@ def test__juniper_bgp_one_safi_policy(mocker):
         "set policy-options route-filter-list PF-LOOPBACK_IPV6 2001:db8:1::/128 exact\n"
         "delete policy-options policy-statement RM-TEST\n"
         "delete policy-options policy-statement AUTOGENERATED::RM-TEST::IPV4_UNICAST\n"
-        "delete policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST\n"
-        "delete policy-options policy-statement AUTOGENERATED::RM-TEST::L2VPN_EVPN\n"
         "set policy-options policy-statement RM-TEST term 10 from route-filter-list PF-LOOPBACK_IPV4\n"
         "set policy-options policy-statement RM-TEST term 10 from protocol direct\n"
         "set policy-options policy-statement RM-TEST term 10 from local-preference 1234\n"
@@ -184,9 +182,105 @@ def test__juniper_bgp_one_safi_policy(mocker):
         "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV4_UNICAST term 10 then reject\n"
         "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV4_UNICAST then reject\n"
         "delete policy-options policy-statement RM-TEST-OUT\n"
-        "delete policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::IPV4_UNICAST\n"
-        "delete policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::IPV6_UNICAST\n"
-        "delete policy-options policy-statement AUTOGENERATED::RM-TEST-OUT::L2VPN_EVPN\n"
+        "set policy-options policy-statement RM-TEST-OUT term 10 then reject\n"
+        "set policy-options policy-statement RM-TEST-OUT then reject"
+    )
+
+@_mock_then_clean
+def test__juniper_bgp_one_safi_policy_ipv6(mocker):
+    """Test Juniper when only a SAFI has a policy in import only."""
+    bgp_config = {
+        "bgp": {
+            "neighbors": {
+                "neighbor": [
+                    {
+                        "neighbor-address": "2001:db8::.1",
+                        "config": {
+                            "peer-group": "PG-DEFAULT",
+                            "neighbor-address": "2001:db8::.1",
+                            "enabled": True,
+                            "peer-as": 65001,
+                            "local-as": 65000,
+                            "auth-password": "",
+                            "description": "my_neighbor",
+                            "peer-type": None,
+                            "remove-private-as": None,
+                            "send-community": "NONE",
+                        },
+                        "afi-safis": {
+                            "afi-safi": [
+                                {
+                                    "afi-safi-name": "IPV6_UNICAST",
+                                    "config": {"afi-safi-name": "IPV6_UNICAST", "enabled": True},
+                                    "apply-policy": {"config": {"import-policy": ["RM-TEST"]}},
+                                    "ipv4-unicast": {
+                                        "prefix-limit": {"config": {"max-prefixes": 0}}
+                                    },
+                                }
+                            ]
+                        },
+                    },
+                ],
+            },
+        },
+    }
+
+    fake_data, _ = _get_data_and_expected_result("junos")
+
+    out = STATE_MOD._generate_routing_policy_config(
+        fake_data["routing-policy"], bgp_config["bgp"], None, saltenv="base"
+    )
+    assert out == (
+        "delete policy-options community CL-LOCAL\n"
+        "set policy-options community CL-LOCAL members 65000:100.\n"
+        "delete policy-options community CL-MAIN\n"
+        "set policy-options community CL-MAIN members 649..:20000\n"
+        "delete policy-options community CL-SERVICE\n"
+        "set policy-options community CL-SERVICE members 65000:5....\n"
+        "delete policy-options community CL-DEFAULT\n"
+        "set policy-options community CL-DEFAULT members 65000:60000\n"
+        "delete policy-options community CL-LOCATION\n"
+        "set policy-options community CL-LOCATION members 65000:65001\n"
+        "delete policy-options community CL-CLOS_INFRA\n"
+        "set policy-options community CL-CLOS_INFRA members 65000:10100\n"
+        "delete policy-options community CL-SERVER\n"
+        "set policy-options community CL-SERVER members 65000:10200\n"
+        "delete policy-options route-filter-list PF-LOOPBACK_IPV4\n"
+        "set policy-options route-filter-list PF-LOOPBACK_IPV4 10.0.0.0/22 exact\n"
+        "set policy-options route-filter-list PF-LOOPBACK_IPV4 10.0.1.0/22 exact\n"
+        "delete policy-options route-filter-list PF-LOOPBACK_IPV6\n"
+        "set policy-options route-filter-list PF-LOOPBACK_IPV6 2001:db8:1::/64 exact\n"
+        "set policy-options route-filter-list PF-LOOPBACK_IPV6 2001:db8:1::/128 exact\n"
+        "delete policy-options policy-statement RM-TEST\n"
+        "delete policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST\n"
+        "set policy-options policy-statement RM-TEST term 10 from route-filter-list PF-LOOPBACK_IPV4\n"
+        "set policy-options policy-statement RM-TEST term 10 from protocol direct\n"
+        "set policy-options policy-statement RM-TEST term 10 from local-preference 1234\n"
+        "set policy-options policy-statement RM-TEST term 10 from community CL-LOCAL\n"
+        "set policy-options policy-statement RM-TEST term 10 then origin egp\n"
+        "set policy-options policy-statement RM-TEST term 10 then local-preference 5678\n"
+        "set policy-options policy-statement RM-TEST term 10 then metric 250\n"
+        "set policy-options policy-statement RM-TEST term 10 then as-path-prepend 65000 65000 65000\n"
+        "delete policy-options community AUTOGENERATED::RM-TEST:10\n"
+        "set policy-options community AUTOGENERATED::RM-TEST:10 members 65000:50000\n"
+        "set policy-options policy-statement RM-TEST term 10 then community set AUTOGENERATED::RM-TEST:10\n"
+        "set policy-options policy-statement RM-TEST term 10 then reject\n"
+        "set policy-options policy-statement RM-TEST then reject\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 from family inet6\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 from route-filter-list PF-LOOPBACK_IPV4\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 from protocol direct\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 from local-preference 1234\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 from community CL-LOCAL\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 then origin egp\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 then local-preference 5678\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 then metric 250\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 then as-path-prepend 65000 65000 65000\n"
+        "delete policy-options community AUTOGENERATED::RM-TEST:10\n"
+        "set policy-options community AUTOGENERATED::RM-TEST:10 members 65000:50000\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 then community set AUTOGENERATED::RM-TEST:10\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST term 10 then reject\n"
+        "set policy-options policy-statement AUTOGENERATED::RM-TEST::IPV6_UNICAST then reject\n"
+        "delete policy-options policy-statement RM-TEST-OUT\n"
         "set policy-options policy-statement RM-TEST-OUT term 10 then reject\n"
         "set policy-options policy-statement RM-TEST-OUT then reject"
     )