Skip to content

Commit

Permalink
Add support for emptyDir volume type (knative#11669)
Browse files Browse the repository at this point in the history
* support emptyDir volume

* fixes

* reorder feature
  • Loading branch information
skonto authored and markusthoemmes committed Aug 23, 2021
1 parent 45b6853 commit 3e31166
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 30 deletions.
1 change: 1 addition & 0 deletions config/core/300-resources/configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ spec:
secretName:
description: 'Name of the secret in the pod''s namespace to use. More info: https://kubernetes.io/docs/concepts/storage/volumes#secret'
type: string
x-kubernetes-preserve-unknown-fields: true
x-kubernetes-preserve-unknown-fields: true
status:
description: ConfigurationStatus communicates the observed state of the Configuration (from the controller).
Expand Down
1 change: 1 addition & 0 deletions config/core/300-resources/revision.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,7 @@ spec:
secretName:
description: 'Name of the secret in the pod''s namespace to use. More info: https://kubernetes.io/docs/concepts/storage/volumes#secret'
type: string
x-kubernetes-preserve-unknown-fields: true
x-kubernetes-preserve-unknown-fields: true
status:
description: RevisionStatus communicates the observed state of the Revision (from the controller).
Expand Down
1 change: 1 addition & 0 deletions config/core/300-resources/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,7 @@ spec:
secretName:
description: 'Name of the secret in the pod''s namespace to use. More info: https://kubernetes.io/docs/concepts/storage/volumes#secret'
type: string
x-kubernetes-preserve-unknown-fields: true
x-kubernetes-preserve-unknown-fields: true
traffic:
description: Traffic specifies how to distribute traffic over a collection of revisions and configurations.
Expand Down
7 changes: 6 additions & 1 deletion config/core/configmaps/features.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ metadata:
labels:
serving.knative.dev/release: devel
annotations:
knative.dev/example-checksum: "8c1f8302"
knative.dev/example-checksum: "3bac317a"
data:
_example: |-
################################
Expand Down Expand Up @@ -131,3 +131,8 @@ data:
# 1. Enabled: http2 connection will be attempted via upgrade.
# 2. Disabled: http2 connection will only be attempted when port name is set to "h2c".
autodetect-http2: "disabled"
# Controls whether volume support for EmptyDir is enabled or not.
# 1. Enabled: enabling EmptyDir volume support
# 2. Disabled: disabling EmptyDir volume support
kubernetes.podspec-volumes-emptydir: "disabled"
1 change: 1 addition & 0 deletions hack/schemapatch-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ k8s.io/api/core/v1.Volume:
- Name
- VolumeSource
k8s.io/api/core/v1.VolumeSource:
preserveUnknownFields: true # for feature flagged fields
allowedFields:
- Secret
- ConfigMap
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/config/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func defaultFeaturesConfig() *Features {
PodSpecSecurityContext: Disabled,
ContainerSpecAddCapabilities: Disabled,
PodSpecTolerations: Disabled,
PodSpecVolumesEmptyDir: Disabled,
TagHeaderBasedRouting: Disabled,
AutoDetectHTTP2: Disabled,
}
Expand All @@ -71,6 +72,7 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {
asFlag("kubernetes.podspec-securitycontext", &nc.PodSpecSecurityContext),
asFlag("kubernetes.containerspec-addcapabilities", &nc.ContainerSpecAddCapabilities),
asFlag("kubernetes.podspec-tolerations", &nc.PodSpecTolerations),
asFlag("kubernetes.podspec-volumes-emptydir", &nc.PodSpecVolumesEmptyDir),
asFlag("tag-header-based-routing", &nc.TagHeaderBasedRouting),
asFlag("autodetect-http2", &nc.AutoDetectHTTP2)); err != nil {
return nil, err
Expand All @@ -95,6 +97,7 @@ type Features struct {
PodSpecSecurityContext Flag
ContainerSpecAddCapabilities Flag
PodSpecTolerations Flag
PodSpecVolumesEmptyDir Flag
TagHeaderBasedRouting Flag
AutoDetectHTTP2 Flag
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/apis/config/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,24 @@ func TestFeaturesConfiguration(t *testing.T) {
data: map[string]string{
"tag-header-based-routing": "Enabled",
},
}, {
name: "kubernetes.podspec-volumes-emptyDir Disabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecVolumesEmptyDir: Disabled,
}),
data: map[string]string{
"kubernetes.podspec-volumes-emptydir": "Disabled",
},
}, {
name: "kubernetes.podspec-volumes-emptyDir Enabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecVolumesEmptyDir: Enabled,
}),
data: map[string]string{
"kubernetes.podspec-volumes-emptydir": "Enabled",
},
}}

for _, tt := range configTests {
Expand Down
15 changes: 12 additions & 3 deletions pkg/apis/serving/fieldmask.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,44 @@ import (
// VolumeMask performs a _shallow_ copy of the Kubernetes Volume object to a new
// Kubernetes Volume object bringing over only the fields allowed in the Knative API. This
// does not validate the contents or the bounds of the provided fields.
func VolumeMask(in *corev1.Volume) *corev1.Volume {
func VolumeMask(ctx context.Context, in *corev1.Volume) *corev1.Volume {
if in == nil {
return nil
}
cfg := config.FromContextOrDefaults(ctx)

out := new(corev1.Volume)

// Allowed fields
out.Name = in.Name
out.VolumeSource = in.VolumeSource

if cfg.Features.PodSpecVolumesEmptyDir != config.Disabled {
out.EmptyDir = in.EmptyDir
}

return out
}

// VolumeSourceMask performs a _shallow_ copy of the Kubernetes VolumeSource object to a new
// Kubernetes VolumeSource object bringing over only the fields allowed in the Knative API. This
// does not validate the contents or the bounds of the provided fields.
func VolumeSourceMask(in *corev1.VolumeSource) *corev1.VolumeSource {
func VolumeSourceMask(ctx context.Context, in *corev1.VolumeSource) *corev1.VolumeSource {
if in == nil {
return nil
}

cfg := config.FromContextOrDefaults(ctx)
out := new(corev1.VolumeSource)

// Allowed fields
out.Secret = in.Secret
out.ConfigMap = in.ConfigMap
out.Projected = in.Projected

if cfg.Features.PodSpecVolumesEmptyDir != config.Disabled {
out.EmptyDir = in.EmptyDir
}

// Too many disallowed fields to list

return out
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/serving/fieldmask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestVolumeMask(t *testing.T) {
}
in := want

got := VolumeMask(in)
got := VolumeMask(context.Background(), in)

if &want == &got {
t.Error("Input and output share addresses. Want different addresses")
Expand All @@ -46,7 +46,7 @@ func TestVolumeMask(t *testing.T) {
t.Error("VolumeMask (-want, +got):", diff)
}

if got = VolumeMask(nil); got != nil {
if got = VolumeMask(context.Background(), nil); got != nil {
t.Errorf("VolumeMask(nil) = %v, want: nil", got)
}
}
Expand All @@ -62,7 +62,7 @@ func TestVolumeSourceMask(t *testing.T) {
NFS: &corev1.NFSVolumeSource{},
}

got := VolumeSourceMask(in)
got := VolumeSourceMask(context.Background(), in)

if &want == &got {
t.Error("Input and output share addresses. Want different addresses")
Expand All @@ -74,7 +74,7 @@ func TestVolumeSourceMask(t *testing.T) {
t.Error("VolumeSourceMask (-want, +got):", diff)
}

if got = VolumeSourceMask(nil); got != nil {
if got = VolumeSourceMask(context.Background(), nil); got != nil {
t.Errorf("VolumeSourceMask(nil) = %v, want: nil", got)
}
}
Expand Down
41 changes: 34 additions & 7 deletions pkg/apis/serving/k8s_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,15 @@ var (
)

// ValidateVolumes validates the Volumes of a PodSpec.
func ValidateVolumes(vs []corev1.Volume, mountedVolumes sets.String) (sets.String, *apis.FieldError) {
func ValidateVolumes(ctx context.Context, vs []corev1.Volume, mountedVolumes sets.String) (sets.String, *apis.FieldError) {
volumes := make(sets.String, 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 volumes.Has(volume.Name) {
errs = errs.Also((&apis.FieldError{
Message: fmt.Sprintf("duplicate volume name %q", volume.Name),
Expand All @@ -98,22 +103,22 @@ func ValidateVolumes(vs []corev1.Volume, mountedVolumes sets.String) (sets.Strin
Paths: []string{"name"},
}).ViaIndex(i))
}
errs = errs.Also(validateVolume(volume).ViaIndex(i))
errs = errs.Also(validateVolume(ctx, volume).ViaIndex(i))
volumes.Insert(volume.Name)
}
return volumes, errs
}

func validateVolume(volume corev1.Volume) *apis.FieldError {
errs := apis.CheckDisallowedFields(volume, *VolumeMask(&volume))
func validateVolume(ctx context.Context, volume corev1.Volume) *apis.FieldError {
errs := 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(&vs)))
errs = errs.Also(apis.CheckDisallowedFields(vs, *VolumeSourceMask(ctx, &vs)))
var specified []string
if vs.Secret != nil {
specified = append(specified, "secret")
Expand All @@ -133,8 +138,17 @@ func validateVolume(volume corev1.Volume) *apis.FieldError {
errs = errs.Also(validateProjectedVolumeSource(proj).ViaFieldIndex("projected", i))
}
}
if vs.EmptyDir != nil {
specified = append(specified, "emptyDir")
errs = errs.Also(validateEmptyDirFields(vs.EmptyDir).ViaField("emptyDir"))
}
if len(specified) == 0 {
errs = errs.Also(apis.ErrMissingOneOf("secret", "configMap", "projected"))
fieldPaths := []string{"secret", "configMap", "projected"}
cfg := config.FromContextOrDefaults(ctx)
if cfg.Features.PodSpecVolumesEmptyDir == config.Enabled {
fieldPaths = append(fieldPaths, "emptyDir")
}
errs = errs.Also(apis.ErrMissingOneOf(fieldPaths...))
} else if len(specified) > 1 {
errs = errs.Also(apis.ErrMultipleOneOf(specified...))
}
Expand Down Expand Up @@ -211,6 +225,19 @@ func validateKeyToPath(k2p corev1.KeyToPath) *apis.FieldError {
return errs
}

func validateEmptyDirFields(dir *corev1.EmptyDirVolumeSource) *apis.FieldError {
var errs *apis.FieldError
if dir.Medium != "" && dir.Medium != "Memory" {
errs = errs.Also(apis.ErrInvalidValue(dir.Medium, "medium"))
}
if dir.SizeLimit != nil {
if dir.SizeLimit.Value() < 0 {
errs = errs.Also(apis.ErrInvalidValue(dir.SizeLimit, "sizeLimit"))
}
}
return errs
}

func validateEnvValueFrom(ctx context.Context, source *corev1.EnvVarSource) *apis.FieldError {
if source == nil {
return nil
Expand Down Expand Up @@ -289,7 +316,7 @@ func ValidatePodSpec(ctx context.Context, ps corev1.PodSpec) *apis.FieldError {

errs = errs.Also(ValidatePodSecurityContext(ctx, ps.SecurityContext).ViaField("securityContext"))

volumes, err := ValidateVolumes(ps.Volumes, AllMountedVolumes(ps.Containers))
volumes, err := ValidateVolumes(ctx, ps.Volumes, AllMountedVolumes(ps.Containers))
if err != nil {
errs = errs.Also(err.ViaField("volumes"))
}
Expand Down
45 changes: 38 additions & 7 deletions pkg/apis/serving/k8s_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ func withContainerSpecAddCapabilitiesEnabled() configOption {
}
}

func withPodSpecVolumesEmptyDirEnabled() configOption {
return func(cfg *config.Config) *config.Config {
cfg.Features.PodSpecVolumesEmptyDir = config.Enabled
return cfg
}
}

func TestPodSpecValidation(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -1512,9 +1519,10 @@ func TestContainerValidation(t *testing.T) {

func TestVolumeValidation(t *testing.T) {
tests := []struct {
name string
v corev1.Volume
want *apis.FieldError
name string
v corev1.Volume
want *apis.FieldError
cfgOpts []configOption
}{{
name: "just name",
v: corev1.Volume{
Expand Down Expand Up @@ -1546,11 +1554,26 @@ func TestVolumeValidation(t *testing.T) {
v: corev1.Volume{
Name: "foo",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
EmptyDir: &corev1.EmptyDirVolumeSource{
Medium: "Memory",
SizeLimit: resource.NewQuantity(400, "G"),
},
},
},
cfgOpts: []configOption{withPodSpecVolumesEmptyDirEnabled()},
}, {
name: "invalid emptyDir volume",
v: corev1.Volume{
Name: "foo",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{
Medium: "Memory",
SizeLimit: resource.NewQuantity(-1, "G"),
},
},
},
want: apis.ErrMissingOneOf("secret", "configMap", "projected").Also(
apis.ErrDisallowedFields("emptyDir")),
want: apis.ErrInvalidValue(-1, "emptyDir.sizeLimit"),
cfgOpts: []configOption{withPodSpecVolumesEmptyDirEnabled()},
}, {
name: "no volume source",
v: corev1.Volume{
Expand Down Expand Up @@ -1767,7 +1790,15 @@ func TestVolumeValidation(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := validateVolume(test.v)
ctx := context.Background()
if test.cfgOpts != nil {
cfg := config.FromContextOrDefaults(ctx)
for _, opt := range test.cfgOpts {
cfg = opt(cfg)
}
ctx = config.ToContext(ctx, cfg)
}
got := validateVolume(ctx, test.v)
if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
t.Errorf("validateVolume (-want, +got): \n%s", diff)
}
Expand Down
17 changes: 9 additions & 8 deletions pkg/reconciler/route/resources/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,14 +370,15 @@ 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,
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,
TagHeaderBasedRouting: apiConfig.Disabled,
},
}
}

0 comments on commit 3e31166

Please sign in to comment.