Proposed behavior change to rollback() in nxos_ssh driver

[decoupca]

For the IOS and Junos drivers, calling rollback() will initiate a rollback regardless of any changes that napalm thinks it made. For nxos_ssh however,rollback() has no effect if self.changed is falsy.

I'm using napalm in a workflow that might need to call rollback() in a session that didn't initiate any changes. I can work around this by manually setting device.changed = True, but it seems like this is inconsistent behavior. I think rollback() should work whether napalm thinks it made any changes or not (same way it behaves in IOS/Junos). Happy to submit a PR.

[ktbyers]

On IOS, napalm will on rollback() call, NAPALM will rollback to whatever the contents of "rollback_config.txt" are. This file will be populated with the contents from the running_config just before the last commit_config().

On Junos, napalm will rollback(rb_id=1) (so it will rollback to the previous config before the last commit regardless of whether napalm made the commit or not).

On EOS, napalm will "configure replace flash:rollback-0". flash:rollback-0 will be generated by a commit_config operation. Though it doesn't look like EOS even checks whether the file exists or not. We should verify what happens in the case of the rollback-0 file not existing.

On NX-OS (nx-api) napalm will:

    cmd = "rollback running-config file {}".format(filename)
    self.show(cmd, raw_text=True)

But only if self.changed is True. And self.changed will get set to True on commit_config() operation, but obviously won't persist past a given NAPALM session. Also it doesn't look like there are any checks if "rollback_config.txt" (so if self.changed goes away, then we probably should at least check the file exists). Like IOS, this file would be populated with the contents of the last successful NAPALM commit_config (checkpoint file).

NX-OS (SSH) behaves the same as nx-api.

IOS-XR (XML agent) will execute rollback using XML with a rb_id=1 (so the behavior is the same as Junos). IOS-XR NETCONF looks the same.

I didn't check what EOS (SSH) does.

[ktbyers]

Okay, that sounds reasonably to me that we should remove the changed check and attribute in NX-OS (both for nx-api and for SSH).

We should also add checks into EOS, NX-OS (nx-api), NX-OS (SSH) that the relevant rollback file exists (and probably raise an exception if it doesn't).

We should also verify the behavior on EOS (SSH) is reasonable and verify the relevant rollback file exists.

[decoupca]

I like it. So rollback() will roll back to previous config, regardless of any changes napalm is aware of, but check to ensure the rollback file exists for platforms that use file-based snapshots. I can submit a PR next week

[ktbyers]

Yes, rollback will rollback to the previous saved state which for platforms that have a true candidate config will be the a rollback 1 (i.e. the state before the last commit). For platforms with no true candidate config, it will be the last NAPALM commit (basically the last time napalm's commit_config saved the state into the rollback-file).

Then for platforms counting on the file existing we should just add in more error checking (i.e. does the file actually exist). We probably should check the current NAPALM exceptions and see if there is an exception category that makes sense (or potentially add a new exception category).

[decoupca]

Sounds good. Do you want me to file an issue before the PR?