Skip to content

Commit 18b7410

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #502 from olliewalsh/node_selectors
Ensure nodeSelector logic is consistent for all operators
2 parents 6d9f1d4 + 1a349e3 commit 18b7410

File tree

7 files changed

+150
-18
lines changed

7 files changed

+150
-18
lines changed

‎api/v1beta1/keystoneapi_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ type KeystoneAPISpecCore struct {
138138

139139
// +kubebuilder:validation:Optional
140140
// NodeSelector to target subset of worker nodes running this service
141-
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
141+
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
142142

143143
// +kubebuilder:validation:Optional
144144
// +kubebuilder:default=false

‎api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 7 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/keystone/bootstrap.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ func BootstrapJob(
117117
}
118118
job.Spec.Template.Spec.Containers[0].Env = env.MergeEnvs(job.Spec.Template.Spec.Containers[0].Env, envVars)
119119

120-
if len(instance.Spec.NodeSelector) > 0 {
121-
job.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
120+
if instance.Spec.NodeSelector != nil {
121+
job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
122122
}
123123

124124
return job

‎pkg/keystone/cronjob.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,8 @@ func CronJob(
9898
},
9999
},
100100
}
101-
if len(instance.Spec.NodeSelector) > 0 {
102-
cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
101+
if instance.Spec.NodeSelector != nil {
102+
cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
103103
}
104-
105104
return cronjob
106105
}

‎pkg/keystone/dbsync.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ func DbSyncJob(
9090
},
9191
}
9292

93-
if len(instance.Spec.NodeSelector) > 0 {
94-
job.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
93+
if instance.Spec.NodeSelector != nil {
94+
job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
9595
}
9696

9797
return job

‎pkg/keystone/deployment.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,9 @@ func Deployment(
158158
},
159159
corev1.LabelHostname,
160160
)
161-
if len(instance.Spec.NodeSelector) > 0 {
162-
deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
161+
162+
if instance.Spec.NodeSelector != nil {
163+
deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
163164
}
164165

165166
return deployment, nil

‎tests/functional/keystoneapi_controller_test.go‎

Lines changed: 133 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ var _ = Describe("Keystone controller", func() {
5151
var internalCertSecretName types.NamespacedName
5252
var publicCertSecretName types.NamespacedName
5353
var memcachedSpec memcachedv1.MemcachedSpec
54+
var cronJobName types.NamespacedName
5455

5556
BeforeEach(func() {
5657

@@ -99,6 +100,10 @@ var _ = Describe("Keystone controller", func() {
99100
Replicas: ptr.To(int32(3)),
100101
},
101102
}
103+
cronJobName = types.NamespacedName{
104+
Namespace: keystoneAPIName.Namespace,
105+
Name: "keystone-cron",
106+
}
102107

103108
err := os.Setenv("OPERATOR_TEMPLATES", "../../templates")
104109
Expect(err).NotTo(HaveOccurred())
@@ -609,11 +614,7 @@ var _ = Describe("Keystone controller", func() {
609614
})
610615

611616
It("should create a CronJob for trust flush", func() {
612-
cronJob := types.NamespacedName{
613-
Namespace: keystoneAPIName.Namespace,
614-
Name: fmt.Sprintf("%s-%s", keystoneAPIName.Name, "cron"),
615-
}
616-
GetCronJob(cronJob)
617+
GetCronJob(cronJobName)
617618
})
618619

619620
It("should create a ConfigMap and Secret for client config", func() {
@@ -1189,6 +1190,133 @@ var _ = Describe("Keystone controller", func() {
11891190
})
11901191
})
11911192

1193+
When("A KeystoneAPI is created with nodeSelector", func() {
1194+
BeforeEach(func() {
1195+
spec := GetDefaultKeystoneAPISpec()
1196+
spec["nodeSelector"] = map[string]interface{}{
1197+
"foo": "bar",
1198+
}
1199+
DeferCleanup(
1200+
k8sClient.Delete, ctx, CreateKeystoneMessageBusSecret(namespace, "rabbitmq-secret"))
1201+
keystone := CreateKeystoneAPI(keystoneAPIName, spec)
1202+
DeferCleanup(th.DeleteInstance, keystone)
1203+
DeferCleanup(
1204+
k8sClient.Delete, ctx, CreateKeystoneAPISecret(namespace, SecretName))
1205+
DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, "memcached", memcachedSpec))
1206+
DeferCleanup(
1207+
mariadb.DeleteDBService,
1208+
mariadb.CreateDBService(
1209+
namespace,
1210+
GetKeystoneAPI(keystoneAPIName).Spec.DatabaseInstance,
1211+
corev1.ServiceSpec{
1212+
Ports: []corev1.ServicePort{{Port: 3306}},
1213+
},
1214+
),
1215+
)
1216+
mariadb.SimulateMariaDBAccountCompleted(keystoneAccountName)
1217+
mariadb.SimulateMariaDBDatabaseCompleted(keystoneDatabaseName)
1218+
infra.SimulateTransportURLReady(types.NamespacedName{
1219+
Name: fmt.Sprintf("%s-keystone-transport", keystoneAPIName.Name),
1220+
Namespace: namespace,
1221+
})
1222+
infra.SimulateMemcachedReady(types.NamespacedName{
1223+
Name: "memcached",
1224+
Namespace: namespace,
1225+
})
1226+
th.SimulateJobSuccess(dbSyncJobName)
1227+
th.SimulateJobSuccess(bootstrapJobName)
1228+
th.SimulateDeploymentReplicaReady(deploymentName)
1229+
})
1230+
1231+
It("sets nodeSelector in resource specs", func() {
1232+
Eventually(func(g Gomega) {
1233+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1234+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1235+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1236+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1237+
}, timeout, interval).Should(Succeed())
1238+
})
1239+
1240+
It("updates nodeSelector in resource specs when changed", func() {
1241+
Eventually(func(g Gomega) {
1242+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1243+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1244+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1245+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1246+
}, timeout, interval).Should(Succeed())
1247+
1248+
Eventually(func(g Gomega) {
1249+
keystone := GetKeystoneAPI(keystoneAPIName)
1250+
newNodeSelector := map[string]string{
1251+
"foo2": "bar2",
1252+
}
1253+
keystone.Spec.NodeSelector = &newNodeSelector
1254+
g.Expect(k8sClient.Update(ctx, keystone)).Should(Succeed())
1255+
}, timeout, interval).Should(Succeed())
1256+
1257+
Eventually(func(g Gomega) {
1258+
th.SimulateJobSuccess(dbSyncJobName)
1259+
th.SimulateJobSuccess(bootstrapJobName)
1260+
th.SimulateDeploymentReplicaReady(deploymentName)
1261+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
1262+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
1263+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
1264+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
1265+
}, timeout, interval).Should(Succeed())
1266+
})
1267+
1268+
It("removes nodeSelector from resource specs when cleared", func() {
1269+
Eventually(func(g Gomega) {
1270+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1271+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1272+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1273+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1274+
}, timeout, interval).Should(Succeed())
1275+
1276+
Eventually(func(g Gomega) {
1277+
keystone := GetKeystoneAPI(keystoneAPIName)
1278+
emptyNodeSelector := map[string]string{}
1279+
keystone.Spec.NodeSelector = &emptyNodeSelector
1280+
g.Expect(k8sClient.Update(ctx, keystone)).Should(Succeed())
1281+
}, timeout, interval).Should(Succeed())
1282+
1283+
Eventually(func(g Gomega) {
1284+
th.SimulateJobSuccess(dbSyncJobName)
1285+
th.SimulateJobSuccess(bootstrapJobName)
1286+
th.SimulateDeploymentReplicaReady(deploymentName)
1287+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(BeNil())
1288+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(BeNil())
1289+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(BeNil())
1290+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(BeNil())
1291+
}, timeout, interval).Should(Succeed())
1292+
})
1293+
1294+
It("removes nodeSelector from resource specs when nilled", func() {
1295+
Eventually(func(g Gomega) {
1296+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1297+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1298+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1299+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
1300+
}, timeout, interval).Should(Succeed())
1301+
1302+
Eventually(func(g Gomega) {
1303+
keystone := GetKeystoneAPI(keystoneAPIName)
1304+
keystone.Spec.NodeSelector = nil
1305+
g.Expect(k8sClient.Update(ctx, keystone)).Should(Succeed())
1306+
}, timeout, interval).Should(Succeed())
1307+
1308+
Eventually(func(g Gomega) {
1309+
th.SimulateJobSuccess(dbSyncJobName)
1310+
th.SimulateJobSuccess(bootstrapJobName)
1311+
th.SimulateDeploymentReplicaReady(deploymentName)
1312+
g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(BeNil())
1313+
g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(BeNil())
1314+
g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(BeNil())
1315+
g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(BeNil())
1316+
}, timeout, interval).Should(Succeed())
1317+
})
1318+
})
1319+
11921320
// Run MariaDBAccount suite tests. these are pre-packaged ginkgo tests
11931321
// that exercise standard account create / update patterns that should be
11941322
// common to all controllers that ensure MariaDBAccount CRs.

0 commit comments

Comments
 (0)