Merge pull request #332 from ASBishop/pod-affinity

Set pod affinity to spread services across OCP workers
openstack-k8s-operators · Feb 12, 2024 · 74da8c8 · 74da8c8
2 parents 6572d1b + d169dc3
commit 74da8c8
Show file tree

Hide file tree

Showing 15 changed files with 45 additions and 81 deletions.
diff --git a/api/v1beta1/cinder_webhook.go b/api/v1beta1/cinder_webhook.go
@@ -22,11 +22,11 @@ limitations under the License.
 package v1beta1
 
 import (
+	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 )
 
 // CinderDefaults -

diff --git a/controllers/cinderapi_controller.go b/controllers/cinderapi_controller.go
@@ -714,7 +714,7 @@ func (r *CinderAPIReconciler) reconcileNormal(ctx context.Context, instance *cin
 	//
 	serviceLabels := map[string]string{
 		common.AppSelector:       cinder.ServiceName,
-		common.ComponentSelector: cinderapi.Component,
+		common.ComponentSelector: cinderapi.ComponentName,
 	}
 
 	//

diff --git a/controllers/cinderbackup_controller.go b/controllers/cinderbackup_controller.go
@@ -416,7 +416,7 @@ func (r *CinderBackupReconciler) reconcileNormal(ctx context.Context, instance *
 	//
 	serviceLabels := map[string]string{
 		common.AppSelector:       cinder.ServiceName,
-		common.ComponentSelector: cinderbackup.Component,
+		common.ComponentSelector: cinderbackup.ComponentName,
 	}
 
 	//

diff --git a/controllers/cinderscheduler_controller.go b/controllers/cinderscheduler_controller.go
@@ -415,7 +415,7 @@ func (r *CinderSchedulerReconciler) reconcileNormal(ctx context.Context, instanc
 	//
 	serviceLabels := map[string]string{
 		common.AppSelector:       cinder.ServiceName,
-		common.ComponentSelector: cinderscheduler.Component,
+		common.ComponentSelector: cinderscheduler.ComponentName,
 	}
 
 	//

diff --git a/controllers/cindervolume_controller.go b/controllers/cindervolume_controller.go
@@ -417,8 +417,8 @@ func (r *CinderVolumeReconciler) reconcileNormal(ctx context.Context, instance *
 	//
 	serviceLabels := map[string]string{
 		common.AppSelector:       cinder.ServiceName,
-		common.ComponentSelector: cindervolume.Component,
-		cindervolume.Backend:     instance.Name[len(cindervolume.Component)+1:],
+		common.ComponentSelector: cindervolume.ComponentName,
+		cindervolume.Backend:     instance.Name[len(cindervolume.ComponentName)+1:],
 	}
 
 	//

diff --git a/pkg/cinder/funcs.go b/pkg/cinder/funcs.go
@@ -1,6 +1,12 @@
 package cinder
 
-import "sigs.k8s.io/controller-runtime/pkg/client"
+import (
+	common "github.com/openstack-k8s-operators/lib-common/modules/common"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
+
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
 
 // GetOwningCinderName - Given a CinderAPI, CinderScheduler, CinderBackup or CinderVolume
 // object, returning the parent Cinder object that created it (if any)
@@ -27,3 +33,17 @@ func GetNetworkAttachmentAddrs(namespace string, networkAttachments []string, ne
 
 	return networkAttachmentAddrs
 }
+
+// GetPodAffinity - Returns a corev1.Affinity reference for the specified component.
+func GetPodAffinity(componentName string) *corev1.Affinity {
+	// If possible two pods of the same component (e.g cinder-api) should not
+	// run on the same worker node. If this is not possible they get still
+	// created on the same worker node.
+	return affinity.DistributePods(
+		common.ComponentSelector,
+		[]string{
+			componentName,
+		},
+		corev1.LabelHostname,
+	)
+}
diff --git a/pkg/cinderapi/const.go b/pkg/cinderapi/const.go
@@ -16,8 +16,8 @@ limitations under the License.
 package cinderapi
 
 const (
-	// Component -
-	Component = "cinder-api"
+	// ComponentName -
+	ComponentName = "cinder-api"
 
 	//LogFile -
 	LogFile = "/var/log/cinder/cinder-api.log"

diff --git a/pkg/cinderapi/statefuleset.go b/pkg/cinderapi/statefuleset.go
@@ -19,7 +19,6 @@ import (
 	cinderv1beta1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1"
 	cinder "github.com/openstack-k8s-operators/cinder-operator/pkg/cinder"
 	common "github.com/openstack-k8s-operators/lib-common/modules/common"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
@@ -162,7 +161,7 @@ func StatefulSet(
 							Resources:    instance.Spec.Resources,
 						},
 						{
-							Name: cinder.ServiceName + "-api",
+							Name: ComponentName,
 							Command: []string{
 								"/bin/bash",
 							},
@@ -178,26 +177,13 @@ func StatefulSet(
 							LivenessProbe:  livenessProbe,
 						},
 					},
+					Affinity:     cinder.GetPodAffinity(ComponentName),
 					NodeSelector: instance.Spec.NodeSelector,
 					Volumes:      volumes,
 				},
 			},
 		},
 	}
 
-	// If possible two pods of the same service should not
-	// run on the same worker node. If this is not possible
-	// the get still created on the same worker node.
-	statefulset.Spec.Template.Spec.Affinity = affinity.DistributePods(
-		common.AppSelector,
-		[]string{
-			cinder.ServiceName,
-		},
-		corev1.LabelHostname,
-	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		statefulset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
-	}
-
 	return statefulset, nil
 }
diff --git a/pkg/cinderbackup/const.go b/pkg/cinderbackup/const.go
@@ -16,6 +16,6 @@ limitations under the License.
 package cinderbackup
 
 const (
-	// Component -
-	Component = "cinder-backup"
+	// ComponentName -
+	ComponentName = "cinder-backup"
 )
diff --git a/pkg/cinderbackup/statefulset.go b/pkg/cinderbackup/statefulset.go
@@ -19,7 +19,6 @@ import (
 	cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1"
 	cinder "github.com/openstack-k8s-operators/cinder-operator/pkg/cinder"
 	common "github.com/openstack-k8s-operators/lib-common/modules/common"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -137,7 +136,7 @@ func StatefulSet(
 					HostPID: true,
 					Containers: []corev1.Container{
 						{
-							Name: cinder.ServiceName + "-backup",
+							Name: ComponentName,
 							Command: []string{
 								"/bin/bash",
 							},
@@ -164,26 +163,13 @@ func StatefulSet(
 							VolumeMounts: volumeMounts,
 						},
 					},
+					Affinity:     cinder.GetPodAffinity(ComponentName),
 					NodeSelector: instance.Spec.NodeSelector,
 					Volumes:      volumes,
 				},
 			},
 		},
 	}
 
-	// If possible two pods of the same service should not
-	// run on the same worker node. If this is not possible
-	// the get still created on the same worker node.
-	statefulset.Spec.Template.Spec.Affinity = affinity.DistributePods(
-		common.AppSelector,
-		[]string{
-			cinder.ServiceName,
-		},
-		corev1.LabelHostname,
-	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		statefulset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
-	}
-
 	return statefulset
 }
diff --git a/pkg/cinderscheduler/const.go b/pkg/cinderscheduler/const.go
@@ -16,6 +16,6 @@ limitations under the License.
 package cinderscheduler
 
 const (
-	// Component -
-	Component = "cinder-scheduler"
+	// ComponentName -
+	ComponentName = "cinder-scheduler"
 )
diff --git a/pkg/cinderscheduler/statefulset.go b/pkg/cinderscheduler/statefulset.go
@@ -19,7 +19,6 @@ import (
 	cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1"
 	cinder "github.com/openstack-k8s-operators/cinder-operator/pkg/cinder"
 	common "github.com/openstack-k8s-operators/lib-common/modules/common"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -123,7 +122,7 @@ func StatefulSet(
 					ServiceAccountName: instance.Spec.ServiceAccount,
 					Containers: []corev1.Container{
 						{
-							Name: cinder.ServiceName + "-scheduler",
+							Name: ComponentName,
 							Command: []string{
 								"/bin/bash",
 							},
@@ -149,26 +148,13 @@ func StatefulSet(
 							VolumeMounts: volumeMounts,
 						},
 					},
+					Affinity:     cinder.GetPodAffinity(ComponentName),
 					NodeSelector: instance.Spec.NodeSelector,
 					Volumes:      volumes,
 				},
 			},
 		},
 	}
 
-	// If possible two pods of the same service should not
-	// run on the same worker node. If this is not possible
-	// the get still created on the same worker node.
-	statefulset.Spec.Template.Spec.Affinity = affinity.DistributePods(
-		common.AppSelector,
-		[]string{
-			cinder.ServiceName,
-		},
-		corev1.LabelHostname,
-	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		statefulset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
-	}
-
 	return statefulset
 }
diff --git a/pkg/cindervolume/const.go b/pkg/cindervolume/const.go
@@ -16,8 +16,8 @@ limitations under the License.
 package cindervolume
 
 const (
-	// Component -
-	Component = "cinder-volume"
+	// ComponentName -
+	ComponentName = "cinder-volume"
 	// Backend -
 	Backend = "backend"
 )
diff --git a/pkg/cindervolume/statefulset.go b/pkg/cindervolume/statefulset.go
@@ -19,7 +19,6 @@ import (
 	cinderv1 "github.com/openstack-k8s-operators/cinder-operator/api/v1beta1"
 	cinder "github.com/openstack-k8s-operators/cinder-operator/pkg/cinder"
 	common "github.com/openstack-k8s-operators/lib-common/modules/common"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/affinity"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -137,7 +136,7 @@ func StatefulSet(
 					HostPID: true,
 					Containers: []corev1.Container{
 						{
-							Name: cinder.ServiceName + "-volume",
+							Name: ComponentName,
 							Command: []string{
 								"/bin/bash",
 							},
@@ -164,26 +163,13 @@ func StatefulSet(
 							VolumeMounts: volumeMounts,
 						},
 					},
+					Affinity:     cinder.GetPodAffinity(ComponentName),
 					NodeSelector: instance.Spec.NodeSelector,
 					Volumes:      volumes,
 				},
 			},
 		},
 	}
 
-	// If possible two pods of the same service should not
-	// run on the same worker node. If this is not possible
-	// the get still created on the same worker node.
-	statefulset.Spec.Template.Spec.Affinity = affinity.DistributePods(
-		common.AppSelector,
-		[]string{
-			cinder.ServiceName,
-		},
-		corev1.LabelHostname,
-	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		statefulset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
-	}
-
 	return statefulset
 }
diff --git a/test/kuttl/common/assert_sample_deployment.yaml b/test/kuttl/common/assert_sample_deployment.yaml
@@ -51,10 +51,10 @@ spec:
           - podAffinityTerm:
               labelSelector:
                 matchExpressions:
-                - key: service
+                - key: component
                   operator: In
                   values:
-                  - cinder
+                  - cinder-api
               topologyKey: kubernetes.io/hostname
             weight: 1
       containers: