Junos fix next-hop self in get_bgp_config() (#620)

* junos fix nhs in get_bgp_config() * junos add test case for nhs in get_bgp_config() * junos fix missing policy in existing test case for nhs * junos fix E712 comparison * fix typo * junos remove nhs key in group level in get_bgp_config() * junos rework nhs in get_bgp_config() by fetching all policies in one go * junos fix test case for nhs get_bgp_config() * junos clean code and add comments in nhs get_bgp_config() * junos nhs remove inactive term in mocked data
napalm-automation · Jan 23, 2018 · 7ad5e6a · 7ad5e6a
1 parent a5255d0
commit 7ad5e6a
Show file tree

Hide file tree

Showing 6 changed files with 170 additions and 0 deletions.
diff --git a/napalm/junos/junos.py b/napalm/junos/junos.py
@@ -862,6 +862,16 @@ def _process_pipe(cmd, txt):
 
     def get_bgp_config(self, group='', neighbor=''):
         """Return BGP configuration."""
+        def _check_nhs(policies, nhs_policies):
+            if not isinstance(policies, list):
+                # Make it a list if it is a single policy
+                policies = [policies]
+            # Return True if "next-hop self" was found in any of the policies p
+            for p in policies:
+                if nhs_policies[p] is True:
+                    return True
+            return False
+
         def update_dict(d, u):  # for deep dictionary update
             for k, v in u.items():
                 if isinstance(d, collections.Mapping):
@@ -980,6 +990,19 @@ def build_prefix_limit(**args):
         if neighbor:
             neighbor_ip = napalm.base.helpers.ip(neighbor)
 
+        # Get all policies configured in one go and check if "next-hop self" is found in each policy
+        # Save the result in a dict indexed by policy name (junos policy-statement)
+        # The value is a boolean. True if "next-hop self" was found
+        # The resulting dict (nhs_policies) will be used by _check_nhs to determine if "nhs"
+        # is configured or not in the policies applied to a BGP neighbor
+        policy = junos_views.junos_policy_nhs_config_table(self.device)
+        policy.get()
+        nhs_policies = dict()
+        for policy_name, is_nhs_list in policy.items():
+            # is_nhs_list is a list with one element. Ex: [('is_nhs', True)]
+            is_nhs, boolean = is_nhs_list[0]
+            nhs_policies[policy_name] = boolean if boolean is not None else False
+
         for bgp_group in bgp_items:
             bgp_group_name = bgp_group[0]
             bgp_group_details = bgp_group[1]
@@ -1042,6 +1065,9 @@ def build_prefix_limit(**args):
                     default = _DATATYPE_DEFAULT_.get(datatype)
                     key = elem[0]
                     value = elem[1]
+                    if key in ['export_policy']:
+                        # next-hop self is applied on export IBGP sessions
+                        bgp_peer_details['nhs'] = _check_nhs(value, nhs_policies)
                     if key in ['export_policy', 'import_policy']:
                         if isinstance(value, list):
                             value = ' '.join(value)

diff --git a/napalm/junos/utils/junos_views.yml b/napalm/junos/utils/junos_views.yml
@@ -251,7 +251,17 @@ junos_lldp_neighbors_detail_view:
 ###
 ### BGP config
 ###
+junos_policy_nhs_config_table:
+  get: policy-options/policy-statement
+  item: policy-statement
+  key: name
+  view: junos_policy_nhs_config_view
 
+junos_policy_nhs_config_view:
+  fields:
+    # "next-hop self" can be used with or without a term
+    # Return True if "next-hop self" was configured in any of the 2 ways
+    is_nhs: { term/then/next-hop/self | then/next-hop/self: True=self }
 
 junos_bgp_config_table:
   get: protocols/bgp/group

diff --git a/...s/_configuration__policy_options__policy_statement____policy_options___configuration_.xml b/...s/_configuration__policy_options__policy_statement____policy_options___configuration_.xml
@@ -0,0 +1,90 @@
+<configuration>
+        <policy-options>
+            <policy-statement>
+                <name>PUBLIC-PEER-OUT</name>
+                <then>
+                    <next-hop>
+                        <self/>
+                    </next-hop>
+                    <accept/>
+                </then>
+            </policy-statement>
+            <policy-statement>
+                <name>nhs</name>
+                <term>
+                    <name>0</name>
+                    <then>
+                        <accept/>
+                    </then>
+                </term>
+                <then>
+                    <next-hop>
+                        <self/>
+                    </next-hop>
+                </then>
+            </policy-statement>
+            <policy-statement>
+                <name>nhs2</name>
+                <then>
+                    <next-hop>
+                        <self/>
+                    </next-hop>
+                </then>
+            </policy-statement>
+            <policy-statement>
+                <name>nhs3</name>
+                <term>
+                    <name>1</name>
+                    <then>
+                        <accept/>
+                    </then>
+                </term>
+                <term>
+                    <name>2</name>
+                    <then>
+                        <next>term</next>
+                    </then>
+                </term>
+                <term>
+                    <name>3</name>
+                    <then>
+                        <reject/>
+                    </then>
+                </term>
+                <term>
+                    <name>4</name>
+                    <then>
+                        <next-hop>
+                            <peer-address/>
+                        </next-hop>
+                    </then>
+                </term>
+                <then>
+                    <next-hop>
+                        <self/>
+                    </next-hop>
+                </then>
+            </policy-statement>
+            <policy-statement>
+                <name>static</name>
+                <term>
+                    <name>1</name>
+                    <from>
+                        <protocol>static</protocol>
+                    </from>
+                    <then>
+                        <accept/>
+                    </then>
+                </term>
+            </policy-statement>
+            <community>
+                <name>target</name>
+                <members>6500:1</members>
+            </community>
+            <community>
+                <name>vrf-target</name>
+                <members>target:65000:1</members>
+            </community>
+        </policy-options>
+</configuration>
+
diff --git a/..._config/nhs/_configuration__protocols__bgp__group____bgp___protocols___configuration_.xml b/..._config/nhs/_configuration__protocols__bgp__group____bgp___protocols___configuration_.xml
@@ -0,0 +1,25 @@
+<configuration>
+        <protocols>
+            <bgp>
+                <local-as>
+                    <as-number>65001</as-number>
+                </local-as>
+                <group>
+                    <name>internal</name>
+                    <type>internal</type>
+                    <neighbor>
+                        <name>10.10.10.1</name>
+                        <export>nhs</export>
+                    </neighbor>
+                </group>
+                <group>
+                    <name>internal-2</name>
+                    <type>internal</type>
+                    <neighbor>
+                        <name>10.10.10.2</name>
+                        <export>static</export>
+                    </neighbor>
+                </group>
+            </bgp>
+        </protocols>
+</configuration>
diff --git a/test/junos/mocked_data/test_get_bgp_config/nhs/expected_result.json b/test/junos/mocked_data/test_get_bgp_config/nhs/expected_result.json
@@ -0,0 +1 @@
+{"internal":{"apply_groups":[],"description":"","export_policy":"","import_policy":"","local_address":"","local_as":0,"multihop_ttl":0,"multipath":false,"neighbors":{"10.10.10.1":{"authentication_key":"","description":"","export_policy":"nhs","import_policy":"","local_address":"","local_as":0,"nhs":true,"prefix_limit":{},"remote_as":0,"route_reflector_client":false}},"prefix_limit":{},"remote_as":0,"remove_private_as":false,"type":"internal"},"internal-2":{"apply_groups":[],"description":"","export_policy":"","import_policy":"","local_address":"","local_as":0,"multihop_ttl":0,"multipath":false,"neighbors":{"10.10.10.2":{"authentication_key":"","description":"","export_policy":"static","import_policy":"","local_address":"","local_as":0,"nhs":false,"prefix_limit":{},"remote_as":0,"route_reflector_client":false}},"prefix_limit":{},"remote_as":0,"remove_private_as":false,"type":"internal"}}
diff --git a/...l/_configuration__policy_options__policy_statement____policy_options___configuration_.xml b/...l/_configuration__policy_options__policy_statement____policy_options___configuration_.xml
@@ -0,0 +1,18 @@
+<configuration>
+        <policy-options>
+            <policy-statement>
+                <name>PUBLIC-PEER-IN</name>
+                <then>
+                    <accept/>
+                </then>
+            </policy-statement>
+        </policy-options>
+        <policy-options>
+            <policy-statement>
+                <name>PUBLIC-PEER-OUT</name>
+                <then>
+                    <accept/>
+                </then>
+            </policy-statement>
+        </policy-options>
+</configuration>