Skip to content
This repository has been archived by the owner on Jan 16, 2019. It is now read-only.

Add commit confirmed feature #124

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Add commit confirmed feature #124

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71383f1
Add commit confirmed feature
mirceaulinic Mar 15, 2017
343eacd
wr mem only on definitive commit
mirceaulinic Mar 15, 2017
2e58ecf
config replace requires explicit bootflash:
mirceaulinic Mar 15, 2017
7920b53
Schedule reverting before modifying the running-config
mirceaulinic Mar 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions napalm_ios/ios.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def _commit_hostname_handler(self, cmd):
output = ''
return output

def commit_config(self):
def commit_config(self, confirmed=0):
"""
If replacement operation, perform 'configure replace' for the entire config.

Expand All @@ -354,6 +354,8 @@ def commit_config(self):
raise ReplaceConfigException("Candidate config file does not exist")
if self.auto_rollback_on_error:
cmd = 'configure replace {} force revert trigger error'.format(cfg_file)
elif confirmed:
cmd = 'configure replace bootflash:{} force time {}'.format(cfg_file, confirmed)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't hard-code the file system bootflash. The cfg_file variable already has the file_system which either gets auto detected or set by optional_args.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have hardcoded as I have noticed that it wasn't able to detect the file system and for this command it seems that is mandatory -- but I will double check this & fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Netmiko should automatically try to autodectect by doing a dir. If this fails, however, you can still set it using NAPALM optional_args.

We should create an issue, however, if you are seeing this fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The autodetect happens right as part of the open code in the NAPALM code (IIRC).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most probably I missed something. I will check.

else:
cmd = 'configure replace {} force'.format(cfg_file)
output = self._commit_hostname_handler(cmd)
Expand All @@ -369,15 +371,31 @@ def commit_config(self):
cfg_file = self._gen_full_path(filename)
if not self._check_file_exists(cfg_file):
raise MergeConfigException("Merge source config file does not exist")
cmd = 'copy {} running-config'.format(cfg_file)
self._disable_confirm()
if confirmed:
# Replace running config with the rollback config
# And schedule the revert time
# If not confirmed, it will go back to the current running-config state
cmd = 'configure replace bootflash:{} force time {}'.format(self.rollback_cfg,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't hard-code file system. Use _gen_full_path(self.rollback_cfg) to generate full path using file-system bound to object.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should ensure that the rollback_cfg exactly matches running config at this point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably copy it just before to be sure?

Copy link
Contributor

@ktbyers ktbyers Mar 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about copy and then make sure no diffs (and throw an error if differences are found).

pynet-rtr1#copy system:running-config flash:rollback_config.txt
pynet-rtr1#show archive config differences system:running-config flash:rollback_config.txt
!Contextual Config Diffs:
!No changes were found

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that makes sense to me... didn't know about this command (as many other tricks I have discovered reading the code 😄 ).

confirmed)
output = self.device.send_command_expect(cmd)
# The rest of the commands will be loaded directly through the normal
# merge into the running-config
cmd = 'copy {} running-config'.format(cfg_file)
output = self._commit_hostname_handler(cmd)
self._enable_confirm()
if 'Invalid input detected' in output:
self.rollback()
merge_error = "Configuration merge failed; automatic rollback attempted"
raise MergeConfigException(merge_error)

if not confirmed:
# Save config to startup (both replace and merge)
output += self.device.send_command_expect("write mem")

def commit_confirm(self):
# Confirm replacement of running-config
output = self.device.send_command_expect('configure confirm')
# Save config to startup (both replace and merge)
output += self.device.send_command_expect("write mem")

Expand Down