From 0d376081b7681ebd5b60c33cb9c72122b50d6260 Mon Sep 17 00:00:00 2001 From: Martin Schuppert Date: Wed, 6 Aug 2025 08:13:23 +0200 Subject: [PATCH] Fix tolerations API placement and implementation Move tolerations from ContainerSpec to OperatorSpec level since tolerations are deployment/pod-level configuration, not container-level. Changes: - Move Tolerations field from ContainerSpec to OperatorSpec in API types - Update override logic to use correct field path (opOvr.Tolerations) - Fix test cases to use OperatorSpec.Tolerations instead of ControllerManager.Tolerations - Fix YAML indentation in sample configuration This corrects the API design to follow Kubernetes conventions where tolerations are specified at the pod template level, not per container. Signed-off-by: Martin Schuppert --- .../operator.openstack.org_openstacks.yaml | 32 +++++----- apis/operator/v1beta1/openstack_types.go | 10 +-- .../operator/v1beta1/zz_generated.deepcopy.go | 14 ++-- .../operator.openstack.org_openstacks.yaml | 32 +++++----- ...v1beta1_openstack_tolerations_example.yaml | 56 ++++++++-------- pkg/operator/override.go | 4 +- pkg/operator/override_test.go | 64 ++++++++----------- 7 files changed, 99 insertions(+), 113 deletions(-) diff --git a/apis/bases/operator.openstack.org_openstacks.yaml b/apis/bases/operator.openstack.org_openstacks.yaml index 78d156b307..6097c962eb 100644 --- a/apis/bases/operator.openstack.org_openstacks.yaml +++ b/apis/bases/operator.openstack.org_openstacks.yaml @@ -70,22 +70,6 @@ spec: x-kubernetes-int-or-string: true type: object type: object - tolerations: - items: - properties: - effect: - type: string - key: - type: string - operator: - type: string - tolerationSeconds: - format: int64 - type: integer - value: - type: string - type: object - type: array type: object name: enum: @@ -118,6 +102,22 @@ spec: maximum: 1 minimum: 0 type: integer + tolerations: + items: + properties: + effect: + type: string + key: + type: string + operator: + type: string + tolerationSeconds: + format: int64 + type: integer + value: + type: string + type: object + type: array required: - name type: object diff --git a/apis/operator/v1beta1/openstack_types.go b/apis/operator/v1beta1/openstack_types.go index e6b4f6c8b8..660af02b3b 100644 --- a/apis/operator/v1beta1/openstack_types.go +++ b/apis/operator/v1beta1/openstack_types.go @@ -220,6 +220,11 @@ type OperatorSpec struct { // +kubebuilder:validation:Optional // ControllerManager - tunings for the controller manager container ControllerManager ContainerSpec `json:"controllerManager,omitempty"` + + // +kubebuilder:validation:Optional + // Tolerations - Tolerations for the service operator deployment pods + // https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/ + Tolerations []corev1.Toleration `json:"tolerations,omitempty"` } // ContainerSpec - customization for the container spec @@ -228,11 +233,6 @@ type ContainerSpec struct { // Resources - Compute Resources for the service operator controller manager // https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ Resources corev1.ResourceRequirements `json:"resources,omitempty"` - - // +kubebuilder:validation:Optional - // Tolerations - Tolerations for the service operator controller manager - // https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/ - Tolerations []corev1.Toleration `json:"tolerations,omitempty"` } // OpenStackStatus defines the observed state of OpenStack diff --git a/apis/operator/v1beta1/zz_generated.deepcopy.go b/apis/operator/v1beta1/zz_generated.deepcopy.go index 7cd2cf52f1..64cc6b57c8 100644 --- a/apis/operator/v1beta1/zz_generated.deepcopy.go +++ b/apis/operator/v1beta1/zz_generated.deepcopy.go @@ -30,13 +30,6 @@ import ( func (in *ContainerSpec) DeepCopyInto(out *ContainerSpec) { *out = *in in.Resources.DeepCopyInto(&out.Resources) - if in.Tolerations != nil { - in, out := &in.Tolerations, &out.Tolerations - *out = make([]v1.Toleration, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ContainerSpec. @@ -186,6 +179,13 @@ func (in *OperatorSpec) DeepCopyInto(out *OperatorSpec) { **out = **in } in.ControllerManager.DeepCopyInto(&out.ControllerManager) + if in.Tolerations != nil { + in, out := &in.Tolerations, &out.Tolerations + *out = make([]v1.Toleration, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorSpec. diff --git a/config/crd/bases/operator.openstack.org_openstacks.yaml b/config/crd/bases/operator.openstack.org_openstacks.yaml index 78d156b307..6097c962eb 100644 --- a/config/crd/bases/operator.openstack.org_openstacks.yaml +++ b/config/crd/bases/operator.openstack.org_openstacks.yaml @@ -70,22 +70,6 @@ spec: x-kubernetes-int-or-string: true type: object type: object - tolerations: - items: - properties: - effect: - type: string - key: - type: string - operator: - type: string - tolerationSeconds: - format: int64 - type: integer - value: - type: string - type: object - type: array type: object name: enum: @@ -118,6 +102,22 @@ spec: maximum: 1 minimum: 0 type: integer + tolerations: + items: + properties: + effect: + type: string + key: + type: string + operator: + type: string + tolerationSeconds: + format: int64 + type: integer + value: + type: string + type: object + type: array required: - name type: object diff --git a/config/samples/operator_v1beta1_openstack_tolerations_example.yaml b/config/samples/operator_v1beta1_openstack_tolerations_example.yaml index c11b75d664..74035e5cf2 100644 --- a/config/samples/operator_v1beta1_openstack_tolerations_example.yaml +++ b/config/samples/operator_v1beta1_openstack_tolerations_example.yaml @@ -28,36 +28,34 @@ spec: operatorOverrides: - name: "keystone" # Custom tolerations for keystone operator pods - controllerManager: - tolerations: - - key: "example.com/special-nodes" - operator: "Equal" - value: "keystone" - effect: "NoSchedule" - - key: "node.kubernetes.io/memory-pressure" - operator: "Exists" - effect: "NoExecute" - tolerationSeconds: 300 + tolerations: + - key: "example.com/special-nodes" + operator: "Equal" + value: "keystone" + effect: "NoSchedule" + - key: "node.kubernetes.io/memory-pressure" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 300 - name: "nova" # Example: Override default tolerations and add new ones # Result will be: # 1. node.kubernetes.io/not-ready (OVERRIDDEN - 600s instead of 120s) # 2. node.kubernetes.io/unreachable (OVERRIDDEN - 400s instead of 120s) # 3. node.example.com/compute (ADDED - new toleration) - controllerManager: - tolerations: - - key: "node.kubernetes.io/not-ready" - operator: "Exists" - effect: "NoExecute" - tolerationSeconds: 600 # Override default 120s - - key: "node.kubernetes.io/unreachable" - operator: "Exists" - effect: "NoExecute" - tolerationSeconds: 400 # Override default 120s - - key: "node.example.com/compute" # Add new toleration - operator: "Equal" - value: "true" - effect: "NoSchedule" + tolerations: + - key: "node.kubernetes.io/not-ready" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 600 # Override default 120s + - key: "node.kubernetes.io/unreachable" + operator: "Exists" + effect: "NoExecute" + tolerationSeconds: 400 # Override default 120s + - key: "node.example.com/compute" # Add new toleration + operator: "Equal" + value: "true" + effect: "NoSchedule" - name: "glance" # Custom resource limits AND tolerations example controllerManager: @@ -68,9 +66,9 @@ spec: requests: cpu: "1" memory: "2Gi" - tolerations: - - key: "storage-node" - operator: "Equal" - value: "true" - effect: "NoSchedule" + tolerations: + - key: "storage-node" + operator: "Equal" + value: "true" + effect: "NoSchedule" # Note: Operators not listed (like mariadb, neutron, etc.) will use the default tolerations diff --git a/pkg/operator/override.go b/pkg/operator/override.go index 6de1efb6b7..d62653de49 100644 --- a/pkg/operator/override.go +++ b/pkg/operator/override.go @@ -109,8 +109,8 @@ func SetOverrides(opOvr operatorv1beta1.OperatorSpec, op *Operator) { op.Deployment.Manager.Resources.Requests.Memory = opOvr.ControllerManager.Resources.Requests.Memory().String() } } - if len(opOvr.ControllerManager.Tolerations) > 0 { - op.Deployment.Tolerations = mergeTolerations(op.Deployment.Tolerations, opOvr.ControllerManager.Tolerations) + if len(opOvr.Tolerations) > 0 { + op.Deployment.Tolerations = mergeTolerations(op.Deployment.Tolerations, opOvr.Tolerations) } } diff --git a/pkg/operator/override_test.go b/pkg/operator/override_test.go index 09195e7249..8bfb90d202 100644 --- a/pkg/operator/override_test.go +++ b/pkg/operator/override_test.go @@ -157,10 +157,8 @@ func TestApplyOperatorOverrides(t *testing.T) { }, }, { - Name: "nova", - ControllerManager: operatorv1beta1.ContainerSpec{ - Tolerations: customTolerations, - }, + Name: "nova", + Tolerations: customTolerations, }, } @@ -363,10 +361,8 @@ func TestTolerationsOverride(t *testing.T) { { name: "Add tolerations to empty list", operatorSpec: operatorv1beta1.OperatorSpec{ - Name: "test-operator", - ControllerManager: operatorv1beta1.ContainerSpec{ - Tolerations: testTolerations, - }, + Name: "test-operator", + Tolerations: testTolerations, }, initialTolerations: nil, expectedTolerations: testTolerations, @@ -374,10 +370,8 @@ func TestTolerationsOverride(t *testing.T) { { name: "No custom tolerations, keep defaults unchanged", operatorSpec: operatorv1beta1.OperatorSpec{ - Name: "test-operator", - ControllerManager: operatorv1beta1.ContainerSpec{ - // No tolerations specified - }, + Name: "test-operator", + // No tolerations specified }, initialTolerations: defaultTolerations, expectedTolerations: defaultTolerations, @@ -385,10 +379,8 @@ func TestTolerationsOverride(t *testing.T) { { name: "Merge custom tolerations with defaults (different keys)", operatorSpec: operatorv1beta1.OperatorSpec{ - Name: "test-operator", - ControllerManager: operatorv1beta1.ContainerSpec{ - Tolerations: testTolerations, // Different keys than defaults - }, + Name: "test-operator", + Tolerations: testTolerations, // Different keys than defaults }, initialTolerations: defaultTolerations, expectedTolerations: append(defaultTolerations, testTolerations...), @@ -397,14 +389,12 @@ func TestTolerationsOverride(t *testing.T) { name: "Override default tolerations (same key)", operatorSpec: operatorv1beta1.OperatorSpec{ Name: "test-operator", - ControllerManager: operatorv1beta1.ContainerSpec{ - Tolerations: []corev1.Toleration{ - { - Key: corev1.TaintNodeNotReady, // "node.kubernetes.io/not-ready", // Same key as default - Operator: corev1.TolerationOpExists, - Effect: corev1.TaintEffectNoExecute, - TolerationSeconds: ptr.To[int64](600), // Different value - }, + Tolerations: []corev1.Toleration{ + { + Key: corev1.TaintNodeNotReady, // "node.kubernetes.io/not-ready", // Same key as default + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoExecute, + TolerationSeconds: ptr.To[int64](600), // Different value }, }, }, @@ -428,20 +418,18 @@ func TestTolerationsOverride(t *testing.T) { name: "Mixed scenario: override one default, add new custom", operatorSpec: operatorv1beta1.OperatorSpec{ Name: "test-operator", - ControllerManager: operatorv1beta1.ContainerSpec{ - Tolerations: []corev1.Toleration{ - { - Key: corev1.TaintNodeNotReady, // "node.kubernetes.io/not-ready", // Override default - Operator: corev1.TolerationOpExists, - Effect: corev1.TaintEffectNoExecute, - TolerationSeconds: ptr.To[int64](300), - }, - { - Key: "node.example.com/gpu", // Add new - Operator: corev1.TolerationOpEqual, - Value: "nvidia", - Effect: corev1.TaintEffectNoSchedule, - }, + Tolerations: []corev1.Toleration{ + { + Key: corev1.TaintNodeNotReady, // "node.kubernetes.io/not-ready", // Override default + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoExecute, + TolerationSeconds: ptr.To[int64](300), + }, + { + Key: "node.example.com/gpu", // Add new + Operator: corev1.TolerationOpEqual, + Value: "nvidia", + Effect: corev1.TaintEffectNoSchedule, }, }, },