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 15 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: "1b231253"
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 to "true", 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
# `
#
# enable-pod-anti-affinity-rule: "true"
izabelacg marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 11 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,10 @@ const (
// qpoptions
queueSidecarTokenAudiencesKey = "queue-sidecar-token-audiences"
queueSidecarRooCAKey = "queue-sidecar-rootca"

enablePodAntiAffinityRule = "enable-pod-anti-affinity-rule"

EnablePodAntiAffinityRuleDefault = true
)

var (
Expand Down Expand Up @@ -103,6 +106,7 @@ func defaultConfig() *Config {
DigestResolutionTimeout: digestResolutionTimeoutDefault,
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
}
// 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 @@ -144,6 +148,8 @@ func NewConfigFromMap(configMap map[string]string) (*Config, error) {

cm.AsStringSet(queueSidecarTokenAudiencesKey, &nc.QueueSidecarTokenAudiences),
cm.AsString(queueSidecarRooCAKey, &nc.QueueSidecarRootCA),

cm.AsBool(enablePodAntiAffinityRule, &nc.EnablePodAntiAffinityRule),
); err != nil {
return nil, err
}
Expand Down Expand Up @@ -214,4 +220,8 @@ type Config struct {

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

// EnablePodAntiAffinityRule is a flag that controls if pod anti-affinity rules will be automatically
// applied to the PodSpec of all Knative services.
EnablePodAntiAffinityRule bool
}
50 changes: 50 additions & 0 deletions pkg/deployment/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,49 @@ func TestControllerConfiguration(t *testing.T) {
wantConfig *Config
data map[string]string
}{{
name: "controller configuration with no pod anti-affinity toggle specified",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
DigestResolutionTimeout: digestResolutionTimeoutDefault,
QueueSidecarImage: defaultSidecarImage,
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
},
}, {
name: "controller configuration with empty string for the pod anti-affinity toggle",
wantErr: true,
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
enablePodAntiAffinityRule: "",
},
}, {
name: "controller configuration with wrong type for the pod anti-affinity toggle",
wantErr: true,
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
enablePodAntiAffinityRule: "coconut",
},
}, {
name: "controller configuration with the pod anti-affinity toggle on",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("kind.local", "ko.local", "dev.local"),
DigestResolutionTimeout: digestResolutionTimeoutDefault,
QueueSidecarImage: defaultSidecarImage,
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
EnablePodAntiAffinityRule: true,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
enablePodAntiAffinityRule: "true",
},
}, {
name: "controller configuration with bad registries",
wantConfig: &Config{
RegistriesSkippingTagResolving: sets.New("ko.local", ""),
Expand All @@ -89,6 +132,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New("foo", "bar", "boo-srv"),
ProgressDeadline: ProgressDeadlineDefault,
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -104,6 +148,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: 444 * time.Second,
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -118,6 +163,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -132,6 +178,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarCPURequest: &QueueSidecarCPURequestDefault,
QueueSidecarTokenAudiences: sets.New(""),
ProgressDeadline: ProgressDeadlineDefault,
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand All @@ -151,6 +198,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarMemoryLimit: quantity("654m"),
QueueSidecarEphemeralStorageLimit: quantity("321M"),
QueueSidecarTokenAudiences: sets.New(""),
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
data: map[string]string{
QueueSidecarImageKey: defaultSidecarImage,
Expand Down Expand Up @@ -227,6 +275,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarEphemeralStorageRequest: quantity("9M"),
QueueSidecarEphemeralStorageLimit: quantity("10M"),
QueueSidecarTokenAudiences: sets.New(""),
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
}, {
name: "newer key case takes priority",
Expand Down Expand Up @@ -268,6 +317,7 @@ func TestControllerConfiguration(t *testing.T) {
QueueSidecarEphemeralStorageRequest: quantity("20M"),
QueueSidecarEphemeralStorageLimit: quantity("21M"),
QueueSidecarTokenAudiences: sets.New("foo"),
EnablePodAntiAffinityRule: EnablePodAntiAffinityRuleDefault,
},
}}

Expand Down
20 changes: 20 additions & 0 deletions pkg/reconciler/revision/resources/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,22 @@ func rewriteUserLivenessProbe(p *corev1.Probe, userPort int) {
}
}

func makeDefaultPodAntiAffinity(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 +226,10 @@ func makePodSpec(rev *v1.Revision, cfg *config.Config) (*corev1.PodSpec, error)
}
}

if cfg.Deployment.EnablePodAntiAffinityRule && cfg.Features.PodSpecAffinity == apiconfig.Disabled {
podSpec.Affinity = &corev1.Affinity{PodAntiAffinity: makeDefaultPodAntiAffinity(rev.Name)}
}

return podSpec, nil
}

Expand Down
97 changes: 97 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,20 @@ 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",
},
},
},
}},
}

maxUnavailable = intstr.FromInt(0)
defaultDeployment = &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1409,6 +1423,89 @@ 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 pod anti-affinity rules",
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{
EnablePodAntiAffinityRule: true,
},
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 pod anti-affinity rules toggle off",
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{
EnablePodAntiAffinityRule: false,
},
want: podSpec(
[]corev1.Container{
servingContainer(func(container *corev1.Container) {
container.Image = "busybox@sha256:deadbeef"
}),
queueContainer(),
},
),
}, {
name: "with pod anti-affinity rules toggle on for both users and operators",
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.Enabled,
},
dc: deployment.Config{
EnablePodAntiAffinityRule: true,
},
want: podSpec(
[]corev1.Container{
servingContainer(func(container *corev1.Container) {
container.Image = "busybox@sha256:deadbeef"
}),
queueContainer(),
},
),
}}

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