-
Notifications
You must be signed in to change notification settings - Fork 555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IOSDriver should catch "Rollback aborted" case when using configure replace
#1701
base: develop
Are you sure you want to change the base?
IOSDriver should catch "Rollback aborted" case when using configure replace
#1701
Conversation
Is anything else needed to merge this? |
Do we need a different process here? In this case applying the candidate config failed and the I think we should automatically invoke a NAPALM rollback in this case as we are now in an invalid state (as far as the config). By Thoughts? |
Would the rollback process just end up issuing another |
@ncsurfus Yes, it would try to do a I would expect this should work (otherwise, I would say it is an IOS/IOS-XE bug)...since the config that we are rolling back to is the running configuration before the original configure replace started. We would have to make sure we can't get into a loop (i.e. if the NAPALM rollback also fails, then we raise an exception and tell the user). We would also need to make sure here that if we fail again that we raise a clear message to the end-user. |
I did some manual testing for when this occurs, and yeah running a |
@ktbyers @ncsurfus I pulled the match for this case out into its own block and added a |
@rbcollins123 @ktbyers I pulled in these changes and was able to test it. It worked great and I was able to get successful rollbacks across a couple different devices. I also verified with the command below and only got the expected difference.
|
Ok, let me know if there’s anything else I can do to help get this merged
in @ktbyers
…On Mon, Aug 29, 2022 at 7:29 PM Nathan Surfus ***@***.***> wrote:
@rbcollins123 <https://github.com/rbcollins123> @ktbyers
<https://github.com/ktbyers> I pulled in these changes and was able to
test it. It worked great and I was able to get successful rollbacks across
a couple different devices. I also verified with the command below and only
got the expected difference.
xxxxxx#show archive config differences flash:rollback_config.txt
!Contextual Config Diffs:
+file prompt quiet
—
Reply to this email directly, view it on GitHub
<#1701 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC2S4CTWUDTW4WJTYP4FQKTV3VBUHANCNFSM53MWHLWA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Bump! Anything else I can do to help get this merged? |
I'm not sure this CR should be merged. "configure replace" uses the term rollback in a confusing way. "configure replace" was intended to rollback to a previously saved running configuration, not deploy an entirely new configuration. For this reason "configure replace" uses the term rollback to indicate that it's applying the desired configuration. For instance, when "configure replace" is successful, it'll end with "Rollback Done". The terminology is confusing, but kind of makes sense if you understand the original use case. "Rollback Aborted" also doesn't mean "configure replace" attempted to revert back to the original configuration and failed, but rather it appears to indicate that it's just aborting the deployment. The most common reason, from my testing, is that some of the desired configuration isn't showing up in the running configuration. For instance, if you include "no shutdown" on an interface, then "configure replace" will attempt to apply "no shutdown", and when it doesn't show up in the running configuration after 5 passes, then it "aborts". Some things like SNMPv3 users don't show up in the running configuration and can also cause this output. From my testing, this is generally harmless, but I'm sure there are scenarios where this could break something. |
Code-wise, this would kinda make sense to me. But I can't claim I understand what |
@mirceaulinic @rbcollins123 @ncsurfus I think we should leave the current solution and just close this PR (as per the comments from @ncsurfus). Basically, it sounds like we might be creating larger problems by attempting to auto-rollback on this. I am definitely open to discussions on this, however. |
@ktbyers I don't care if the napalm-level-rollback happens or not, that was only added at your request. My only concern in raising the PR originally was to ensure an exception is raised when this occurs since the config-replace has failed in this scenario. If the |
@rbcollins123 (@ncsurfus) is pointing out that So I would have the same issue with raising an exception (i.e. we would be causing working config files to fail). @rbcollins123 Was this your test merge config to demonstrate the issue:
If not, can you provide a test case on IOS/IOS-XE? |
Yes that was an IOS-XE example and real device test case to force and show
the behavior. Yes that example is a benign but not all cases are. We have
seen similar happen on NXOS cases in real world too.
…On Tue, Mar 26, 2024 at 13:23 Kirk Byers ***@***.***> wrote:
@rbcollins123 <https://github.com/rbcollins123> ***@***.***
<https://github.com/ncsurfus>) is pointing out that rollback aborted is
relatively normal behavior (meaning it is just a dumb string parser and it
is looking for certain text in the output and if the text doesn't show up
then we will get this rollback aborted). This could easily happen due to
default values (like shutdown/no shutdown).
So I would have the same issue with raising an exception (i.e. we would be
causing working config files to fail).
@rbcollins123 <https://github.com/rbcollins123> Was this your test merge
config to demonstrate the issue:
********
!List of Rollback Commands:
interface GigabitEthernet0/0
no negotiation auto
interface GigabitEthernet0/0
shutdown
negotiation auto
end
********
If not, can you provide a test case on IOS/IOS-XE?
—
Reply to this email directly, view it on GitHub
<#1701 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC2S4CQ7UM3HLS3DVWQ6DZDY2GVJTAVCNFSM53MWHLWKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBSGEYDINJWGIYQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Invalid candidate configs may silently fail installation currently because IOS-XE may attempt to rollback upon error but the rollback itself may be invalid.
Here is an example output from a Cat9300 switch on IOS-XE v17.3.3 when the intended config file attempts to make impossible changes to the running-config and reversing those changes are also impossible.
The above should generate an exception within IOSDriver.commit_config