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

Switch default DNS plugin to CoreDNS #566

Closed
justaugustus opened this issue May 7, 2018 · 46 comments
Closed

Switch default DNS plugin to CoreDNS #566

justaugustus opened this issue May 7, 2018 · 46 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team
Milestone

Comments

@justaugustus
Copy link
Member

justaugustus commented May 7, 2018

Feature Description

  • One-line feature description (can be used as a release note): Switch default DNS plugin to CoreDNS
  • Primary contact (assignee): @johnbelamaric
  • Responsible SIGs: sig-network, sig-cluster-lifecycle
  • Design proposal link (community repo): Add coredns proposal community#1100
  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred: @bowei @thockin
  • Approver (likely from SIG/area to which feature belongs): @thockin
  • Feature target (which target equals to which milestone):
    • Stable release target: 1.12

xref: #427

/kind feature
/sig cluster-lifecycle
/sig network
/assign @johnbelamaric

@k8s-ci-robot
Copy link
Contributor

@justaugustus: GitHub didn't allow me to assign the following users: johnbelamaric.

Note that only kubernetes members and repo collaborators can be assigned.

In response to this:

Feature Description

  • One-line feature description (can be used as a release note): Switch default DNS plugin to CoreDNS
  • Primary contact (assignee): @johnbelamaric
  • Responsible SIGs: sig-network, sig-cluster-lifecycle
  • Design proposal link (community repo): Add coredns proposal community#1100
  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred: @bowei @thockin
  • Approver (likely from SIG/area to which feature belongs): @thockin
  • Feature target (which target equals to which milestone):
  • Alpha release target: 1.12

/kind feature
/sig cluster-lifecycle
/sig network
/assign @johnbelamaric

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.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. labels May 7, 2018
@justaugustus justaugustus added the stage/alpha Denotes an issue tracking an enhancement targeted for Alpha status label May 7, 2018
@justaugustus justaugustus added this to the v1.12 milestone May 7, 2018
@luxas
Copy link
Member

luxas commented May 8, 2018

Hi, thanks for filing this feature!
I have a couple of questions/concerns though.
Why is this marked alpha? It sounds more like it should be stable/GA when it's a default.
I don't think this even needs to be a separate features issue. The common practice is to use one issue (in this case #427) to track the full lifecycle.
If your goal is to have CoreDNS as the default DNS plugin in v1.12, I'd just adjust #427 to be "stable/GA target: v1.12"

Cheers 😄

@stp-ip
Copy link
Member

stp-ip commented May 8, 2018

The reasoning behind the split was, that in order to make it default later on the actual plugin needs to be stable/GA already. That was the prerequisite after a bigger discussion. The result was the split into two feature issues and making the default alpha first. (Alpha might not be the best thing so.)

@stp-ip
Copy link
Member

stp-ip commented May 8, 2018

Reading all your other comments on the other thread (#427 (comment)) your comment is much clearer in my head. As CoreDNS is such a central place it make still make sense to track the state of being the default.

@justaugustus
Copy link
Member Author

@johnbelamaric @cmluciano --

It looks like this feature is currently in the Kubernetes 1.12 Milestone.

If that is still accurate, please ensure that this issue is up-to-date with ALL of the following information:

  • One-line feature description (can be used as a release note):
  • Primary contact (assignee):
  • Responsible SIGs:
  • Design proposal link (community repo):
  • Link to e2e and/or unit tests:
  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred:
  • Approver (likely from SIG/area to which feature belongs):
  • Feature target (which target equals to which milestone):
    • Alpha release target (x.y)
    • Beta release target (x.y)
    • Stable release target (x.y)

Set the following:

  • Description
  • Assignee(s)
  • Labels:
    • stage/{alpha,beta,stable}
    • sig/*
    • kind/feature

Please note that the Features Freeze is July 31st, after which any incomplete Feature issues will require an Exception request to be accepted into the milestone.

In addition, please be aware of the following relevant deadlines:

  • Docs deadline (open placeholder PRs): 8/21
  • Test case freeze: 8/28

Please make sure all PRs for features have relevant release notes included as well.

Happy shipping!

/cc @justaugustus @kacole2 @robertsandoval @rajendar38

@luxas
Copy link
Member

luxas commented Jul 30, 2018

I assume this is beta in v1.12? kubeadm has used it on a GA/beta level already since v1.11 fwiw

@justaugustus
Copy link
Member Author

@johnbelamaric / @cmluciano / @luxas / @thockin --
So this is a case where the "graduation" of the feature is actually flipping the bit for it to be the default DNS plugin.
As CoreDNS is already GA, what should we do here?

  • Straight to GA?
  • Set to Beta to soak for a release and then graduate to GA?
  • Something else entirely?

/assign @johnbelamaric
/remove-stage alpha

@k8s-ci-robot k8s-ci-robot removed the stage/alpha Denotes an issue tracking an enhancement targeted for Alpha status label Jul 31, 2018
@stp-ip
Copy link
Member

stp-ip commented Jul 31, 2018

As the point for switching the flip was mainly to separate GA of CoreDNS and the actual config switch it seems like moving to beta is another step, that might not be necessary especially considering it's already GA/beta in kubeadm.

Should go GA in v1.12 in my view fwiw.

@justaugustus
Copy link
Member Author

justaugustus commented Jul 31, 2018

Just to clarify for passerbys...
Adding CoreDNS as a DNS plugin was tracked on #427. There is a process for moving features through stages (alpha --> beta --> stable), which again was tracked on #427.

This Feature issue is unique in the fact that here, we're not actually tracking the development state of the feature (as it's already been developed), but instead we're tracking the process of deciding in which release it should be enabled as the default DNS plugin. AFAIK, we don't have an official graduation process for "flipping a bit".

While developing kubeadm is a separate concern, @stp-ip is right; this has already had some soak time as CoreDNS was selected as the default DNS plugin for kubeadm in Kubernetes 1.11.

FWIW, I would also vote to mark this as GA for 1.12, but I'll wait for the powers that be (@kubernetes/sig-network-misc) to make a call on that.

@chrisohaver
Copy link

How does this process relate to the KEP process?

The KEP for graduating CoreDNS to default DNS server, is here ...
https://github.com/kubernetes/community/blob/master/keps/sig-network/0012-20180518-coredns-default-proposal.md

@cmluciano
Copy link

I agree that switching to default based on kubeadm results is great progress. Do we have any preliminary results from cloud-providers or other large deploys on stability.

It may be a "flipping in bit" in docs, but often features are not used in k8s until they reach beta stage.

@justaugustus
Copy link
Member Author

@johnbelamaric @cmluciano -- we still need to make a call on this. I'm tentatively setting this to GA on the sheet, but let me know if you feel differently.

/stage stable
cc: @kacole2 @wadadli @robertsandoval @rajendar38

@k8s-ci-robot k8s-ci-robot added the stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status label Aug 4, 2018
@justaugustus justaugustus added the tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team label Aug 4, 2018
@chrisohaver
Copy link

The KEP lists the following criteria for graduating coredns to "default"...

  • Add CoreDNS image in a Kubernetes repository (To Be Defined) and ensure a workflow/process to update the CoreDNS versions in the future.
  • Have a certain number (To Be Defined) of clusters of significant size (To Be Defined) adopting and running CoreDNS as their default DNS.

The first is, I believe, satisfied.
For the second, we (CoreDNS team + CNCF) are conducting a survey of k8s+coredns users to gauge coredns adoption in large clusters. Just pushed it out recently. We do not have survey results yet.

@fturib
Copy link

fturib commented Aug 6, 2018

@justaugustus : I try to push an email on this subject with Tim, John, I will add @cmluciano .. but I cannot find any email for yourself. Can your provide one ? Thanks !

@fturib
Copy link

fturib commented Aug 6, 2018

for info, the survey for feedback on CoreDNS adoption is available here : https://www.surveymonkey.com/r/SKZQSLK

@johnbelamaric
Copy link
Member

Going GA with this feature means deprecating kube-dns, I think we need to wait until 1.13 at the earliest for this, to gather enough data.

@chrisohaver
Copy link

I’m looking into the auto scale and debugging articles.

@johnbelamaric
Copy link
Member

@justaugustus I think we can wait a cycle or so to deprecate kube-dns. Originally I was pairing the two but that's not strictly necessary.

@justaugustus
Copy link
Member Author

@justaugustus I think we can wait a cycle or so to deprecate kube-dns. Originally I was pairing the two but that's not strictly necessary.

@johnbelamaric -- Got it. Noting again that we need to coalesce a plan for kube-dns deprecation sooner rather than later, as it's a multi-release event.

@tfogo
Copy link

tfogo commented Sep 7, 2018

Hi @rajansandeep, @johnbelamaric,

How are docs looking for this? Is there a PR open in k/website?

@rajansandeep
Copy link
Contributor

I forgot to refer the PR for this.
kubernetes/website#10228 is ready for review.

@chrisohaver
Copy link

@tpepper
Copy link
Member

tpepper commented Sep 13, 2018

warning this is now at risk, see:

kubernetes/kubernetes#68629
kubernetes/kubernetes#68613

@kacole2
Copy link
Contributor

kacole2 commented Sep 17, 2018

/milestone v1.13

@AishSundar
Copy link

@bowei @chrisohaver can you plz update this issue with your level of confidence, whats pending in terms of PR, test and docs for this feature to wrap in 1.13? Considering the last minute Scalability and memory constraints we ran into last cycle, it will be great to land the open PRs sooner in this cycle to we have enough time to watch the CI signals and stabilize.

Currently Code Slush for 1.13 is Nov 9th and Freeze starts on Nov 15th. Thanks

@fturib
Copy link

fturib commented Oct 4, 2018

@AishSundar : We have been investigating the scale issue and memory constraint. We found some optimization and know now we can make it. However we need to wrap up the changes and publish a new release of CoreDNS.
I cannot provide a date yet, but I expect to be by Oct 15th.

We then need:

  • re-publish the PR for CoreDNS as default, with the new image. (pretty simple - no delay)
  • e2e test will validate after that merge. (2 PR already presented - see below)
  • about doc, I think all docs expected were finally merged in 1.12 (because still valid whatever CoreDNS is default or not for GCE). We may need to have another look.

I agree completely on the need to have this feature checked-in asap. We had a sync-up with @thockin from SIG-Network last Tuesday on this subject.
Thanks to remind the code slush date. I plan to be much ahead of these dates.

PR needed for validation e2e tests at scale:

@AishSundar
Copy link

Thanks @fturib for the detailed status and good to know that we have a path forward. I will check on the status again post Oct mid.

@tfogo
Copy link

tfogo commented Nov 3, 2018

Hi @fturib, I know we had a bunch of docs land for this in 1.12, but I just wanted to check if there be any updates to the docs necessary for 1.13? The deadline for placeholder PRs for the 1.13 release is November 8. So it's important to make a docs PR as soon as possible if needed.

@AishSundar
Copy link

@fturib @chrisohaver @wojtek-t are there any specific docs / release notes to callout any metrics / resource changes for CoreDNS

@marpaia as FYI for release notes

@fturib
Copy link

fturib commented Nov 5, 2018

@AishSundar, @tfogo : let me sync-up on this topic with @chrisohaver, @rajansandeep and come later today or tomorrow on any need for the doc for this feature. Thanks!

@fturib
Copy link

fturib commented Nov 8, 2018

We have few updates for some docs. Only minor changes and a pointer for a documentation page create by @chrisohaver for tuning CoreDNS when resources are limited on the cluster.

There is no resource change for CoreDNS.

@rajansandeep is preparing a PR for these changes in kubernetes/website.

@AishSundar
Copy link

Thanks @fturib for the confirmation. @rajansandeep once you have the doc PRs plz link them here as well. Thanks.

@rajansandeep
Copy link
Contributor

@AishSundar The PR is here. kubernetes/website#10923

@AishSundar
Copy link

@tfogo

@luxas
Copy link
Member

luxas commented Dec 4, 2018

v1.13 is now released, and this is hence fully implemented 🎉

@luxas luxas closed this as completed Dec 4, 2018
@kacole2 kacole2 added tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team and removed tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team labels Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/network Categorizes an issue or PR as relevant to SIG Network. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status tracked/no Denotes an enhancement issue is NOT actively being tracked by the Release Team
Projects
None yet
Development

No branches or pull requests