Skip to content

Commit 73233e7

Browse files
ChunyiLyuChunyiLyu
authored
Merge pull request #394 from rabbitmq/configuration-updates
Configuration updates
2 parents b131459 + 9a481bc commit 73233e7

14 files changed

+166
-94
lines changed

‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ list: ## list Makefile targets
1212
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-20s\033[0m %s\n", $$1, $$2}'
1313

1414
unit-tests: install-tools generate fmt vet manifests ## Run unit tests
15-
ginkgo -r api/ internal/
15+
ginkgo -r --randomizeAllSpecs api/ internal/
1616

1717
integration-tests: install-tools generate fmt vet manifests ## Run integration tests
1818
ginkgo -r controllers/

‎controllers/rabbitmqcluster_controller.go

+84-42
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ const (
6060
deletionFinalizer = "deletion.finalizers.rabbitmqclusters.rabbitmq.com"
6161
pluginsUpdateAnnotation = "rabbitmq.com/pluginsUpdatedAt"
6262
queueRebalanceAnnotation = "rabbitmq.com/queueRebalanceNeededAt"
63+
serverConfAnnotation = "rabbitmq.com/serverConfUpdatedAt"
64+
stsRestartAnnotation = "rabbitmq.com/lastRestartAt"
6365
)
6466

6567
// RabbitmqClusterReconciler reconciles a RabbitmqCluster object
@@ -103,28 +105,27 @@ func (r *RabbitmqClusterReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er
103105
return ctrl.Result{}, nil
104106
}
105107

106-
// Resource has been marked for deletion
108+
// Check if the resource has been marked for deletion
107109
if !rabbitmqCluster.ObjectMeta.DeletionTimestamp.IsZero() {
108110
logger.Info("Deleting RabbitmqCluster",
109111
"namespace", rabbitmqCluster.Namespace,
110112
"name", rabbitmqCluster.Name)
111-
// Stop reconciliation as the item is being deleted
112113
return ctrl.Result{}, r.prepareForDeletion(ctx, rabbitmqCluster)
113114
}
114115

116+
// Ensure the resource have a deletion marker
117+
if err := r.addFinalizerIfNeeded(ctx, rabbitmqCluster); err != nil {
118+
return ctrl.Result{}, err
119+
}
120+
115121
// TLS: check if specified, and if secret exists
116122
if rabbitmqCluster.TLSEnabled() {
117123
if result, err := r.checkTLSSecrets(ctx, rabbitmqCluster); err != nil {
118124
return result, err
119125
}
120126
}
121127

122-
if err := r.addFinalizerIfNeeded(ctx, rabbitmqCluster); err != nil {
123-
return ctrl.Result{}, err
124-
}
125-
126128
childResources, err := r.getChildResources(ctx, *rabbitmqCluster)
127-
128129
if err != nil {
129130
return ctrl.Result{}, err
130131
}
@@ -195,12 +196,19 @@ func (r *RabbitmqClusterReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er
195196
return ctrl.Result{}, err
196197
}
197198

198-
r.annotatePluginsConfigMapIfUpdated(ctx, builder, operationResult, rabbitmqCluster)
199-
if restarted := r.restartStatefulSetIfNeeded(ctx, builder, operationResult, rabbitmqCluster); restarted {
200-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
199+
if err = r.annotateConfigMapIfUpdated(ctx, builder, operationResult, rabbitmqCluster); err != nil {
200+
return ctrl.Result{}, err
201201
}
202202
}
203203

204+
requeueAfter, err := r.restartStatefulSetIfNeeded(ctx, rabbitmqCluster)
205+
if err != nil {
206+
return ctrl.Result{}, err
207+
}
208+
if requeueAfter > 0 {
209+
return ctrl.Result{RequeueAfter: requeueAfter}, nil
210+
}
211+
204212
// Set ReconcileSuccess to true here because all CRUD operations to Kube API related
205213
// to child resources returned no error
206214
rabbitmqCluster.Status.SetCondition(status.ReconcileSuccess, corev1.ConditionTrue, "Success", "Created or Updated all child resources")
@@ -216,7 +224,7 @@ func (r *RabbitmqClusterReconciler) Reconcile(req ctrl.Request) (ctrl.Result, er
216224

217225
// By this point the StatefulSet may have finished deploying. Run any
218226
// post-deploy steps if so, or requeue until the deployment is finished.
219-
requeueAfter, err := r.runPostDeployStepsIfNeeded(ctx, rabbitmqCluster)
227+
requeueAfter, err = r.runPostDeployStepsIfNeeded(ctx, rabbitmqCluster)
220228
if err != nil {
221229
return ctrl.Result{}, err
222230
}
@@ -338,7 +346,7 @@ func (r *RabbitmqClusterReconciler) runPostDeployStepsIfNeeded(ctx context.Conte
338346
}
339347

340348
// Retrieve the plugins config map, if it exists.
341-
pluginsConfig, err := r.pluginsConfigMap(ctx, rmq)
349+
pluginsConfig, err := r.configMap(ctx, rmq, rmq.ChildResourceName(resource.PluginsConfigName))
342350
if client.IgnoreNotFound(err) != nil {
343351
return 0, err
344352
}
@@ -419,16 +427,32 @@ func (r *RabbitmqClusterReconciler) runSetPluginsCommand(ctx context.Context, rm
419427
return nil
420428
}
421429

422-
// Adds an arbitrary annotation (rabbitmq.com/lastRestartAt) to the StatefulSet PodTemplate to trigger a StatefulSet restart
423-
// if builder requires StatefulSet to be updated.
424-
func (r *RabbitmqClusterReconciler) restartStatefulSetIfNeeded(
425-
ctx context.Context,
426-
builder resource.ResourceBuilder,
427-
operationResult controllerutil.OperationResult,
428-
rmq *rabbitmqv1beta1.RabbitmqCluster) (restarted bool) {
430+
// Adds an arbitrary annotation to the sts PodTemplate to trigger a sts restart
431+
// it compares annotation "rabbitmq.com/serverConfUpdatedAt" from server-conf configMap and annotation "rabbitmq.com/lastRestartAt" from sts
432+
// to determine whether to restart sts
433+
func (r *RabbitmqClusterReconciler) restartStatefulSetIfNeeded(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster) (time.Duration, error) {
434+
serverConf, err := r.configMap(ctx, rmq, rmq.ChildResourceName(resource.ServerConfigMapName))
435+
if err != nil {
436+
// requeue request after 10s if unable to find server-conf configmap, else return the error
437+
return 10 * time.Second, client.IgnoreNotFound(err)
438+
}
429439

430-
if !(builder.UpdateRequiresStsRestart() && operationResult == controllerutil.OperationResultUpdated) {
431-
return false
440+
serverConfigUpdatedAt, ok := serverConf.Annotations[serverConfAnnotation]
441+
if !ok {
442+
// server-conf configmap hasn't been updated; no need to restart sts
443+
return 0, nil
444+
}
445+
446+
sts, err := r.statefulSet(ctx, rmq)
447+
if err != nil {
448+
// requeue request after 10s if unable to find sts, else return the error
449+
return 10 * time.Second, client.IgnoreNotFound(err)
450+
}
451+
452+
stsRestartedAt, ok := sts.Spec.Template.ObjectMeta.Annotations[stsRestartAnnotation]
453+
if ok && stsRestartedAt > serverConfigUpdatedAt {
454+
// sts was updated after the last server-conf configmap update; no need to restart sts
455+
return 0, nil
432456
}
433457

434458
if err := clientretry.RetryOnConflict(clientretry.DefaultRetry, func() error {
@@ -439,19 +463,21 @@ func (r *RabbitmqClusterReconciler) restartStatefulSetIfNeeded(
439463
if sts.Spec.Template.ObjectMeta.Annotations == nil {
440464
sts.Spec.Template.ObjectMeta.Annotations = make(map[string]string)
441465
}
442-
sts.Spec.Template.ObjectMeta.Annotations["rabbitmq.com/lastRestartAt"] = time.Now().Format(time.RFC3339)
466+
sts.Spec.Template.ObjectMeta.Annotations[stsRestartAnnotation] = time.Now().Format(time.RFC3339)
443467
return r.Update(ctx, sts)
444468
}); err != nil {
445469
msg := fmt.Sprintf("failed to restart StatefulSet %s of Namespace %s; rabbitmq.conf configuration may be outdated", rmq.ChildResourceName("server"), rmq.Namespace)
446470
r.Log.Error(err, msg)
447471
r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedUpdate", msg)
448-
return false
472+
// failed to restart sts; return error to requeue request
473+
return 0, err
449474
}
450475

451476
msg := fmt.Sprintf("restarted StatefulSet %s of Namespace %s", rmq.ChildResourceName("server"), rmq.Namespace)
452477
r.Log.Info(msg)
453478
r.Recorder.Event(rmq, corev1.EventTypeNormal, "SuccessfulUpdate", msg)
454-
return true
479+
480+
return 0, nil
455481
}
456482

457483
func (r *RabbitmqClusterReconciler) statefulSet(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster) (*appsv1.StatefulSet, error) {
@@ -462,43 +488,59 @@ func (r *RabbitmqClusterReconciler) statefulSet(ctx context.Context, rmq *rabbit
462488
return sts, nil
463489
}
464490

465-
func (r *RabbitmqClusterReconciler) pluginsConfigMap(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster) (*corev1.ConfigMap, error) {
491+
func (r *RabbitmqClusterReconciler) configMap(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster, name string) (*corev1.ConfigMap, error) {
466492
configMap := &corev1.ConfigMap{}
467-
if err := r.Get(ctx, types.NamespacedName{Namespace: rmq.Namespace, Name: rmq.ChildResourceName(resource.PluginsConfig)}, configMap); err != nil {
493+
if err := r.Get(ctx, types.NamespacedName{Namespace: rmq.Namespace, Name: name}, configMap); err != nil {
468494
return nil, err
469495
}
470496
return configMap, nil
471497
}
472498

473-
// Annotates the plugins ConfigMap if it was updated such that 'rabbitmq-plugins set' will be called on the RabbitMQ nodes at a later point in time
474-
func (r *RabbitmqClusterReconciler) annotatePluginsConfigMapIfUpdated(
475-
ctx context.Context,
476-
builder resource.ResourceBuilder,
477-
operationResult controllerutil.OperationResult,
478-
rmq *rabbitmqv1beta1.RabbitmqCluster) {
499+
// Annotates the plugins ConfigMap or the server-conf ConfigMap
500+
// annotations later used to indicate whether to call 'rabbitmq-plugins set' or to restart the sts
501+
func (r *RabbitmqClusterReconciler) annotateConfigMapIfUpdated(ctx context.Context, builder resource.ResourceBuilder, operationResult controllerutil.OperationResult, rmq *rabbitmqv1beta1.RabbitmqCluster) error {
502+
if operationResult != controllerutil.OperationResultUpdated {
503+
return nil
504+
}
479505

480-
if _, ok := builder.(*resource.RabbitmqPluginsConfigMapBuilder); !ok {
481-
return
506+
var configMap, annotationKey string
507+
switch builder.(type) {
508+
case *resource.RabbitmqPluginsConfigMapBuilder:
509+
configMap = rmq.ChildResourceName(resource.PluginsConfigName)
510+
annotationKey = pluginsUpdateAnnotation
511+
case *resource.ServerConfigMapBuilder:
512+
configMap = rmq.ChildResourceName(resource.ServerConfigMapName)
513+
annotationKey = serverConfAnnotation
514+
default:
515+
return nil
482516
}
483-
if operationResult != controllerutil.OperationResultUpdated {
484-
return
517+
518+
if err := r.annotateConfigMap(ctx, rmq.Namespace, configMap, annotationKey, time.Now().Format(time.RFC3339)); err != nil {
519+
msg := fmt.Sprintf("Failed to annotate ConfigMap %s of Namespace %s; %s may be outdated", configMap, rmq.Namespace, rmq.Name)
520+
r.Log.Error(err, msg)
521+
r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedUpdate", msg)
522+
return err
485523
}
486524

525+
r.Log.Info("successfully annotated", "ConfigMap", configMap, "Namespace", rmq.Namespace)
526+
return nil
527+
}
528+
529+
func (r *RabbitmqClusterReconciler) annotateConfigMap(ctx context.Context, namespace, name, key, value string) error {
487530
if retryOnConflictErr := clientretry.RetryOnConflict(clientretry.DefaultRetry, func() error {
488531
configMap := corev1.ConfigMap{}
489-
if err := r.Get(ctx, types.NamespacedName{Namespace: rmq.Namespace, Name: rmq.ChildResourceName(resource.PluginsConfig)}, &configMap); err != nil {
532+
if err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, &configMap); err != nil {
490533
return client.IgnoreNotFound(err)
491534
}
492535
if configMap.Annotations == nil {
493536
configMap.Annotations = make(map[string]string)
494537
}
495-
configMap.Annotations[pluginsUpdateAnnotation] = time.Now().Format(time.RFC3339)
538+
configMap.Annotations[key] = value
496539
return r.Update(ctx, &configMap)
497540
}); retryOnConflictErr != nil {
498-
msg := fmt.Sprintf("Failed to annotate ConfigMap %s of Namespace %s; enabled_plugins may be outdated", rmq.ChildResourceName(resource.PluginsConfig), rmq.Namespace)
499-
r.Log.Error(retryOnConflictErr, msg)
500-
r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedUpdate", msg)
541+
return retryOnConflictErr
501542
}
543+
return nil
502544
}
503545

504546
func (r *RabbitmqClusterReconciler) exec(namespace, podName, containerName string, command ...string) (string, string, error) {
@@ -600,8 +642,8 @@ func (r *RabbitmqClusterReconciler) addRabbitmqDeletionLabel(ctx context.Context
600642
return nil
601643
}
602644

645+
// addFinalizerIfNeeded adds a deletion finalizer if the RabbitmqCluster does not have one yet and is not marked for deletion
603646
func (r *RabbitmqClusterReconciler) addFinalizerIfNeeded(ctx context.Context, rabbitmqCluster *rabbitmqv1beta1.RabbitmqCluster) error {
604-
// The RabbitmqCluster is not marked for deletion (no deletion timestamp) but does not have the deletion finalizer
605647
if rabbitmqCluster.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(rabbitmqCluster, deletionFinalizer) {
606648
controllerutil.AddFinalizer(rabbitmqCluster, deletionFinalizer)
607649
if err := r.Client.Update(ctx, rabbitmqCluster); err != nil {

‎controllers/rabbitmqcluster_controller_test.go

+75-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"k8s.io/apimachinery/pkg/util/intstr"
1919

2020
. "github.com/onsi/ginkgo"
21+
. "github.com/onsi/ginkgo/extensions/table"
2122
. "github.com/onsi/gomega"
2223
rabbitmqv1beta1 "github.com/rabbitmq/cluster-operator/api/v1beta1"
2324
"github.com/rabbitmq/cluster-operator/internal/resource"
@@ -1403,7 +1404,7 @@ var _ = Describe("RabbitmqClusterController", func() {
14031404
})
14041405
})
14051406

1406-
Context("Cluster restarts", func() {
1407+
Context("Cluster post deploy steps", func() {
14071408
var annotations map[string]string
14081409

14091410
BeforeEach(func() {
@@ -1501,7 +1502,7 @@ var _ = Describe("RabbitmqClusterController", func() {
15011502
BeforeEach(func() {
15021503
cluster = &rabbitmqv1beta1.RabbitmqCluster{
15031504
ObjectMeta: metav1.ObjectMeta{
1504-
Name: "rabbitmq-three",
1505+
Name: "rabbitmq-three-no-post-deploy",
15051506
Namespace: defaultNamespace,
15061507
},
15071508
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
@@ -1561,11 +1562,11 @@ var _ = Describe("RabbitmqClusterController", func() {
15611562
})
15621563
})
15631564

1564-
When("the cluster is only 1 node large", func() {
1565+
When("the cluster is a single node cluster", func() {
15651566
BeforeEach(func() {
15661567
cluster = &rabbitmqv1beta1.RabbitmqCluster{
15671568
ObjectMeta: metav1.ObjectMeta{
1568-
Name: "rabbitmq-one",
1569+
Name: "rabbitmq-one-rebalance",
15691570
Namespace: defaultNamespace,
15701571
},
15711572
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
@@ -1627,6 +1628,76 @@ var _ = Describe("RabbitmqClusterController", func() {
16271628
})
16281629

16291630
})
1631+
1632+
DescribeTable("Server configurations updates",
1633+
func(testCase string) {
1634+
// create rabbitmqcluster
1635+
cluster = &rabbitmqv1beta1.RabbitmqCluster{
1636+
ObjectMeta: metav1.ObjectMeta{
1637+
Name: "rabbitmq-" + testCase,
1638+
Namespace: defaultNamespace,
1639+
},
1640+
Spec: rabbitmqv1beta1.RabbitmqClusterSpec{
1641+
Replicas: &one,
1642+
},
1643+
}
1644+
Expect(client.Create(ctx, cluster)).To(Succeed())
1645+
waitForClusterCreation(ctx, cluster, client)
1646+
1647+
// ensure that configMap and statefulSet does not have annotations set when configurations haven't changed
1648+
configMap, err := clientSet.CoreV1().ConfigMaps(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server-conf"), metav1.GetOptions{})
1649+
Expect(err).To(Not(HaveOccurred()))
1650+
Expect(configMap.Annotations).ShouldNot(HaveKey("rabbitmq.com/serverConfUpdatedAt"))
1651+
1652+
sts, err := clientSet.AppsV1().StatefulSets(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{})
1653+
Expect(err).To(Not(HaveOccurred()))
1654+
Expect(sts.Annotations).ShouldNot(HaveKey("rabbitmq.com/lastRestartAt"))
1655+
1656+
// update rabbitmq server configurations
1657+
Expect(updateWithRetry(cluster, func(r *rabbitmqv1beta1.RabbitmqCluster) {
1658+
if testCase == "additional-config" {
1659+
r.Spec.Rabbitmq.AdditionalConfig = "test_config=0"
1660+
}
1661+
if testCase == "advanced-config" {
1662+
r.Spec.Rabbitmq.AdvancedConfig = "sample-advanced-config."
1663+
}
1664+
if testCase == "env-config" {
1665+
r.Spec.Rabbitmq.EnvConfig = "some-env-variable"
1666+
}
1667+
})).To(Succeed())
1668+
1669+
By("annotating the server-conf ConfigMap")
1670+
// ensure annotations from the server-conf ConfigMap
1671+
var annotations map[string]string
1672+
Eventually(func() map[string]string {
1673+
configMap, err := clientSet.CoreV1().ConfigMaps(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server-conf"), metav1.GetOptions{})
1674+
Expect(err).To(Not(HaveOccurred()))
1675+
annotations = configMap.Annotations
1676+
return annotations
1677+
}, 5).Should(HaveKey("rabbitmq.com/serverConfUpdatedAt"))
1678+
_, err = time.Parse(time.RFC3339, annotations["rabbitmq.com/serverConfUpdatedAt"])
1679+
Expect(err).NotTo(HaveOccurred(), "Annotation rabbitmq.com/serverConfUpdatedAt was not a valid RFC3339 timestamp")
1680+
1681+
By("annotating the sts podTemplate")
1682+
// ensure statefulSet annotations
1683+
Eventually(func() map[string]string {
1684+
sts, err := clientSet.AppsV1().StatefulSets(cluster.Namespace).Get(ctx, cluster.ChildResourceName("server"), metav1.GetOptions{})
1685+
Expect(err).To(Not(HaveOccurred()))
1686+
annotations = sts.Spec.Template.Annotations
1687+
return annotations
1688+
}, 5).Should(HaveKey("rabbitmq.com/lastRestartAt"))
1689+
_, err = time.Parse(time.RFC3339, annotations["rabbitmq.com/lastRestartAt"])
1690+
Expect(err).NotTo(HaveOccurred(), "Annotation rabbitmq.com/lastRestartAt was not a valid RFC3339 timestamp")
1691+
1692+
// delete rmq cluster
1693+
Expect(client.Delete(ctx, cluster)).To(Succeed())
1694+
waitForClusterDeletion(ctx, cluster, client)
1695+
},
1696+
1697+
Entry("spec.rabbitmq.additionalConfig is updated", "additional-config"),
1698+
Entry("spec.rabbitmq.advancedConfig is updated", "advanced-config"),
1699+
Entry("spec.rabbitmq.envConfig is updated", "env-config"),
1700+
)
16301701
})
16311702

16321703
func extractContainer(containers []corev1.Container, containerName string) corev1.Container {

‎internal/resource/client_service.go

-4
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,6 @@ type ClientServiceBuilder struct {
3535
Scheme *runtime.Scheme
3636
}
3737

38-
func (builder *ClientServiceBuilder) UpdateRequiresStsRestart() bool {
39-
return false
40-
}
41-
4238
func (builder *ClientServiceBuilder) Build() (runtime.Object, error) {
4339
return &corev1.Service{
4440
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)