diff --git a/Gopkg.lock b/Gopkg.lock index 0c5d149b8ecc..23a0c1b5a139 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -435,7 +435,7 @@ revision = "3fc06fd3c9880a9ebb5c401f4b20cf6666cc7bc0" [[projects]] - digest = "1:b7ccc2a66d28cfacf4cdd97fcb90662c7ce4feb9a68944975c6b06bbdfc8757e" + digest = "1:6243ebb8894d4bcd54dc2dfdb2fbeeb3e49c7742ec9dfe9a8118ab642e98aac7" name = "github.com/knative/pkg" packages = [ "apis", @@ -487,7 +487,7 @@ "websocket", ] pruneopts = "NUT" - revision = "04154dda9a1b8ef17939d30a628272b201330ddb" + revision = "0f749ef7d5e65aba4e8a76aa34b4744685981a91" [[projects]] branch = "master" diff --git a/Gopkg.toml b/Gopkg.toml index 473db527c6d3..f36023141b16 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -24,8 +24,8 @@ required = [ [[override]] name = "github.com/knative/pkg" - # HEAD as of 2019-03-25 - revision = "04154dda9a1b8ef17939d30a628272b201330ddb" + # HEAD as of 2019-03-26 + revision = "0f749ef7d5e65aba4e8a76aa34b4744685981a91" [[override]] name = "go.uber.org/zap" diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index bd4f0f72ba70..538d6482636e 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "context" "flag" "log" @@ -31,6 +32,7 @@ import ( "github.com/knative/pkg/version" "github.com/knative/pkg/webhook" kpa "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1" + apiconfig "github.com/knative/serving/pkg/apis/config" net "github.com/knative/serving/pkg/apis/networking/v1alpha1" "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/knative/serving/pkg/logging" @@ -83,6 +85,10 @@ func main() { // Watch the logging config map and dynamically update logging levels. configMapWatcher := configmap.NewInformedWatcher(kubeClient, system.Namespace()) configMapWatcher.Watch(logging.ConfigMapName(), logging.UpdateLevelFromConfigMap(logger, atomicLevel, component)) + + store := apiconfig.NewStore(logger.Named("config-store")) + store.WatchConfigs(configMapWatcher) + if err = configMapWatcher.Start(stopCh); err != nil { logger.Fatalw("Failed to start the ConfigMap watcher", zap.Error(err)) } @@ -110,6 +116,11 @@ func main() { }, Logger: logger, DisallowUnknownFields: true, + + // Decorate contexts with the current state of the config. + WithContext: func(ctx context.Context) context.Context { + return store.ToContext(ctx) + }, } if err = controller.Run(stopCh); err != nil { logger.Fatalw("Failed to start the admission controller", zap.Error(err)) diff --git a/config/config-defaults.yaml b/config/config-defaults.yaml new file mode 100644 index 000000000000..7b9a526330b7 --- /dev/null +++ b/config/config-defaults.yaml @@ -0,0 +1,46 @@ +# Copyright 2019 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: v1 +kind: ConfigMap +metadata: + name: config-defaults + namespace: knative-serving + labels: + serving.knative.dev/release: devel + +data: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ + + # This block is not actually functional configuration, + # but serves to illustrate the available configuration + # options and document them in a way that is accessible + # to users that `kubectl edit` this config map. + # + # These sample configuration options may be copied out of + # this block and unindented to actually change the configuration. + + # revision-timeout-seconds contains the default number of + # seconds to use for the revision's per-request timeout, if + # none is specified. + revision-timeout-seconds: "300" # 5 minutes + + # revision-cpu-limit contains the cpu allocation to assign + # to revisions by default. + revision-cpu-limit: "400m" # 0.4 of a CPU (aka 400 milli-CPU) diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index d60e8df9084e..469d9bbc3b43 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -46,6 +46,7 @@ ${CODEGEN_PKG}/generate-groups.sh "deepcopy,client,informer,lister" \ ${GOPATH}/bin/deepcopy-gen \ -O zz_generated.deepcopy \ --go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt \ + -i github.com/knative/serving/pkg/apis/config \ -i github.com/knative/serving/pkg/reconciler/v1alpha1/clusteringress/config \ -i github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/config \ -i github.com/knative/serving/pkg/reconciler/v1alpha1/revision/config \ diff --git a/pkg/apis/config/defaults.go b/pkg/apis/config/defaults.go new file mode 100644 index 000000000000..bf084edfe39d --- /dev/null +++ b/pkg/apis/config/defaults.go @@ -0,0 +1,99 @@ +/* +Copyright 2019 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. +*/ + +package config + +import ( + "strconv" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +const ( + // DefaultsConfigName is the name of config map for the defaults. + DefaultsConfigName = "config-defaults" +) + +// NewDefaultsConfigFromMap creates a Defaults from the supplied Map +func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) { + nc := &Defaults{} + + // Process int64 fields + for _, i64 := range []struct { + key string + field *int64 + // specified exactly when optional + defaultValue int64 + }{{ + key: "revision-timeout-seconds", + field: &nc.RevisionTimeoutSeconds, + defaultValue: DefaultRevisionTimeoutSeconds, + }} { + if raw, ok := data[i64.key]; !ok { + *i64.field = i64.defaultValue + } else if val, err := strconv.ParseInt(raw, 10, 64); err != nil { + return nil, err + } else { + *i64.field = val + } + } + + // Process resource quantity fields + for _, rsrc := range []struct { + key string + field *resource.Quantity + // specified exactly when optional + defaultValue resource.Quantity + }{{ + key: "revision-cpu-limit", + field: &nc.RevisionCPULimit, + defaultValue: DefaultRevisionCPULimit, + }} { + if raw, ok := data[rsrc.key]; !ok { + *rsrc.field = rsrc.defaultValue + } else if val, err := resource.ParseQuantity(raw); err != nil { + return nil, err + } else { + *rsrc.field = val + } + } + + return nc, nil +} + +// NewDefaultsConfigFromConfigMap creates a Defaults from the supplied configMap +func NewDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*Defaults, error) { + return NewDefaultsConfigFromMap(config.Data) +} + +const ( + // DefaultRevisionTimeoutSeconds will be set if timeoutSeconds not specified. + DefaultRevisionTimeoutSeconds = 5 * 60 +) + +// Pseudo-constants +var ( + // DefaultRevisionCPULimit will be set if resources.limits.cpu is not specified. + DefaultRevisionCPULimit = resource.MustParse("400m") +) + +// Defaults includes the default values to be populated by the webhook. +type Defaults struct { + RevisionTimeoutSeconds int64 + + RevisionCPULimit resource.Quantity +} diff --git a/pkg/apis/config/defaults_test.go b/pkg/apis/config/defaults_test.go new file mode 100644 index 000000000000..ac947e79fa7e --- /dev/null +++ b/pkg/apis/config/defaults_test.go @@ -0,0 +1,119 @@ +/* +Copyright 2019 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 config + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/knative/pkg/system" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + . "github.com/knative/serving/pkg/reconciler/testing" +) + +func TestDefaultsConfigurationFromFile(t *testing.T) { + cm, example := ConfigMapsFromTestFile(t, DefaultsConfigName) + + if _, err := NewDefaultsConfigFromConfigMap(cm); err != nil { + t.Errorf("NewDefaultsConfigFromConfigMap(actual) = %v", err) + } + + if _, err := NewDefaultsConfigFromConfigMap(example); err != nil { + t.Errorf("NewDefaultsConfigFromConfigMap(example) = %v", err) + } +} + +func TestDefaultsConfiguration(t *testing.T) { + configTests := []struct { + name string + wantErr bool + wantDefaults interface{} + config *corev1.ConfigMap + }{{ + name: "defaults configuration", + wantErr: false, + wantDefaults: &Defaults{ + RevisionTimeoutSeconds: DefaultRevisionTimeoutSeconds, + RevisionCPULimit: DefaultRevisionCPULimit, + }, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: DefaultsConfigName, + }, + Data: map[string]string{}, + }, + }, { + name: "specified values", + wantErr: false, + wantDefaults: &Defaults{ + RevisionTimeoutSeconds: 123, + RevisionCPULimit: resource.MustParse("123m"), + }, + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: DefaultsConfigName, + }, + Data: map[string]string{ + "revision-timeout-seconds": "123", + "revision-cpu-limit": "123m", + }, + }, + }, { + name: "bad revision timeout", + wantErr: true, + wantDefaults: (*Defaults)(nil), + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: DefaultsConfigName, + }, + Data: map[string]string{ + "revision-timeout-seconds": "asdf", + }, + }, + }, { + name: "bad resource", + wantErr: true, + wantDefaults: (*Defaults)(nil), + config: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: system.Namespace(), + Name: DefaultsConfigName, + }, + Data: map[string]string{ + "revision-cpu-limit": "bad", + }, + }, + }} + + for _, tt := range configTests { + actualDefaults, err := NewDefaultsConfigFromConfigMap(tt.config) + + if (err != nil) != tt.wantErr { + t.Fatalf("Test: %q; NewDefaultsConfigFromConfigMap() error = %v, WantErr %v", tt.name, err, tt.wantErr) + } + + if diff := cmp.Diff(actualDefaults, tt.wantDefaults, ignoreResourceQuantity); diff != "" { + t.Fatalf("Test: %q; want %v, but got %v", tt.name, tt.wantDefaults, actualDefaults) + } + } +} diff --git a/pkg/apis/config/doc.go b/pkg/apis/config/doc.go new file mode 100644 index 000000000000..8a8d1a75801c --- /dev/null +++ b/pkg/apis/config/doc.go @@ -0,0 +1,21 @@ +/* +Copyright 2019 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. +*/ + +// +k8s:deepcopy-gen=package + +// Package config holds the typed objects that define the schemas for +// ConfigMap objects that pertain to our API objects. +package config diff --git a/pkg/apis/config/store.go b/pkg/apis/config/store.go new file mode 100644 index 000000000000..7d7f519d9acd --- /dev/null +++ b/pkg/apis/config/store.go @@ -0,0 +1,92 @@ +/* +Copyright 2019 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 config + +import ( + "context" + + "github.com/knative/pkg/configmap" +) + +type cfgKey struct{} + +// Config holds the collection of configurations that we attach to contexts. +// +k8s:deepcopy-gen=false +type Config struct { + Defaults *Defaults +} + +// FromContext extracts a Config from the provided context. +func FromContext(ctx context.Context) *Config { + x, ok := ctx.Value(cfgKey{}).(*Config) + if ok { + return x + } + return nil +} + +// 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 + } + defaults, _ := NewDefaultsConfigFromMap(map[string]string{}) + return &Config{ + Defaults: defaults, + } +} + +// ToContext attaches the provided Config to the provided context, returning the +// new context with the Config attached. +func ToContext(ctx context.Context, c *Config) context.Context { + return context.WithValue(ctx, cfgKey{}, c) +} + +// Store is a typed wrapper around configmap.Untyped store to handle our configmaps. +// +k8s:deepcopy-gen=false +type Store struct { + *configmap.UntypedStore +} + +// NewStore creates a new store of Configs and optionally calls functions when ConfigMaps are updated. +func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store { + store := &Store{ + UntypedStore: configmap.NewUntypedStore( + "defaults", + logger, + configmap.Constructors{ + DefaultsConfigName: NewDefaultsConfigFromConfigMap, + }, + onAfterStore..., + ), + } + + return store +} + +// ToContext attaches the current Config state to the provided context. +func (s *Store) ToContext(ctx context.Context) context.Context { + return ToContext(ctx, s.Load()) +} + +// Load creates a Config from the current config state of the Store. +func (s *Store) Load() *Config { + return &Config{ + Defaults: s.UntypedLoad(DefaultsConfigName).(*Defaults).DeepCopy(), + } +} diff --git a/pkg/apis/config/store_test.go b/pkg/apis/config/store_test.go new file mode 100644 index 000000000000..f43097ff397c --- /dev/null +++ b/pkg/apis/config/store_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2019 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 config + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + logtesting "github.com/knative/pkg/logging/testing" + . "github.com/knative/serving/pkg/reconciler/testing" + "k8s.io/apimachinery/pkg/api/resource" +) + +var ignoreResourceQuantity = cmpopts.IgnoreUnexported(resource.Quantity{}) + +func TestStoreLoadWithContext(t *testing.T) { + defer logtesting.ClearAll() + store := NewStore(logtesting.TestLogger(t)) + + defaultsConfig := ConfigMapFromTestFile(t, DefaultsConfigName) + + store.OnConfigChanged(defaultsConfig) + + config := FromContextOrDefaults(store.ToContext(context.Background())) + + t.Run("defaults", func(t *testing.T) { + expected, _ := NewDefaultsConfigFromConfigMap(defaultsConfig) + if diff := cmp.Diff(expected, config.Defaults, ignoreResourceQuantity); diff != "" { + t.Errorf("Unexpected defaults config (-want, +got): %v", diff) + } + }) +} + +func TestStoreLoadWithContextOrDefaults(t *testing.T) { + defer logtesting.ClearAll() + + defaultsConfig := ConfigMapFromTestFile(t, DefaultsConfigName) + config := FromContextOrDefaults(context.Background()) + + t.Run("defaults", func(t *testing.T) { + expected, _ := NewDefaultsConfigFromConfigMap(defaultsConfig) + if diff := cmp.Diff(expected, config.Defaults, ignoreResourceQuantity); diff != "" { + t.Errorf("Unexpected defaults config (-want, +got): %v", diff) + } + }) +} + +func TestStoreImmutableConfig(t *testing.T) { + defer logtesting.ClearAll() + store := NewStore(logtesting.TestLogger(t)) + + store.OnConfigChanged(ConfigMapFromTestFile(t, DefaultsConfigName)) + + config := store.Load() + + config.Defaults.RevisionTimeoutSeconds = 1234 + + newConfig := store.Load() + + if newConfig.Defaults.RevisionTimeoutSeconds == 1234 { + t.Error("Defaults config is not immutable") + } +} diff --git a/pkg/apis/config/testdata/config-defaults.yaml b/pkg/apis/config/testdata/config-defaults.yaml new file mode 120000 index 000000000000..4f00ffb571b4 --- /dev/null +++ b/pkg/apis/config/testdata/config-defaults.yaml @@ -0,0 +1 @@ +../../../../config/config-defaults.yaml \ No newline at end of file diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go new file mode 100644 index 000000000000..44f6ce0e5290 --- /dev/null +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -0,0 +1,38 @@ +// +build !ignore_autogenerated + +/* +Copyright 2019 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. +*/ + +// This file was autogenerated by deepcopy-gen. Do not edit it manually! + +package config + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Defaults) DeepCopyInto(out *Defaults) { + *out = *in + out.RevisionCPULimit = in.RevisionCPULimit.DeepCopy() + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Defaults. +func (in *Defaults) DeepCopy() *Defaults { + if in == nil { + return nil + } + out := new(Defaults) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/apis/serving/v1alpha1/configuration_defaults_test.go b/pkg/apis/serving/v1alpha1/configuration_defaults_test.go index 67d23dd1710e..d4d0449e593d 100644 --- a/pkg/apis/serving/v1alpha1/configuration_defaults_test.go +++ b/pkg/apis/serving/v1alpha1/configuration_defaults_test.go @@ -24,6 +24,8 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + + "github.com/knative/serving/pkg/apis/config" ) var ( @@ -47,7 +49,7 @@ func TestConfigurationDefaulting(t *testing.T) { Spec: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, diff --git a/pkg/apis/serving/v1alpha1/revision_defaults.go b/pkg/apis/serving/v1alpha1/revision_defaults.go index 6f1fda40392b..f7c68cfc4d32 100644 --- a/pkg/apis/serving/v1alpha1/revision_defaults.go +++ b/pkg/apis/serving/v1alpha1/revision_defaults.go @@ -20,12 +20,8 @@ import ( "context" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) -const ( - // defaultTimeoutSeconds will be set if timeoutSeconds not specified. - defaultTimeoutSeconds = 5 * 60 + "github.com/knative/serving/pkg/apis/config" ) func (r *Revision) SetDefaults(ctx context.Context) { @@ -33,6 +29,8 @@ func (r *Revision) SetDefaults(ctx context.Context) { } func (rs *RevisionSpec) SetDefaults(ctx context.Context) { + cfg := config.FromContextOrDefaults(ctx) + // When ConcurrencyModel is specified but ContainerConcurrency // is not (0), use the ConcurrencyModel value. if rs.DeprecatedConcurrencyModel == RevisionRequestConcurrencyModelSingle && rs.ContainerConcurrency == 0 { @@ -40,14 +38,14 @@ func (rs *RevisionSpec) SetDefaults(ctx context.Context) { } if rs.TimeoutSeconds == 0 { - rs.TimeoutSeconds = defaultTimeoutSeconds + rs.TimeoutSeconds = cfg.Defaults.RevisionTimeoutSeconds } if rs.Container.Resources.Requests == nil { rs.Container.Resources.Requests = corev1.ResourceList{} } if _, ok := rs.Container.Resources.Requests[corev1.ResourceCPU]; !ok { - rs.Container.Resources.Requests[corev1.ResourceCPU] = resource.MustParse("400m") + rs.Container.Resources.Requests[corev1.ResourceCPU] = cfg.Defaults.RevisionCPULimit } vms := rs.Container.VolumeMounts diff --git a/pkg/apis/serving/v1alpha1/revision_defaults_test.go b/pkg/apis/serving/v1alpha1/revision_defaults_test.go index 9207e114ff45..b055792a73a0 100644 --- a/pkg/apis/serving/v1alpha1/revision_defaults_test.go +++ b/pkg/apis/serving/v1alpha1/revision_defaults_test.go @@ -21,7 +21,11 @@ import ( "testing" "github.com/google/go-cmp/cmp" + logtesting "github.com/knative/pkg/logging/testing" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/knative/serving/pkg/apis/config" ) func TestRevisionDefaulting(t *testing.T) { @@ -29,13 +33,39 @@ func TestRevisionDefaulting(t *testing.T) { name string in *Revision want *Revision + wc func(context.Context) context.Context }{{ name: "empty", in: &Revision{}, want: &Revision{ Spec: RevisionSpec{ ContainerConcurrency: 0, - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, + Container: corev1.Container{ + Resources: defaultResources, + }, + }, + }, + }, { + name: "with context", + in: &Revision{}, + wc: func(ctx context.Context) context.Context { + s := config.NewStore(logtesting.TestLogger(t)) + s.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.DefaultsConfigName, + }, + Data: map[string]string{ + "revision-timeout-seconds": "123", + }, + }) + + return s.ToContext(ctx) + }, + want: &Revision{ + Spec: RevisionSpec{ + ContainerConcurrency: 0, + TimeoutSeconds: 123, Container: corev1.Container{ Resources: defaultResources, }, @@ -94,7 +124,7 @@ func TestRevisionDefaulting(t *testing.T) { want: &Revision{ Spec: RevisionSpec{ ContainerConcurrency: 0, - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, @@ -112,7 +142,7 @@ func TestRevisionDefaulting(t *testing.T) { Spec: RevisionSpec{ DeprecatedConcurrencyModel: "Single", ContainerConcurrency: 1, - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, @@ -123,7 +153,11 @@ func TestRevisionDefaulting(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { got := test.in - got.SetDefaults(context.Background()) + ctx := context.Background() + if test.wc != nil { + ctx = test.wc(ctx) + } + got.SetDefaults(ctx) if diff := cmp.Diff(test.want, got, ignoreUnexportedResources); diff != "" { t.Errorf("SetDefaults (-want, +got) = %v", diff) } diff --git a/pkg/apis/serving/v1alpha1/service_defaults_test.go b/pkg/apis/serving/v1alpha1/service_defaults_test.go index 7e7e43720a55..9beef2d85db8 100644 --- a/pkg/apis/serving/v1alpha1/service_defaults_test.go +++ b/pkg/apis/serving/v1alpha1/service_defaults_test.go @@ -22,6 +22,8 @@ import ( "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" + + "github.com/knative/serving/pkg/apis/config" ) func TestServiceDefaulting(t *testing.T) { @@ -60,7 +62,7 @@ func TestServiceDefaulting(t *testing.T) { Configuration: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, @@ -79,7 +81,7 @@ func TestServiceDefaulting(t *testing.T) { RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ ContainerConcurrency: 1, - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, }, }, }, @@ -93,7 +95,7 @@ func TestServiceDefaulting(t *testing.T) { RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ ContainerConcurrency: 1, - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, @@ -116,7 +118,7 @@ func TestServiceDefaulting(t *testing.T) { Configuration: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, @@ -172,7 +174,7 @@ func TestServiceDefaulting(t *testing.T) { Configuration: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ - TimeoutSeconds: defaultTimeoutSeconds, + TimeoutSeconds: config.DefaultRevisionTimeoutSeconds, Container: corev1.Container{ Resources: defaultResources, }, diff --git a/vendor/github.com/knative/pkg/webhook/webhook.go b/vendor/github.com/knative/pkg/webhook/webhook.go index b75f6b64a893..b07d90781eb7 100644 --- a/vendor/github.com/knative/pkg/webhook/webhook.go +++ b/vendor/github.com/knative/pkg/webhook/webhook.go @@ -121,9 +121,14 @@ type AdmissionController struct { Handlers map[schema.GroupVersionKind]GenericCRD Logger *zap.SugaredLogger + WithContext func(context.Context) context.Context DisallowUnknownFields bool } +func nop(ctx context.Context) context.Context { + return ctx +} + // GenericCRD is the interface definition that allows us to perform the generic // CRD actions like deciding whether to increment generation and so forth. type GenericCRD interface { @@ -205,32 +210,32 @@ func getOrGenerateKeyCertsFromSecret(ctx context.Context, client kubernetes.Inte // validate checks whether "new" and "old" implement HasImmutableFields and checks them, // it then delegates validation to apis.Validatable on "new". -func validate(old GenericCRD, new GenericCRD) error { +func validate(ctx context.Context, old, new GenericCRD) error { if immutableNew, ok := new.(apis.Immutable); ok && old != nil { // Copy the old object and set defaults so that we don't reject our own // defaulting done earlier in the webhook. old = old.DeepCopyObject().(GenericCRD) // TODO(mattmoor): Plumb through a real context - old.SetDefaults(context.TODO()) + old.SetDefaults(ctx) immutableOld, ok := old.(apis.Immutable) if !ok { return fmt.Errorf("unexpected type mismatch %T vs. %T", old, new) } // TODO(mattmoor): Plumb through a real context - if err := immutableNew.CheckImmutableFields(context.TODO(), immutableOld); err != nil { + if err := immutableNew.CheckImmutableFields(ctx, immutableOld); err != nil { return err } } // Can't just `return new.Validate()` because it doesn't properly nil-check. // TODO(mattmoor): Plumb through a real context - if err := new.Validate(context.TODO()); err != nil { + if err := new.Validate(ctx); err != nil { return err } return nil } -func setAnnotations(patches duck.JSONPatch, new, old GenericCRD, ui *authenticationv1.UserInfo) (duck.JSONPatch, error) { +func setAnnotations(ctx context.Context, patches duck.JSONPatch, new, old GenericCRD, ui *authenticationv1.UserInfo) (duck.JSONPatch, error) { // Nowhere to set the annotations. if new == nil { return patches, nil @@ -246,7 +251,7 @@ func setAnnotations(patches duck.JSONPatch, new, old GenericCRD, ui *authenticat b, a := new.DeepCopyObject().(apis.Annotatable), na // TODO(mattmoor): Plumb through a real context - a.AnnotateUserInfo(context.TODO(), oa, ui) + a.AnnotateUserInfo(ctx, oa, ui) patch, err := duck.CreatePatch(b, a) if err != nil { return nil, err @@ -255,10 +260,10 @@ func setAnnotations(patches duck.JSONPatch, new, old GenericCRD, ui *authenticat } // setDefaults simply leverages apis.Defaultable to set defaults. -func setDefaults(patches duck.JSONPatch, crd GenericCRD) (duck.JSONPatch, error) { +func setDefaults(ctx context.Context, patches duck.JSONPatch, crd GenericCRD) (duck.JSONPatch, error) { before, after := crd.DeepCopyObject(), crd // TODO(mattmoor): Plumb through a real context - after.SetDefaults(context.TODO()) + after.SetDefaults(ctx) patch, err := duck.CreatePatch(before, after) if err != nil { @@ -456,7 +461,13 @@ func (ac *AdmissionController) ServeHTTP(w http.ResponseWriter, r *http.Request) zap.String(logkey.Resource, fmt.Sprint(review.Request.Resource)), zap.String(logkey.SubResource, fmt.Sprint(review.Request.SubResource)), zap.String(logkey.UserInfo, fmt.Sprint(review.Request.UserInfo))) - reviewResponse := ac.admit(logging.WithLogger(r.Context(), logger), review.Request) + ctx := logging.WithLogger(r.Context(), logger) + + if ac.WithContext != nil { + ctx = ac.WithContext(ctx) + } + + reviewResponse := ac.admit(ctx, review.Request) var response admissionv1beta1.AdmissionReview if reviewResponse != nil { response.Response = reviewResponse @@ -561,14 +572,14 @@ func (ac *AdmissionController) mutate(ctx context.Context, req *admissionv1beta1 patches = append(patches, rtp...) } - if patches, err = setDefaults(patches, newObj); err != nil { + if patches, err = setDefaults(ctx, patches, newObj); err != nil { logger.Errorw("Failed the resource specific defaulter", zap.Error(err)) // Return the error message as-is to give the defaulter callback // discretion over (our portion of) the message that the user sees. return nil, err } - if patches, err = setAnnotations(patches, newObj, oldObj, &req.UserInfo); err != nil { + if patches, err = setAnnotations(ctx, patches, newObj, oldObj, &req.UserInfo); err != nil { logger.Errorw("Failed the resource annotator", zap.Error(err)) return nil, perrors.Wrap(err, "error setting annotations") } @@ -577,7 +588,7 @@ func (ac *AdmissionController) mutate(ctx context.Context, req *admissionv1beta1 if newObj == nil { return nil, errMissingNewObject } - if err := validate(oldObj, newObj); err != nil { + if err := validate(ctx, oldObj, newObj); err != nil { logger.Errorw("Failed the resource specific validation", zap.Error(err)) // Return the error message as-is to give the validation callback // discretion over (our portion of) the message that the user sees.