diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index d76a2806f699..7e7677556986 100644 --- a/config/core/configmaps/features.yaml +++ b/config/core/configmaps/features.yaml @@ -23,7 +23,7 @@ metadata: app.kubernetes.io/version: devel serving.knative.dev/release: devel annotations: - knative.dev/example-checksum: "2897f625" + knative.dev/example-checksum: "d9e300ba" data: _example: |- ################################ @@ -149,3 +149,13 @@ data: # 1. Enabled: enabling init containers support # 2. Disabled: disabling init containers support kubernetes.podspec-init-containers: "disabled" + + # Controls whether persistent volume claim support is enabled or not. + # 1. Enabled: enabling persistent volume claim support + # 2. Disabled: disabling persistent volume claim support + kubernetes.podspec-persistent-volume-claim: "disabled" + + # Controls whether write access for persistent volumes is enabled or not. + # 1. Enabled: enabling write access for persistent volumes + # 2. Disabled: disabling write access for persistent volumes + kubernetes.podspec-persistent-volume-write: "disabled" diff --git a/openshift/ci-operator/knative-test-images/emptydir/Dockerfile b/openshift/ci-operator/knative-test-images/volumes/Dockerfile similarity index 59% rename from openshift/ci-operator/knative-test-images/emptydir/Dockerfile rename to openshift/ci-operator/knative-test-images/volumes/Dockerfile index 8875e1b9d4ab..a23a8030f8c9 100644 --- a/openshift/ci-operator/knative-test-images/emptydir/Dockerfile +++ b/openshift/ci-operator/knative-test-images/volumes/Dockerfile @@ -2,5 +2,5 @@ FROM openshift/origin-base USER 65532 -ADD emptydir /ko-app/emptydir -ENTRYPOINT ["/ko-app/emptydir"] +ADD volumes /ko-app/volumes +ENTRYPOINT ["/ko-app/volumes"] \ No newline at end of file diff --git a/openshift/e2e-common.sh b/openshift/e2e-common.sh index 828fe12bdbd2..467706125913 100644 --- a/openshift/e2e-common.sh +++ b/openshift/e2e-common.sh @@ -15,7 +15,7 @@ readonly OLM_NAMESPACE="openshift-marketplace" if [ -n "$OPENSHIFT_BUILD_NAMESPACE" ]; then TEST_IMAGE_TEMPLATE=$(cat <<-END {{- with .Name }} -{{- if eq . "emptydir"}}$KNATIVE_SERVING_TEST_EMPTYDIR{{end -}} +{{- if eq . "volumes"}}$KNATIVE_SERVING_TEST_VOLUMES{{end -}} {{- if eq . "readiness"}}$KNATIVE_SERVING_TEST_READINESS{{end -}} {{- if eq . "pizzaplanetv1"}}$KNATIVE_SERVING_TEST_PIZZAPLANETV1{{end -}} {{- if eq . "pizzaplanetv2"}}$KNATIVE_SERVING_TEST_PIZZAPLANETV2{{end -}} @@ -377,6 +377,22 @@ function run_e2e_tests(){ --skip-cleanup-on-fail \ --resolvabledomain || failed=1 + # Run PVC test + oc -n ${SYSTEM_NAMESPACE} patch knativeserving/knative-serving --type=merge --patch='{"spec": {"config": { "features": {"kubernetes.podspec-persistent-volume-claim": "enabled"}}}}' || fail_test + oc -n ${SYSTEM_NAMESPACE} patch knativeserving/knative-serving --type=merge --patch='{"spec": {"config": { "features": {"kubernetes.podspec-persistent-volume-write": "enabled"}}}}' || fail_test + oc -n ${SYSTEM_NAMESPACE} patch knativeserving/knative-serving --type=merge --patch='{"spec": {"config": { "features": {"kubernetes.podspec-securitycontext": "enabled"}}}}' || fail_test + oc apply -f ./test/config/pvc/pvc.yaml || return $? + go_test_e2e -timeout=5m ./test/e2e/pvc \ + --kubeconfig "$KUBECONFIG" \ + --imagetemplate "$TEST_IMAGE_TEMPLATE" \ + --enable-alpha \ + --https \ + --skip-cleanup-on-fail \ + --resolvabledomain || failed=1 + oc -n ${SYSTEM_NAMESPACE} patch knativeserving/knative-serving --type=merge --patch='{"spec": {"config": { "features": {"kubernetes.podspec-persistent-volume-claim": "disabled"}}}}' || fail_test + oc -n ${SYSTEM_NAMESPACE} patch knativeserving/knative-serving --type=merge --patch='{"spec": {"config": { "features": {"kubernetes.podspec-persistent-volume-write": "disabled"}}}}' || fail_test + oc -n ${SYSTEM_NAMESPACE} patch knativeserving/knative-serving --type=merge --patch='{"spec": {"config": { "features": {"kubernetes.podspec-securitycontext": "disabled"}}}}' || fail_test + # Run the helloworld test with an image pulled into the internal registry. local image_to_tag=$KNATIVE_SERVING_TEST_HELLOWORLD oc tag -n serving-tests "$image_to_tag" "helloworld:latest" --reference-policy=local diff --git a/pkg/apis/config/features.go b/pkg/apis/config/features.go index db4f212d6cd0..0998025e3898 100644 --- a/pkg/apis/config/features.go +++ b/pkg/apis/config/features.go @@ -54,6 +54,8 @@ func defaultFeaturesConfig() *Features { ContainerSpecAddCapabilities: Disabled, PodSpecTolerations: Disabled, PodSpecVolumesEmptyDir: Disabled, + PodSpecPersistentVolumeClaim: Disabled, + PodSpecPersistentVolumeWrite: Disabled, PodSpecInitContainers: Disabled, TagHeaderBasedRouting: Disabled, AutoDetectHTTP2: Disabled, @@ -79,6 +81,8 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) { asFlag("kubernetes.podspec-tolerations", &nc.PodSpecTolerations), asFlag("kubernetes.podspec-volumes-emptydir", &nc.PodSpecVolumesEmptyDir), asFlag("kubernetes.podspec-init-containers", &nc.PodSpecInitContainers), + asFlag("kubernetes.podspec-persistent-volume-claim", &nc.PodSpecPersistentVolumeClaim), + asFlag("kubernetes.podspec-persistent-volume-write", &nc.PodSpecPersistentVolumeWrite), asFlag("tag-header-based-routing", &nc.TagHeaderBasedRouting), asFlag("autodetect-http2", &nc.AutoDetectHTTP2)); err != nil { return nil, err @@ -107,6 +111,8 @@ type Features struct { PodSpecTolerations Flag PodSpecVolumesEmptyDir Flag PodSpecInitContainers Flag + PodSpecPersistentVolumeClaim Flag + PodSpecPersistentVolumeWrite Flag TagHeaderBasedRouting Flag AutoDetectHTTP2 Flag } diff --git a/pkg/apis/config/features_test.go b/pkg/apis/config/features_test.go index e38419f5f78b..16dcee7d584c 100644 --- a/pkg/apis/config/features_test.go +++ b/pkg/apis/config/features_test.go @@ -354,6 +354,42 @@ func TestFeaturesConfiguration(t *testing.T) { data: map[string]string{ "kubernetes.podspec-volumes-emptydir": "Enabled", }, + }, { + name: "kubernetes.podspec-persistent-volume-claim Disabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecPersistentVolumeClaim: Disabled, + }), + data: map[string]string{ + "kubernetes.podspec-persistent-volume-claim": "Disabled", + }, + }, { + name: "kubernetes.podspec-persistent-volume-claim Enabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecPersistentVolumeClaim: Enabled, + }), + data: map[string]string{ + "kubernetes.podspec-persistent-volume-claim": "Enabled", + }, + }, { + name: "kubernetes.podspec-persistent-volume-write Disabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecPersistentVolumeWrite: Disabled, + }), + data: map[string]string{ + "kubernetes.podspec-persistent-volume-write": "Disabled", + }, + }, { + name: "kubernetes.podspec-persistent-volume-claim Enabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecPersistentVolumeWrite: Enabled, + }), + data: map[string]string{ + "kubernetes.podspec-persistent-volume-write": "Enabled", + }, }, { name: "kubernetes.podspec-init-containers Disabled", wantErr: false, diff --git a/pkg/apis/serving/fieldmask.go b/pkg/apis/serving/fieldmask.go index 4ea939b298a4..32723a1f8586 100644 --- a/pkg/apis/serving/fieldmask.go +++ b/pkg/apis/serving/fieldmask.go @@ -45,6 +45,10 @@ func VolumeMask(ctx context.Context, in *corev1.Volume) *corev1.Volume { out.EmptyDir = in.EmptyDir } + if cfg.Features.PodSpecPersistentVolumeClaim != config.Disabled { + out.PersistentVolumeClaim = in.PersistentVolumeClaim + } + return out } @@ -67,6 +71,10 @@ func VolumeSourceMask(ctx context.Context, in *corev1.VolumeSource) *corev1.Volu out.EmptyDir = in.EmptyDir } + if cfg.Features.PodSpecPersistentVolumeClaim != config.Disabled { + out.PersistentVolumeClaim = in.PersistentVolumeClaim + } + // Too many disallowed fields to list return out diff --git a/pkg/apis/serving/k8s_validation.go b/pkg/apis/serving/k8s_validation.go index 608afa78a217..ac36212ad80b 100644 --- a/pkg/apis/serving/k8s_validation.go +++ b/pkg/apis/serving/k8s_validation.go @@ -85,12 +85,7 @@ var ( func ValidateVolumes(ctx context.Context, vs []corev1.Volume, mountedVolumes sets.String) (map[string]corev1.Volume, *apis.FieldError) { volumes := make(map[string]corev1.Volume, len(vs)) var errs *apis.FieldError - features := config.FromContextOrDefaults(ctx).Features for i, volume := range vs { - if volume.EmptyDir != nil && features.PodSpecVolumesEmptyDir != config.Enabled { - errs = errs.Also((&apis.FieldError{Message: fmt.Sprintf("EmptyDir volume support is off, "+ - "but found EmptyDir volume %s", volume.Name)}).ViaIndex(i)) - } if _, ok := volumes[volume.Name]; ok { errs = errs.Also((&apis.FieldError{ Message: fmt.Sprintf("duplicate volume name %q", volume.Name), @@ -109,14 +104,36 @@ func ValidateVolumes(ctx context.Context, vs []corev1.Volume, mountedVolumes set return volumes, errs } +func validatePersistentVolumeClaims(volume corev1.VolumeSource, features *config.Features) *apis.FieldError { + var errs *apis.FieldError + if volume.PersistentVolumeClaim == nil { + return nil + } + if features.PodSpecPersistentVolumeClaim != config.Enabled { + errs = errs.Also(&apis.FieldError{Message: fmt.Sprintf("Persistent volume claim support is disabled, "+ + "but found persistent volume claim %s", volume.PersistentVolumeClaim.ClaimName)}) + } + isWriteEnabled := features.PodSpecPersistentVolumeWrite == config.Enabled + if !volume.PersistentVolumeClaim.ReadOnly && !isWriteEnabled { + errs = errs.Also(&apis.FieldError{Message: fmt.Sprintf("Persistent volume write support is disabled, "+ + "but found persistent volume claim %s that is not read-only", volume.PersistentVolumeClaim.ClaimName)}) + } + return errs +} + func validateVolume(ctx context.Context, volume corev1.Volume) *apis.FieldError { - errs := apis.CheckDisallowedFields(volume, *VolumeMask(ctx, &volume)) + features := config.FromContextOrDefaults(ctx).Features + errs := validatePersistentVolumeClaims(volume.VolumeSource, features) + if volume.EmptyDir != nil && features.PodSpecVolumesEmptyDir != config.Enabled { + errs = errs.Also(&apis.FieldError{Message: fmt.Sprintf("EmptyDir volume support is disabled, "+ + "but found EmptyDir volume %s", volume.Name)}) + } + errs = errs.Also(apis.CheckDisallowedFields(volume, *VolumeMask(ctx, &volume))) if volume.Name == "" { errs = apis.ErrMissingField("name") } else if len(validation.IsDNS1123Label(volume.Name)) != 0 { errs = apis.ErrInvalidValue(volume.Name, "name") } - vs := volume.VolumeSource errs = errs.Also(apis.CheckDisallowedFields(vs, *VolumeSourceMask(ctx, &vs))) var specified []string @@ -142,12 +159,20 @@ func validateVolume(ctx context.Context, volume corev1.Volume) *apis.FieldError specified = append(specified, "emptyDir") errs = errs.Also(validateEmptyDirFields(vs.EmptyDir).ViaField("emptyDir")) } + + if vs.PersistentVolumeClaim != nil { + specified = append(specified, "persistentVolumeClaim") + } + if len(specified) == 0 { fieldPaths := []string{"secret", "configMap", "projected"} cfg := config.FromContextOrDefaults(ctx) if cfg.Features.PodSpecVolumesEmptyDir == config.Enabled { fieldPaths = append(fieldPaths, "emptyDir") } + if cfg.Features.PodSpecPersistentVolumeClaim == config.Enabled { + fieldPaths = append(fieldPaths, "persistentVolumeClaim") + } errs = errs.Also(apis.ErrMissingOneOf(fieldPaths...)) } else if len(specified) > 1 { errs = errs.Also(apis.ErrMultipleOneOf(specified...)) @@ -597,8 +622,21 @@ func validateVolumeMounts(mounts []corev1.VolumeMount, volumes map[string]corev1 } seenMountPath.Insert(filepath.Clean(vm.MountPath)) - if volumes[vm.Name].EmptyDir == nil && !vm.ReadOnly { - errs = errs.Also(apis.ErrMissingField("readOnly").ViaIndex(i)) + shouldCheckReadOnlyVolume := volumes[vm.Name].EmptyDir == nil && volumes[vm.Name].PersistentVolumeClaim == nil + if shouldCheckReadOnlyVolume && !vm.ReadOnly { + errs = errs.Also((&apis.FieldError{ + Message: "volume mount should be readOnly for this type of volume", + Paths: []string{"readOnly"}, + }).ViaIndex(i)) + } + + if volumes[vm.Name].PersistentVolumeClaim != nil { + if volumes[vm.Name].PersistentVolumeClaim.ReadOnly && !vm.ReadOnly { + errs = errs.Also((&apis.FieldError{ + Message: "volume is readOnly but volume mount is not", + Paths: []string{"readOnly"}, + }).ViaIndex(i)) + } } } return errs diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go index e9c1283b54ce..95d884056888 100644 --- a/pkg/apis/serving/k8s_validation_test.go +++ b/pkg/apis/serving/k8s_validation_test.go @@ -113,6 +113,20 @@ func withPodSpecVolumesEmptyDirEnabled() configOption { } } +func withPodSpecPersistentVolumeClaimEnabled() configOption { + return func(cfg *config.Config) *config.Config { + cfg.Features.PodSpecPersistentVolumeClaim = config.Enabled + return cfg + } +} + +func withPodSpecPersistentVolumeWriteEnabled() configOption { + return func(cfg *config.Config) *config.Config { + cfg.Features.PodSpecPersistentVolumeWrite = config.Enabled + return cfg + } +} + func withPodSpecPriorityClassNameEnabled() configOption { return func(cfg *config.Config) *config.Config { cfg.Features.PodSpecPriorityClassName = config.Enabled @@ -417,6 +431,142 @@ func TestPodSpecValidation(t *testing.T) { Message: `volume with name "debugging-support-files" not mounted`, Paths: []string{"volumes[0].name"}, }, + }, { + name: "PVC not read-only, mount read-only, write disabled by default", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + ReadOnly: true, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: false, + }, + }}, + }}, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled()}, + want: &apis.FieldError{ + Message: `Persistent volume write support is disabled, but found persistent volume claim myclaim that is not read-only`, + }, + }, { + name: "PVC not read-only, mount not read-only, write disabled by default", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + ReadOnly: false, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: false, + }, + }}, + }}, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled()}, + want: &apis.FieldError{ + Message: `Persistent volume write support is disabled, but found persistent volume claim myclaim that is not read-only`, + }, + }, { + name: "PVC read-only, mount not read-only, write disabled by default", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + ReadOnly: false, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: true, + }, + }}, + }}, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled()}, + want: &apis.FieldError{ + Message: "volume is readOnly but volume mount is not", + Paths: []string{"containers[0].volumeMounts[0].readOnly"}, + }, + }, { + name: "PVC read-only, write disabled by default", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + ReadOnly: true, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: true, + }, + }}, + }}, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled()}, + }, { + name: "PVC not read-only, write enabled", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + ReadOnly: true, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: false, + }, + }}, + }}, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled()}, + }, { + name: "PVC read-only, write enabled", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + ReadOnly: false, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: false, + }, + }}, + }}, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled()}, }} for _, test := range tests { @@ -1700,7 +1850,10 @@ func getCommonContainerValidationTestCases() []containerValidationTestCase { Message: "volumeMount has no matching volume", Paths: []string{"name"}, }).ViaFieldIndex("volumeMounts", 0).Also( - apis.ErrMissingField("readOnly").ViaFieldIndex("volumeMounts", 0)).Also( + (&apis.FieldError{ + Message: "volume mount should be readOnly for this type of volume", + Paths: []string{"readOnly"}, + }).ViaFieldIndex("volumeMounts", 0)).Also( apis.ErrMissingField("mountPath").ViaFieldIndex("volumeMounts", 0)).Also( apis.ErrDisallowedFields("mountPropagation").ViaFieldIndex("volumeMounts", 0)), }, { @@ -2030,6 +2183,34 @@ func TestVolumeValidation(t *testing.T) { }, want: apis.ErrInvalidValue(-1, "emptyDir.sizeLimit"), cfgOpts: []configOption{withPodSpecVolumesEmptyDirEnabled()}, + }, { + name: "valid PVC with PVC feature enabled", + v: corev1.Volume{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: true, + }, + }, + }, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled()}, + }, { + name: "valid PVC with PVC feature disabled", + v: corev1.Volume{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: false, + }, + }}, + want: (&apis.FieldError{ + Message: `Persistent volume claim support is disabled, but found persistent volume claim myclaim`, + }).Also(&apis.FieldError{ + Message: `Persistent volume write support is disabled, but found persistent volume claim myclaim that is not read-only`, + }).Also( + &apis.FieldError{Message: "must not set the field(s)", Paths: []string{"persistentVolumeClaim"}}), }, { name: "no volume source", v: corev1.Volume{ diff --git a/pkg/apis/serving/v1/revision_defaults.go b/pkg/apis/serving/v1/revision_defaults.go index 88724199f93e..354b12d89d6f 100644 --- a/pkg/apis/serving/v1/revision_defaults.go +++ b/pkg/apis/serving/v1/revision_defaults.go @@ -121,7 +121,7 @@ func (rs *RevisionSpec) applyDefault(ctx context.Context, container *corev1.Cont vNames := make(sets.String) for _, v := range rs.PodSpec.Volumes { - if v.EmptyDir != nil { + if v.EmptyDir != nil || v.PersistentVolumeClaim != nil { vNames.Insert(v.Name) } } diff --git a/pkg/reconciler/revision/table_test.go b/pkg/reconciler/revision/table_test.go index 780acb5cc335..b84631e4bbab 100644 --- a/pkg/reconciler/revision/table_test.go +++ b/pkg/reconciler/revision/table_test.go @@ -655,6 +655,30 @@ func TestReconcile(t *testing.T) { }}, Key: "foo/first-reconcile", Ctx: defaultconfig.ToContext(context.Background(), &defaultconfig.Config{Features: &defaultconfig.Features{PodSpecInitContainers: defaultconfig.Enabled}}), + }, { + Name: "first revision reconciliation with PVC, PVC enabled", + // Test the simplest successful reconciliation flow. + // We feed in a well formed Revision where none of its sub-resources exist, + // and we expect it to create them and initialize the Revision's status. + Objects: []runtime.Object{ + Revision("foo", "first-reconcile", WithRevisionPVC()), + }, + WantCreates: []runtime.Object{ + // The first reconciliation of a Revision creates the following resources. + pa("foo", "first-reconcile"), + deploy(t, "foo", "first-reconcile", WithRevisionPVC()), + image("foo", "first-reconcile")}, + WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ + Object: Revision("foo", "first-reconcile", + // The first reconciliation Populates the following status properties. + WithLogURL, allUnknownConditions, MarkDeploying("Deploying"), + withDefaultContainerStatuses(), WithRevisionObservedGeneration(1), WithRevisionPVC()), + }}, + Key: "foo/first-reconcile", + Ctx: defaultconfig.ToContext(context.Background(), &defaultconfig.Config{Features: &defaultconfig.Features{ + PodSpecPersistentVolumeClaim: defaultconfig.Enabled, + PodSpecPersistentVolumeWrite: defaultconfig.Enabled, + }}), }} table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, _ configmap.Watcher) controller.Reconciler { diff --git a/pkg/reconciler/route/resources/service_test.go b/pkg/reconciler/route/resources/service_test.go index 8c439babd0b0..2996506024fd 100644 --- a/pkg/reconciler/route/resources/service_test.go +++ b/pkg/reconciler/route/resources/service_test.go @@ -421,18 +421,20 @@ func testConfig() *config.Config { TagTemplate: network.DefaultTagTemplate, }, Features: &apiConfig.Features{ - MultiContainer: apiConfig.Disabled, - PodSpecAffinity: apiConfig.Disabled, - PodSpecFieldRef: apiConfig.Disabled, - PodSpecDryRun: apiConfig.Enabled, - PodSpecHostAliases: apiConfig.Disabled, - PodSpecNodeSelector: apiConfig.Disabled, - PodSpecTolerations: apiConfig.Disabled, - PodSpecVolumesEmptyDir: apiConfig.Disabled, - PodSpecInitContainers: apiConfig.Disabled, - PodSpecPriorityClassName: apiConfig.Disabled, - PodSpecSchedulerName: apiConfig.Disabled, - TagHeaderBasedRouting: apiConfig.Disabled, + MultiContainer: apiConfig.Disabled, + PodSpecAffinity: apiConfig.Disabled, + PodSpecFieldRef: apiConfig.Disabled, + PodSpecDryRun: apiConfig.Enabled, + PodSpecHostAliases: apiConfig.Disabled, + PodSpecNodeSelector: apiConfig.Disabled, + PodSpecTolerations: apiConfig.Disabled, + PodSpecVolumesEmptyDir: apiConfig.Disabled, + PodSpecPersistentVolumeClaim: apiConfig.Disabled, + PodSpecPersistentVolumeWrite: apiConfig.Disabled, + PodSpecInitContainers: apiConfig.Disabled, + PodSpecPriorityClassName: apiConfig.Disabled, + PodSpecSchedulerName: apiConfig.Disabled, + TagHeaderBasedRouting: apiConfig.Disabled, }, } } diff --git a/pkg/testing/v1/revision.go b/pkg/testing/v1/revision.go index 3445dc0ab9dc..6a5cdfa82071 100644 --- a/pkg/testing/v1/revision.go +++ b/pkg/testing/v1/revision.go @@ -232,6 +232,22 @@ func WithRevisionInitContainers() RevisionOption { } } +func WithRevisionPVC() RevisionOption { + return func(r *v1.Revision) { + r.Spec.Volumes = []corev1.Volume{{ + Name: "claimvolume", + VolumeSource: corev1.VolumeSource{PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + ReadOnly: false, + }}}, + } + r.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{{ + Name: "claimvolume", + MountPath: "/data", + }} + } +} + // Revision creates a revision object with given ns/name and options. func Revision(namespace, name string, ro ...RevisionOption) *v1.Revision { r := &v1.Revision{ diff --git a/test/config/pvc/pvc.yaml b/test/config/pvc/pvc.yaml new file mode 100644 index 000000000000..ff8c6d20d703 --- /dev/null +++ b/test/config/pvc/pvc.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: knative-pv-claim + namespace: serving-tests +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi diff --git a/test/conformance.go b/test/conformance.go index 57782de5d500..526d57e61f76 100644 --- a/test/conformance.go +++ b/test/conformance.go @@ -36,7 +36,6 @@ import ( const ( // Test image names Autoscale = "autoscale" - EmptyDir = "emptydir" Failing = "failing" GRPCPing = "grpc-ping" HelloHTTP2 = "hellohttp2" @@ -53,6 +52,7 @@ const ( SidecarContainer = "sidecarcontainer" SingleThreadedImage = "singlethreaded" Timeout = "timeout" + Volumes = "volumes" WorkingDir = "workingdir" // Constants for test image output. diff --git a/test/conformance/api/v1/service_test.go b/test/conformance/api/v1/service_test.go index 90bb0089f458..1192280c6137 100644 --- a/test/conformance/api/v1/service_test.go +++ b/test/conformance/api/v1/service_test.go @@ -744,7 +744,7 @@ func TestServiceCreateWithEmptyDir(t *testing.T) { names := test.ResourceNames{ Service: test.ObjectNameForTest(t), - Image: test.EmptyDir, + Image: test.Volumes, } quantity := resource.MustParse("100M") diff --git a/test/e2e-common.sh b/test/e2e-common.sh index 514814fb547d..65c27cf70840 100644 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -55,6 +55,8 @@ export SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-$(uuidgen | tr 'A-Z' 'a-z')}" readonly REPLICAS=3 readonly BUCKETS=10 +export PVC=${PVC:-1} + # Receives the latest serving version and searches for the same version with major and minor and searches for the latest patch function latest_net_istio_version() { local serving_version=$1 @@ -286,6 +288,10 @@ function install() { YTT_FILES+=("${REPO_ROOT_DIR}/test/config/ytt/kind/ingress/${ingress}-kind.yaml") fi + if (( PVC )); then + YTT_FILES+=("${REPO_ROOT_DIR}/test/config/pvc/pvc.yaml") + fi + local ytt_result=$(mktemp) local ytt_post_install_result=$(mktemp) local ytt_flags="" diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 2425a36225a7..fc92f3c295b4 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -125,6 +125,15 @@ go_test_e2e -timeout=2m ./test/e2e/initcontainers ${TEST_OPTIONS} || failed=1 toggle_feature kubernetes.podspec-init-containers Disabled toggle_feature kubernetes.podspec-volumes-emptydir Disabled +# RUN PVC tests with default storage class. +toggle_feature kubernetes.podspec-persistent-volume-claim Enabled +toggle_feature kubernetes.podspec-persistent-volume-write Enabled +toggle_feature kubernetes.podspec-securitycontext Enabled +go_test_e2e -timeout=5m ./test/e2e/pvc ${TEST_OPTIONS} || failed=1 +toggle_feature kubernetes.podspec-securitycontext Disabled +toggle_feature kubernetes.podspec-persistent-volume-write Disabled +toggle_feature kubernetes.podspec-persistent-volume-claim Disabled + # Run HA tests separately as they're stopping core Knative Serving pods. # Define short -spoofinterval to ensure frequent probing while stopping pods. toggle_feature autocreateClusterDomainClaims true config-network || fail_test diff --git a/test/e2e-upgrade-tests.sh b/test/e2e-upgrade-tests.sh index b1f08d2d4921..c30014c3f5f7 100755 --- a/test/e2e-upgrade-tests.sh +++ b/test/e2e-upgrade-tests.sh @@ -42,8 +42,9 @@ function stage_test_resources() { # Skip installing istio as an add-on. # Temporarily increasing the cluster size for serving tests to rule out # resource/eviction as causes of flakiness. -# Pin to 1.20 since scale test is super flakey on 1.21 -initialize "$@" --skip-istio-addon --min-nodes=4 --max-nodes=4 --cluster-version=1.20 \ +# Pin to 1.20 since scale test is super flaky on 1.21 +# Skip installing a pvc as it is not used in upgrade tests +PVC=0 initialize "$@" --skip-istio-addon --min-nodes=4 --max-nodes=4 --cluster-version=1.21 \ --install-latest-release # TODO(#2656): Reduce the timeout after we get this test to consistently passing. diff --git a/test/e2e/initcontainers/initcontainers_test.go b/test/e2e/initcontainers/initcontainers_test.go index cdb92ffa15ff..1d392d9890e3 100644 --- a/test/e2e/initcontainers/initcontainers_test.go +++ b/test/e2e/initcontainers/initcontainers_test.go @@ -44,7 +44,7 @@ func TestInitContainers(t *testing.T) { names := test.ResourceNames{ Service: test.ObjectNameForTest(t), - Image: test.EmptyDir, + Image: test.Volumes, } test.EnsureTearDown(t, clients, &names) @@ -65,7 +65,7 @@ func TestInitContainers(t *testing.T) { withInitContainer := WithInitContainer(corev1.Container{ Name: "initsetup", - Image: pkgTest.ImagePath(test.EmptyDir), + Image: pkgTest.ImagePath(test.Volumes), VolumeMounts: []corev1.VolumeMount{{ Name: "data", MountPath: "/data", diff --git a/test/e2e/pvc/pvc_test.go b/test/e2e/pvc/pvc_test.go new file mode 100644 index 000000000000..7708fd45223e --- /dev/null +++ b/test/e2e/pvc/pvc_test.go @@ -0,0 +1,93 @@ +//go:build e2e +// +build e2e + +/* +Copyright 2022 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pvc + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/ptr" + pkgTest "knative.dev/pkg/test" + "knative.dev/pkg/test/spoof" + v1 "knative.dev/serving/pkg/apis/serving/v1" + . "knative.dev/serving/pkg/testing/v1" + "knative.dev/serving/test" + v1test "knative.dev/serving/test/v1" +) + +const ( + unprivilegedUserID = 65532 +) + +// TestPersistentVolumeClaims tests pvc support. +func TestPersistentVolumeClaims(t *testing.T) { + if !test.ServingFlags.EnableAlphaFeatures { + t.Skip("Alpha features not enabled") + } + t.Parallel() + clients := test.Setup(t) + + names := test.ResourceNames{ + Service: test.ObjectNameForTest(t), + Image: test.Volumes, + } + + test.EnsureTearDown(t, clients, &names) + + t.Log("Creating a new Service") + + withVolume := WithVolume("data", "/data", corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "knative-pv-claim", + ReadOnly: false, + }, + }) + + // make sure default user can access the written file + withPodSecurityContext := WithPodSecurityContext(corev1.PodSecurityContext{ + FSGroup: ptr.Int64(unprivilegedUserID), + }) + + resources, err := v1test.CreateServiceReady(t, clients, &names, withVolume, withPodSecurityContext) + if err != nil { + t.Fatalf("Failed to create initial Service: %v: %v", names.Service, err) + } + + url := resources.Route.Status.URL.URL() + if _, err := pkgTest.CheckEndpointState( + context.Background(), + clients.KubeClient, + t.Logf, + url, + spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.EmptyDirText)), + "PVCText", + test.ServingFlags.ResolvableDomain, + test.AddRootCAtoTransport(context.Background(), t.Logf, clients, test.ServingFlags.HTTPS), + ); err != nil { + t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.EmptyDirText, err) + } +} + +func WithPodSecurityContext(secCtx corev1.PodSecurityContext) ServiceOption { + return func(s *v1.Service) { + s.Spec.Template.Spec.SecurityContext = &secCtx + } +} diff --git a/test/test_images/initcontainers/service.yaml b/test/test_images/initcontainers/service.yaml index 20553c84534b..d89a4c347db9 100644 --- a/test/test_images/initcontainers/service.yaml +++ b/test/test_images/initcontainers/service.yaml @@ -22,7 +22,7 @@ spec: spec: containers: - imagePullPolicy: Always - image: ko://knative.dev/serving/test/test_images/emptydir + image: ko://knative.dev/serving/test/test_images/volumes volumeMounts: - name: data mountPath: /data @@ -31,7 +31,7 @@ spec: value: /data initContainers: - imagePullPolicy: Always - image: ko://knative.dev/serving/test/test_images/emptydir + image: ko://knative.dev/serving/test/test_images/volumes volumeMounts: - name: data mountPath: /data diff --git a/test/test_images/emptydir/emptydir.go b/test/test_images/volumes/emptydir.go similarity index 100% rename from test/test_images/emptydir/emptydir.go rename to test/test_images/volumes/emptydir.go diff --git a/test/test_images/emptydir/service.yaml b/test/test_images/volumes/service-empty-dir.yaml similarity index 93% rename from test/test_images/emptydir/service.yaml rename to test/test_images/volumes/service-empty-dir.yaml index 6e351a779262..849d09963b1e 100644 --- a/test/test_images/emptydir/service.yaml +++ b/test/test_images/volumes/service-empty-dir.yaml @@ -22,7 +22,7 @@ spec: spec: containers: - imagePullPolicy: Always - image: ko://knative.dev/serving/test/test_images/emptydir + image: ko://knative.dev/serving/test/test_images/volumes volumeMounts: - name: data mountPath: /data diff --git a/test/test_images/volumes/service-pvc.yaml b/test/test_images/volumes/service-pvc.yaml new file mode 100644 index 000000000000..345f8f3177ee --- /dev/null +++ b/test/test_images/volumes/service-pvc.yaml @@ -0,0 +1,47 @@ +# Copyright 2022 The Knative Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: serving.knative.dev/v1 +kind: Service +metadata: + name: event-display +spec: + template: + spec: + containers: + - image: ko://knative.dev/serving/test/test_images/volumes + name: event-display + volumeMounts: + - mountPath: /data + name: mydata + readOnly: false + env: + - name: DATA_PATH + value: /data + volumes: + - name: mydata + persistentVolumeClaim: + claimName: knative-pv-claim + readOnly: false +--- +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: knative-pv-claim +spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1Gi