Merge pull request #287 from olliewalsh/node_selectors

openshift-merge-bot[bot] · web-flow · commit 4b457715f74e · 2024-11-20T16:05:42.000Z
Ensure nodeSelector logic is consistent for all operators
diff --git a/api/v1beta1/galera_types.go b/api/v1beta1/galera_types.go
@@ -64,7 +64,7 @@ type GaleraSpecCore struct {
 	Replicas *int32 `json:"replicas"`
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 	// +kubebuilder:validation:Optional
 	// Customize config using this parameter to change service defaults,
 	// or overwrite rendered information using raw MariaDB config format.
diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/controllers/mariadbaccount_controller.go b/controllers/mariadbaccount_controller.go
@@ -282,7 +282,7 @@ func (r *MariaDBAccountReconciler) reconcileCreate(
 
 	log.Info(fmt.Sprintf("Running account create '%s' MariaDBDatabase '%s'", instance.Name, mariadbDatabaseName))
 
-	jobDef, err := mariadb.CreateDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName)
+	jobDef, err := mariadb.CreateDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -507,7 +507,7 @@ func (r *MariaDBAccountReconciler) reconcileDelete(
 
 	log.Info(fmt.Sprintf("Running account delete '%s' MariaDBDatabase '%s'", instance.Name, mariadbDatabaseName))
 
-	jobDef, err := mariadb.DeleteDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName)
+	jobDef, err := mariadb.DeleteDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
diff --git a/controllers/mariadbdatabase_controller.go b/controllers/mariadbdatabase_controller.go
@@ -195,7 +195,7 @@ func (r *MariaDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	)
 
 	// Define a new Job object (hostname, password, containerImage)
-	jobDef, err := mariadb.DbDatabaseJob(instance, dbHostname, dbSecret, dbContainerImage, serviceAccount, useTLS)
+	jobDef, err := mariadb.DbDatabaseJob(instance, dbHostname, dbSecret, dbContainerImage, serviceAccount, useTLS, dbGalera.Spec.NodeSelector)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
diff --git a/pkg/mariadb/account.go b/pkg/mariadb/account.go
@@ -18,7 +18,7 @@ type accountCreateOrDeleteOptions struct {
 	RequireTLS            string
 }
 
-func CreateDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string) (*batchv1.Job, error) {
+func CreateDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
 	var tlsStatement string
 	if account.Spec.RequireTLS {
 		tlsStatement = " REQUIRE SSL"
@@ -90,10 +90,14 @@ func CreateDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st
 		},
 	}
 
+	if nodeSelector != nil {
+		job.Spec.Template.Spec.NodeSelector = *nodeSelector
+	}
+
 	return job, nil
 }
 
-func DeleteDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string) (*batchv1.Job, error) {
+func DeleteDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
 
 	opts := accountCreateOrDeleteOptions{account.Spec.UserName, databaseName, databaseHostName, "root", ""}
 
@@ -140,5 +144,9 @@ func DeleteDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st
 		},
 	}
 
+	if nodeSelector != nil {
+		job.Spec.Template.Spec.NodeSelector = *nodeSelector
+	}
+
 	return job, nil
 }
diff --git a/pkg/mariadb/database.go b/pkg/mariadb/database.go
@@ -20,7 +20,7 @@ type dbCreateOptions struct {
 }
 
 // DbDatabaseJob -
-func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, useTLS bool) (*batchv1.Job, error) {
+func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, useTLS bool, nodeSelector *map[string]string) (*batchv1.Job, error) {
 	var tlsStatement string
 	if useTLS {
 		tlsStatement = " REQUIRE SSL"
@@ -115,11 +115,15 @@ func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName s
 		},
 	}
 
+	if nodeSelector != nil && len(*nodeSelector) > 0 {
+		job.Spec.Template.Spec.NodeSelector = *nodeSelector
+	}
+
 	return job, nil
 }
 
 // DeleteDbDatabaseJob -
-func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string) (*batchv1.Job, error) {
+func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, nodeSelector *map[string]string) (*batchv1.Job, error) {
 
 	opts := dbCreateOptions{
 		database.Spec.Name,
@@ -206,5 +210,9 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost
 		},
 	}
 
+	if nodeSelector != nil {
+		job.Spec.Template.Spec.NodeSelector = *nodeSelector
+	}
+
 	return job, nil
 }
diff --git a/pkg/mariadb/statefulset.go b/pkg/mariadb/statefulset.go
@@ -78,8 +78,8 @@ func StatefulSet(g *mariadbv1.Galera, configHash string) *appsv1.StatefulSet {
 		},
 		corev1.LabelHostname,
 	)
-	if len(g.Spec.NodeSelector) > 0 {
-		sts.Spec.Template.Spec.NodeSelector = g.Spec.NodeSelector
+	if g.Spec.NodeSelector != nil {
+		sts.Spec.Template.Spec.NodeSelector = *g.Spec.NodeSelector
 	}
 
 	return sts
diff --git a/tests/kuttl/tests/account_create/01-assert.yaml b/tests/kuttl/tests/account_create/01-assert.yaml
@@ -13,6 +13,8 @@ spec:
   replicas: 1
   secret: osp-secret
   storageRequest: 500M
+  nodeSelector:
+    node-role.kubernetes.io/worker: ""
 status:
   bootstrapped: true
   conditions:
@@ -72,6 +74,8 @@ spec:
         cr: galera-openstack
         galera/name: openstack
     spec:
+      nodeSelector:
+        node-role.kubernetes.io/worker: ""
       containers:
       - command:
         - /usr/bin/dumb-init
@@ -96,6 +100,9 @@ apiVersion: v1
 kind: Pod
 metadata:
   name: openstack-galera-0
+spec:
+  nodeSelector:
+    node-role.kubernetes.io/worker: ""
 ---
 apiVersion: v1
 kind: Service
diff --git a/tests/kuttl/tests/account_create/01-deploy_galera.yaml b/tests/kuttl/tests/account_create/01-deploy_galera.yaml
@@ -7,3 +7,5 @@ spec:
   storageClass: local-storage
   storageRequest: 500M
   replicas: 1
+  nodeSelector:
+    node-role.kubernetes.io/worker: ""
diff --git a/tests/kuttl/tests/account_create/02-assert.yaml b/tests/kuttl/tests/account_create/02-assert.yaml
@@ -3,5 +3,10 @@ apiVersion: batch/v1
 kind: Job
 metadata:
   name: kuttldb-accounttest-db-create
+spec:
+  template:
+    spec:
+      nodeSelector:
+        node-role.kubernetes.io/worker: ""
 status:
   succeeded: 1
diff --git a/tests/kuttl/tests/account_create/05-assert.yaml b/tests/kuttl/tests/account_create/05-assert.yaml
@@ -7,3 +7,15 @@ commands:
       ${MARIADB_KUTTL_DIR:-tests/kuttl/tests}/../common/scripts/check_db_account.sh openstack-galera-0 kuttldb_accounttest someuser dbsecret1
       # ensure db users are configured without TLS connection restriction
       oc rsh -n ${NAMESPACE} -c galera openstack-galera-0 /bin/sh -c 'mysql -uroot -p${DB_ROOT_PASSWORD} -e "show grants for \`someuser\`@\`%\`;"' | grep 'GRANT USAGE' | grep -v 'REQUIRE SSL'
+---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: someuser-account-create
+spec:
+  template:
+    spec:
+      nodeSelector:
+        node-role.kubernetes.io/worker: ""
+status:
+  succeeded: 1

Original file line number	Diff line number	Diff line change
`@@ -282,7 +282,7 @@ func (r *MariaDBAccountReconciler) reconcileCreate(`
`282`	`282`
`283`	`283`	`log.Info(fmt.Sprintf("Running account create '%s' MariaDBDatabase '%s'", instance.Name, mariadbDatabaseName))`
`284`	`284`
`285`		`- jobDef, err := mariadb.CreateDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName)`
	`285`	`+ jobDef, err := mariadb.CreateDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)`
`286`	`286`	`if err != nil {`
`287`	`287`	`return ctrl.Result{}, err`
`288`	`288`	`}`
`@@ -507,7 +507,7 @@ func (r *MariaDBAccountReconciler) reconcileDelete(`
`507`	`507`
`508`	`508`	`log.Info(fmt.Sprintf("Running account delete '%s' MariaDBDatabase '%s'", instance.Name, mariadbDatabaseName))`
`509`	`509`
`510`		`- jobDef, err := mariadb.DeleteDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName)`
	`510`	`+ jobDef, err := mariadb.DeleteDbAccountJob(instance, mariadbDatabase.Spec.Name, dbHostname, dbAdminSecret, dbContainerImage, serviceAccountName, dbGalera.Spec.NodeSelector)`
`511`	`511`	`if err != nil {`
`512`	`512`	`return ctrl.Result{}, err`
`513`	`513`	`}`
Original file line number	Diff line number	Diff line change
`@@ -195,7 +195,7 @@ func (r *MariaDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ`
`195`	`195`	`)`
`196`	`196`
`197`	`197`	`// Define a new Job object (hostname, password, containerImage)`
`198`		`- jobDef, err := mariadb.DbDatabaseJob(instance, dbHostname, dbSecret, dbContainerImage, serviceAccount, useTLS)`
	`198`	`+ jobDef, err := mariadb.DbDatabaseJob(instance, dbHostname, dbSecret, dbContainerImage, serviceAccount, useTLS, dbGalera.Spec.NodeSelector)`
`199`	`199`	`if err != nil {`
`200`	`200`	`return ctrl.Result{}, err`
`201`	`201`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ type accountCreateOrDeleteOptions struct {`
`18`	`18`	`RequireTLS string`
`19`	`19`	`}`
`20`	`20`
`21`		`-func CreateDbAccountJob(account databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string) (batchv1.Job, error) {`
	`21`	`+func CreateDbAccountJob(account databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, nodeSelector map[string]string) (*batchv1.Job, error) {`
`22`	`22`	`var tlsStatement string`
`23`	`23`	`if account.Spec.RequireTLS {`
`24`	`24`	`tlsStatement = " REQUIRE SSL"`
`@@ -90,10 +90,14 @@ func CreateDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st`
`90`	`90`	`},`
`91`	`91`	`}`
`92`	`92`
	`93`	`+ if nodeSelector != nil {`
	`94`	`+ job.Spec.Template.Spec.NodeSelector = *nodeSelector`
	`95`	`+ }`
	`96`	`+`
`93`	`97`	`return job, nil`
`94`	`98`	`}`
`95`	`99`
`96`		`-func DeleteDbAccountJob(account databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string) (batchv1.Job, error) {`
	`100`	`+func DeleteDbAccountJob(account databasev1beta1.MariaDBAccount, databaseName string, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, nodeSelector map[string]string) (*batchv1.Job, error) {`
`97`	`101`
`98`	`102`	`opts := accountCreateOrDeleteOptions{account.Spec.UserName, databaseName, databaseHostName, "root", ""}`
`99`	`103`
`@@ -140,5 +144,9 @@ func DeleteDbAccountJob(account *databasev1beta1.MariaDBAccount, databaseName st`
`140`	`144`	`},`
`141`	`145`	`}`
`142`	`146`
	`147`	`+ if nodeSelector != nil {`
	`148`	`+ job.Spec.Template.Spec.NodeSelector = *nodeSelector`
	`149`	`+ }`
	`150`	`+`
`143`	`151`	`return job, nil`
`144`	`152`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ type dbCreateOptions struct {`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`// DbDatabaseJob -`
`23`		`-func DbDatabaseJob(database databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, useTLS bool) (batchv1.Job, error) {`
	`23`	`+func DbDatabaseJob(database databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, useTLS bool, nodeSelector map[string]string) (*batchv1.Job, error) {`
`24`	`24`	`var tlsStatement string`
`25`	`25`	`if useTLS {`
`26`	`26`	`tlsStatement = " REQUIRE SSL"`
`@@ -115,11 +115,15 @@ func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName s`
`115`	`115`	`},`
`116`	`116`	`}`
`117`	`117`
	`118`	`+ if nodeSelector != nil && len(*nodeSelector) > 0 {`
	`119`	`+ job.Spec.Template.Spec.NodeSelector = *nodeSelector`
	`120`	`+ }`
	`121`	`+`
`118`	`122`	`return job, nil`
`119`	`123`	`}`
`120`	`124`
`121`	`125`	`// DeleteDbDatabaseJob -`
`122`		`-func DeleteDbDatabaseJob(database databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string) (batchv1.Job, error) {`
	`126`	`+func DeleteDbDatabaseJob(database databasev1beta1.MariaDBDatabase, databaseHostName string, databaseSecret string, containerImage string, serviceAccountName string, nodeSelector map[string]string) (*batchv1.Job, error) {`
`123`	`127`
`124`	`128`	`opts := dbCreateOptions{`
`125`	`129`	`database.Spec.Name,`
`@@ -206,5 +210,9 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost`
`206`	`210`	`},`
`207`	`211`	`}`
`208`	`212`
	`213`	`+ if nodeSelector != nil {`
	`214`	`+ job.Spec.Template.Spec.NodeSelector = *nodeSelector`
	`215`	`+ }`
	`216`	`+`
`209`	`217`	`return job, nil`
`210`	`218`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,8 +78,8 @@ func StatefulSet(g mariadbv1.Galera, configHash string) appsv1.StatefulSet {`
`78`	`78`	`},`
`79`	`79`	`corev1.LabelHostname,`
`80`	`80`	`)`
`81`		`- if len(g.Spec.NodeSelector) > 0 {`
`82`		`- sts.Spec.Template.Spec.NodeSelector = g.Spec.NodeSelector`
	`81`	`+ if g.Spec.NodeSelector != nil {`
	`82`	`+ sts.Spec.Template.Spec.NodeSelector = *g.Spec.NodeSelector`
`83`	`83`	`}`
`84`	`84`
`85`	`85`	`return sts`