Skip to content

Commit b127e59

Browse files
ChunyiLyuChunyiLyu
committed
Stop updating label/annotation on server conf configMap
- updates to server-conf configMap triggers pod restart - instance labels and annotations are still applied at creation
1 parent 660aa43 commit b127e59

File tree

3 files changed

+77
-112
lines changed

3 files changed

+77
-112
lines changed

‎controllers/rabbitmqcluster_controller_test.go

+1-22
Original file line numberDiff line numberDiff line change
@@ -534,34 +534,13 @@ var _ = Describe("RabbitmqClusterController", func() {
534534
}, 3).Should(HaveKeyWithValue(annotationKey, annotationValue))
535535
})
536536

537-
It("updates annotations for config maps", func() {
538-
Eventually(func() map[string]string {
539-
roleBinding, err := clientSet.CoreV1().ConfigMaps(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server-conf"), metav1.GetOptions{})
540-
Expect(err).NotTo(HaveOccurred())
541-
return roleBinding.Annotations
542-
}, 3).Should(HaveKeyWithValue(annotationKey, annotationValue))
543-
544-
Eventually(func() map[string]string {
545-
roleBinding, err := clientSet.CoreV1().ConfigMaps(cluster.Namespace).Get(ctx, cluster.ChildResourceName("plugins-conf"), metav1.GetOptions{})
546-
Expect(err).NotTo(HaveOccurred())
547-
return roleBinding.Annotations
548-
}, 3).Should(HaveKeyWithValue(annotationKey, annotationValue))
549-
})
550-
551-
It("updates annotations for config maps", func() {
552-
Eventually(func() map[string]string {
553-
roleBinding, err := clientSet.CoreV1().ConfigMaps(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server-conf"), metav1.GetOptions{})
554-
Expect(err).NotTo(HaveOccurred())
555-
return roleBinding.Annotations
556-
}, 3).Should(HaveKeyWithValue(annotationKey, annotationValue))
557-
537+
It("updates annotations for plugins config map", func() {
558538
Eventually(func() map[string]string {
559539
roleBinding, err := clientSet.CoreV1().ConfigMaps(cluster.Namespace).Get(ctx, cluster.ChildResourceName("plugins-conf"), metav1.GetOptions{})
560540
Expect(err).NotTo(HaveOccurred())
561541
return roleBinding.Annotations
562542
}, 3).Should(HaveKeyWithValue(annotationKey, annotationValue))
563543
})
564-
565544
})
566545

567546
It("service type is updated", func() {

‎internal/resource/configmap.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,16 @@ func (builder *RabbitmqResourceBuilder) ServerConfigMap() *ServerConfigMapBuilde
5555
func (builder *ServerConfigMapBuilder) Build() (runtime.Object, error) {
5656
return &corev1.ConfigMap{
5757
ObjectMeta: metav1.ObjectMeta{
58-
Name: builder.Instance.ChildResourceName(ServerConfigMapName),
59-
Namespace: builder.Instance.Namespace,
58+
Name: builder.Instance.ChildResourceName(ServerConfigMapName),
59+
Namespace: builder.Instance.Namespace,
60+
Labels: metadata.GetLabels(builder.Instance.Name, builder.Instance.Labels),
61+
Annotations: metadata.ReconcileAndFilterAnnotations(nil, builder.Instance.Annotations),
6062
},
6163
}, nil
6264
}
6365

6466
func (builder *ServerConfigMapBuilder) Update(object runtime.Object) error {
6567
configMap := object.(*corev1.ConfigMap)
66-
configMap.Labels = metadata.GetLabels(builder.Instance.Name, builder.Instance.Labels)
67-
configMap.Annotations = metadata.ReconcileAndFilterAnnotations(configMap.GetAnnotations(), builder.Instance.Annotations)
6868

6969
ini.PrettySection = false // Remove trailing new line because rabbitmq.conf has only a default section.
7070
cfg, err := ini.Load([]byte(defaultRabbitmqConf))

‎internal/resource/configmap_test.go

+72-86
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,27 @@ var _ = Describe("GenerateServerConfigMap", func() {
6666
var configMap *corev1.ConfigMap
6767

6868
BeforeEach(func() {
69+
instance = rabbitmqv1beta1.RabbitmqCluster{
70+
ObjectMeta: metav1.ObjectMeta{
71+
Name: "rabbit-example",
72+
},
73+
}
74+
instance.Labels = map[string]string{
75+
"app.kubernetes.io/foo": "bar",
76+
"foo": "bar",
77+
"rabbitmq": "is-great",
78+
"foo/app.kubernetes.io": "edgecase",
79+
}
80+
instance.Annotations = map[string]string{
81+
"my-annotation": "i-like-this",
82+
"kubernetes.io/name": "i-do-not-like-this",
83+
"kubectl.kubernetes.io/name": "i-do-not-like-this",
84+
"k8s.io/name": "i-do-not-like-this",
85+
"kubernetes.io/other": "i-do-not-like-this",
86+
"kubectl.kubernetes.io/other": "i-do-not-like-this",
87+
"k8s.io/other": "i-do-not-like-this",
88+
}
89+
6990
obj, err := configMapBuilder.Build()
7091
configMap = obj.(*corev1.ConfigMap)
7192
Expect(err).NotTo(HaveOccurred())
@@ -75,6 +96,32 @@ var _ = Describe("GenerateServerConfigMap", func() {
7596
Expect(configMap.Name).To(Equal(builder.Instance.ChildResourceName("server-conf")))
7697
Expect(configMap.Namespace).To(Equal(builder.Instance.Namespace))
7798
})
99+
100+
It("adds labels from the instance and default labels", func() {
101+
Expect(len(configMap.Labels)).To(Equal(6))
102+
Expect(configMap.Labels).To(SatisfyAll(
103+
HaveKeyWithValue("foo", "bar"),
104+
HaveKeyWithValue("rabbitmq", "is-great"),
105+
HaveKeyWithValue("foo/app.kubernetes.io", "edgecase"),
106+
HaveKeyWithValue("app.kubernetes.io/name", instance.Name),
107+
HaveKeyWithValue("app.kubernetes.io/component", "rabbitmq"),
108+
HaveKeyWithValue("app.kubernetes.io/part-of", "rabbitmq"),
109+
Not(HaveKey("app.kubernetes.io/foo")),
110+
))
111+
})
112+
113+
It("adds annotations from the instance", func() {
114+
Expect(len(configMap.Annotations)).To(Equal(1))
115+
Expect(configMap.Annotations).To(SatisfyAll(
116+
HaveKeyWithValue("my-annotation", "i-like-this"),
117+
Not(HaveKey("kubernetes.io/name")),
118+
Not(HaveKey("kubectl.kubernetes.io/name")),
119+
Not(HaveKey("k8s.io/name")),
120+
Not(HaveKey("kubernetes.io/other")),
121+
Not(HaveKey("kubectl.kubernetes.io/other")),
122+
Not(HaveKey("k8s.io/other")),
123+
))
124+
})
78125
})
79126

80127
Context("Update", func() {
@@ -284,94 +331,33 @@ ssl_options.verify = verify_peer
284331
})
285332
})
286333

287-
Context("labels", func() {
288-
BeforeEach(func() {
289-
instance = rabbitmqv1beta1.RabbitmqCluster{
290-
ObjectMeta: metav1.ObjectMeta{
291-
Name: "rabbit-labelled",
292-
},
293-
}
294-
instance.Labels = map[string]string{
295-
"app.kubernetes.io/foo": "bar",
296-
"foo": "bar",
297-
"rabbitmq": "is-great",
298-
"foo/app.kubernetes.io": "edgecase",
299-
}
300-
301-
configMap = &corev1.ConfigMap{
302-
ObjectMeta: metav1.ObjectMeta{
303-
Labels: map[string]string{
304-
"app.kubernetes.io/name": instance.Name,
305-
"app.kubernetes.io/part-of": "rabbitmq",
306-
"this-was-the-previous-label": "should-be-deleted",
307-
},
308-
},
309-
}
310-
err := configMapBuilder.Update(configMap)
311-
Expect(err).NotTo(HaveOccurred())
312-
})
313-
314-
It("adds labels from the CR", func() {
315-
testLabels(configMap.Labels)
316-
})
317-
318-
It("restores the default labels", func() {
319-
labels := configMap.Labels
320-
Expect(labels["app.kubernetes.io/name"]).To(Equal(instance.Name))
321-
Expect(labels["app.kubernetes.io/component"]).To(Equal("rabbitmq"))
322-
Expect(labels["app.kubernetes.io/part-of"]).To(Equal("rabbitmq"))
323-
})
324-
325-
It("deletes the labels that are removed from the CR", func() {
326-
Expect(configMap.Labels).NotTo(HaveKey("this-was-the-previous-label"))
327-
})
334+
// this is to ensure that pods are not restarted when instance labels are updated
335+
It("does not update labels on the config map", func() {
336+
configMap.Labels = map[string]string{
337+
"app.kubernetes.io/name": instance.Name,
338+
"app.kubernetes.io/component": "rabbitmq",
339+
"app.kubernetes.io/part-of": "rabbitmq",
340+
}
341+
instance.Labels = map[string]string{
342+
"new-label": "test",
343+
}
344+
Expect(configMapBuilder.Update(configMap)).To(Succeed())
345+
Expect(len(configMap.Labels)).To(Equal(3))
346+
Expect(configMap.Labels).To(SatisfyAll(
347+
HaveKeyWithValue("app.kubernetes.io/name", instance.Name),
348+
HaveKeyWithValue("app.kubernetes.io/component", "rabbitmq"),
349+
HaveKeyWithValue("app.kubernetes.io/part-of", "rabbitmq"),
350+
Not(HaveKey("new-label")),
351+
))
328352
})
329353

330-
Context("instance annotations", func() {
331-
BeforeEach(func() {
332-
instance = rabbitmqv1beta1.RabbitmqCluster{
333-
ObjectMeta: metav1.ObjectMeta{
334-
Name: "rabbit-labelled",
335-
},
336-
}
337-
instance.Annotations = map[string]string{
338-
"my-annotation": "i-like-this",
339-
"kubernetes.io/name": "i-do-not-like-this",
340-
"kubectl.kubernetes.io/name": "i-do-not-like-this",
341-
"k8s.io/name": "i-do-not-like-this",
342-
"kubernetes.io/other": "i-do-not-like-this",
343-
"kubectl.kubernetes.io/other": "i-do-not-like-this",
344-
"k8s.io/other": "i-do-not-like-this",
345-
}
346-
347-
configMap = &corev1.ConfigMap{
348-
ObjectMeta: metav1.ObjectMeta{
349-
Annotations: map[string]string{
350-
"my-annotation": "i-will-not-stay",
351-
"old-annotation": "old-value",
352-
"im-here-to-stay.kubernetes.io": "for-a-while",
353-
"kubernetes.io/name": "should-stay",
354-
"kubectl.kubernetes.io/name": "should-stay",
355-
"k8s.io/name": "should-stay",
356-
},
357-
},
358-
}
359-
err := configMapBuilder.Update(configMap)
360-
Expect(err).NotTo(HaveOccurred())
361-
})
362-
363-
It("updates config map annotations", func() {
364-
expectedAnnotations := map[string]string{
365-
"my-annotation": "i-like-this",
366-
"old-annotation": "old-value",
367-
"im-here-to-stay.kubernetes.io": "for-a-while",
368-
"kubernetes.io/name": "should-stay",
369-
"kubectl.kubernetes.io/name": "should-stay",
370-
"k8s.io/name": "should-stay",
371-
}
372-
373-
Expect(configMap.Annotations).To(Equal(expectedAnnotations))
374-
})
354+
// this is to ensure that pods are not restarted when instance annotations are updated
355+
It("does not update labels on the config map", func() {
356+
instance.Annotations = map[string]string{
357+
"new-annotation": "test",
358+
}
359+
Expect(configMapBuilder.Update(configMap)).To(Succeed())
360+
Expect(configMap.Annotations).To(BeEmpty())
375361
})
376362
})
377363
})

0 commit comments

Comments
 (0)