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

Detect if OpenShift cluster is single stack ipv6 #2386

Merged
merged 10 commits into from
Aug 9, 2023
Merged

Detect if OpenShift cluster is single stack ipv6 #2386

merged 10 commits into from
Aug 9, 2023

Conversation

dharmit
Copy link
Contributor

@dharmit dharmit commented Jun 14, 2023 •
edited

What this PR does / why we need it:
Checks if HCO is running on OCP configured with single stack ipv6 setup, and fail starting the Pod if so.

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-28584

Release note:

Detect and alert if OCP is single stack IPv6

NOTE:
We need to get kubevirt/monitoring#186 merged before opening this up for review because it will keep failing the following functional test in monitoring_test.go otherwise:

It("Alert rules should have all the requried annotations", func() {

@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. 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 14, 2023
@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 requested a review from sradco June 14, 2023 11:34
@openshift-ci
Copy link

openshift-ci bot commented Jun 14, 2023

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

dharmit
dharmit commented Jun 14, 2023
View reviewed changes
pkg/util/cluster.go Outdated Show resolved Hide resolved
@dharmit dharmit force-pushed the fix-cnv-28584 branch 2 times, most recently from d99a1bd to 9f4ce41 Compare June 20, 2023 13:04
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 28, 2023
@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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 Jul 5, 2023
machadovilaca
machadovilaca requested changes Jul 5, 2023
View reviewed changes
Copy link
Member

@machadovilaca machadovilaca left a comment

Choose a reason for hiding this comment

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

please also add a functional test for the alert

controllers/alerts/alerts.go Outdated Show resolved Hide resolved
controllers/alerts/alerts.go Outdated Show resolved Hide resolved
controllers/alerts/alerts.go Outdated Show resolved Hide resolved
@dharmit dharmit mentioned this pull request Jul 6, 2023
Merged
@dharmit dharmit force-pushed the fix-cnv-28584 branch 5 times, most recently from 07964dd to f642e36 Compare July 7, 2023 12:56
@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. size/XL and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/L labels Jul 10, 2023
@dharmit
Add unit tests for single stack ipv6 check
c5d7f96 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Vendor changes in tests directory
1cd899a 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Metric and alert for single stack ipv6
351a7d3 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Get 'networks' for HCO serviceaccount
30789b5 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Functional tests for the alert
0ba7215 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Sanity updates
704ba81 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Change rule name; shorten alert name
1082b28 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Let the sanity prevail (make sanity)
286c1f3 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@dharmit
Use DynamicClient, not RestClient
4bf8311 
Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@sonarcloud
Copy link

sonarcloud bot commented Aug 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@hco-bot
Copy link
Collaborator

hco-bot commented Aug 8, 2023

hco-e2e-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-sno-azure
okd-hco-e2e-upgrade-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-upgrade-operator-sdk-gcp
okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-azure, ci/prow/okd-hco-e2e-operator-sdk-gcp, ci/prow/okd-hco-e2e-upgrade-operator-sdk-gcp

In response to this:

hco-e2e-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-operator-sdk-sno-azure
okd-hco-e2e-upgrade-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-upgrade-operator-sdk-gcp
okd-hco-e2e-operator-sdk-aws lane succeeded.
/override ci/prow/okd-hco-e2e-operator-sdk-gcp

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/test-infra repository.

@hco-bot
Copy link
Collaborator

hco-bot commented Aug 8, 2023

hco-e2e-upgrade-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure
hco-e2e-upgrade-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-operator-sdk-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-operator-sdk-azure, ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure

In response to this:

hco-e2e-upgrade-operator-sdk-sno-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure
hco-e2e-upgrade-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-operator-sdk-azure

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/test-infra repository.

@tiraboschi
Copy link
Member

/retest

@hco-bot
Copy link
Collaborator

hco-bot commented Aug 8, 2023

hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

@kubevirt-bot
Copy link
Contributor

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

In response to this:

hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure

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/test-infra repository.

@openshift-ci
Copy link

openshift-ci bot commented Aug 8, 2023 •
edited

@dharmit: 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-operator-sdk-azure 4bf8311 link true /test hco-e2e-upgrade-operator-sdk-azure
ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure 4bf8311 link false /test hco-e2e-upgrade-operator-sdk-sno-azure
ci/prow/hco-e2e-kv-smoke-azure 4bf8311 link true /test hco-e2e-kv-smoke-azure
ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure 4bf8311 link true /test hco-e2e-upgrade-prev-operator-sdk-azure
ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-aws 4bf8311 link false /test hco-e2e-upgrade-prev-operator-sdk-sno-aws

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/test-infra repository. I understand the commands that are listed here.

@hco-bot
Copy link
Collaborator

hco-bot commented Aug 8, 2023

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

@kubevirt-bot
Copy link
Contributor

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

In response to this:

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

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/test-infra repository.

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2023
@tiraboschi
Copy link
Member

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tiraboschi

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 Aug 9, 2023
@kubevirt-bot kubevirt-bot merged commit fc878c5 into kubevirt:main Aug 9, 2023
33 checks passed
dharmit added a commit to dharmit/hyperconverged-cluster-operator that referenced this pull request Aug 9, 2023
@dharmit
Detect if OpenShift cluster is single stack ipv6 (kubevirt#2386)
4dc5c99 
* Detect if OpenShift cluster is single stack ipv6

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Add unit tests for single stack ipv6 check

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Vendor changes in tests directory

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Metric and alert for single stack ipv6

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Get 'networks' for HCO serviceaccount

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Functional tests for the alert

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Sanity updates

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Change rule name; shorten alert name

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Let the sanity prevail (make sanity)

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

* Use DynamicClient, not RestClient

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

---------

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
kubevirt-bot pushed a commit that referenced this pull request Aug 10, 2023
@dharmit
Detect if OpenShift cluster is single stack ipv6 (#2386) (#2467)
b710013 
* Detect if OpenShift cluster is single stack ipv6



* Add unit tests for single stack ipv6 check



* Vendor changes in tests directory



* Metric and alert for single stack ipv6



* Get 'networks' for HCO serviceaccount



* Functional tests for the alert



* Sanity updates



* Change rule name; shorten alert name



* Let the sanity prevail (make sanity)



* Use DynamicClient, not RestClient



---------

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@machadovilaca machadovilaca machadovilaca requested changes

@tiraboschi tiraboschi tiraboschi approved these changes

@sradco sradco Awaiting requested review from sradco

Assignees

@tiraboschi tiraboschi

@machadovilaca machadovilaca

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/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@dharmit @kubevirt-bot @coveralls @hco-bot @tiraboschi @sradco @machadovilaca