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

kubeadm: update coredns to 1.8.4 #102466

Merged
merged 2 commits into from
Jun 22, 2021
Merged

Conversation

pacoxu
Copy link
Member

@pacoxu pacoxu commented Jun 1, 2021

What type of PR is this?

/kind feature
/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

xref #99751

Special notes for your reviewer:

Does this PR introduce a user-facing change?

kubeadm: update CoreDNS to 1.8.4. Grant CoreDNS permissions to "list" and "watch" EndpointSlice objects to accommodate dual-stack support.

@k8s-ci-robot
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

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. area/dependency Issues or PRs related to dependency changes area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 1, 2021
@k8s-ci-robot k8s-ci-robot requested review from hasheddan, logicalhan and a team June 1, 2021 03:00
@pacoxu pacoxu mentioned this pull request Jun 1, 2021
12 tasks
Copy link
Contributor

@rajansandeep rajansandeep left a comment

Choose a reason for hiding this comment

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

You would want to add the changes in #101955 in this PR to add permissions for endpointslices

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 2, 2021
@pacoxu
Copy link
Member Author

pacoxu commented Jun 2, 2021

You would want to add the changes in #101955 in this PR to add permissions for endpointslices

@rajansandeep I cherry-picked it to this PR.

@neolit123
Copy link
Member

the coredns migration library was just updated in this PR:
#102830

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 14, 2021
@pacoxu pacoxu marked this pull request as ready for review June 15, 2021 03:55
@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 Jun 15, 2021
@BenTheElder
Copy link
Member

no worries, thanks for updating the PR description, I mostly wanted to avoid automatic issue closure in this case 😅
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 16, 2021
@pacoxu
Copy link
Member Author

pacoxu commented Jun 16, 2021

/assign @fabriziopandini

@pacoxu
Copy link
Member Author

pacoxu commented Jun 21, 2021

/cc neolit123

@k8s-ci-robot k8s-ci-robot requested a review from neolit123 June 21, 2021 14:33
Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

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

/approve
/hold
/priority important-soon

LGTM, please update the release note to something like:

kubeadm: update CoreDNS to 1.8.4. Grant CoreDNS permissions to "list" and "watch" EndpointSlice objects to accommodate dual-stack support.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 21, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, neolit123, pacoxu

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 Jun 21, 2021
@neolit123
Copy link
Member

/retitle kubeadm: update coredns to 1.8.4

@k8s-ci-robot k8s-ci-robot changed the title kubeadm: upgrade coredns 1.8.4 and corefile-migration to v1.0.12 kubeadm: update coredns to 1.8.4 Jun 21, 2021
@neolit123
Copy link
Member

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 21, 2021
@pacoxu
Copy link
Member Author

pacoxu commented Jun 21, 2021

Updated

@neolit123
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 21, 2021
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@pacoxu
Copy link
Member Author

pacoxu commented Jun 21, 2021

/retest

@chrisohaver
Copy link
Contributor

chrisohaver commented Sep 9, 2021

FYI: @neolit123

Last week a regression was reported for 1.8.4 for the rewrite plugin. There is a workaround. FYI AFAIK, AKS has decided to defer rolling out 1.8.4, skipping over the version, since a fix will be in 1.8.5 (already merged).
Describing the regression as succinctly as I can:

If using the rewrite plugin, a rewrite rule that does not also rewrite the response (using the answer option) will cause clients to reject any response rewritten by the rule (due to question section mismatch). A work around is to append answer auto to affected rules. All exact name rewrites are not affected since they automatically perform a response rewrite.

FWIW, this has always been the case for some clients (some clients are extra picky about question mismatch), but the regression caused it to affect most clients (if not all). My attempt to explain in more detail here.

Also, regarding the workaround, answer auto is only valid in 1.8.4, so a user cannot use that option to adjust their corefile before upgrading to avoid potential downtime. If that is desired, any affected rule should be expressed as a regex rule with a response rewrite, which would be valid in 1.84 and prior versions.

@pacoxu
Copy link
Member Author

pacoxu commented Sep 9, 2021

@chrisohaver Kubeadm default coredns corefile doesn’t include ‘rewrite’ plug-in.

However, it Will affect user using rewrite plugin.

@chrisohaver
Copy link
Contributor

@chrisohaver Kubeadm default coredns corefile doesn’t include ‘rewrite’ plug-in.

However, it Will affect user using rewrite plugin.

Yes. That's correct. Hard to know how many people use it, and how many people who use it are not already doing a response rewrite (as would be prudent).

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. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants