napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

Proposal: change compare_config on merge operation #96

Closed ktbyers closed 7 years ago

ktbyers commented 7 years ago

Cisco IOS has the following:

show archive config incremental-diffs flash:/merge_config.txt

It compares against the running config. This looks like a better solution than what we currently have (which is just to echo the config commands in merge file).

I have a working proof-of-concept of this.

Here is what I observed for behavior:

Test merge file that had:

logging buffered 12000
no logging console

Where this matched what was in current config:

What Cisco IOS returns:

!List of Commands:
end
!No changes were found

What napalm-ios would return:

null string

Test merge file:

logging buffered 10000
no logging console

logging buffer size is different than current config

What Cisco IOS returns:

!List of Commands:
logging buffered 10000
end

What napalm-ios would return:

+logging buffered 10000

Test merge file:

logging buffered 12000
no logging console

router bgp 42
 no neighbor 10.220.88.38
 neighbor 10.220.88.39 remote-as 45
 neighbor 10.220.88.40 remote-as 46

What router current config is:

pynet-rtr1#show run | inc logging
logging buffered 12000
no logging console
pynet-rtr1#show run | section bgp
router bgp 42
 bgp router-id 10.220.88.20
 bgp log-neighbor-changes
 neighbor 10.220.88.38 remote-as 44

What Cisco IOS shows:

!List of Commands:
router bgp 42
 no neighbor 10.220.88.38
 neighbor 10.220.88.39 remote-as 45
 neighbor 10.220.88.40 remote-as 46
end

What napalm-ios would show:

+router bgp 42
- no neighbor 10.220.88.38
+ neighbor 10.220.88.39 remote-as 45
+ neighbor 10.220.88.40 remote-as 46

Note, the '-' prefixing is handled in napalm-ios and is relatively stupid i.e. if the command line starts with a 'no ' it will be prefixed with a minus sign.

ktbyers commented 7 years ago

One more example, an ACL change where the order of the ACL has changed, but all the lines entirely match for ACL entries.

Router config:

ip access-list extended TEST1
 permit ip any host 1.1.1.1
 permit ip any host 1.1.1.2
 permit ip any host 1.1.1.3
 permit ip any host 1.1.1.4
 permit ip any host 1.1.1.5
 permit ip any host 2.2.2.2

New merge file:

logging buffered 12000
no logging console

ip access-list extended TEST1
 permit ip any host 2.2.2.2
 permit ip any host 1.1.1.1
 permit ip any host 1.1.1.2
 permit ip any host 1.1.1.3
 permit ip any host 1.1.1.4
 permit ip any host 1.1.1.5

What Cisco IOS reports:

show archive config incremental-diffs flash:/merge_config.txt
!List of Commands:
!
!The following order-dependent line(s) were re-ordered
!ip access-list extended TEST1
! permit ip any host 1.1.1.1
! permit ip any host 1.1.1.2
! permit ip any host 1.1.1.3
! permit ip any host 1.1.1.4
! permit ip any host 1.1.1.5
end
!No changes were found

What napalm-ios would show:

+!The following order-dependent line(s) were re-ordered
+!ip access-list extended TEST1
+! permit ip any host 1.1.1.1
+! permit ip any host 1.1.1.2
+! permit ip any host 1.1.1.3
+! permit ip any host 1.1.1.4
+! permit ip any host 1.1.1.5
ktbyers commented 7 years ago

If the 'show archive config incremental-diffs flash:/merge_config.txt', then it will fall back to old method and print some warning about this.

mirceaulinic commented 7 years ago

That's fantastic @ktbyers!

I would be curious if NX-OS has something similar we could use. I will have a look soon and check that out!

jedelman8 commented 7 years ago

Very cool. Good find @ktbyers with this incremental-diffs command.

@mirceaulinic nothing like this I'm aware of on NX-OS unless we use a system generated checkpoint file (which is how napalm works today for a config replace).

dbarrosop commented 7 years ago

+100000

This is great! Any caveat like OS support or something that we should add to the documentation?

mirceaulinic commented 7 years ago

Thanks for info @jedelman8!

ktbyers commented 7 years ago

@dbarrosop I didn't see any new caveats, but let me dig into it some more. I did add a handler so if the command fails (i.e. '% Invalid' it will fall back to the old way).

ktbyers commented 7 years ago

Working branch is here:

https://github.com/ktbyers/napalm-ios/tree/devel

Note, I still have some print debugging in the code and some other minor things need cleaned up.

ktbyers commented 7 years ago

Implemented here: https://github.com/napalm-automation/napalm-ios/pull/100