napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

Add commit confirmed feature #124

Open mirceaulinic opened 7 years ago

mirceaulinic commented 7 years ago

Based on the suggestions from https://github.com/napalm-automation/napalm-ios/issues/121, submitting this changes to try an implementation for commit confirmed.

For load_merge, when confirmed is required, a candidate config file is saved on the flash that is used afterwards to replace and schedule the rollback time.

mirceaulinic commented 7 years ago

I have tested and it seems to work as expected:

>>> i.load_merge_candidate(config='ntp server 1.2.3.6')
>>> i.commit_config(confirmed=1)

produces the logs:

*Mar 15 15:31:36.052: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_BACKUP: Backing up current running config to bootflash:archive-Mar-15-15-31-36.011-15
*Mar 15 15:31:36.062: Rollback:Acquired Configuration lock.
*Mar 15 15:31:36.081: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_START_ABSTIMER: User: napalm(Priv: 15, View: 0): Scheduled to rollback to config bootflash:archive-Mar-15-15-31-36.011-15 in 1 minutes
*Mar 15 15:31:36.081: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_WARNING_ABSTIMER: System will rollback to config bootflash:archive-Mar-15-15-31-36.011-15 in one minute. Enter "configure confirm" if you wish to keep what you've configured
*Mar 15 15:32:36.081: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_ROLLBACK_START: Start rolling to: bootflash:archive-Mar-15-15-31-36.011-15
*Mar 15 15:32:36.093: Rollback:Acquired Configuration lock.

And indeed reverted the change I requested.

Confirming the commit:

>>> i.load_merge_candidate(config='ntp server 1.2.3.6')
>>> i.commit_config(confirmed=1)
>>> i.commit_confirm()

produces the logs:

*Mar 15 15:34:16.442: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_BACKUP: Backing up current running config to bootflash:archive-Mar-15-15-34-16.400-16
*Mar 15 15:34:16.446: Rollback:Acquired Configuration lock.
*Mar 15 15:34:16.465: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_START_ABSTIMER: User: napalm(Priv: 15, View: 0): Scheduled to rollback to config bootflash:archive-Mar-15-15-34-16.400-16 in 1 minutes
*Mar 15 15:34:16.467: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_WARNING_ABSTIMER: System will rollback to config bootflash:archive-Mar-15-15-34-16.400-16 in one minute. Enter "configure confirm" if you wish to keep what you've configured
*Mar 15 15:34:17.329: %PARSER-4-BADCFG: Unexpected end of configuration file.

*Mar 15 15:34:20.244: %SYS-5-CONFIG_I: Configured from console by napalm on vty1 (172.31.13.136)
*Mar 15 15:34:25.842: %ARCHIVE_DIFF-5-ROLLBK_CNFMD_CHG_CONFIRM: User: napalm: Confirm the configuration change

and saved the running config.

dbarrosop commented 7 years ago

LGTM but I will defer to @ktbyers as he is our resident expert on everything ios.

ktbyers commented 7 years ago

@mirceaulinic Nice! I added some comments (mainly pertaining to the hard-coding of the file system).

Also I would like to test this prior to merging.

ktbyers commented 7 years ago

We also need unit tests for this on real devices (i.e. not the mocked unit tests / the real unit tests).

mirceaulinic commented 7 years ago

I have tested this on a 2811 so I can run the tests after we implement them, no problem.

ktbyers commented 7 years ago

Yes, I will probably try to work on the tests also...though it might be the end of next week.

ktbyers commented 6 years ago

This will be reimplemented post-reunification.