UPSTREAM: 37228: kubelet: storage: teardown terminated pod volumes #12669
Conversation
sjenning
force-pushed
the
pick-37228
branch
2 times, most recently
from
135d0f3
to
66d5600
Compare
|
[test] while waiting for review |
|
continuous-integration/openshift-jenkins/test Waiting: Determining build queue position |
| @@ -473,6 +473,9 @@ type KubeletConfiguration struct { | |||
| // (binaries, etc.) to mount the volume are available on the underlying node. If the check is enabled | |||
| // and fails the mount operation fails. | |||
| ExperimentalCheckNodeCapabilitiesBeforeMount bool `json:"ExperimentalCheckNodeCapabilitiesBeforeMount,omitempty"` | |||
| // This flag, if set, instructs the kubelet to keep volumes from terminated pods mounted to the node. | |||
| // This can be useful for debugging volume related issues. | |||
| KeepTerminatedPodVolumes bool `json:"KeepTerminatedPodVolumes,omitempty"` | |||
There was a problem hiding this comment.
The upstream PR doesn't have json tags but this does?
There was a problem hiding this comment.
Ok I see the others in this file do as well (upstream has since cleared them from internal). Nevermind on this for now.
| @@ -511,6 +511,9 @@ type KubeletConfiguration struct { | |||
| // (binaries, etc.) to mount the volume are available on the underlying node. If the check is enabled | |||
| // and fails the mount operation fails. | |||
| ExperimentalCheckNodeCapabilitiesBeforeMount bool `json:"ExperimentalCheckNodeCapabilitiesBeforeMount,omitempty"` | |||
| // This flag, if set, instructs the kubelet to keep volumes from terminated pods mounted to the node. | |||
| // This can be useful for debugging volume related issues. | |||
| KeepTerminatedPodVolumes bool `json:"KeepTerminatedPodVolumes,omitempty"` | |||
There was a problem hiding this comment.
json tag should start with a lowercase k
|
2 comments to address then lgtm |
|
@liggitt @smarterclayton ptal, the json tags are a bit funky as kube upstream has removed internal json tags and also fixed the capitalization issue in componentconfig. Are you ok merging this as is? |
|
Evaluated for origin test up to 4dab27c |
|
I want to run this against GCE first. |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13352/) (Base Commit: 3e38a69) |
|
GCE test will be here once the previous build wraps up: https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_gce/200 |
|
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_gce/195/#showFailuresLink was the test, which passed. |
|
@smarterclayton any objections to merging? |
|
no, merge away. |
|
[merge]
…On Fri, Jan 27, 2017 at 4:37 PM Clayton Coleman ***@***.***> wrote:
no, merge away.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12669 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAABYh9Yu55rEsOjoqWWhbSO6ZGmIYKBks5rWmOZgaJpZM4Lt_Gc>
.
|
|
Evaluated for origin merge up to 4dab27c |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13390/) (Base Commit: ae7e34f) (Image: devenv-rhel7_5798) |
|
@sjenning - we need to ensure that upstream does not change this flag between now and release of k8s 1.6. also we will need updates to the documentation for out of resource handling to ensure that operators know about this flag... |
kubernetes/kubernetes#37228
xref https://bugzilla.redhat.com/show_bug.cgi?id=1349311
@derekwaynecarr @ncdc