Merge pull request #1892 from napalm-automation/iss905

eos: use peer group dict to populate neighbors
napalm-automation · Apr 5, 2023 · 24ea5d2 · 24ea5d2
2 parents 02837c0 + 4097d29
commit 24ea5d2
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 25 deletions.
diff --git a/napalm/eos/eos.py b/napalm/eos/eos.py
@@ -994,6 +994,7 @@ def get_bgp_config(self, group="", neighbor=""):
             "local-v4-addr": "local_address",
             "local-v6-addr": "local_address",
             "local-as": "local_as",
+            "next-hop-self": "nhs",
             "description": "description",
             "import-policy": "import_policy",
             "export-policy": "export_policy",
@@ -1051,7 +1052,7 @@ def default_group_dict(local_as):
             )  # few more default values
             return group_dict
 
-        def default_neighbor_dict(local_as):
+        def default_neighbor_dict(local_as, group_dict):
             neighbor_dict = {}
             neighbor_dict.update(
                 {
@@ -1062,6 +1063,13 @@ def default_neighbor_dict(local_as):
             neighbor_dict.update(
                 {"prefix_limit": {}, "local_as": local_as, "authentication_key": ""}
             )  # few more default values
+            neighbor_dict.update(
+                {
+                    key: group_dict.get(key)
+                    for key in _GROUP_FIELD_MAP_.values()
+                    if key in group_dict and key in _PEER_FIELD_MAP_.values()
+                }
+            )  # copy in values from group dict if present
             return neighbor_dict
 
         def parse_options(options, default_value=False):
@@ -1152,14 +1160,20 @@ def parse_options(options, default_value=False):
                 ipaddress.ip_address(group_or_neighbor)
                 # if passes the test => it is an IP Address, thus a Neighbor!
                 peer_address = group_or_neighbor
-                if peer_address not in bgp_neighbors:
-                    bgp_neighbors[peer_address] = default_neighbor_dict(local_as)
+                group_name = None
                 if options[0] == "peer-group":
-                    bgp_neighbors[peer_address]["__group"] = options[1]
+                    group_name = options[1]
                 # EOS > 4.23.0 only supports the new syntax
                 # https://www.arista.com/en/support/advisories-notices/fieldnotices/7097-field-notice-39
                 elif options[0] == "peer" and options[1] == "group":
-                    bgp_neighbors[peer_address]["__group"] = options[2]
+                    group_name = options[2]
+                if peer_address not in bgp_neighbors:
+                    bgp_neighbors[peer_address] = default_neighbor_dict(
+                        local_as, bgp_config.get(group_name, {})
+                    )
+
+                if group_name:
+                    bgp_neighbors[peer_address]["__group"] = group_name
 
                 # in the config, neighbor details are lister after
                 # the group is specified for the neighbor:
@@ -1197,6 +1211,10 @@ def parse_options(options, default_value=False):
                 bgp_config[peer_group] = default_group_dict(local_as)
             bgp_config[peer_group]["neighbors"][peer] = peer_details
 
+        [
+            v.pop("nhs", None) for v in bgp_config.values()
+        ]  # remove NHS from group-level dictionary
+
         if local_as == 0:
             # BGP not running
             return {}

diff --git a/.../eos/mocked_data/test_get_bgp_config/issue1504_alt_peer_group_syntax/expected_result.json b/.../eos/mocked_data/test_get_bgp_config/issue1504_alt_peer_group_syntax/expected_result.json
@@ -35,8 +35,8 @@
         "local_as": 64496,
         "nhs": false,
         "route_reflector_client": false,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "reject-all",
         "authentication_key": "",
         "prefix_limit": {}
       },
@@ -47,8 +47,8 @@
         "local_as": 64496,
         "nhs": false,
         "route_reflector_client": false,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "reject-all",
         "authentication_key": "",
         "prefix_limit": {}
       }

diff --git a/test/eos/mocked_data/test_get_bgp_config/issue_1113_dot_asn/expected_result.json b/test/eos/mocked_data/test_get_bgp_config/issue_1113_dot_asn/expected_result.json
@@ -35,8 +35,8 @@
         "local_as": 4266524237,
         "nhs": false,
         "route_reflector_client": false,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "4-public-peer-anycast-out",
         "authentication_key": "",
         "prefix_limit": {}
       },
@@ -47,8 +47,8 @@
         "local_as": 4266524237,
         "nhs": false,
         "route_reflector_client": false,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "4-public-peer-anycast-out",
         "authentication_key": "",
         "prefix_limit": {}
       }
@@ -75,8 +75,8 @@
         "local_as": 4266524237,
         "nhs": false,
         "route_reflector_client": false,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "reject-all",
         "authentication_key": "",
         "prefix_limit": {}
       },
@@ -87,8 +87,8 @@
         "local_as": 4266524237,
         "nhs": false,
         "route_reflector_client": false,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "reject-all",
         "authentication_key": "",
         "prefix_limit": {}
       }

diff --git a/test/eos/mocked_data/test_get_bgp_config/issue_905_group_copydown/expected_result.json b/test/eos/mocked_data/test_get_bgp_config/issue_905_group_copydown/expected_result.json
@@ -0,0 +1,57 @@
+{
+  "_": {
+    "type": "",
+    "multipath": false,
+    "apply_groups": [],
+    "remove_private_as": false,
+    "multihop_ttl": 0,
+    "remote_as": 0,
+    "local_address": "",
+    "local_as": 65534,
+    "description": "",
+    "import_policy": "",
+    "export_policy": "",
+    "prefix_limit": {},
+    "neighbors": {}
+  },
+  "FOO-GROUP": {
+    "local_address": "",
+    "description": "FOO",
+    "type": "",
+    "local_as": 65534,
+    "apply_groups": [],
+    "multihop_ttl": 0,
+    "remove_private_as": false,
+    "remote_as": 65534,
+    "import_policy": "",
+    "export_policy": "",
+    "neighbors": {
+      "192.0.2.2": {
+        "local_address": "",
+        "authentication_key": "",
+        "description": "FOO",
+        "nhs": false,
+        "local_as": 65534,
+        "route_reflector_client": false,
+        "remote_as": 65534,
+        "import_policy": "",
+        "export_policy": "",
+        "prefix_limit": {}
+      },
+      "192.0.2.3": {
+        "local_address": "",
+        "authentication_key": "",
+        "description": "SECOND-PEER",
+        "nhs": true,
+        "local_as": 65534,
+        "route_reflector_client": false,
+        "remote_as": 65534,
+        "import_policy": "",
+        "export_policy": "",
+        "prefix_limit": {}
+      }
+    },
+    "prefix_limit": {},
+    "multipath": false
+  }
+}
diff --git a/...est_get_bgp_config/issue_905_group_copydown/show_running_config___section_router_bgp.text b/...est_get_bgp_config/issue_905_group_copydown/show_running_config___section_router_bgp.text
@@ -0,0 +1,11 @@
+router bgp 65534
+   router-id 192.0.2.1
+   neighbor FOO-GROUP peer group
+   neighbor FOO-GROUP next-hop-self
+   neighbor FOO-GROUP description FOO
+   neighbor FOO-GROUP remote-as 65534
+   neighbor 192.0.2.2 peer group FOO-GROUP
+   no neighbor 192.0.2.2 next-hop-self
+   neighbor 192.0.2.3 peer group FOO-GROUP
+   neighbor 192.0.2.3 description SECOND-PEER
+!
diff --git a/test/eos/mocked_data/test_get_bgp_config/normal/expected_result.json b/test/eos/mocked_data/test_get_bgp_config/normal/expected_result.json
@@ -34,8 +34,8 @@
         "local_as": 13335,
         "route_reflector_client": false,
         "remote_as": 13414,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "4-public-peer-anycast-out",
         "prefix_limit": {}
       },
       "192.168.0.1": {
@@ -46,8 +46,8 @@
         "local_as": 13335,
         "route_reflector_client": false,
         "remote_as": 32934,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "4-public-peer-anycast-out",
         "prefix_limit": {}
       }
     },
@@ -74,8 +74,8 @@
         "local_as": 13335,
         "route_reflector_client": false,
         "remote_as": 54113,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "reject-all",
         "prefix_limit": {}
       },
       "2001:db8::0:1": {
@@ -86,8 +86,8 @@
         "local_as": 13335,
         "route_reflector_client": false,
         "remote_as": 8403,
-        "import_policy": "",
-        "export_policy": "",
+        "import_policy": "reject-all",
+        "export_policy": "reject-all",
         "prefix_limit": {}
       }
     },