napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

Support configuration lock for IOS on merge operation #21

Closed jwbensley closed 7 years ago

jwbensley commented 8 years ago

We can't really use this for a complete config replacement, just a merge I think (well technically we could, but the user would have to upload a file using many "no" clauses to negate existing config if they wanted to replace the running config with a completely different configuration, it's basically high risk / high likelihood of failure).

The current NAPALM IOS driver is using "configure replace" on the CLI for a complete replace operation and "copy running-config" for a merge operation. For the merge operation we could have an optional argument that instead enters into "configure terminal" mode and locks the configuration and then streams the new config file in as CLI input.

One can lock the configuration in IOS by using "configure terminal lock [revert timer N]", the optional end part means we can provide an optional automatic rollback if the configuration isn't confirmed within N minutes. The configuration unlocks when the current users leaves "conf t" mode.

So in the case of a merge operation we can wrap the proposed config file in a "configure terminal lock" command, then stream in the file to the CLI, then finish with an "end" to exit "conf t" mode and release the configuration lock (although I think ending the IOS config with an "end" is a pre-req anyway right?).

We could also optionally wrap it in "configure terminal lock revert timer N". If after N minutes the users hasn't left "conf t" mode and entered "configure confirm" back in enable mode, IOS will auto rollback the change. This would make the operation safer.

This whole feature requires the local config archive feature to be enabled on the IOS device, when one enters “configure terminal lock revert timer N” a backup of the running config is made to the archive destination (such as “flash:”) before the configuration is locked and the user placed into “configure terminal” mode, so that is has something to roll back to. This but is also a pre-req of the NAPALM IOS driver too right?

So I think both pre-reqs for this proposed enhancement are implicitly met if one is using NAPALM-IOS.

dbarrosop commented 8 years ago

@ktbyers thoughts? Sounds good to me, it's similar to what other drivers do.

ktbyers commented 8 years ago

@dbarrosop @jwbensley Yes, I like this idea particularly that we might get an automatic rollback capability.

We could do an optional arg... "merge_mode" that would either be 'scp' or 'cli'.

Should be easy to do with the netmiko driver as it has a method that streams the commands in via CLI (either from a file or from a Python iterable).

mirceaulinic commented 8 years ago

To keep things consistent - most probably you are aware of this detail, but please let me make sure we have an agreement: IOS-XR and JunOS have an optional argument config_lock: http://napalm.readthedocs.io/en/latest/support/index.html#list-of-supported-optional-arguments napalm-iosxr and napalm-junos by default will lock the config DB after establishing the connection. If the user does not want to lock the DB, but only when loading a config, the arg config_lock must be set to False. I think we should apply the same pattern here.

dbarrosop commented 8 years ago

Two things:

  1. I agree with @mirceaulinic. I think we should control that with the config_lock.
  2. I don't like the automatic rollback. That's not how the rest of the drivers work. Rollback has to be triggered by the user or be an explicit optional argument.
ktbyers commented 8 years ago

I think config_lock should default to False (since it wouldn't be available for the 'replace' operation and SCP should probably be the default operation for merge).

Agreed we should be consistent on automatic rollback (i.e. it shouldn't happen by default) and should require an explicit action (somehow) by the user to activate it.

mirceaulinic commented 8 years ago

I think config_lock should default to False (since it wouldn't be available for the 'replace' operation and SCP should probably be the default operation for merge).

Indeed, the logic says that config_lock should default to False. No doubt about that. It is the other way around for historical reasons only: until a couple of months ago this arg did not exist and all connections were locked by default, no matter what. To not broke the existing usage we had to leave this default behaviour. And if the user wants to lock only when applying changes, has to specify that. Therefore config_lock defaults to True.

ktbyers commented 8 years ago

@mirceaulinic I am not sure I understood you?

Are you proposing it should default to False or to True for IOS? Current default for IOS is False since there is no current config locking.

dbarrosop commented 8 years ago

@ktbyers let's default to False to avoid breaking current assumptions on IOS. We can change it later to make it consistent but let's not bother about that right now.

mirceaulinic commented 8 years ago

@ktbyers I'm just saying that config_lock currently defaults to True for other drivers. If we are going to use the same param with a different value, means that we completely miss the consistency goals. I am fine to update napalm-junos and napalm-iosxr if necessary and keep the mess away!

dbarrosop commented 8 years ago

@mirceaulinic don't rush on this. I think that the detail is so small (lock vs no lock) that is fine to have the deviation for a while. I definitively want to make sure there is no deviation but I think we can live with this one for a while. And I think the default should be no locking because most network engineers don't even know that the configuration can be locked :P

mirceaulinic commented 8 years ago

And I think the default should be no locking because most network engineers don't even know that the configuration can be locked :P

Totally agree with you - as previously said, I would also find it more natural to lock the config only when necessary. Glad we have an agreement on this! @dbarrosop @ktbyers please keep me posted; once this is ready I'll update junos and iosxr drivers accordingly 😸

ktbyers commented 8 years ago

Okay, let's see if we can figure out a way to do config lock for both replace and merge operations for IOS.

If we can do this, then we can eliminate my main concern.

At that point, we can make sure the default option for config lock is consistent between the various drivers.

jwbensley commented 8 years ago

@ktbyers @dbarrosop @mirceaulinic Is there somewhere these discrepancies and variances are documented within the central NAPALM docs? OK it’s a little bit annoying if the IOS driver has a different locking default to other drivers but it is expected to be “fixed” later, its more annoying to have the difference between drivers and not have it documented anywhere so it gets forgotten.

@ktbyers For locking on replace, this is default when using the CLI command "configuration replace flash:/candidate_config.txt", you have to explicity disable it;

R1#configure replace flash:1.txt ? force Forcibly replace without prompting for user input ignorecase Ignore case list List the commands applied in each pass nolock Do not acquire config lock revert Options for reverting back to the original config time Time for which to wait for confirmation

R1#show ver | i 15 Cisco IOS Software, 7200 Software (C7200-ADVIPSERVICESK9-M), Version 15.2(4)M7, RELEASE SOFTWARE (fc2)
dbarrosop commented 8 years ago

Is there somewhere these discrepancies and variances are documented

There should be none. If you notice any, please, report it. However, some vendors have some limitations and those are documented here:

http://napalm.readthedocs.io/en/latest/support/index.html#caveats

ktbyers commented 7 years ago

Revisiting this from a while back.

@jwbensley I think you have a misunderstanding of the configure replace operation at least with what you stated here:

but the user would have to upload a file using many "no" clauses to negate existing 
config if they wanted to replace the running config with a completely different 
configuration

configure replace already acquires an automatic lock during its operation (as pointed out by @jwbensley) so the only thing that is missing is a lock for configuration merge operation.

I think it is a mistake to try to stream the commands in using configure terminal. I say this because it will end up being meaningfully slower and more error prone than doing a merge operation.

We could do something like the following (although it strikes me as a kludge and I question whether it is worth it).

pynet-rtr1#configure terminal lock 
Configuration session is locked. The lock will be cleared once you exit out of configuration mode.
Enter configuration commands, one per line.  End with CNTL/Z.

pynet-rtr1(config)#do copy flash:merge_config.txt running-config 
Destination filename [running-config]? 
25 bytes copied in 0.008 secs (3125 bytes/sec)

pynet-rtr1(config)#do show run | inc logging
logging buffered 20000
no logging console
pynet-rtr1(config)#exit

Here is what a merge operation looks like if a configuration lock is obtained in another window:

pynet-rtr1#copy flash:/merge_config.txt system:/running-config
Destination filename [running-config]? 
Command execution is locked, Please try later.
25 bytes copied in 0.008 secs (3125 bytes/sec)

So the general question is...is this worth doing?

ktbyers commented 7 years ago

I am going to close this and someone can re-open it (if there is active interest in working on it).