Skip to content

Commit 37d5a9b

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #186 from olliewalsh/node_selectors
Ensure nodeSelector logic is consistent for all operators
2 parents 6febb2d + 5af1b7b commit 37d5a9b

File tree

9 files changed

+182
-17
lines changed

9 files changed

+182
-17
lines changed

‎api/v1beta1/barbican_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ type BarbicanSpecBase struct {
8484
// +kubebuilder:validation:Optional
8585
// NodeSelector to target subset of worker nodes running this component. Setting here overrides
8686
// any global NodeSelector settings within the Barbican CR.
87-
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
87+
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
8888

8989
// +kubebuilder:validation:Optional
9090
// CustomServiceConfig - customize the service config using this parameter to change service defaults,

‎api/v1beta1/common_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type BarbicanComponentTemplate struct {
5858
// +kubebuilder:validation:Optional
5959
// NodeSelector to target subset of worker nodes running this component. Setting here overrides
6060
// any global NodeSelector settings within the Barbican CR.
61-
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
61+
NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
6262

6363
// +kubebuilder:validation:Optional
6464
// +kubebuilder:default=1

‎api/v1beta1/zz_generated.deepcopy.go‎

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

‎controllers/barbican_controller.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -687,8 +687,8 @@ func (r *BarbicanReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, in
687687

688688
// If NodeSelector is not specified in BarbicanAPITemplate, the current
689689
// API instance inherits the value from the top-level CR.
690-
if apiSpec.BarbicanAPITemplate.BarbicanAPITemplateCore.BarbicanComponentTemplate.NodeSelector == nil {
691-
apiSpec.BarbicanAPITemplate.BarbicanAPITemplateCore.BarbicanComponentTemplate.NodeSelector = instance.Spec.BarbicanSpecBase.NodeSelector
690+
if apiSpec.NodeSelector == nil {
691+
apiSpec.NodeSelector = instance.Spec.NodeSelector
692692
}
693693

694694
deployment := &barbicanv1beta1.BarbicanAPI{
@@ -731,8 +731,8 @@ func (r *BarbicanReconciler) workerDeploymentCreateOrUpdate(ctx context.Context,
731731

732732
// If NodeSelector is not specified in BarbicanWorkerTemplate, the current
733733
// Worker instance inherits the value from the top-level CR.
734-
if workerSpec.BarbicanWorkerTemplate.BarbicanWorkerTemplateCore.BarbicanComponentTemplate.NodeSelector == nil {
735-
workerSpec.BarbicanWorkerTemplate.BarbicanWorkerTemplateCore.BarbicanComponentTemplate.NodeSelector = instance.Spec.BarbicanSpecBase.NodeSelector
734+
if workerSpec.NodeSelector == nil {
735+
workerSpec.NodeSelector = instance.Spec.NodeSelector
736736
}
737737

738738
deployment := &barbicanv1beta1.BarbicanWorker{
@@ -774,8 +774,8 @@ func (r *BarbicanReconciler) keystoneListenerDeploymentCreateOrUpdate(ctx contex
774774

775775
// If NodeSelector is not specified in BarbicanKeystoneListenerTemplate, the current
776776
// KeystoneListener instance inherits the value from the top-level CR.
777-
if keystoneListenerSpec.BarbicanKeystoneListenerTemplate.BarbicanKeystoneListenerTemplateCore.BarbicanComponentTemplate.NodeSelector == nil {
778-
keystoneListenerSpec.BarbicanKeystoneListenerTemplate.BarbicanKeystoneListenerTemplateCore.BarbicanComponentTemplate.NodeSelector = instance.Spec.BarbicanSpecBase.NodeSelector
777+
if keystoneListenerSpec.NodeSelector == nil {
778+
keystoneListenerSpec.NodeSelector = instance.Spec.NodeSelector
779779
}
780780

781781
deployment := &barbicanv1beta1.BarbicanKeystoneListener{

‎pkg/barbican/dbsync.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,5 +114,9 @@ func DbSyncJob(instance *barbicanv1beta1.Barbican, labels map[string]string, ann
114114
dbSyncVolume...,
115115
)
116116

117+
if instance.Spec.NodeSelector != nil {
118+
job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
119+
}
120+
117121
return job
118122
}

‎pkg/barbicanapi/deployment.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ func Deployment(
128128
},
129129
Spec: corev1.PodSpec{
130130
ServiceAccountName: instance.Spec.ServiceAccount,
131-
NodeSelector: instance.Spec.NodeSelector,
132131
Containers: []corev1.Container{
133132
{
134133
Name: instance.Name + "-log",
@@ -183,5 +182,9 @@ func Deployment(
183182
instance.Spec.CustomServiceConfigSecrets),
184183
apiVolumes...)
185184

185+
if instance.Spec.NodeSelector != nil {
186+
deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
187+
}
188+
186189
return deployment, nil
187190
}

‎pkg/barbicankeystonelistener/deployment.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func Deployment(
8080
},
8181
Spec: corev1.PodSpec{
8282
ServiceAccountName: instance.Spec.ServiceAccount,
83-
NodeSelector: instance.Spec.NodeSelector,
8483
Containers: []corev1.Container{
8584
{
8685
Name: instance.Name + "-log",
@@ -129,5 +128,10 @@ func Deployment(
129128
instance.Name,
130129
instance.Spec.CustomServiceConfigSecrets),
131130
keystoneListenerVolumes...)
131+
132+
if instance.Spec.NodeSelector != nil {
133+
deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
134+
}
135+
132136
return deployment
133137
}

‎pkg/barbicanworker/deployment.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ func Deployment(
104104
},
105105
Spec: corev1.PodSpec{
106106
ServiceAccountName: instance.Spec.ServiceAccount,
107-
NodeSelector: instance.Spec.NodeSelector,
108107
Containers: []corev1.Container{
109108
{
110109
Name: instance.Name + "-log",
@@ -157,5 +156,10 @@ func Deployment(
157156
instance.Name,
158157
instance.Spec.CustomServiceConfigSecrets),
159158
workerVolumes...)
159+
160+
if instance.Spec.NodeSelector != nil {
161+
deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
162+
}
163+
160164
return deployment
161165
}

‎tests/functional/barbican_controller_test.go‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,148 @@ var _ = Describe("Barbican controller", func() {
280280
})
281281
})
282282

283+
When("A Barbican with nodeSelector is created", func() {
284+
BeforeEach(func() {
285+
spec := GetDefaultBarbicanSpec()
286+
spec["nodeSelector"] = map[string]interface{}{
287+
"foo": "bar",
288+
}
289+
spec["barbicanAPI"] = GetDefaultBarbicanAPISpec()
290+
DeferCleanup(k8sClient.Delete, ctx, CreateBarbicanMessageBusSecret(barbicanTest.Instance.Namespace, "rabbitmq-secret"))
291+
DeferCleanup(th.DeleteInstance, CreateBarbican(barbicanTest.Instance, spec))
292+
DeferCleanup(k8sClient.Delete, ctx, CreateKeystoneAPISecret(barbicanTest.Instance.Namespace, SecretName))
293+
294+
DeferCleanup(
295+
k8sClient.Delete, ctx, CreateBarbicanSecret(barbicanTest.Instance.Namespace, "test-osp-secret-barbican"))
296+
297+
DeferCleanup(
298+
mariadb.DeleteDBService,
299+
mariadb.CreateDBService(
300+
barbicanTest.Instance.Namespace,
301+
GetBarbican(barbicanTest.Instance).Spec.DatabaseInstance,
302+
corev1.ServiceSpec{
303+
Ports: []corev1.ServicePort{{Port: 3306}},
304+
},
305+
),
306+
)
307+
infra.SimulateTransportURLReady(barbicanTest.BarbicanTransportURL)
308+
DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(barbicanTest.Instance.Namespace))
309+
mariadb.SimulateMariaDBAccountCompleted(barbicanTest.BarbicanDatabaseAccount)
310+
mariadb.SimulateMariaDBDatabaseCompleted(barbicanTest.BarbicanDatabaseName)
311+
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
312+
})
313+
314+
It("sets nodeSelector in resource specs", func() {
315+
Eventually(func(g Gomega) {
316+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
317+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
318+
}, timeout, interval).Should(Succeed())
319+
})
320+
321+
It("updates nodeSelector in resource specs when changed", func() {
322+
Eventually(func(g Gomega) {
323+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
324+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
325+
}, timeout, interval).Should(Succeed())
326+
327+
Eventually(func(g Gomega) {
328+
barbican := GetBarbican(barbicanName)
329+
newNodeSelector := map[string]string{
330+
"foo2": "bar2",
331+
}
332+
barbican.Spec.NodeSelector = &newNodeSelector
333+
g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
334+
}, timeout, interval).Should(Succeed())
335+
336+
Eventually(func(g Gomega) {
337+
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
338+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
339+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
340+
}, timeout, interval).Should(Succeed())
341+
})
342+
343+
It("removes nodeSelector from resource specs when cleared", func() {
344+
Eventually(func(g Gomega) {
345+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
346+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
347+
}, timeout, interval).Should(Succeed())
348+
349+
Eventually(func(g Gomega) {
350+
barbican := GetBarbican(barbicanName)
351+
emptyNodeSelector := map[string]string{}
352+
barbican.Spec.NodeSelector = &emptyNodeSelector
353+
g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
354+
}, timeout, interval).Should(Succeed())
355+
356+
Eventually(func(g Gomega) {
357+
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
358+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(BeNil())
359+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(BeNil())
360+
}, timeout, interval).Should(Succeed())
361+
})
362+
363+
It("removes nodeSelector from resource specs when nilled", func() {
364+
Eventually(func(g Gomega) {
365+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
366+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
367+
}, timeout, interval).Should(Succeed())
368+
369+
Eventually(func(g Gomega) {
370+
barbican := GetBarbican(barbicanName)
371+
barbican.Spec.NodeSelector = nil
372+
g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
373+
}, timeout, interval).Should(Succeed())
374+
375+
Eventually(func(g Gomega) {
376+
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
377+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(BeNil())
378+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(BeNil())
379+
}, timeout, interval).Should(Succeed())
380+
})
381+
382+
It("allows nodeSelector service override", func() {
383+
Eventually(func(g Gomega) {
384+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
385+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
386+
}, timeout, interval).Should(Succeed())
387+
388+
Eventually(func(g Gomega) {
389+
barbican := GetBarbican(barbicanName)
390+
apiNodeSelector := map[string]string{
391+
"foo": "api",
392+
}
393+
barbican.Spec.BarbicanAPI.NodeSelector = &apiNodeSelector
394+
g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
395+
}, timeout, interval).Should(Succeed())
396+
397+
Eventually(func(g Gomega) {
398+
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
399+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
400+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "api"}))
401+
}, timeout, interval).Should(Succeed())
402+
})
403+
404+
It("allows nodeSelector service override to empty", func() {
405+
Eventually(func(g Gomega) {
406+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
407+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
408+
}, timeout, interval).Should(Succeed())
409+
410+
Eventually(func(g Gomega) {
411+
barbican := GetBarbican(barbicanName)
412+
emptyNodeSelector := map[string]string{}
413+
barbican.Spec.BarbicanAPI.NodeSelector = &emptyNodeSelector
414+
g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
415+
}, timeout, interval).Should(Succeed())
416+
417+
Eventually(func(g Gomega) {
418+
th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
419+
g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
420+
g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(BeNil())
421+
}, timeout, interval).Should(Succeed())
422+
})
423+
})
424+
283425
// Run MariaDBAccount suite tests. these are pre-packaged ginkgo tests
284426
// that exercise standard account create / update patterns that should be
285427
// common to all controllers that ensure MariaDBAccount CRs.

0 commit comments

Comments
 (0)