Promote mount propagation to GA #67255
Conversation
k8s-ci-robot
added
size/M
k8s-ci-robot
added
needs-rebase
bertinatto
force-pushed
the
promote_mount_propagation
branch
from
e1e7da4 to
d258177
Compare
k8s-ci-robot
removed
the
needs-rebase
|
/sig storage |
|
/ok-to-test Is sig aws correct? This feature should work across all Linux environments |
k8s-ci-robot
added
sig/storage
bertinatto
force-pushed
the
promote_mount_propagation
branch
from
d258177 to
a311228
Compare
|
@msau42: sorry, I assigned to sig aws by mistake. |
|
/remove-sig aws |
|
I heard no objections since the last sig-storage meeting |
k8s-ci-robot
added
the
lgtm
|
/assign @liggitt @Random-Liu @tallclair for approval. Lot of code that checked for the feature is being removed. |
| @@ -371,7 +365,6 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS | |||
| PodPriority: {Default: true, PreRelease: utilfeature.Beta}, | |||
| EnableEquivalenceClassCache: {Default: false, PreRelease: utilfeature.Alpha}, | |||
| TaintNodesByCondition: {Default: true, PreRelease: utilfeature.Beta}, | |||
| MountPropagation: {Default: true, PreRelease: utilfeature.Beta}, | |||
There was a problem hiding this comment.
should be changed to GA and defaulted true, right?
There was a problem hiding this comment.
@kubernetes/sig-architecture-pr-reviews - does the expected lifecycle of a feature gate stop honoring the gate being disabled once it reaches GA?
There was a problem hiding this comment.
I didn't find anything about removing feature gate in community repo. Did I miss it?
I found issue #46404 that suggests we should remove all gate checks, report the feature as GA for 2 releases and then remove it.
#65862 followed this approach, but they remove the gate checks when removing the feature, not when it was declared GA.
There was a problem hiding this comment.
That's not completely true. Advanced Audit Logging is actually moves to GA, I remove all gate checks and remove also old pipeline that is not possible to execute without disabling Advanced Audit Logging (because it in GA)
There was a problem hiding this comment.
Yes, I think the expectation should be that once a feature is GA it cannot be disabled, even though we need to leave the gate in place.
There was a problem hiding this comment.
See the thread at https://groups.google.com/d/msg/kubernetes-sig-architecture/qJeJ7ANYzS0/0KnJgirWDgAJ
Promoting to GA and announcing deprecation of the gate in 1.12, then removing it and the checks of it in 1.13 would be consistent with our deprecation policy.
There was a problem hiding this comment.
@liggitt, done. Can you take a look again, please?
k8s-ci-robot
added
the
sig/architecture
|
LGTM other than the question about the GA feature gate |
bertinatto
force-pushed
the
promote_mount_propagation
branch
from
a311228 to
448d600
Compare
k8s-ci-robot
added
area/kubelet
and removed
lgtm
|
@liggitt, can you take a look, please? I added the feature gate back and now I emit a warning if it's disabled (as per the discussion in the thread above). |
| if !utilfeature.DefaultFeatureGate.Enabled(features.MountPropagation) { | ||
| allErrs = append(allErrs, field.Forbidden(fldPath, "mount propagation is disabled by feature-gate")) | ||
| return allErrs | ||
| glog.Warning("MountPropagation can't be disabled via feature gate") |
There was a problem hiding this comment.
warning here, per request validation, isn't the right place... we'd want to warn at startup if someone set this to false
There was a problem hiding this comment.
Moved the warning to kubelet.go.
bertinatto
force-pushed
the
promote_mount_propagation
branch
2 times, most recently
from
3de8bf1 to
731f17d
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
bertinatto
force-pushed
the
promote_mount_propagation
branch
from
731f17d to
b87a57a
Compare
k8s-ci-robot
added
size/XS
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
also updated the release note to indicate the deprecation. feel free to go ahead and open up the PR to remove the gate and the checks and we can merge that as soon as 1.13 opens |
|
cc @tallclair for approval |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bertinatto, jsafrane, liggitt, tallclair 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 |
k8s-ci-robot
added
the
approved
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 65251, 67255, 67224, 67297, 68105). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md. |
What this PR does / why we need it:
This PR promotes mount propagation to GA.
Website PR: kubernetes/website#9823
Release note: