-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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 workaround for klog/coredns crash issue #85212
Conversation
/cc @rajansandeep |
/retest |
@@ -129,6 +129,8 @@ spec: | |||
- name: config-volume | |||
mountPath: /etc/coredns | |||
readOnly: true | |||
- name: tmp | |||
mountPath: /tmp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra indent needs to be fixed.
- name: tmp
mountPath: /tmp
@@ -163,6 +165,8 @@ spec: | |||
readOnlyRootFilesystem: true | |||
dnsPolicy: Default | |||
volumes: | |||
- name: tmp | |||
emptyDir: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here as well, extra indent
- name: tmp
emptyDir: {}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - same copy paste error in the other files too.
And there was also a tab that snuck into manifests.go.
/kind bug |
Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
1adfe3b
to
ca1b0b5
Compare
/lgtm |
/approve |
thanks @chrisohaver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrisohaver, MrHohn, neolit123 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 |
Thanks for the PR. This fixes the issue indeed. |
Is there anything blocking merge of this? Really hoping to see this fix in the next patch release. |
/assign @tpepper needs sign off from the patch release team |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change lgtm, was it not possible to use the automated cherry pick (cherry_pick_pull.sh) for this one?
See also https://github.com/kubernetes/community/blob/master/contributors/devel/sig-release/cherry-picks.md It's not necessary to re-do this via the cherrypick automation, but just want to FYI in the event in the future you have a choice between using the automation versus manual. |
@tpepper yes, the manual pick looks correct now, agreed no need for re-do. In general though, automated cherry-pick should be preferred when there are no merge conflicts. Helps avoid the manual errors that happened here and git tooling plays better when the original commit hash is maintained. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Adding a /tmp volume mount resolves the issue in CoreDNS 1.3.1 where CoreDNS will crash when the Kubernetes API shuts down.
This is the same fix as #82128, which was merged into the 1.14 branch. CoreDNS 1.3.1 is installed by kubeadm/kubeup in K8s 1.14 and 1.15, so the fix should be in 1.15 too.
Fixes #85205
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: