Skip to content
2 changes: 1 addition & 1 deletion config/config-network.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ data:
# For more information, visit
# https://istio.io/docs/tasks/traffic-management/egress/
#
istio.sidecar.includeOutboundIPRanges: "*"
istio.sidecar.includeOutboundIPRanges: "10.0.0.1/24"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was probably unintentionally included with this PR. I have the same local change and have almost accidentally included it in a PR as well 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops. That's embarrassing. Thank for catching it, @bbrowning.

12 changes: 12 additions & 0 deletions pkg/apis/serving/v1alpha1/revision_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,18 @@ type RevisionSpec struct {
// https://github.com/knative/serving/issues/627
// +optional
Container corev1.Container `json:"container,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"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This api change (and the Tolerations below it) should include a corresponding change to the spec at https://github.com/knative/serving/blob/master/docs/spec/spec.md and the conformance tests at https://github.com/knative/serving/tree/master/test/conformance, if applicable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbrowning I updated docs just now.

I'm really not sure whether the conformance tests are applicable here, and if so, I'm not sure where to begin.


// If specified, the revision's tolerations.
// +optional
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
}

// RevisionConditionType is used to communicate the status of the reconciliation process.
Expand Down
130 changes: 130 additions & 0 deletions pkg/apis/serving/v1alpha1/revision_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@ limitations under the License.
package v1alpha1

import (
"fmt"
"strings"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation"

"github.com/knative/pkg/apis"
)
Expand All @@ -44,6 +48,16 @@ func (rs *RevisionSpec) Validate() *apis.FieldError {
if err := validateContainer(rs.Container); err != nil {
return err.ViaField("container")
}
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(
fmt.Sprintf("tolerations[%d]", i),
)
}
}
return rs.ConcurrencyModel.Validate().ViaField("concurrencyModel")
}

Expand Down Expand Up @@ -142,3 +156,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 {
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
// 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"},
}
}
}
Loading