Skip to content
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

Add affinity-rules feature to configmap config-deployment #15250

Merged
merged 23 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ee19514
add affinity-rules to config-deployment configmap
izabelacg May 24, 2024
3b9f7aa
run ./hack/update-codegen.sh
izabelacg May 27, 2024
dd0d599
change affinity rules property to be a flag
izabelacg May 27, 2024
0d0bdc7
run ./hack/update-codegen.sh
izabelacg May 27, 2024
4d73f10
add default pod anti-affinity rules to PodSpec
izabelacg May 27, 2024
92f0ab5
re-arrange imports
izabelacg May 27, 2024
a633eba
enable pod anti affinity by default
izabelacg May 27, 2024
7d0f070
fix value in config-deployment
izabelacg May 28, 2024
cc096da
fix condition for adding pod anti-affinity based on presence of a label
izabelacg May 28, 2024
bad694c
run ./hack/update-codegen.sh
izabelacg May 28, 2024
e56c559
clean up deploy tests
izabelacg May 28, 2024
9ef849b
change property name
izabelacg May 28, 2024
8daaa67
enable pod anti affinity by default
izabelacg May 29, 2024
a2610e4
update deployment.yaml
izabelacg May 29, 2024
905443c
adds new default for enable pod anti affinity to existing tests
izabelacg May 29, 2024
def1a18
change affinity type from toggle to string
izabelacg Jun 3, 2024
da265ce
run ./hack/update-codegen.sh
izabelacg Jun 3, 2024
1e5f200
fix condition to apply podspec
izabelacg Jun 3, 2024
3470332
tweak when applying the defaults
izabelacg Jun 5, 2024
376cd87
simplify condition that apply affinity defaults
izabelacg Jun 5, 2024
1f4fde2
rename new field to default-affinity-type
izabelacg Jun 5, 2024
7dec43d
replace usage of old name affinity
izabelacg Jun 5, 2024
6c9252f
rename test cases
izabelacg Jun 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion config/core/configmaps/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ metadata:
app.kubernetes.io/component: controller
app.kubernetes.io/version: devel
annotations:
knative.dev/example-checksum: "ed77183a"
knative.dev/example-checksum: "e2f637c6"
data:
# This is the Go import path for the binary that is containerized
# and substituted here.
Expand Down Expand Up @@ -91,3 +91,20 @@ data:
# Sets rootCA for the queue proxy - used by QPOptions
# If omitted, or empty, no rootCA is added to the golang rootCAs
queue-sidecar-rootca: ""

# If set, it automatically configures pod anti-affinity requirements for all Knative services.
# It employs the `preferredDuringSchedulingIgnoredDuringExecution` weighted pod affinity term,
# aligning with the Knative revision label. It yields the configuration below in all workloads' deployments:
# `
# affinity:
# podAntiAffinity:
# preferredDuringSchedulingIgnoredDuringExecution:
# - podAffinityTerm:
# topologyKey: kubernetes.io/hostname
# labelSelector:
# matchLabels:
# serving.knative.dev/revision: {{revision-name}}
# weight: 100
# `
# This may be "none" or "prefer-spread-revision-over-nodes" (default)
# default-affinity-type: "prefer-spread-revision-over-nodes"
28 changes: 27 additions & 1 deletion pkg/deployment/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/sets"

cm "knative.dev/pkg/configmap"
)

Expand Down Expand Up @@ -68,6 +67,9 @@ const (
// qpoptions
queueSidecarTokenAudiencesKey = "queue-sidecar-token-audiences"
queueSidecarRooCAKey = "queue-sidecar-rootca"

defaultAffinityTypeKey = "default-affinity-type"
defaultAffinityTypeValue = PreferSpreadRevisionOverNodes
)

var (
Expand Down Expand Up @@ -103,6 +105,7 @@ func defaultConfig() *Config {
DigestResolutionTimeout: digestResolutionTimeoutDefault,
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
DefaultAffinityType: defaultAffinityTypeValue,
}
// The following code is needed for ConfigMap testing.
// defaultConfig must match the example in deployment.yaml which includes: `queue-sidecar-token-audiences: ""`
Expand Down Expand Up @@ -164,6 +167,14 @@ func NewConfigFromMap(configMap map[string]string) (*Config, error) {
return nil, fmt.Errorf("digest-resolution-timeout cannot be a non-positive duration, was %v", nc.DigestResolutionTimeout)
}

if affinity, ok := configMap[defaultAffinityTypeKey]; ok {
switch opt := AffinityType(affinity); opt {
case None, PreferSpreadRevisionOverNodes:
nc.DefaultAffinityType = opt
default:
return nil, fmt.Errorf("unsupported %s value: %q", defaultAffinityTypeKey, affinity)
}
}
return nc, nil
}

Expand All @@ -172,6 +183,17 @@ func NewConfigFromConfigMap(config *corev1.ConfigMap) (*Config, error) {
return NewConfigFromMap(config.Data)
}

// AffinityType specifies which affinity requirements will be automatically applied to the PodSpec of all Knative services.
type AffinityType string

const (
// None is used for deactivating affinity configuration for user workloads.
None AffinityType = "none"

// PreferSpreadRevisionOverNodes is used to set pod anti-affinity requirements for user workloads.
PreferSpreadRevisionOverNodes AffinityType = "prefer-spread-revision-over-nodes"
)

// Config includes the configurations for the controller.
type Config struct {
// QueueSidecarImage is the name of the image used for the queue sidecar
Expand Down Expand Up @@ -214,4 +236,8 @@ type Config struct {

// QueueSidecarRootCA is a root certificate to be trusted by the queue proxy sidecar qpoptions.
QueueSidecarRootCA string

// DefaultAffinityType is a string that controls what affinity rules will be automatically
// applied to the PodSpec of all Knative services.
DefaultAffinityType AffinityType
}
65 changes: 65 additions & 0 deletions pkg/deployment/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,64 @@ func TestControllerConfiguration(t *testing.T) {
wantConfig *Config
data map[string]string
}{{
name: "controller configuration with no default affinity type specified",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
DigestResolutionTimeout: digestResolutionTimeoutDefault,
QueueSidecarImage: defaultSidecarImage,
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
},
}, {
name: "controller configuration with empty string set for the default affinity type",
wantErr: true,
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
defaultAffinityTypeKey: "",
},
}, {
name: "controller configuration with unsupported value for default affinity type",
wantErr: true,
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
defaultAffinityTypeKey: "coconut",
},
}, {
name: "controller configuration with the default affinity type set",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
DigestResolutionTimeout: digestResolutionTimeoutDefault,
QueueSidecarImage: defaultSidecarImage,
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
defaultAffinityTypeKey: string(PreferSpreadRevisionOverNodes),
},
}, {
name: "controller configuration with default affinity type deactivated",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
DigestResolutionTimeout: digestResolutionTimeoutDefault,
QueueSidecarImage: defaultSidecarImage,
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
DefaultAffinityType: None,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
defaultAffinityTypeKey: string(None),
},
}, {
name: "controller configuration with bad registries",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("ko.local", ""),
Expand All @@ -89,6 +147,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New("foo", "bar", "boo-srv"),
ProgressDeadline: ProgressDeadlineDefault,
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -104,6 +163,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: 444 * time.Second,
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -118,6 +178,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -132,6 +193,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -151,6 +213,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarMemoryLimit: quantity("654m"),
QueueSidecarEphemeralStorageLimit: quantity("321M"),
QueueSidecarTokenAudiences: sets.New(""),
DefaultAffinityType: defaultAffinityTypeValue,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand Down Expand Up @@ -227,6 +290,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarEphemeralStorageRequest: quantity("9M"),
QueueSidecarEphemeralStorageLimit: quantity("10M"),
QueueSidecarTokenAudiences: sets.New(""),
DefaultAffinityType: defaultAffinityTypeValue,
},
}, {
name: "newer key case takes priority",
Expand Down Expand Up @@ -268,6 +332,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarEphemeralStorageRequest: quantity("20M"),
QueueSidecarEphemeralStorageLimit: quantity("21M"),
QueueSidecarTokenAudiences: sets.New("foo"),
DefaultAffinityType: defaultAffinityTypeValue,
},
}}

Expand Down
21 changes: 21 additions & 0 deletions pkg/reconciler/revision/resources/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"

apiconfig "knative.dev/serving/pkg/apis/config"
deploymentconfig "knative.dev/serving/pkg/deployment"
)

const certVolumeName = "server-certs"
Expand Down Expand Up @@ -150,6 +151,22 @@ func rewriteUserLivenessProbe(p *corev1.Probe, userPort int) {
}
}

func makePreferSpreadRevisionOverNodes(revisionLabelValue string) *corev1.PodAntiAffinity {
return &corev1.PodAntiAffinity{
PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{{
Weight: 100,
PodAffinityTerm: corev1.PodAffinityTerm{
TopologyKey: corev1.LabelHostname,
LabelSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
serving.RevisionLabelKey: revisionLabelValue,
},
},
},
}},
}
}

func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error) {
queueContainer, err := makeQueueContainer(rev, cfg)
tokenVolume := varTokenVolume.DeepCopy()
Expand Down Expand Up @@ -210,6 +227,10 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error)
}
}

if cfg.Deployment.DefaultAffinityType == deploymentconfig.PreferSpreadRevisionOverNodes && rev.Spec.Affinity == nil {
podSpec.Affinity = &corev1.Affinity{PodAntiAffinity: makePreferSpreadRevisionOverNodes(rev.Name)}
}

return podSpec, nil
}

Expand Down
117 changes: 117 additions & 0 deletions pkg/reconciler/revision/resources/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,31 @@ var (
EnableServiceLinks: ptr.Bool(false),
}

defaultPodAntiAffinityRules = &corev1.PodAntiAffinity{
PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{{
Weight: 100,
PodAffinityTerm: corev1.PodAffinityTerm{
TopologyKey: "kubernetes.io/hostname",
LabelSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"serving.knative.dev/revision": "bar",
},
},
},
}},
}

userDefinedPodAntiAffinityRules = &corev1.PodAntiAffinity{
RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{
TopologyKey: "kubernetes.io/hostname",
LabelSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{
"serving.knative.dev/revision": "bar",
},
},
}},
}

maxUnavailable = intstr.FromInt(0)
defaultDeployment = &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1409,6 +1434,98 @@ func TestMakePodSpec(t *testing.T) {
withEnvVar("SERVING_READINESS_PROBE", `[{"httpGet":{"path":"/","port":8080,"host":"127.0.0.1","scheme":"HTTP"}},{"httpGet":{"path":"/","port":8090,"host":"127.0.0.1","scheme":"HTTP"}}]`),
),
}),
}, {
name: "with default affinity type set",
rev: revision("bar", "foo",
withContainers([]corev1.Container{{
Name: servingContainerName,
Image: "busybox",
ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort),
}}),
WithContainerStatuses([]v1.ContainerStatus{{
ImageDigest: "busybox@sha256:deadbeef",
}}),
),
fc: apicfg.Features{
PodSpecAffinity: apicfg.Disabled,
},
dc: deployment.Config{
DefaultAffinityType: deployment.PreferSpreadRevisionOverNodes,
},
want: podSpec(
[]corev1.Container{
servingContainer(func(container *corev1.Container) {
container.Image = "busybox@sha256:deadbeef"
}),
queueContainer(),
},
func(p *corev1.PodSpec) {
p.Affinity = &corev1.Affinity{
PodAntiAffinity: defaultPodAntiAffinityRules,
}
},
),
}, {
name: "with default affinity type deactivated",
rev: revision("bar", "foo",
withContainers([]corev1.Container{{
Name: servingContainerName,
Image: "busybox",
ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort),
}}),
WithContainerStatuses([]v1.ContainerStatus{{
ImageDigest: "busybox@sha256:deadbeef",
}}),
),
fc: apicfg.Features{
PodSpecAffinity: apicfg.Disabled,
},
dc: deployment.Config{
DefaultAffinityType: deployment.None,
},
want: podSpec(
[]corev1.Container{
servingContainer(func(container *corev1.Container) {
container.Image = "busybox@sha256:deadbeef"
}),
queueContainer(),
},
),
}, {
name: "with affinity rules set by both the user and the operator",
rev: revision("bar", "foo",
withContainers([]corev1.Container{{
Name: servingContainerName,
Image: "busybox",
ReadinessProbe: withTCPReadinessProbe(v1.DefaultUserPort),
}}),
WithContainerStatuses([]v1.ContainerStatus{{
ImageDigest: "busybox@sha256:deadbeef",
}}),
func(r *v1.Revision) {
r.Spec.Affinity = &corev1.Affinity{
PodAntiAffinity: userDefinedPodAntiAffinityRules,
}
}),
fc: apicfg.Features{
PodSpecAffinity: apicfg.Enabled,
},
dc: deployment.Config{
DefaultAffinityType: deployment.PreferSpreadRevisionOverNodes,
},
want: podSpec(
[]corev1.Container{
servingContainer(func(container *corev1.Container) {
container.Image = "busybox@sha256:deadbeef"
}),
queueContainer(),
},
func(p *corev1.PodSpec) {
p.Affinity = &corev1.Affinity{
PodAntiAffinity: userDefinedPodAntiAffinityRules,
}
},
),
}}

for _, test := range tests {
Expand Down
Loading