Skip to content

Commit

Permalink
add feature falgs for affinity/nodeSelector/tolerations
Browse files Browse the repository at this point in the history
  • Loading branch information
emaildanwilson committed Jul 15, 2020
1 parent ece4ca8 commit 1e59637
Show file tree
Hide file tree
Showing 8 changed files with 302 additions and 44 deletions.
11 changes: 10 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: "43bdc81c"
knative.dev/example-checksum: "2977d6de"
data:
_example: |
################################
Expand All @@ -41,6 +41,15 @@ data:
# Indicates whether multi container support is enabled
multi-container: "disabled"
# Indicates whether Kubernetes affinity support is enabled
kubernetes.podspec-affinity: "disabled"
# Indicates whether Kubernetes nodeSelector support is enabled
kubernetes.podspec-nodeselector: "disabled"
# Indicates whether Kubernetes tolerations support is enabled
kubernetes.podspec-tolerations: "disabled"
# Indicates whether Kubernetes FieldRef support is enabled
kubernetes.podspec-fieldref: "disabled"
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/config/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ const (
func defaultFeaturesConfig() *Features {
return &Features{
MultiContainer: Disabled,
PodSpecAffinity: Disabled,
PodSpecFieldRef: Disabled,
PodSpecDryRun: Allowed,
PodSpecNodeSelector: Disabled,
PodSpecTolerations: Disabled,
ResponsiveRevisionGC: Disabled,
}
}
Expand All @@ -53,8 +56,11 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) {

if err := cm.Parse(data,
asFlag("multi-container", &nc.MultiContainer),
asFlag("kubernetes.podspec-affinity", &nc.PodSpecAffinity),
asFlag("kubernetes.podspec-fieldref", &nc.PodSpecFieldRef),
asFlag("kubernetes.podspec-dryrun", &nc.PodSpecDryRun),
asFlag("kubernetes.podspec-nodeselector", &nc.PodSpecNodeSelector),
asFlag("kubernetes.podspec-tolerations", &nc.PodSpecTolerations),
asFlag("responsive-revision-gc", &nc.ResponsiveRevisionGC)); err != nil {
return nil, err
}
Expand All @@ -69,8 +75,11 @@ func NewFeaturesConfigFromConfigMap(config *corev1.ConfigMap) (*Features, error)
// Features specifies which features are allowed by the webhook.
type Features struct {
MultiContainer Flag
PodSpecAffinity Flag
PodSpecFieldRef Flag
PodSpecDryRun Flag
PodSpecNodeSelector Flag
PodSpecTolerations Flag
ResponsiveRevisionGC Flag
}

Expand Down
93 changes: 90 additions & 3 deletions pkg/apis/config/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,19 @@ func TestFeaturesConfiguration(t *testing.T) {
wantErr: false,
wantFeatures: defaultWith(&Features{
MultiContainer: Enabled,
PodSpecAffinity: Enabled,
PodSpecDryRun: Enabled,
PodSpecNodeSelector: Enabled,
PodSpecTolerations: Enabled,
ResponsiveRevisionGC: Enabled,
}),
data: map[string]string{
"multi-container": "Enabled",
"kubernetes.podspec-dryrun": "Enabled",
"responsive-revision-gc": "Enabled",
"multi-container": "Enabled",
"kubernetes.podspec-affinity": "Enabled",
"kubernetes.podspec-dryrun": "Enabled",
"kubernetes.podspec-nodeselector": "Enabled",
"kubernetes.podspec-tolerations": "Enabled",
"responsive-revision-gc": "Enabled",
},
}, {
name: "multi-container Allowed",
Expand All @@ -86,6 +92,33 @@ func TestFeaturesConfiguration(t *testing.T) {
data: map[string]string{
"multi-container": "Disabled",
},
}, {
name: "kubernetes.podspec-affinity Allowed",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecAffinity: Allowed,
}),
data: map[string]string{
"kubernetes.podspec-affinity": "Allowed",
},
}, {
name: "kubernetes.podspec-affinity Enabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecAffinity: Enabled,
}),
data: map[string]string{
"kubernetes.podspec-affinity": "Enabled",
},
}, {
name: "kubernetes.podspec-affinity Disabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecAffinity: Disabled,
}),
data: map[string]string{
"kubernetes.podspec-affinity": "Disabled",
},
}, {
name: "kubernetes.podspec-fieldref Allowed",
wantErr: false,
Expand Down Expand Up @@ -113,6 +146,60 @@ func TestFeaturesConfiguration(t *testing.T) {
data: map[string]string{
"kubernetes.podspec-fieldref": "Disabled",
},
}, {
name: "kubernetes.podspec-nodeselector Allowed",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecNodeSelector: Allowed,
}),
data: map[string]string{
"kubernetes.podspec-nodeselector": "Allowed",
},
}, {
name: "kubernetes.podspec-nodeselector Enabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecNodeSelector: Enabled,
}),
data: map[string]string{
"kubernetes.podspec-nodeselector": "Enabled",
},
}, {
name: "kubernetes.podspec-nodeselector Disabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecNodeSelector: Disabled,
}),
data: map[string]string{
"kubernetes.podspec-nodeselector": "Disabled",
},
}, {
name: "kubernetes.podspec-tolerations Allowed",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecTolerations: Allowed,
}),
data: map[string]string{
"kubernetes.podspec-tolerations": "Allowed",
},
}, {
name: "kubernetes.podspec-tolerations Enabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecTolerations: Enabled,
}),
data: map[string]string{
"kubernetes.podspec-tolerations": "Enabled",
},
}, {
name: "kubernetes.podspec-tolerations Disabled",
wantErr: false,
wantFeatures: defaultWith(&Features{
PodSpecTolerations: Disabled,
}),
data: map[string]string{
"kubernetes.podspec-tolerations": "Disabled",
},
}, {
name: "responsive-revision-gc Allowed",
wantErr: false,
Expand Down
27 changes: 18 additions & 9 deletions pkg/apis/config/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,26 @@ func FromContext(ctx context.Context) *Config {
// FromContextOrDefaults is like FromContext, but when no Config is attached it
// returns a Config populated with the defaults for each of the Config fields.
func FromContextOrDefaults(ctx context.Context) *Config {
if cfg := FromContext(ctx); cfg != nil {
return cfg
cfg := FromContext(ctx)
if cfg == nil {
cfg = &Config{}
}
defaults, _ := NewDefaultsConfigFromMap(map[string]string{})
features, _ := NewFeaturesConfigFromMap(map[string]string{})
autoscaler, _ := autoscalerconfig.NewConfigFromMap(map[string]string{})
return &Config{
Defaults: defaults,
Features: features,
Autoscaler: autoscaler,

if cfg.Defaults == nil {
defaults, _ := NewDefaultsConfigFromMap(map[string]string{})
cfg.Defaults = defaults
}

if cfg.Features == nil {
features, _ := NewFeaturesConfigFromMap(map[string]string{})
cfg.Features = features
}

if cfg.Autoscaler == nil {
autoscaler, _ := autoscalerconfig.NewConfigFromMap(map[string]string{})
cfg.Autoscaler = autoscaler
}
return cfg
}

// ToContext attaches the provided Config to the provided context, returning the
Expand Down
17 changes: 13 additions & 4 deletions pkg/apis/serving/fieldmask.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package serving

import (
corev1 "k8s.io/api/core/v1"
"knative.dev/serving/pkg/apis/config"
)

// VolumeMask performs a _shallow_ copy of the Kubernetes Volume object to a new
Expand Down Expand Up @@ -136,7 +137,7 @@ func KeyToPathMask(in *corev1.KeyToPath) *corev1.KeyToPath {
// PodSpecMask performs a _shallow_ copy of the Kubernetes PodSpec object to a new
// Kubernetes PodSpec 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 PodSpecMask(in *corev1.PodSpec) *corev1.PodSpec {
func PodSpecMask(in *corev1.PodSpec, features *config.Features) *corev1.PodSpec {
if in == nil {
return nil
}
Expand All @@ -150,14 +151,24 @@ func PodSpecMask(in *corev1.PodSpec) *corev1.PodSpec {
out.ImagePullSecrets = in.ImagePullSecrets
out.EnableServiceLinks = in.EnableServiceLinks

// Feature fields
if features.PodSpecAffinity != config.Disabled {
out.Affinity = in.Affinity
}
if features.PodSpecNodeSelector != config.Disabled {
out.NodeSelector = in.NodeSelector
}
if features.PodSpecTolerations != config.Disabled {
out.Tolerations = in.Tolerations
}

// Disallowed fields
// This list is unnecessary, but added here for clarity
out.InitContainers = nil
out.RestartPolicy = ""
out.TerminationGracePeriodSeconds = nil
out.ActiveDeadlineSeconds = nil
out.DNSPolicy = ""
out.NodeSelector = nil
out.AutomountServiceAccountToken = nil
out.NodeName = ""
out.HostNetwork = false
Expand All @@ -167,9 +178,7 @@ func PodSpecMask(in *corev1.PodSpec) *corev1.PodSpec {
out.SecurityContext = nil
out.Hostname = ""
out.Subdomain = ""
out.Affinity = nil
out.SchedulerName = ""
out.Tolerations = nil
out.HostAliases = nil
out.PriorityClassName = ""
out.Priority = nil
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/serving/fieldmask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"knative.dev/pkg/kmp"
"knative.dev/pkg/ptr"
"knative.dev/serving/pkg/apis/config"
)

func TestVolumeMask(t *testing.T) {
Expand Down Expand Up @@ -117,7 +118,8 @@ func TestPodSpecMask(t *testing.T) {
}},
}

got := PodSpecMask(in)
features, _ := config.NewFeaturesConfigFromMap(make(map[string]string))
got := PodSpecMask(in, features)

if &want == &got {
t.Error("Input and output share addresses. Want different addresses")
Expand All @@ -129,7 +131,7 @@ func TestPodSpecMask(t *testing.T) {
t.Errorf("PodSpecMask (-want, +got): %s", diff)
}

if got = PodSpecMask(nil); got != nil {
if got = PodSpecMask(nil, features); got != nil {
t.Errorf("PodSpecMask(nil) = %v, want: nil", got)
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/serving/k8s_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ func ValidatePodSpec(ctx context.Context, ps corev1.PodSpec) *apis.FieldError {
// return apis.ErrMissingField(apis.CurrentField)
// }

errs := apis.CheckDisallowedFields(ps, *PodSpecMask(&ps))
features := config.FromContextOrDefaults(ctx).Features
errs := apis.CheckDisallowedFields(ps, *PodSpecMask(&ps, features))

volumes, err := ValidateVolumes(ps.Volumes, AllMountedVolumes(ps.Containers))
if err != nil {
Expand Down
Loading

0 comments on commit 1e59637

Please sign in to comment.