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.

Sign up for GitHub

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

Jump to bottom

CNAO, KubevirtIpamController: Support component by a feature gate #3006

Merged
merged 1 commit into from
Jun 26, 2024
Merged

CNAO, KubevirtIpamController: Support component by a feature gate #3006

merged 1 commit into from
Jun 26, 2024

Conversation

oshoval
Copy link
Contributor

@oshoval oshoval commented Jun 23, 2024 •
edited
Loading

What this PR does / why we need it:

KubevirtIpamController allows having persistent IPs for Kubevirt VM user defined networks.
OVN k8s implements the SDN side, hence required to be deployed as well.
FG name deployKubevirtIpamController.

Since the feature is alpha, this PR presents a feature gate flag, allows
installing the component via CNAO (default value is false).
The feature is also feature gated on OCP itself [1].

Note that if used on K8s cluster (non OCP), a cert manager is required to be deployed,
cert-manager is recommended.

Feature design openshift/enhancements#1456

[1] openshift/api#1913

Reviewer Checklist

Reviewers are supposed to review the PR for every aspect below one by one. To check an item means the PR is either "OK" or "Not Applicable" in terms of that item. All items are supposed to be checked before merging a PR.

  • PR Message
  • Commit Messages
  • How to test
  • Unit Tests
  • Functional Tests
  • User Documentation
  • Developer Documentation
  • Upgrade Scenario
  • Uninstallation Scenario
  • Backward Compatibility
  • Troubleshooting Friendly

Jira Ticket:

https://issues.redhat.com/browse/CNV-42721

Release note:

Support deploying KubevirtIpamController via a feature gate

@kubevirt-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 23, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jun 23, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/L release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 23, 2024
@oshoval oshoval force-pushed the ipam branch 5 times, most recently from 1e1e4de to 889b3ad Compare June 23, 2024 10:11
nunnatsa
nunnatsa reviewed Jun 23, 2024
View reviewed changes
Copy link
Collaborator

@nunnatsa nunnatsa left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @oshoval

I think it's premature. Let's wait for CANO release.

Added a few comment inline.

Please also add more info in the PR description. This description should convince the reader that a new API is required, and explain the context, the requirements and so on.

@oshoval oshoval force-pushed the ipam branch 3 times, most recently from 065da9a to 58fa942 Compare June 23, 2024 13:49
@oshoval
Copy link
Contributor Author

oshoval commented Jun 23, 2024

Addressed comments, lets wait for CNAO to run the tests and so on
Thanks

@oshoval oshoval force-pushed the ipam branch 2 times, most recently from fa1262d to df86020 Compare June 25, 2024 07:43
@oshoval oshoval changed the title WIP CNAO, KubevirtIpamController: Support component by a feature gate CNAO, KubevirtIpamController: Support component by a feature gate Jun 25, 2024
@oshoval oshoval marked this pull request as ready for review June 25, 2024 07:43
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2024
@oshoval
Copy link
Contributor Author

oshoval commented Jun 25, 2024

Thanks @nunnatsa
rebased, now it is ready please
lets see all pass

@coveralls
Copy link
Collaborator

coveralls commented Jun 25, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9658449603

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 85.862%
Totals Coverage Status
Change from base Build 9658355966: 0.02%
Covered Lines: 5223
Relevant Lines: 6083
💛 - Coveralls

@oshoval oshoval requested a review from nunnatsa June 25, 2024 08:09
@coveralls
Copy link
Collaborator

coveralls commented Jun 25, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9658771855

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 85.846%
Totals Coverage Status
Change from base Build 9658355966: 0.007%
Covered Lines: 5222
Relevant Lines: 6083
💛 - Coveralls

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jun 25, 2024
@oshoval
CNAO, KubevirtIpamController: Support component by a feature gate
33604bd 
Signed-off-by: Or Shoval <oshoval@redhat.com>
nunnatsa
nunnatsa approved these changes Jun 25, 2024
View reviewed changes
Copy link
Collaborator

@nunnatsa nunnatsa left a comment

Choose a reason for hiding this comment

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

/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nunnatsa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2024
@oshoval
Copy link
Contributor Author

oshoval commented Jun 25, 2024

Thanks !

@coveralls
Copy link
Collaborator

coveralls commented Jun 25, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9659567755

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 85.846%
Totals Coverage Status
Change from base Build 9658355966: 0.007%
Covered Lines: 5222
Relevant Lines: 6083
💛 - Coveralls

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jun 25, 2024

@oshoval: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws 33604bd link true /test hco-e2e-upgrade-prev-operator-sdk-aws
ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure 33604bd link true /test hco-e2e-consecutive-operator-sdk-upgrades-azure
ci/prow/hco-e2e-kv-smoke-azure 33604bd link true /test hco-e2e-kv-smoke-azure

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@hco-bot
Copy link
Collaborator

hco-bot commented Jun 25, 2024

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure
hco-e2e-consecutive-operator-sdk-upgrades-aws lane succeeded.
/override ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure
hco-e2e-upgrade-prev-operator-sdk-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure, ci/prow/hco-e2e-kv-smoke-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure
hco-e2e-consecutive-operator-sdk-upgrades-aws lane succeeded.
/override ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure
hco-e2e-upgrade-prev-operator-sdk-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@oshoval
Copy link
Contributor Author

oshoval commented Jun 26, 2024

/retest

@kubevirt-bot kubevirt-bot merged commit 6bc3cbc into kubevirt:main Jun 26, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nunnatsa nunnatsa nunnatsa approved these changes

@machadovilaca machadovilaca Awaiting requested review from machadovilaca

@orenc1 orenc1 Awaiting requested review from orenc1

@avlitman avlitman Awaiting requested review from avlitman

Assignees

@nunnatsa nunnatsa

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@oshoval @kubevirt-bot @coveralls @hco-bot @nunnatsa