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

OPNET-329: add DualStackIPv6Primary for vSphere #42899

Merged
merged 4 commits into from Oct 5, 2023
Merged

OPNET-329: add DualStackIPv6Primary for vSphere #42899

merged 4 commits into from Oct 5, 2023

Conversation

rbbratta
Copy link
Contributor

@rbbratta rbbratta commented Sep 1, 2023 •
edited by openshift-ci bot

OPNET-198 added IPv6-primary dual stack

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 1, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 1, 2023 •
edited by openshift-ci bot

@rbbratta: This pull request references OPNET-329 which is a valid jira issue.

In response to this:

OPNET-198 added IPv6-primary dual stack

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.

1 similar comment
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 1, 2023 •
edited by openshift-ci bot

@rbbratta: This pull request references OPNET-329 which is a valid jira issue.

In response to this:

OPNET-198 added IPv6-primary dual stack

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.

@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from e6d1526 to da9585b Compare September 1, 2023 00:48
@rbbratta
Copy link
Contributor Author

rbbratta commented Sep 1, 2023

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack

@rbbratta
Copy link
Contributor Author

rbbratta commented Sep 1, 2023

/cc @mkowalski

@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from da9585b to 2dbd25e Compare September 1, 2023 01:15
@rbbratta
Copy link
Contributor Author

rbbratta commented Sep 1, 2023

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack

@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from 2dbd25e to 15d4ecc Compare September 1, 2023 15:24
@rbbratta
Copy link
Contributor Author

rbbratta commented Sep 1, 2023

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack

mkowalski
mkowalski reviewed Sep 1, 2023
View reviewed changes
ci-operator/step-registry/ovn/conf/vsphere-dualstack/ovn-conf-vsphere-dualstack-ref.yaml Outdated
Comment on lines 12 to 14
env:
- name: IP_FAMILIES
default: "DualStackIPv6Primary"
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure about this part... It modifies the ovn-conf-vsphere-dualstack step which may be used by a lot of jobs and it sets the default family to DualStackIPv6Primary which doesn't have to be true because there are dual-stack jobs with IPv4-primary. Wouldn't it be better to not have a default at the step level and just set it for the specific job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a question of coverage. Every other job tests IPv4. This is our only IPv6 primary test? We need at least 75% of vSphere testing to be with IPv6.

OPNET-198 is done. IPv6 primary is supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't figured out how to handle the 4.13 jobs yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I didn't mean the coverage, I meant that this is a reusable step so I feel that we should not set any default here and every test that uses this step should set this value explicitly.

Otherwise we are assuming implicitly that everyone doing vsphere-dualstack will do IPv6-primary which may or may not be true.

It's not like I have something against IPv6-primary to become our model way of doing dual-stack because I would be very happy too, it's just that I feel doing it at the step level is a stretch for anyone outside of the "you and me" circle

@rbbratta rbbratta changed the title OPNET-329: add DualStackIPv6Primary for vSphere [WIP] OPNET-329: add DualStackIPv6Primary for vSphere Sep 12, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 12, 2023
@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from 15d4ecc to e75a35f Compare September 18, 2023 12:45
@rbbratta rbbratta changed the title [WIP] OPNET-329: add DualStackIPv6Primary for vSphere OPNET-329: add DualStackIPv6Primary for vSphere Sep 18, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 18, 2023
@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from e75a35f to e3a6113 Compare September 18, 2023 12:47
@rbbratta
Copy link
Contributor Author

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack

@rbbratta
Copy link
Contributor Author

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack-primaryv6

@rbbratta
Copy link
Contributor Author

Looks like install succeeded.

 ++++ ip_hint=fd65:a1a8:60ad:271c::e

--node-ip="fd65:a1a8:60ad:271c::e,192.168.108.10"

@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from 1046527 to af54341 Compare September 29, 2023 15:11
@rbbratta
Copy link
Contributor Author

/pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack-primaryv6

@rbbratta
Copy link
Contributor Author

rbbratta commented Oct 2, 2023 

: [sig-network] [Feature:IPv6DualStack] should create a single stack service with cluster ip from primary service range [Suite:openshift/conformance/parallel] [Suite:k8s] expand_less 	9s
{  fail [test/e2e/network/dual_stack.go:704]: service e2e-dualstack-3953/defaultclusterip expected family IPv4 at index[0] got IPv6
Error: exit with code 1
Ginkgo exit error 1: exit with code 1}

Something is not set for IPv6Primary.

@rbbratta
Copy link
Contributor Author

rbbratta commented Oct 2, 2023

openshift/origin#28292 to enable IPv6 tests.

@mkowalski
Copy link
Contributor

mkowalski commented Oct 2, 2023 •
edited

Something is not set for IPv6Primary.

This is a bug in test framework, not in the product that is being tested. I mean, you install dual-stack primary-v6 cluster but your test errors with

service e2e-dualstack-3953/defaultclusterip expected family IPv4 at index[0] got IPv6

so your test expected IPv4 even though we have a primary-v6 cluster.

@rbbratta
Copy link
Contributor Author

rbbratta commented Oct 2, 2023

@mkowalski Is there a way to unblock this for clusterbot launches without having e2e pass?

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 2, 2023 •
edited

@rbbratta: 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/build-farm/build09-dry 1046527a8335d1a3197f0200be461867e93b5e8e link true /test build09-dry
ci/rehearse/openshift/cluster-network-operator/master/e2e-vsphere-ovn-dualstack-primaryv6 af54341af6bb11db25340fb9e96cbaa0b93c9718 link unknown /pj-rehearse pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack-primaryv6

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.

@rbbratta rbbratta force-pushed the OPNET-329-DualStackIPv6Primary branch from d1982d2 to c096f42 Compare October 3, 2023 00:06
@openshift-ci-robot
Copy link
Contributor

[REHEARSALNOTIFIER]
@rbbratta: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack-primaryv6 openshift/cluster-network-operator presubmit Presubmit changed
pull-ci-openshift-cluster-network-operator-release-4.14-e2e-vsphere-ovn-dualstack-primaryv6 openshift/cluster-network-operator presubmit Presubmit changed
pull-ci-openshift-cluster-network-operator-release-4.15-e2e-vsphere-ovn-dualstack-primaryv6 openshift/cluster-network-operator presubmit Presubmit changed
pull-ci-openshift-cluster-network-operator-master-e2e-vsphere-ovn-dualstack openshift/cluster-network-operator presubmit Registry content changed
pull-ci-openshift-cluster-network-operator-release-4.16-e2e-vsphere-ovn-dualstack openshift/cluster-network-operator presubmit Registry content changed
pull-ci-openshift-cluster-network-operator-release-4.15-e2e-vsphere-ovn-dualstack openshift/cluster-network-operator presubmit Registry content changed
pull-ci-openshift-cluster-network-operator-release-4.14-e2e-vsphere-ovn-dualstack openshift/cluster-network-operator presubmit Registry content changed
pull-ci-openshift-cluster-network-operator-release-4.13-e2e-vsphere-ovn-dualstack openshift/cluster-network-operator presubmit Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-stable-vsphere-ipi-ovn-dualstack-f14 N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-vsphere-ipi-ovn-dualstack-f28 N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.13-amd64-stable-vsphere-ipi-ovn-dualstack-f14 N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-vsphere-ipi-ovn-dualstack-f28-ui N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-vsphere-ipi-ovn-dualstack-f28-destructive N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-ovn-dualstack-f28-destructive N/A periodic Registry content changed
periodic-ci-openshift-verification-tests-master-installer-rehearse-4.13-installer-rehearse-vsphere N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-ovn-dualstack-f28 N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.13-amd64-nightly-vsphere-ipi-ovn-dualstack-f14-ui N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-ovn-dualstack-f28-ui N/A periodic Registry content changed
periodic-ci-openshift-openshift-tests-private-release-4.13-amd64-nightly-vsphere-ipi-ovn-dualstack-f14 N/A periodic Registry content changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 10 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 20 rehearsals
Comment: /pj-rehearse max to run up to 35 rehearsals
Comment: /pj-rehearse auto-ack to run up to 10 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse abort to abort all active rehearsals

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@openshift-merge-robot
Copy link
Contributor

@rbbratta: all tests passed!

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.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 3, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 3, 2023 •
edited by openshift-ci bot

@rbbratta: This pull request references OPNET-329 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.15.0" version, but no target version was set.

In response to this:

OPNET-198 added IPv6-primary dual stack

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.

@mkowalski
Copy link
Contributor

/cc @danwinship
Owner of CNO and CNO in priv

/cc @vrutkovs
Owner of o/release

@mkowalski
Copy link
Contributor

/pj-rehearse ack
We are happy enough with the results

@openshift-ci-robot openshift-ci-robot added the rehearsals-ack Signifies that rehearsal jobs have been acknowledged label Oct 3, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 3, 2023 •
edited by openshift-ci bot

@rbbratta: This pull request references OPNET-329 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.15.0" version, but no target version was set.

In response to this:

OPNET-198 added IPv6-primary dual stack

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.

vrutkovs
vrutkovs approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

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

/approve

@rbbratta
Copy link
Contributor Author

rbbratta commented Oct 4, 2023

/cc @cybertron

@openshift-ci openshift-ci bot requested a review from cybertron October 4, 2023 18:43
danwinship
danwinship reviewed Oct 5, 2023
View reviewed changes
Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

This looks fine I guess but I don't really know the e2e stuff that well any more

ci-operator/step-registry/ovn/conf/vsphere-dualstack/ovn-conf-vsphere-dualstack-commands.sh
@@ -35,3 +35,12 @@ networking:
- 172.30.0.0/16
- fd65:172:16::/112
EOF

if [[ "${IP_FAMILIES}" == "DualStackIPv6Primary" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

It's weird that you have IP_FAMILIES but it's always either "" or "DualStackIPv6Primary"...

Maybe make it PRIMARY_IP_FAMILY and have it default to IPv4?

@knobunc
Copy link
Contributor

knobunc commented Oct 5, 2023

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knobunc, mkowalski, rbbratta, vrutkovs

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2023
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 5, 2023 •
edited by openshift-ci bot

@rbbratta: This pull request references OPNET-329 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.15.0" version, but no target version was set.

In response to this:

OPNET-198 added IPv6-primary dual stack

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 openshift-ci bot merged commit 6047f25 into openshift:master Oct 5, 2023
18 checks passed
@rbbratta rbbratta mentioned this pull request Oct 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danwinship danwinship danwinship left review comments

@vrutkovs vrutkovs vrutkovs approved these changes

@mkowalski mkowalski mkowalski approved these changes

@knobunc knobunc Awaiting requested review from knobunc

@trozet trozet Awaiting requested review from trozet

@cybertron cybertron Awaiting requested review from cybertron

Assignees

@vrutkovs vrutkovs

@mkowalski mkowalski

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. rehearsals-ack Signifies that rehearsal jobs have been acknowledged
Projects
None yet
Milestone
No milestone
7 participants
@rbbratta @openshift-ci-robot @mkowalski @openshift-merge-robot @knobunc @danwinship @vrutkovs