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

Add _check_ovn_support for ovn-chassis, ovn-central and ovn-dedicated-chassis #331

Merged

Conversation

rgildein
Copy link
Contributor

Add _check_ovn_support sanity check for ovn-chassis, ovn-central and ovn-dedicated-chassis, instead of calling it directly in pre_upgrade_steps.

based on: #328
fixes: #330

@rgildein rgildein added the bug Something isn't working label Mar 25, 2024
@rgildein rgildein self-assigned this Mar 25, 2024
@rgildein rgildein marked this pull request as ready for review March 26, 2024 13:33
@rgildein rgildein requested a review from a team as a code owner March 26, 2024 13:33
@rgildein rgildein requested review from Pjack, aieri, agileshaw and jneo8 March 26, 2024 13:33
cou/apps/auxiliary.py Show resolved Hide resolved
cou/apps/auxiliary_subordinate.py Show resolved Hide resolved
@rgildein rgildein force-pushed the bug/BSENG-2237/validate_ovn_in_sanity_checks branch from 480f694 to cef63a4 Compare March 27, 2024 15:30
@rgildein rgildein requested a review from agileshaw March 27, 2024 15:30
Copy link
Member

@gabrielcocenza gabrielcocenza left a comment

Choose a reason for hiding this comment

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

LGTM. I'm wondering if it worth to have a OVN class that we could share some functionality between ovn-subordinates and ovn-principals. This might be good for maintainability because we have some duplicated code on those classes and might be good to have a single source. If you folks agree, we can target this in another PR.

cou/apps/auxiliary.py Show resolved Hide resolved
Copy link
Contributor

@agileshaw agileshaw left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@rgildein rgildein merged commit 649f44d into canonical:main Mar 28, 2024
4 checks passed
@rgildein rgildein deleted the bug/BSENG-2237/validate_ovn_in_sanity_checks branch March 28, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The validate_ovn_support should be part of upgrade_plan_sanity_checks
3 participants