-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support for NodeSelector and Tolerations. #3467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,16 +20,18 @@ import ( | |
| "fmt" | ||
| "path/filepath" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| "github.com/google/go-containerregistry/pkg/name" | ||
| "github.com/knative/pkg/apis" | ||
| "github.com/knative/pkg/kmp" | ||
| networkingv1alpha1 "github.com/knative/serving/pkg/apis/networking/v1alpha1" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/equality" | ||
| "k8s.io/apimachinery/pkg/util/intstr" | ||
| "k8s.io/apimachinery/pkg/util/sets" | ||
| "k8s.io/apimachinery/pkg/util/validation" | ||
|
|
||
| "github.com/google/go-containerregistry/pkg/name" | ||
| "github.com/knative/pkg/apis" | ||
| "github.com/knative/pkg/kmp" | ||
| networkingv1alpha1 "github.com/knative/serving/pkg/apis/networking/v1alpha1" | ||
| ) | ||
|
|
||
| var ( | ||
|
|
@@ -83,6 +85,17 @@ func (rs *RevisionSpec) Validate() *apis.FieldError { | |
| rs.ContainerConcurrency, rs.DeprecatedConcurrencyModel)) | ||
| } | ||
|
|
||
| if err := validateNodeSelector(rs.NodeSelector); err != nil { | ||
| return err.ViaField("nodeSelector") | ||
| } | ||
| for i, toleration := range rs.Tolerations { | ||
| if err := validateToleration(toleration); err != nil { | ||
| return err.ViaField( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Append to |
||
| fmt.Sprintf("tolerations[%d]", i), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return errs.Also(validateTimeoutSeconds(rs.TimeoutSeconds)) | ||
| } | ||
|
|
||
|
|
@@ -389,3 +402,119 @@ 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 errStrs := | ||
| validation.IsQualifiedName(key); len(errStrs) > 0 { | ||
| return apis.ErrInvalidKeyName( | ||
| key, | ||
| key, | ||
| strings.Join(errStrs, "; "), | ||
| ) | ||
| } | ||
| if errStrs := | ||
| validation.IsValidLabelValue(value); len(errStrs) != 0 { | ||
| return &apis.FieldError{ | ||
| Message: fmt.Sprintf("invalid value %q", value), | ||
| Details: strings.Join(errStrs, "; "), | ||
| 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 { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if toleration.Key != "" { | ||
| if errStrs := | ||
| validation.IsQualifiedName(toleration.Key); len(errStrs) > 0 { | ||
| return &apis.FieldError{ | ||
| Message: fmt.Sprintf("invalid value %q", toleration.Key), | ||
| Details: strings.Join(errStrs, "; "), | ||
| Paths: []string{"key"}, | ||
| } | ||
| } | ||
| } | ||
| // 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| // 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"}, | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Append to
errsinstead of returning?