diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 2c855345a143..5a046651fe9b 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -166,6 +166,21 @@ spec: timeoutSeconds: ... serviceAccountName: ... # Name of the service account the code should run as. + # +optional. Influence scheduling by requiring nodes with certain attributes + # See: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector + nodeSelector: + key: value + ... + + # +optional. Influence scheduling by tolerating specific node "smells" + # See: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ + tolerations: + - key: ... + operator: Equals | Exists + value: ... + effect: NoSchedule | PreferNoSchedule | NoExecute + - ... + status: # the latest created and ready to serve. Watched by Route latestReadyRevisionName: abc @@ -239,6 +254,21 @@ spec: # Many higher-level systems impose a per-request response deadline. timeoutSeconds: ... + # +optional. Influence scheduling by requiring nodes with certain attributes + # See: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector + nodeSelector: + key: value + ... + + # +optional. Influence scheduling by tolerating specific node "smells" + # See: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ + tolerations: + - key: ... + operator: Equals | Exists + value: ... + effect: NoSchedule | PreferNoSchedule | NoExecute + - ... + status: # This is a copy of metadata from the container image or grafeas, # indicating the provenance of the revision. This is based on the @@ -305,6 +335,19 @@ spec: # One of "runLatest", "release", "pinned" (DEPRECATED), or "manual" containerConcurrency: ... # Optional timeoutSeconds: ... # Optional serviceAccountName: ... # Name of the service account the code should run as + # +optional. Influence scheduling by requiring nodes with certain attributes + # See: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector + nodeSelector: + key: value + ... + # +optional. Influence scheduling by tolerating specific node "smells" + # See: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ + tolerations: + - key: ... + operator: Equals | Exists + value: ... + effect: NoSchedule | PreferNoSchedule | NoExecute + - ... # Example, only one of "runLatest", "release", "pinned" (DEPRECATED), or "manual" can be set in practice. pinned: @@ -318,6 +361,19 @@ spec: # One of "runLatest", "release", "pinned" (DEPRECATED), or "manual" containerConcurrency: ... # Optional timeoutSeconds: ... # Optional serviceAccountName: ... # Name of the service account the code should run as + # +optional. Influence scheduling by requiring nodes with certain attributes + # See: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector + nodeSelector: + key: value + ... + # +optional. Influence scheduling by tolerating specific node "smells" + # See: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ + tolerations: + - key: ... + operator: Equals | Exists + value: ... + effect: NoSchedule | PreferNoSchedule | NoExecute + - ... # Example, only one of "runLatest", "release", "pinned" (DEPRECATED), or "manual" can be set in practice. release: @@ -334,6 +390,19 @@ spec: # One of "runLatest", "release", "pinned" (DEPRECATED), or "manual" containerConcurrency: ... # Optional timeoutSeconds: ... # Optional serviceAccountName: ... # Name of the service account the code should run as + # +optional. Influence scheduling by requiring nodes with certain attributes + # See: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector + nodeSelector: + key: value + ... + # +optional. Influence scheduling by tolerating specific node "smells" + # See: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ + tolerations: + - key: ... + operator: Equals | Exists + value: ... + effect: NoSchedule | PreferNoSchedule | NoExecute + - ... # Example, only one of "runLatest", "release", "pinned" (DEPRECATED), or "manual" can be set in practice. # Manual has no fields. It enables direct access to modify a previously created diff --git a/pkg/apis/serving/v1alpha1/revision_types.go b/pkg/apis/serving/v1alpha1/revision_types.go index 01efa4ea5a34..be9afa72c5eb 100644 --- a/pkg/apis/serving/v1alpha1/revision_types.go +++ b/pkg/apis/serving/v1alpha1/revision_types.go @@ -220,6 +220,18 @@ type RevisionSpec struct { // TimeoutSeconds holds the max duration the instance is allowed for responding to a request. // +optional TimeoutSeconds int64 `json:"timeoutSeconds,omitempty"` + + // NodeSelector is a selector which must be true for the revision's pod(s) to + // fit on a node. + // Selector which must match a node's labels for the pod to be scheduled on + // that node. + // More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ + // +optional + NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // If specified, the revision's tolerations. + // +optional + Tolerations []corev1.Toleration `json:"tolerations,omitempty"` } const ( diff --git a/pkg/apis/serving/v1alpha1/revision_validation.go b/pkg/apis/serving/v1alpha1/revision_validation.go index 2bf2151fe3fe..45464e8b8a7b 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation.go +++ b/pkg/apis/serving/v1alpha1/revision_validation.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "fmt" "strconv" + "strings" "github.com/google/go-containerregistry/pkg/name" "github.com/knative/pkg/apis" @@ -58,6 +59,17 @@ func (rs *RevisionSpec) Validate() *apis.FieldError { if err := validateTimeoutSeconds(rs.TimeoutSeconds); err != nil { errs = errs.Also(err) } + + if err := validateNodeSelector(rs.NodeSelector); err != nil { + errs = errs.Also(err.ViaField("nodeSelector")) + } + + for i, toleration := range rs.Tolerations { + if err := validateToleration(toleration); err != nil { + errs = errs.Also(err.ViaField(fmt.Sprintf("tolerations[%d]", i))) + } + } + return errs } @@ -303,3 +315,109 @@ func (current *Revision) CheckImmutableFields(og apis.Immutable) *apis.FieldErro return nil } + +func validateNodeSelector(nodeSelector map[string]string) *apis.FieldError { + for key, value := range nodeSelector { + if verrs := validation.IsQualifiedName(key); len(verrs) > 0 { + return apis.ErrInvalidKeyName(key, key, verrs...) + } + if verrs := + validation.IsValidLabelValue(value); len(verrs) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", value), + Details: strings.Join(verrs, ", "), + Paths: []string{key}, + } + } + } + return nil +} + +// validateToleration duplicates and adapts logic from +// k8s.io/kubernetes/pkg/apis/core/validation. Although relevant functions from +// that package are exported, they're not usable here because they are for +// unversioned resources. +func validateToleration(toleration corev1.Toleration) *apis.FieldError { + if toleration.Key != "" { + if verrs := validation.IsQualifiedName(toleration.Key); len(verrs) > 0 { + return apis.ErrInvalidKeyName(toleration.Key, "key", verrs...) + } + } + // empty toleration key with Exists operator means match all taints + if toleration.Key == "" && + toleration.Operator != corev1.TolerationOpExists { + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", toleration.Operator), + Details: "operator must be Exists when `key` is empty, which means " + + `"match all values and all keys"`, + Paths: []string{"operator"}, + } + } + if toleration.TolerationSeconds != nil && + toleration.Effect != corev1.TaintEffectNoExecute { + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", toleration.Effect), + Details: "effect must be 'NoExecute' when `tolerationSeconds` is set", + Paths: []string{"effect"}, + } + } + // validate toleration operator and value + switch toleration.Operator { + // empty operator means Equal + case corev1.TolerationOpEqual, "": + if errStrs := + validation.IsValidLabelValue(toleration.Value); len(errStrs) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", toleration.Value), + Details: strings.Join(errStrs, "; "), + Paths: []string{"value"}, + } + } + case corev1.TolerationOpExists: + if toleration.Value != "" { + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", toleration.Value), + Details: "value must be empty when `operator` is 'Exists'", + Paths: []string{"value"}, + } + } + default: + validValues := []string{ + string(corev1.TolerationOpEqual), + string(corev1.TolerationOpExists), + } + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", toleration.Operator), + Details: fmt.Sprintf( + "allowed values are: %s", + strings.Join(validValues, ", "), + ), + Paths: []string{"operator"}, + } + } + // validate toleration effect, empty toleration effect means match all taint + // effects + switch toleration.Effect { + // TODO: Uncomment TaintEffectNoScheduleNoAdmit once it is implemented. + case "", corev1.TaintEffectNoSchedule, corev1.TaintEffectPreferNoSchedule, + corev1.TaintEffectNoExecute: // corev1.TaintEffectNoScheduleNoAdmit + return nil + default: + validValues := []string{ + string(corev1.TaintEffectNoSchedule), + string(corev1.TaintEffectPreferNoSchedule), + string(corev1.TaintEffectNoExecute), + // TODO: Uncomment next line when TaintEffectNoScheduleNoAdmit is + // implemented. + // string(corev1.TaintEffectNoScheduleNoAdmit), + } + return &apis.FieldError{ + Message: fmt.Sprintf("invalid value %q", toleration.Effect), + Details: fmt.Sprintf( + "allowed values are: %s", + strings.Join(validValues, ", "), + ), + Paths: []string{"effect"}, + } + } +} diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index 8173b5a0cbe5..9b56b2a1139d 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/pkg/apis/serving/v1alpha1/revision_validation_test.go @@ -564,6 +564,36 @@ func TestRevisionSpecValidation(t *testing.T) { want: apis.ErrOutOfBoundsValue("-30s", "0s", fmt.Sprintf("%ds", int(netv1alpha1.DefaultTimeout.Seconds())), "timeoutSeconds"), + }, { + name: "bad nodeSelector", + rs: &RevisionSpec{ + Container: corev1.Container{ + Image: "helloworld", + }, + NodeSelector: map[string]string{"foo*bar": "bat"}, + }, + want: &apis.FieldError{ + Message: "invalid key name \"foo*bar\"", + Paths: []string{"nodeSelector.foo*bar"}, + Details: `name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`, + }, + }, { + name: "bad toleration", + rs: &RevisionSpec{ + Container: corev1.Container{ + Image: "helloworld", + }, + Tolerations: []corev1.Toleration{ + { + Key: "foo*bar", + }, + }, + }, + want: &apis.FieldError{ + Message: `invalid key name "foo*bar"`, + Paths: []string{"tolerations[0].key"}, + Details: `name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`, + }, }} for _, test := range tests { @@ -882,3 +912,192 @@ func TestImmutableFields(t *testing.T) { }) } } + +func TestNodeSelectorValidation(t *testing.T) { + tests := []struct { + name string + nodeSelector map[string]string + want *apis.FieldError + }{ + { + name: "invalid key", + nodeSelector: map[string]string{ + "foo*bar": "bat", + }, + want: &apis.FieldError{ + Message: `invalid key name "foo*bar"`, + Paths: []string{"foo*bar"}, + }, + }, + { + name: "invalid value", + nodeSelector: map[string]string{ + "foo": "bar*bat", + }, + want: &apis.FieldError{ + Message: `invalid value "bar*bat"`, + Paths: []string{"foo"}, + }, + }, + { + name: "value key/value pair", + nodeSelector: map[string]string{ + "foo": "bar", + }, + want: nil, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := validateNodeSelector(test.nodeSelector) + if (got == nil && test.want != nil) || + (got != nil && test.want == nil) { + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("Validate (-want, +got) = %v", diff) + } + } else if got != nil && test.want != nil { + if diff := cmp.Diff(test.want.Message, got.Message); diff != "" { + t.Errorf("Validate Message (-want, +got) = %v", diff) + } + if diff := cmp.Diff(test.want.Paths, got.Paths); diff != "" { + t.Errorf("Validate Paths (-want, +got) = %v", diff) + } + } + }) + } +} + +func TestTolerationValidation(t *testing.T) { + tests := []struct { + name string + toleration corev1.Toleration + want *apis.FieldError + }{ + { + name: "invalid key", + toleration: corev1.Toleration{ + Key: "foo*bar", + }, + want: &apis.FieldError{ + Message: `invalid key name "foo*bar"`, + Paths: []string{"key"}, + }, + }, + { + name: "empty key with operator is not Exists", + toleration: corev1.Toleration{ + Operator: corev1.TolerationOpEqual, + }, + want: &apis.FieldError{ + Message: `invalid value "Equal"`, + Paths: []string{"operator"}, + }, + }, + { + name: "tolerationSeconds specified with effect is not NoExecute", + toleration: corev1.Toleration{ + Operator: corev1.TolerationOpExists, + TolerationSeconds: &[]int64{10}[0], + Effect: corev1.TaintEffectNoSchedule, + }, + want: &apis.FieldError{ + Message: `invalid value "NoSchedule"`, + Paths: []string{"effect"}, + }, + }, + { + name: "operator is empty and value is invalid", + toleration: corev1.Toleration{ + Key: "foo", + Value: "foo*bar", + }, + want: &apis.FieldError{ + Message: `invalid value "foo*bar"`, + Paths: []string{"value"}, + }, + }, + { + name: "operator is Equal and value is invalid", + toleration: corev1.Toleration{ + Key: "foo", + Operator: corev1.TolerationOpEqual, + Value: "foo*bar", + }, + want: &apis.FieldError{ + Message: `invalid value "foo*bar"`, + Paths: []string{"value"}, + }, + }, + { + name: "operator is Exists and value is not empty", + toleration: corev1.Toleration{ + Operator: corev1.TolerationOpExists, + Value: "foo", + }, + want: &apis.FieldError{ + Message: `invalid value "foo"`, + Paths: []string{"value"}, + }, + }, + { + name: "operator is invalid", + toleration: corev1.Toleration{ + Key: "foo", + Operator: corev1.TolerationOperator("bar"), + }, + want: &apis.FieldError{ + Message: `invalid value "bar"`, + Paths: []string{"operator"}, + }, + }, + { + name: "effect is invalid", + toleration: corev1.Toleration{ + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffect("foo"), + }, + want: &apis.FieldError{ + Message: `invalid value "foo"`, + Paths: []string{"effect"}, + }, + }, + { + name: "effect is invalid", + toleration: corev1.Toleration{ + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffect("foo"), + }, + want: &apis.FieldError{ + Message: `invalid value "foo"`, + Paths: []string{"effect"}, + }, + }, + { + name: "valid toleration", + toleration: corev1.Toleration{ + Key: "foo", + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoSchedule, + }, + want: nil, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := validateToleration(test.toleration) + if (got == nil && test.want != nil) || + (got != nil && test.want == nil) { + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("Validate (-want, +got) = %v", diff) + } + } else if got != nil && test.want != nil { + if diff := cmp.Diff(test.want.Message, got.Message); diff != "" { + t.Errorf("Validate Message (-want, +got) = %v", diff) + } + if diff := cmp.Diff(test.want.Paths, got.Paths); diff != "" { + t.Errorf("Validate Paths (-want, +got) = %v", diff) + } + } + }) + } +} diff --git a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go index 3504d03a30a6..201640efccce 100644 --- a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go @@ -305,6 +305,20 @@ func (in *RevisionSpec) DeepCopyInto(out *RevisionSpec) { } } in.Container.DeepCopyInto(&out.Container) + if in.NodeSelector != nil { + in, out := &in.NodeSelector, &out.NodeSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Tolerations != nil { + in, out := &in.Tolerations, &out.Tolerations + *out = make([]v1.Toleration, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/reconciler/v1alpha1/revision/resources/deploy.go b/pkg/reconciler/v1alpha1/revision/resources/deploy.go index 79b45390b060..64312add304a 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/deploy.go +++ b/pkg/reconciler/v1alpha1/revision/resources/deploy.go @@ -145,6 +145,8 @@ func makePodSpec(rev *v1alpha1.Revision, loggingConfig *logging.Config, observab Volumes: []corev1.Volume{varLogVolume}, ServiceAccountName: rev.Spec.ServiceAccountName, TerminationGracePeriodSeconds: &revisionTimeout, + NodeSelector: rev.Spec.NodeSelector, + Tolerations: rev.Spec.Tolerations, } // Add Fluentd sidecar and its config map volume if var log collection is enabled. diff --git a/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go b/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go index 9a6bd5bac9eb..01bc7fed1788 100644 --- a/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go +++ b/pkg/reconciler/v1alpha1/revision/resources/deploy_test.go @@ -154,7 +154,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -245,7 +245,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -339,7 +339,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -437,7 +437,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -544,7 +544,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -649,7 +649,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -756,7 +756,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -859,7 +859,7 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), }, }, { @@ -1027,6 +1027,18 @@ func TestMakePodSpec(t *testing.T) { TerminationMessagePolicy: corev1.TerminationMessageReadFile, }, TimeoutSeconds: 45, + NodeSelector: map[string]string{ + "kubernetes.io/role": "agent", + "beta.kubernetes.io/os": "linux", + "type": "virtual-kubelet", + }, + Tolerations: []corev1.Toleration{ + corev1.Toleration{ + Key: "virtual-kubelet.github.io", + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoSchedule, + }, + }, }, }, lc: &logging.Config{}, @@ -1119,8 +1131,20 @@ func TestMakePodSpec(t *testing.T) { Value: system.Namespace(), }}, }}, - Volumes: []corev1.Volume{varLogVolume}, + Volumes: []corev1.Volume{varLogVolume}, TerminationGracePeriodSeconds: refInt64(45), + NodeSelector: map[string]string{ + "kubernetes.io/role": "agent", + "beta.kubernetes.io/os": "linux", + "type": "virtual-kubelet", + }, + Tolerations: []corev1.Toleration{ + corev1.Toleration{ + Key: "virtual-kubelet.github.io", + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoSchedule, + }, + }, }, }}