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

Graduate IPv6 support to beta #1139

Merged
merged 2 commits into from
Aug 9, 2019
Merged

Conversation

aojea
Copy link
Member

@aojea aojea commented Jul 14, 2019

Graduate IPv6 support to beta, it was in alpha status since 1.9 and a CI system was added to avoid regressions and guarantee the quality

Ref: #1138

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/network Categorizes an issue or PR as relevant to SIG Network. labels Jul 14, 2019
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 14, 2019
@aojea aojea mentioned this pull request Jul 14, 2019
@aojea
Copy link
Member Author

aojea commented Jul 14, 2019

/cc @lachie83 @BenTheElder

@aojea aojea changed the title Graduate IPv6 support to beta [WIP] Graduate IPv6 support to beta Jul 20, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2019
@aojea
Copy link
Member Author

aojea commented Jul 20, 2019

I have to add a section to include the ipv6 jobs at least as part of the presubmits and to the release to avoid situations like #80388

@dims
Copy link
Member

dims commented Jul 20, 2019

@aojea @khenidak Can we make sure at least two or more cloud providers work well with IPv6 (and have CI coverage)?

@aojea
Copy link
Member Author

aojea commented Jul 22, 2019

@dims let me add it to the KEP so it can be discussed in the reviews, I will try to finish it this week 😄

@aojea aojea force-pushed the ipv6Beta branch 2 times, most recently from 4cb2f23 to 8d625ce Compare July 23, 2019 13:52
@aojea aojea changed the title [WIP] Graduate IPv6 support to beta Graduate IPv6 support to beta Jul 23, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 23, 2019
@aojea aojea force-pushed the ipv6Beta branch 4 times, most recently from f926d20 to 4be84c3 Compare July 23, 2019 21:52
## Proposal

* Make kind IPv6 e2e jobs mandatory for all PRs
* Have signal on IPv6 e2e jobs running in at least one Cloud Provider
Copy link
Member Author

Choose a reason for hiding this comment

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

@thockin @lachie83 is this feasible?
are those jobs available for all kubernetes members or are they only configured by the companies?

Copy link
Member

Choose a reason for hiding this comment

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

anyone can publish conformance signal to testgrid, we already have a KEP for that.
anyone can also donate resources for their cloud to be used by prow / kubernetes's own CI

... we apparently even have EKS clusters on prow.k8s.io today.

however, the fact that someone could do this doesn't mean they will ...

## Proposal

* Make kind IPv6 e2e jobs mandatory for all PRs
* Have signal on IPv6 e2e jobs running in at least one Cloud Provider
Copy link
Member

Choose a reason for hiding this comment

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

I'd request this to be changed to at least 2 cloud providers. please explicitly say that we need the results in the testgrid as well for the release team. ("signal" is not expressive enough i feel)

## Proposal

* Make kind IPv6 e2e jobs mandatory for all PRs
* Create IPv6 e2e jobs on at least 2 Cloud Providers and publish the results on testgrid
Copy link
Member

Choose a reason for hiding this comment

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

IMHO this (2 cloud provider IPv6 jobs) is potentially a pretty significant hurdle.

You can't just create e2e jobs on a provider, you need credits from the provider and a working system to deploy IPv6 clusters on them besides the CI glue.

Today we have some AWS quota and lots of GCP quota, for which we have kops tooling on AWS and a few options on GCE, neither of which sets of (tools, provider) do IPv6 clusters AFAIK.

Is there a concrete plan for this particular point? Otherwise I suspect this will block progress.

Copy link
Member

Choose a reason for hiding this comment

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

@BenTheElder we need to shop this around in the sig-cloudprovider and get volunteers to help

Copy link
Member

Choose a reason for hiding this comment

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

the main reason for asking this is - since beta means it's on by default, if its on by default but does not work on most of the cloud providers, it's not really useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, technically is on by default, you don't have to enable anything, you configure ipv6 address, it work in ipv6, you configure ipv4 address it works in ipv4.

Copy link
Member

Choose a reason for hiding this comment

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

@aojea we don't have a feature flag? hmmm

Copy link
Member Author

@aojea aojea Jul 26, 2019

Choose a reason for hiding this comment

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

Right, I see that misunderstanding lately, this is not a feature as in typical software development. When you develop your application you usually don't care about the network protocol, what @leblancd and others implemented is that k8s will use ipv6 addresses instead of ipv4, you don't have to turn on anything,

Copy link
Member

Choose a reason for hiding this comment

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

@aojea Ack thanks. I am ok with just one cloud provider in that case for beta.


* Make kind IPv6 e2e jobs mandatory for all PRs
* Create IPv6 e2e jobs on at least 2 Cloud Providers and publish the results on testgrid
* Move all IPv6 e2e conformance jobs (kind + Cloud Providers) into release blocking
Copy link
Member

Choose a reason for hiding this comment

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

(following the standard process to move things into blocking following proving that they are stable and contacting SIG-release of course)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, thanks, will reword and explain it better

@lachie83
Copy link
Member

cc @khenidak

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 27, 2019
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 28, 2019
@dims
Copy link
Member

dims commented Jul 29, 2019

/assign @johnbelamaric @thockin

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 9, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, thockin

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 9, 2019
@k8s-ci-robot k8s-ci-robot merged commit df77bac into kubernetes:master Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/network Categorizes an issue or PR as relevant to SIG Network. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants