Add contextual diff computation mode

[angely-dev]

Problem

As you may know, the current implementation of _get_merge_diff() is not really sufficient.

Something as simple as:

Candidate	Running (extract)
# interface GigabitEthernet0/0/2 undo portswitch undo shutdown l2 binding vsi CUST # bgp 1234 router-id 1.2.3.4 ipv4-family vpn-instance SOME-VPN import-route direct #	# interface GigabitEthernet0/0/1 description CustomerA undo portswitch l2 binding vsi CUST # interface GigabitEthernet0/0/2 description CustomerB shutdown # bgp 1234 router-id 1.2.3.4 ipv4-family vpn-instance SOME-VPN import-route static #

Using the basic Python sample:

from napalm import get_network_driver

driver = get_network_driver('huawei_vrp')
device = driver(hostname='switch', username='user', password='pass')
device.open()

device.load_merge_candidate(config=candidate)
print(device.compare_config())

Will result in:

 undo shutdown
  import-route direct

Inconsistencies:

• New items from GigabitEthernet0/0/2 are not displayed
• New items are displayed (if found) without any context

In other words, the diff is unable to compute changes per-context (per-indented block).

Proposed solution

I ended up doing DiffPlus, a lightweight module to compute an incremental and contextual diff between two indented configs. Taking about a hundred lines of code, it only relies on Python builtins and has no extra dependencies.

Though I did it originally for this specific use case, the module is generic enough to work with any indented config, theoretically. I ran it successfully on multiple Huawei platforms (S5732, NE20, NE05, CE6810) having from 2K to 16K number of lines.

Demo

The module is now on PyPI.

Diff only

Using the same configs as above:

from diffplus import IndentedConfig, IncrementalDiff

candidate = open('candidate.txt').read()
running = open('running.txt').read()

candidate = IndentedConfig(candidate, sanitize=True) # sanitize to remove comments, blank lines, etc.
running = IndentedConfig(running, sanitize=True)
diff = IncrementalDiff(candidate, running, colored=True) # colored mode is optional

print(diff)

Will output:

 interface GigabitEthernet0/0/2
+ undo portswitch
+ undo shutdown
+ l2 binding vsi CUST
 bgp 1234
  ipv4-family vpn-instance SOME-VPN
+  import-route direct

Merging

Alternatively, we can merge to get a preview of the full config before applying it:

diff = IncrementalDiff(candidate, running, colored=True, merge=True)

Will output:

 interface GigabitEthernet0/0/1
  description CustomerA
  undo portswitch
  l2 binding vsi CUST
 interface GigabitEthernet0/0/2
  description CustomerB
  shutdown
+ undo portswitch
+ undo shutdown
+ l2 binding vsi CUST
 bgp 1234
  router-id 1.2.3.4
  ipv4-family vpn-instance SOME-VPN
   import-route static
+  import-route direct

Limitations

It is important to understand this module does NOT check syntax or semantic. By essence, it computes the diff between two indented texts, which makes it both simple and generic.

Also, deletions cannot be computed. I explain why here with an example. To sum up, it would require to re-implement the config logic in some way. Though it is tempting, that is far from easy and very platform-dependent.

The diff in NAPALM Cisco IOS actually fakes deletions this way: a no command is considered a deletion even if it won't negate any existing command. This is very debatable and does not make sense to me.

Changes in NAPALM Huawei VRP

Option 1: modify _get_merge_diff()

You may find this option too radical.

def _get_merge_diff(self):
    running_config = self.get_config(retrieve='running')['running']
    running_config = IndentedConfig(running_config, sanitize=True)
    merge_candidate = IndentedConfig(self.merge_candidate, sanitize=True)
    diff = IncrementalDiff(merge_candidate, running_config)
    return diff

Option 2: add an optional_args

This option is more prudent. We let users choose the diff computation mode: either they stick with the actual mode (by default, for legacy purposes) or they experiment the contextual mode through an optional argument contextual_diff.

device = driver(
    hostname='switch',
    username='user',
    password='pass', 
    optional_args={'contextual_diff': True}
)

This is what I implemented in the pull request and I'd be glad to have your view.

[angely-dev]

Any updates on my PR?

[codingnetworksb]

Any updates on my PR?

Hi @angely-dev sorry i haven't replied before. I will read and review your proposal in detail this weekend.

[angely-dev]

Thanks @codingnetworksb. I am available for any question or to resolve the conflict which just appeared after the recent activity.

[codingnetworksb]

Hi @angely-dev, this weekend i started to update the library because it was very outdated compared with original napalm library. I'm making sure it in complete compliance with getters structure, output, etc. I'm also implementing mocked data to for automatic testing when we make pull request.

I couldn't review your pull request. I will do it as soon as i finish updating, hope during the week or weekend. And i'll let you know at that time to check the conflicts.

[codingnetworksb]

Hi @angely-dev, i'm done until now with my updates. Could you check and fix the conflicts?

[angely-dev]

Thanks for maintaining the repo @codingnetworksb. Please allow some time to fix the conflicts which appeared.

[angely-dev]

So I checked and fixed the conflict @codingnetworksb. It was just a reformatting issue (see 502e930).

[codingnetworksb]

Hi @vladislav-tenishchev

The new code failed the Checks i have included recently. It's failing in Black format check. As napalm original project, we have included Black formatter check to this project, please format the code using Black formatter and push the code again.

We appreciate your contribution to this project.

[angely-dev]

So I did run the Black code formatter @codingnetworksb and committed the changes in my fork (a8a8bf7). Is it OK for you at this point? Thanks!

Edit 1: I'm now seeing that the "type checker" workflow step is failing. I'm on it.

Edit 2: OK this is fixed. Just like for the other modules, I ignored mypy missing imports for the diffplus module in the commit 4877340. The workflow has passed!

[angely-dev]

Any updates @codingnetworksb? Or some concerns about the PR?

Not only I fixed the failed checks, but I also added a workflow in my project as well 😊 It includes the Black code formatting check and it ensures unit tests have passed (from Python 3.8 to 3.12). I also released the new version on PyPI.

[codingnetworksb]

Any updates @codingnetworksb? Or some concerns about the PR?

Not only I fixed the failed checks, but I also added a workflow in my project as well 😊 It includes the Black code formatting check and it ensures unit tests have passed (from Python 3.8 to 3.12). I also released the new version on PyPI.

No concern about the PR. I got distracted with other projects, but this weekend I'll catch up on these PRs

[codingnetworksb]

Hi @angely-dev ,

I was testing the PR in my local environment, using a Virtual NE40E and seems to work as you explain it. Too sad that deletions cannot be computed. I understood the complexity of this.

Thank you for documenting this PR so well.

I will approve and merge today.

Regards,

[codingnetworksb]

Approved.

[angely-dev]

Hi @codingnetworksb,

Thanks for the feedback and for approving the PR. I hope users will take benefit of it despite the limitations.

I realized I didn't include an update of the README in the PR. We could update it this way:

• Make a brief mention of the contextual_diff option in the Quick Start section and link it to the PR for reference:

from napalm import get_network_driver
driver = get_network_driver('huawei_vrp')
device = driver(
    hostname='192.168.76.10',
    username='admin',
    password='this_is_not_a_secure_password',
    optional_args={'contextual_diff': True}  # enable contextual diff mode
)
device.open()

# Send Any CLI command
# …

Please refer to PR #23 for more information about the contextual_diff mode.

• Or make it more explicit by adding a new subsection like this: Diff mechanism

I prefer the second way, but I let you choose if you are willing to update the README directly. Otherwise, I could do another PR just for this update.

[codingnetworksb]

Hi @angely-dev ,

Both options sounds good to me. Please submit a new PR just that update. If you create a new subsection make it as summary as possible.