From 1e202d7aed52c5cdd6a7978c95ab3064ab253077 Mon Sep 17 00:00:00 2001 From: Francesco Pantano <fpantano@redhat.com> Date: Mon, 18 Nov 2024 15:48:47 +0100 Subject: [PATCH 1/2] Expose PodAffinity and PodAntiAffinity struct and build overrides This patch introduces a first, basic implementation that allows to expose the Pod Affinity/Antiaffinity interface as part of the CR spec and allows to override it. Signed-off-by: Francesco Pantano <fpantano@redhat.com> --- modules/common/affinity/affinity.go | 49 ++++++++++++- modules/common/affinity/types.go | 38 ++++++++++ .../common/affinity/zz_generated.deepcopy.go | 71 +++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 modules/common/affinity/types.go create mode 100644 modules/common/affinity/zz_generated.deepcopy.go diff --git a/modules/common/affinity/affinity.go b/modules/common/affinity/affinity.go index 0e188c47..02334efd 100644 --- a/modules/common/affinity/affinity.go +++ b/modules/common/affinity/affinity.go @@ -17,8 +17,12 @@ limitations under the License. package affinity import ( + "encoding/json" + "fmt" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/strategicpatch" ) // DistributePods - returns rule to ensure that two replicas of the same selector @@ -27,8 +31,9 @@ func DistributePods( selectorKey string, selectorValues []string, topologyKey string, + overrides *AffinityOverrideSpec, ) *corev1.Affinity { - return &corev1.Affinity{ + defaultAffinity := &corev1.Affinity{ PodAntiAffinity: &corev1.PodAntiAffinity{ // This rule ensures that two replicas of the same selector // should not run if possible on the same worker node @@ -53,4 +58,46 @@ func DistributePods( }, }, } + // patch the default affinity Object with the data passed as input + if overrides != nil { + patchedAffinity, _ := toCoreAffinity(defaultAffinity, overrides) + return patchedAffinity + } + return defaultAffinity +} + +func toCoreAffinity( + affinity *v1.Affinity, + override *AffinityOverrideSpec, +) (*v1.Affinity, error) { + + aff := &v1.Affinity{ + PodAntiAffinity: affinity.PodAntiAffinity, + PodAffinity: affinity.PodAffinity, + } + if override != nil { + if override != nil { + origAffinit, err := json.Marshal(affinity) + if err != nil { + return aff, fmt.Errorf("error marshalling Affinity Spec: %w", err) + } + patch, err := json.Marshal(override) + if err != nil { + return aff, fmt.Errorf("error marshalling Affinity Spec: %w", err) + } + + patchedJSON, err := strategicpatch.StrategicMergePatch(origAffinit, patch, v1.Affinity{}) + if err != nil { + return aff, fmt.Errorf("error patching Affinity Spec: %w", err) + } + + patchedSpec := v1.Affinity{} + err = json.Unmarshal(patchedJSON, &patchedSpec) + if err != nil { + return aff, fmt.Errorf("error unmarshalling patched Service Spec: %w", err) + } + aff = &patchedSpec + } + } + return aff, nil } diff --git a/modules/common/affinity/types.go b/modules/common/affinity/types.go new file mode 100644 index 00000000..1f4247fa --- /dev/null +++ b/modules/common/affinity/types.go @@ -0,0 +1,38 @@ +/* +Copyright 2024 Red Hat + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// +kubebuilder:object:generate:=true + +package affinity + +import ( + corev1 "k8s.io/api/core/v1" +) + +// OverrideSpec - service override configuration for the Affinity propagated to the Pods +// Allows for the manifest of the created StatefulSet to be overwritten with custom Pod affinity configuration. +type OverrideSpec struct { + Spec *AffinityOverrideSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` +} + +type AffinityOverrideSpec struct { + // Describes pod affinity scheduling rules (e.g. co-locate this pod in the same node, zone, etc. as some other pod(s)). + // +optional + PodAffinity *corev1.PodAffinity `json:"podAffinity,omitempty" protobuf:"bytes,2,opt,name=podAffinity"` + // Describes pod anti-affinity scheduling rules (e.g. avoid putting this pod in the same node, zone, etc. as some other pod(s)). + // +optional + PodAntiAffinity *corev1.PodAntiAffinity `json:"podAntiAffinity,omitempty" protobuf:"bytes,3,opt,name=podAntiAffinity"` +} diff --git a/modules/common/affinity/zz_generated.deepcopy.go b/modules/common/affinity/zz_generated.deepcopy.go new file mode 100644 index 00000000..b72204df --- /dev/null +++ b/modules/common/affinity/zz_generated.deepcopy.go @@ -0,0 +1,71 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* + + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package affinity + +import ( + "k8s.io/api/core/v1" +) + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AffinityOverrideSpec) DeepCopyInto(out *AffinityOverrideSpec) { + *out = *in + if in.PodAffinity != nil { + in, out := &in.PodAffinity, &out.PodAffinity + *out = new(v1.PodAffinity) + (*in).DeepCopyInto(*out) + } + if in.PodAntiAffinity != nil { + in, out := &in.PodAntiAffinity, &out.PodAntiAffinity + *out = new(v1.PodAntiAffinity) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AffinityOverrideSpec. +func (in *AffinityOverrideSpec) DeepCopy() *AffinityOverrideSpec { + if in == nil { + return nil + } + out := new(AffinityOverrideSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OverrideSpec) DeepCopyInto(out *OverrideSpec) { + *out = *in + if in.Spec != nil { + in, out := &in.Spec, &out.Spec + *out = new(AffinityOverrideSpec) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OverrideSpec. +func (in *OverrideSpec) DeepCopy() *OverrideSpec { + if in == nil { + return nil + } + out := new(OverrideSpec) + in.DeepCopyInto(out) + return out +} From 3c061cf6e768ad1932579a4ab3fcd74d57675523 Mon Sep 17 00:00:00 2001 From: Francesco Pantano <fpantano@redhat.com> Date: Tue, 19 Nov 2024 11:10:21 +0100 Subject: [PATCH 2/2] Simplify the Affinity struct exposed to service operators This patch introduces some more wrapping around corev1.PodAffinitySpec to simplify what is exposed in the service operators. In particular, it is possible to consume the AffinityOverrides struct that can implement either affinity or antiaffinity rules. They uses the same structs behind the scenes (PodAffinityTerm and WeightedPodAffinityTerm), for both RequiredDuringSchedulingIgnoredDuringExecution and PreferredDuringSchedulingIgnoredDuringExecution, but they have a different semantic when included in the corev1.Affinity k8s object. The old behavior of distributing Pods is currently preserved for operators where we do not want to provide this interface. Signed-off-by: Francesco Pantano <fpantano@redhat.com> --- modules/common/affinity/affinity.go | 180 ++++++++++++++---- modules/common/affinity/affinity_test.go | 2 +- modules/common/affinity/types.go | 40 +++- .../common/affinity/zz_generated.deepcopy.go | 72 +++++-- 4 files changed, 235 insertions(+), 59 deletions(-) diff --git a/modules/common/affinity/affinity.go b/modules/common/affinity/affinity.go index 02334efd..c0607e7b 100644 --- a/modules/common/affinity/affinity.go +++ b/modules/common/affinity/affinity.go @@ -20,7 +20,6 @@ import ( "encoding/json" "fmt" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/strategicpatch" ) @@ -31,49 +30,51 @@ func DistributePods( selectorKey string, selectorValues []string, topologyKey string, - overrides *AffinityOverrideSpec, -) *corev1.Affinity { - defaultAffinity := &corev1.Affinity{ - PodAntiAffinity: &corev1.PodAntiAffinity{ - // This rule ensures that two replicas of the same selector - // should not run if possible on the same worker node - PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{ - { - PodAffinityTerm: corev1.PodAffinityTerm{ - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: selectorKey, - Operator: metav1.LabelSelectorOpIn, - Values: selectorValues, - }, - }, - }, - // usually corev1.LabelHostname "kubernetes.io/hostname" - // https://github.com/kubernetes/api/blob/master/core/v1/well_known_labels.go#L20 - TopologyKey: topologyKey, - }, - Weight: 100, - }, - }, + overrides *Overrides, +) (*corev1.Affinity, error) { + // By default apply an anti-affinity policy using corev1.LabelHostname as + // preferred scheduling policy: this maintains backward compatibility with + // an already deployed environment + defaultAffinity := DefaultAffinity( + Rules{ + SelectorKey: selectorKey, + SelectorValues: selectorValues, + TopologyKey: topologyKey, + Weight: DefaultPreferredWeight, }, + ) + if overrides == nil || (overrides.Affinity == nil && overrides.AntiAffinity == nil) { + return defaultAffinity, nil } - // patch the default affinity Object with the data passed as input - if overrides != nil { - patchedAffinity, _ := toCoreAffinity(defaultAffinity, overrides) - return patchedAffinity + + affinityPatch := corev1.Affinity{} + if overrides.Affinity != nil { + affinityPatch = NewAffinity(overrides.Affinity) } - return defaultAffinity + + antiAffinityPatch := corev1.Affinity{} + if overrides.AntiAffinity != nil { + antiAffinityPatch = NewAntiAffinity(overrides.AntiAffinity) + } + + overridesSpec := &OverrideSpec{ + PodAffinity: affinityPatch.PodAffinity, + PodAntiAffinity: antiAffinityPatch.PodAntiAffinity, + } + + // patch the default affinity Object with the data passed as input + patchedAffinity, err := toCoreAffinity(defaultAffinity, overridesSpec) + return patchedAffinity, err } +// toCoreAffinity - func toCoreAffinity( - affinity *v1.Affinity, - override *AffinityOverrideSpec, -) (*v1.Affinity, error) { - - aff := &v1.Affinity{ + affinity *corev1.Affinity, + override *OverrideSpec, +) (*corev1.Affinity, error) { + aff := &corev1.Affinity{ PodAntiAffinity: affinity.PodAntiAffinity, - PodAffinity: affinity.PodAffinity, + PodAffinity: affinity.PodAffinity, } if override != nil { if override != nil { @@ -85,13 +86,11 @@ func toCoreAffinity( if err != nil { return aff, fmt.Errorf("error marshalling Affinity Spec: %w", err) } - - patchedJSON, err := strategicpatch.StrategicMergePatch(origAffinit, patch, v1.Affinity{}) + patchedJSON, err := strategicpatch.StrategicMergePatch(origAffinit, patch, corev1.Affinity{}) if err != nil { return aff, fmt.Errorf("error patching Affinity Spec: %w", err) } - - patchedSpec := v1.Affinity{} + patchedSpec := corev1.Affinity{} err = json.Unmarshal(patchedJSON, &patchedSpec) if err != nil { return aff, fmt.Errorf("error unmarshalling patched Service Spec: %w", err) @@ -101,3 +100,102 @@ func toCoreAffinity( } return aff, nil } + +// WeightedPodAffinityTerm - returns a WeightedPodAffinityTerm that is assigned +// to the Affinity or AntiAffinity rule +func (affinity *Rules) WeightedPodAffinityTerm() []corev1.WeightedPodAffinityTerm { + if affinity == nil { + return []corev1.WeightedPodAffinityTerm{} + } + affinityTerm := []corev1.WeightedPodAffinityTerm{ + { + Weight: affinity.Weight, + PodAffinityTerm: corev1.PodAffinityTerm{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: affinity.SelectorKey, + Operator: metav1.LabelSelectorOpIn, + Values: affinity.SelectorValues, + }, + }, + }, + TopologyKey: affinity.TopologyKey, + }, + }, + } + return affinityTerm +} + +// PodAffinityTerm - +func (affinity *Rules) PodAffinityTerm() []corev1.PodAffinityTerm { + if affinity == nil { + return []corev1.PodAffinityTerm{} + } + affinityTerm := []corev1.PodAffinityTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: affinity.SelectorKey, + Operator: metav1.LabelSelectorOpIn, + Values: affinity.SelectorValues, + }, + }, + }, + TopologyKey: affinity.TopologyKey, + }, + } + return affinityTerm +} + +// NewAffinity - +func NewAffinity(p *PodScheduling) corev1.Affinity { + aff := &corev1.Affinity{ + PodAffinity: &corev1.PodAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: p.RequiredScheduling.PodAffinityTerm(), + PreferredDuringSchedulingIgnoredDuringExecution: p.PreferredScheduling.WeightedPodAffinityTerm(), + }, + } + return *aff +} + +// NewAntiAffinity - +func NewAntiAffinity(p *PodScheduling) corev1.Affinity { + aff := &corev1.Affinity{ + PodAntiAffinity: &corev1.PodAntiAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: p.RequiredScheduling.PodAffinityTerm(), + PreferredDuringSchedulingIgnoredDuringExecution: p.PreferredScheduling.WeightedPodAffinityTerm(), + }, + } + return *aff +} + +// DefaultAffinity - +func DefaultAffinity(aff Rules) *corev1.Affinity { + return &corev1.Affinity{ + PodAntiAffinity: &corev1.PodAntiAffinity{ + // This rule ensures that two replicas of the same selector + // should not run if possible on the same worker node + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.WeightedPodAffinityTerm{ + { + PodAffinityTerm: corev1.PodAffinityTerm{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: aff.SelectorKey, + Operator: metav1.LabelSelectorOpIn, + Values: aff.SelectorValues, + }, + }, + }, + // usually corev1.LabelHostname "kubernetes.io/hostname" + // https://github.com/kubernetes/api/blob/master/core/v1/well_known_labels.go#L20 + TopologyKey: aff.TopologyKey, + }, + Weight: aff.Weight, + }, + }, + }, + } +} diff --git a/modules/common/affinity/affinity_test.go b/modules/common/affinity/affinity_test.go index 16a142dd..59849573 100644 --- a/modules/common/affinity/affinity_test.go +++ b/modules/common/affinity/affinity_test.go @@ -52,7 +52,7 @@ func TestDistributePods(t *testing.T) { t.Run("Default pod distribution", func(t *testing.T) { g := NewWithT(t) - d := DistributePods("ThisSelector", []string{"selectorValue1", "selectorValue2"}, "ThisTopologyKey") + d, _ := DistributePods("ThisSelector", []string{"selectorValue1", "selectorValue2"}, "ThisTopologyKey", nil) g.Expect(d).To(BeEquivalentTo(affinityObj)) }) diff --git a/modules/common/affinity/types.go b/modules/common/affinity/types.go index 1f4247fa..b12a448b 100644 --- a/modules/common/affinity/types.go +++ b/modules/common/affinity/types.go @@ -22,13 +22,13 @@ import ( corev1 "k8s.io/api/core/v1" ) -// OverrideSpec - service override configuration for the Affinity propagated to the Pods -// Allows for the manifest of the created StatefulSet to be overwritten with custom Pod affinity configuration. -type OverrideSpec struct { - Spec *AffinityOverrideSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"` -} +const ( + // DefaultPreferredWeight - + DefaultPreferredWeight = 100 +) -type AffinityOverrideSpec struct { +// OverrideSpec - +type OverrideSpec struct { // Describes pod affinity scheduling rules (e.g. co-locate this pod in the same node, zone, etc. as some other pod(s)). // +optional PodAffinity *corev1.PodAffinity `json:"podAffinity,omitempty" protobuf:"bytes,2,opt,name=podAffinity"` @@ -36,3 +36,31 @@ type AffinityOverrideSpec struct { // +optional PodAntiAffinity *corev1.PodAntiAffinity `json:"podAntiAffinity,omitempty" protobuf:"bytes,3,opt,name=podAntiAffinity"` } + +// Rules - +// +kubebuilder:object:generate:=true +type Rules struct { + // +kubebuilder:validation:Optional + SelectorKey string `json:"selectorKey,omitempty" protobuf:"bytes,2,opt,name=selectorKey"` + // +kubebuilder:validation:Optional + SelectorValues []string `json:"selectorValues,omitempty" protobuf:"bytes,2,opt,name=selectorValues"` + // https://github.com/kubernetes/api/blob/master/core/v1/well_known_labels.go#L20 + // +kubebuilder:validation:Optional + TopologyKey string `json:"topologyKey,omitempty" protobuf:"bytes,2,opt,name=topologyKey"` + // +kubebuilder:validation:Optional + Weight int32 `json:"weight,omitempty" protobuf:"bytes,2,opt,name=weight"` +} + +// PodScheduling - +// +kubebuilder:object:generate:=true +type PodScheduling struct { + RequiredScheduling *Rules `json:"required,omitempty" protobuf:"bytes,2,opt,name=required"` + PreferredScheduling *Rules `json:"preferred,omitempty" protobuf:"bytes,2,opt,name=referred"` +} + +// Overrides - +// +kubebuilder:object:generate:=true +type Overrides struct { + Affinity *PodScheduling `json:"affinity,omitempty"` + AntiAffinity *PodScheduling `json:"antiAffinity,omitempty"` +} diff --git a/modules/common/affinity/zz_generated.deepcopy.go b/modules/common/affinity/zz_generated.deepcopy.go index b72204df..0924f7f8 100644 --- a/modules/common/affinity/zz_generated.deepcopy.go +++ b/modules/common/affinity/zz_generated.deepcopy.go @@ -26,7 +26,7 @@ import ( ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AffinityOverrideSpec) DeepCopyInto(out *AffinityOverrideSpec) { +func (in *OverrideSpec) DeepCopyInto(out *OverrideSpec) { *out = *in if in.PodAffinity != nil { in, out := &in.PodAffinity, &out.PodAffinity @@ -40,32 +40,82 @@ func (in *AffinityOverrideSpec) DeepCopyInto(out *AffinityOverrideSpec) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AffinityOverrideSpec. -func (in *AffinityOverrideSpec) DeepCopy() *AffinityOverrideSpec { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OverrideSpec. +func (in *OverrideSpec) DeepCopy() *OverrideSpec { if in == nil { return nil } - out := new(AffinityOverrideSpec) + out := new(OverrideSpec) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *OverrideSpec) DeepCopyInto(out *OverrideSpec) { +func (in *Overrides) DeepCopyInto(out *Overrides) { *out = *in - if in.Spec != nil { - in, out := &in.Spec, &out.Spec - *out = new(AffinityOverrideSpec) + if in.Affinity != nil { + in, out := &in.Affinity, &out.Affinity + *out = new(PodScheduling) + (*in).DeepCopyInto(*out) + } + if in.AntiAffinity != nil { + in, out := &in.AntiAffinity, &out.AntiAffinity + *out = new(PodScheduling) (*in).DeepCopyInto(*out) } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OverrideSpec. -func (in *OverrideSpec) DeepCopy() *OverrideSpec { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Overrides. +func (in *Overrides) DeepCopy() *Overrides { if in == nil { return nil } - out := new(OverrideSpec) + out := new(Overrides) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PodScheduling) DeepCopyInto(out *PodScheduling) { + *out = *in + if in.RequiredScheduling != nil { + in, out := &in.RequiredScheduling, &out.RequiredScheduling + *out = new(Rules) + (*in).DeepCopyInto(*out) + } + if in.PreferredScheduling != nil { + in, out := &in.PreferredScheduling, &out.PreferredScheduling + *out = new(Rules) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodScheduling. +func (in *PodScheduling) DeepCopy() *PodScheduling { + if in == nil { + return nil + } + out := new(PodScheduling) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Rules) DeepCopyInto(out *Rules) { + *out = *in + if in.SelectorValues != nil { + in, out := &in.SelectorValues, &out.SelectorValues + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Rules. +func (in *Rules) DeepCopy() *Rules { + if in == nil { + return nil + } + out := new(Rules) in.DeepCopyInto(out) return out }