Skip to content

Commit 100ade5

Browse files
elijah-rouelijah-rou
committed
enable mount prop
run update schema add units remove useless val tests
1 parent 6265a8e commit 100ade5

File tree

12 files changed

+86
-22
lines changed

12 files changed

+86
-22
lines changed

‎cmd/schema-tweak/overrides.go

+4
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ func revSpecOverrides(prefixPath string) []entry {
227227
"mountPath",
228228
"subPath",
229229
),
230+
featureFlagFields: []flagField{{
231+
name: "mountPropagation",
232+
flag: config.FeaturePodSpecMountPropagation,
233+
}},
230234
}, {
231235
path: "volumes",
232236
allowedFields: sets.New(

‎config/core/300-resources/configuration.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,10 @@ spec:
926926
Path within the container at which the volume should be mounted. Must
927927
not contain ':'.
928928
type: string
929+
mountPropagation:
930+
description: |-
931+
This is accessible behind a feature flag - kubernetes.podspec-mount-propagation
932+
type: string
929933
name:
930934
description: This must match the Name of a Volume.
931935
type: string

‎config/core/300-resources/revision.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,10 @@ spec:
902902
Path within the container at which the volume should be mounted. Must
903903
not contain ':'.
904904
type: string
905+
mountPropagation:
906+
description: |-
907+
This is accessible behind a feature flag - kubernetes.podspec-mount-propagation
908+
type: string
905909
name:
906910
description: This must match the Name of a Volume.
907911
type: string

‎config/core/300-resources/service.yaml

+4
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,10 @@ spec:
944944
Path within the container at which the volume should be mounted. Must
945945
not contain ':'.
946946
type: string
947+
mountPropagation:
948+
description: |-
949+
This is accessible behind a feature flag - kubernetes.podspec-mount-propagation
950+
type: string
947951
name:
948952
description: This must match the Name of a Volume.
949953
type: string

‎config/core/configmaps/features.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,11 @@ data:
223223
# 2. Disabled: disabling write access for persistent volumes
224224
kubernetes.podspec-persistent-volume-write: "disabled"
225225
226+
# Controls whether volume mount propagation support is enabled or not.
227+
# 1. Enabled: enabling volume mount propagation support
228+
# 2. Disabled: disabling volume mount propagation support
229+
kubernetes.podspec-mount-propagation: "disabled"
230+
226231
# Controls if the queue proxy podInfo feature is enabled, allowed or disabled
227232
#
228233
# This feature should be enabled/allowed when using queue proxy Options (Extensions)

‎pkg/apis/config/features.go

+4
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ const (
6666
FeaturePodSpecHostPID = "kubernetes.podspec-hostpid"
6767
FeaturePodSpecHostPath = "kubernetes.podspec-volumes-hostpath"
6868
FeaturePodSpecInitContainers = "kubernetes.podspec-init-containers"
69+
FeaturePodSpecMountPropagation = "kubernetes.podspec-mount-propagation"
6970
FeaturePodSpecNodeSelector = "kubernetes.podspec-nodeselector"
7071
FeaturePodSpecPVClaim = "kubernetes.podspec-persistent-volume-claim"
7172
FeaturePodSpecPriorityClassName = "kubernetes.podspec-priorityclassname"
@@ -99,6 +100,7 @@ func defaultFeaturesConfig() *Features {
99100
PodSpecTolerations: Disabled,
100101
PodSpecVolumesEmptyDir: Enabled,
101102
PodSpecVolumesHostPath: Disabled,
103+
PodSpecVolumeMountPropagation: Disabled,
102104
PodSpecPersistentVolumeClaim: Disabled,
103105
PodSpecPersistentVolumeWrite: Disabled,
104106
QueueProxyMountPodInfo: Disabled,
@@ -139,6 +141,7 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {
139141
asFlag(FeaturePodSpecHostPID, &nc.PodSpecHostPID),
140142
asFlag(FeaturePodSpecHostPath, &nc.PodSpecVolumesHostPath),
141143
asFlag(FeaturePodSpecInitContainers, &nc.PodSpecInitContainers),
144+
asFlag(FeaturePodSpecMountPropagation, &nc.PodSpecVolumeMountPropagation),
142145
asFlag(FeaturePodSpecNodeSelector, &nc.PodSpecNodeSelector),
143146
asFlag(FeaturePodSpecPVClaim, &nc.PodSpecPersistentVolumeClaim),
144147
asFlag(FeaturePodSpecPriorityClassName, &nc.PodSpecPriorityClassName),
@@ -181,6 +184,7 @@ type Features struct {
181184
PodSpecTolerations Flag
182185
PodSpecVolumesEmptyDir Flag
183186
PodSpecVolumesHostPath Flag
187+
PodSpecVolumeMountPropagation Flag
184188
PodSpecInitContainers Flag
185189
PodSpecPersistentVolumeClaim Flag
186190
PodSpecPersistentVolumeWrite Flag

‎pkg/apis/config/features_test.go

+18
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,24 @@ func TestFeaturesConfiguration(t *testing.T) {
473473
data: map[string]string{
474474
"kubernetes.podspec-persistent-volume-claim": "Enabled",
475475
},
476+
}, {
477+
name: "kubernetes.podspec-mount-propagation Disabled",
478+
wantErr: false,
479+
wantFeatures: defaultWith(&Features{
480+
PodSpecVolumeMountPropagation: Disabled,
481+
}),
482+
data: map[string]string{
483+
"kubernetes.podspec-mount-propagation": "Disabled",
484+
},
485+
}, {
486+
name: "kubernetes.podspec-mount-propagation Enabled",
487+
wantErr: false,
488+
wantFeatures: defaultWith(&Features{
489+
PodSpecVolumeMountPropagation: Enabled,
490+
}),
491+
data: map[string]string{
492+
"kubernetes.podspec-mount-propagation": "Enabled",
493+
},
476494
}, {
477495
name: "kubernetes.podspec-persistent-volume-write Disabled",
478496
wantErr: false,

‎pkg/apis/serving/fieldmask.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -333,22 +333,26 @@ func ContainerMask(in *corev1.Container) *corev1.Container {
333333
// VolumeMountMask performs a _shallow_ copy of the Kubernetes VolumeMount object to a new
334334
// Kubernetes VolumeMount object bringing over only the fields allowed in the Knative API. This
335335
// does not validate the contents or the bounds of the provided fields.
336-
func VolumeMountMask(in *corev1.VolumeMount) *corev1.VolumeMount {
336+
func VolumeMountMask(ctx context.Context, in *corev1.VolumeMount) *corev1.VolumeMount {
337337
if in == nil {
338338
return nil
339339
}
340340

341+
cfg := config.FromContextOrDefaults(ctx)
341342
out := new(corev1.VolumeMount)
342343

343344
// Allowed fields
344345
out.Name = in.Name
345346
out.ReadOnly = in.ReadOnly
346347
out.MountPath = in.MountPath
347348
out.SubPath = in.SubPath
349+
if cfg.Features.PodSpecVolumeMountPropagation != config.Disabled {
350+
out.MountPropagation = in.MountPropagation
351+
}
348352

349353
// Disallowed fields
350354
// This list is unnecessary, but added here for clarity
351-
out.MountPropagation = nil
355+
out.RecursiveReadOnly = nil
352356

353357
return out
354358
}

‎pkg/apis/serving/fieldmask_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ func TestVolumeMountMask(t *testing.T) {
331331
MountPropagation: &mode,
332332
}
333333

334-
got := VolumeMountMask(in)
334+
got := VolumeMountMask(nil, in)
335335

336336
if &want == &got {
337337
t.Error("Input and output share addresses. Want different addresses")
@@ -343,7 +343,7 @@ func TestVolumeMountMask(t *testing.T) {
343343
t.Error("VolumeMountMask (-want, +got):", diff)
344344
}
345345

346-
if got = VolumeMountMask(nil); got != nil {
346+
if got = VolumeMountMask(nil, nil); got != nil {
347347
t.Errorf("VolumeMountMask(nil) = %v, want: nil", got)
348348
}
349349
}

‎pkg/apis/serving/k8s_validation.go

+12-3
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ func validate(ctx context.Context, container corev1.Container, volumes map[strin
616616
errs = errs.Also(apis.ErrInvalidValue(container.TerminationMessagePolicy, "terminationMessagePolicy"))
617617
}
618618
// VolumeMounts
619-
errs = errs.Also(validateVolumeMounts(container.VolumeMounts, volumes).ViaField("volumeMounts"))
619+
errs = errs.Also(validateVolumeMounts(ctx, container.VolumeMounts, volumes).ViaField("volumeMounts"))
620620

621621
return errs
622622
}
@@ -659,15 +659,15 @@ func validateSecurityContext(ctx context.Context, sc *corev1.SecurityContext) *a
659659
return errs
660660
}
661661

662-
func validateVolumeMounts(mounts []corev1.VolumeMount, volumes map[string]corev1.Volume) *apis.FieldError {
662+
func validateVolumeMounts(ctx context.Context, mounts []corev1.VolumeMount, volumes map[string]corev1.Volume) *apis.FieldError {
663663
var errs *apis.FieldError
664664
// Check that volume mounts match names in "volumes", that "volumes" has 100%
665665
// coverage, and the field restrictions.
666666
seenName := make(sets.Set[string], len(mounts))
667667
seenMountPath := make(sets.Set[string], len(mounts))
668668
for i := range mounts {
669669
vm := mounts[i]
670-
errs = errs.Also(apis.CheckDisallowedFields(vm, *VolumeMountMask(&vm)).ViaIndex(i))
670+
errs = errs.Also(apis.CheckDisallowedFields(vm, *VolumeMountMask(ctx, &vm)).ViaIndex(i))
671671
// This effectively checks that Name is non-empty because Volume name must be non-empty.
672672
if _, ok := volumes[vm.Name]; !ok {
673673
errs = errs.Also((&apis.FieldError{
@@ -700,6 +700,15 @@ func validateVolumeMounts(mounts []corev1.VolumeMount, volumes map[string]corev1
700700
}).ViaIndex(i))
701701
}
702702

703+
if vm.MountPropagation != nil {
704+
if *vm.MountPropagation != corev1.MountPropagationNone || *vm.MountPropagation != corev1.MountPropagationHostToContainer || *vm.MountPropagation != corev1.MountPropagationBidirectional {
705+
errs = errs.Also((&apis.FieldError{
706+
Message: "mount propagation should be set to None, HostToContainer or Bidirectional",
707+
Paths: []string{"mountPropagation"},
708+
}).ViaIndex(i))
709+
}
710+
}
711+
703712
if volumes[vm.Name].PersistentVolumeClaim != nil {
704713
if volumes[vm.Name].PersistentVolumeClaim.ReadOnly && !vm.ReadOnly {
705714
errs = errs.Also((&apis.FieldError{

‎pkg/apis/serving/k8s_validation_test.go

+7
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ func withPodSpecPersistentVolumeWriteEnabled() configOption {
143143
}
144144
}
145145

146+
func withPodSpecMountPropagationEnabled() configOption {
147+
return func(cfg *config.Config) *config.Config {
148+
cfg.Features.PodSpecVolumeMountPropagation = config.Enabled
149+
return cfg
150+
}
151+
}
152+
146153
func withPodSpecPriorityClassNameEnabled() configOption {
147154
return func(cfg *config.Config) *config.Config {
148155
cfg.Features.PodSpecPriorityClassName = config.Enabled

‎pkg/reconciler/route/resources/service_test.go

+16-15
Original file line numberDiff line numberDiff line change
@@ -426,21 +426,22 @@ func testConfig() *config.Config {
426426
SystemInternalTLS: netcfg.EncryptionDisabled,
427427
},
428428
Features: &apiConfig.Features{
429-
MultiContainer: apiConfig.Disabled,
430-
PodSpecAffinity: apiConfig.Disabled,
431-
PodSpecFieldRef: apiConfig.Disabled,
432-
PodSpecDryRun: apiConfig.Enabled,
433-
PodSpecHostAliases: apiConfig.Disabled,
434-
PodSpecNodeSelector: apiConfig.Disabled,
435-
PodSpecTolerations: apiConfig.Disabled,
436-
PodSpecVolumesEmptyDir: apiConfig.Disabled,
437-
PodSpecVolumesHostPath: apiConfig.Disabled,
438-
PodSpecPersistentVolumeClaim: apiConfig.Disabled,
439-
PodSpecPersistentVolumeWrite: apiConfig.Disabled,
440-
PodSpecInitContainers: apiConfig.Disabled,
441-
PodSpecPriorityClassName: apiConfig.Disabled,
442-
PodSpecSchedulerName: apiConfig.Disabled,
443-
TagHeaderBasedRouting: apiConfig.Disabled,
429+
MultiContainer: apiConfig.Disabled,
430+
PodSpecAffinity: apiConfig.Disabled,
431+
PodSpecFieldRef: apiConfig.Disabled,
432+
PodSpecDryRun: apiConfig.Enabled,
433+
PodSpecHostAliases: apiConfig.Disabled,
434+
PodSpecNodeSelector: apiConfig.Disabled,
435+
PodSpecTolerations: apiConfig.Disabled,
436+
PodSpecVolumesEmptyDir: apiConfig.Disabled,
437+
PodSpecVolumesHostPath: apiConfig.Disabled,
438+
PodSpecPersistentVolumeClaim: apiConfig.Disabled,
439+
PodSpecPersistentVolumeWrite: apiConfig.Disabled,
440+
PodSpecVolumeMountPropagation: apiConfig.Disabled,
441+
PodSpecInitContainers: apiConfig.Disabled,
442+
PodSpecPriorityClassName: apiConfig.Disabled,
443+
PodSpecSchedulerName: apiConfig.Disabled,
444+
TagHeaderBasedRouting: apiConfig.Disabled,
444445
},
445446
}
446447
}

0 commit comments

Comments
 (0)