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 kind dual presubmits #21605

Closed
wants to merge 1 commit into from
Closed

add kind dual presubmits #21605

wants to merge 1 commit into from

Conversation

aojea
Copy link
Member

@aojea aojea commented Apr 1, 2021

Now that dualstack is supported in KIND "officially" , and that those jobs were running for a while without issues we can run them in presubmits.

https://testgrid.k8s.io/sig-network-kind#sig-network-kind,%20dual,%20master&width=20

There are 2 dual-stack presubmit jobs depending on the kube-proxy implementation.
The ipvs one will run only if some ipvs code has changed.
The iptables one will run on every commit, but it will be optional without reporting status to let it soak.

xref: kubernetes-sigs/kind#2172

@aojea
Copy link
Member Author

aojea commented Apr 1, 2021

/assign @thockin @BenTheElder

@k8s-ci-robot k8s-ci-robot added area/config Issues or PRs related to code in /config area/jobs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/testgrid sig/network Categorizes an issue or PR as relevant to SIG Network. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Apr 1, 2021
@aojea
Copy link
Member Author

aojea commented Apr 1, 2021

/retest

@aojea
Copy link
Member Author

aojea commented Apr 1, 2021

is this something am I doing wrong?
pull-test-infra-gubernator — Job failed.

...............WARNING:root:label event with no labels (multiple changes?)
...ERROR:root:webhook failed signature check
.ERROR:root:webhook failed signature check
........................ERROR:root:unable to fetch 'http://testbed.example.com/_ah/gcs/foo/quux': status code 404
....ERROR:root:failed to get user name: status 403
.ERROR:root:failed to vend token: status 403

@aojea
Copy link
Member Author

aojea commented Apr 1, 2021

/retest

@k8s-ci-robot k8s-ci-robot added the area/gubernator Issues or PRs related to code in /gubernator label Apr 6, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aojea
To complete the pull request process, please ask for approval from bentheelder after the PR has been reviewed.

The full list of commands accepted by this bot can be found 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

@aojea
Copy link
Member Author

aojea commented Apr 6, 2021

Ben is out this week
/assign @spiffxp

always_run: false
skip_report: false
always_run: true
skip_report: true
Copy link
Member

Choose a reason for hiding this comment

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

Three always run kind jobs is a lot. Can we replace the other two with this? Or at least one of them? We should definitely not skip reporting unless the intent is to gain confidence to replace

Copy link
Member Author

Choose a reason for hiding this comment

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

this is to gain confidence ...
we need the ipv6 only because is the only job that gates on ipv6 ...
the dual is a superset of the ipv4, and we have all the other jobs for ipv4 ...
I'd like to hear from Ben too

@thockin thockin removed their assignment Apr 7, 2021
@aojea
Copy link
Member Author

aojea commented Apr 28, 2021

/hold
let's wait until dual-stack goes GA, tentative this cycle, and we can switch pull-kind for pull-kind-dual

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 28, 2021
@BenTheElder
Copy link
Member

let's wait until dual-stack goes GA, tentative this cycle, and we can switch pull-kind for pull-kind-dual

I feel like of the 3 scenarios people are most likely to have ipv4, or dual, not ipv6 and dual. Let's switch the ipv6 presubmit for dual?

@aojea
Copy link
Member Author

aojea commented May 19, 2021

I feel like of the 3 scenarios people are most likely to have ipv4, or dual, not ipv6 and dual. Let's switch the ipv6 presubmit for dual?

the dual job test exercises the same codepath that the ipv4, since its first ipFamily is IPv4 ... we don't have any other gate for ipv6. I prefer switching the ipv4 to dual, so we don't loose any coverage ... and gce and the others are still ipv4 only

@BenTheElder
Copy link
Member

and gce and the others are still ipv4 only

yes, which is why I'm not thrilled to not have a closely matching kind job, we need to be able to answer the question if flakes are due to kind (or GCE!) or something else (like totally different cluster mode / tests)

@BenTheElder
Copy link
Member

longterm I personally still lean towards GCE -> not in k/k presubmit, except maybe the moderate scale tests. we want to drive down test time and avoid testing vendors versus k8s itself.

but until then, we have to compare kind to GCE as the defacto prior presubmit.

@aojea
Copy link
Member Author

aojea commented Jun 22, 2021

we can use the periodics job to get signal, if we start to see bugs or regressions I'll reopen the debate
Thanks

@aojea aojea closed this Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config Issues or PRs related to code in /config area/gubernator Issues or PRs related to code in /gubernator area/jobs area/testgrid cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants