Skip to content
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

Issu sw install #632

Merged
merged 8 commits into from
Dec 13, 2016
Merged

Issu sw install #632

merged 8 commits into from
Dec 13, 2016

Conversation

vnitinv
Copy link
Contributor

@vnitinv vnitinv commented Dec 8, 2016

function added _issu_nssu_requirement_validation

@coveralls
Copy link

coveralls commented Dec 8, 2016

Coverage Status

Coverage decreased (-0.5%) to 95.643% when pulling f9db2c0 on vnitinv:issu-sw-install into e3af698 on Juniper:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 95.643% when pulling f9db2c0 on vnitinv:issu-sw-install into e3af698 on Juniper:master.

Copy link
Contributor

@stacywsmith stacywsmith left a comment

Choose a reason for hiding this comment

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

Minor changes suggested.

ss.run('cli', '> ', timeout=5)
data = ss.run('show system switchover', '> ', timeout=5)
output = data[1]
ss.run('exit')
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this cleanly exit the request routing-engine login in all cases (both shell user and cli user)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as we are using context manager, StartShell creates a new connection and at the end the connection is closed. Hence no issues. If we put more ss.run('exit'), we need to be much more careful. closing connection clears all. If you suggest, we can change it as per your review.

:returns:
* ``True`` if validation passes.
* * ``False`` otherwise
"""
self.log('Checking GRES status')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest changing to: self.log('Checking GRES configuration')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if conf.find('system/commit/synchronize') is None:
self.log('Requirement FAILED: commit synchronize is not Enabled in configuration')
return False
self.log('Checking NSR status')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest changing to: self.log('Checking NSR configuration')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

'commit-scripts': 'apply'})
if conf.find('routing-options/nonstop-routing') is None:
self.log('Requirement FAILED: NSR is not Enabled in configuration')
return False
self.log('Verifying that NSR is configured on the current Routing Engine\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest changing to: self.log('Verifying that GRES status on the current Routing Engine\n'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -248,11 +253,6 @@ def _issu_requirement_validation(self):
* The master Routing Engine and backup Routing Engine must be
running the same software version before you can perform a
unified ISSU.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add:

  • All checks from _issu_nssu_requirement_validation()

@coveralls
Copy link

coveralls commented Dec 9, 2016

Coverage Status

Coverage decreased (-0.5%) to 95.643% when pulling 366f27a on vnitinv:issu-sw-install into e3af698 on Juniper:master.

Copy link
Contributor

@stacywsmith stacywsmith left a comment

Choose a reason for hiding this comment

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

Looks good.

@vnitinv vnitinv merged commit 1f9817a into Juniper:master Dec 13, 2016
@vnitinv vnitinv deleted the issu-sw-install branch February 18, 2019 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants