From 9c4744e77cb6cb17354bbaeabd8db5f471104281 Mon Sep 17 00:00:00 2001 From: srteam2020 Date: Sun, 9 May 2021 19:42:01 -0500 Subject: [PATCH 1/5] fix issue-314 by checking zookeeperCluster resource version before updating sts --- .../zookeepercluster_controller.go | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller.go b/pkg/controller/zookeepercluster/zookeepercluster_controller.go index f05f3ec73..34f4e6706 100644 --- a/pkg/controller/zookeepercluster/zookeepercluster_controller.go +++ b/pkg/controller/zookeepercluster/zookeepercluster_controller.go @@ -164,6 +164,28 @@ func (r *ReconcileZookeeperCluster) Reconcile(request reconcile.Request) (reconc return reconcile.Result{RequeueAfter: ReconcileTime}, nil } +func (r *ReconcileZookeeperCluster) zookeeperClusterUpdated(instance *zookeeperv1beta1.ZookeeperCluster, sts *appsv1.StatefulSet) bool { + labeledRV, ok := sts.Labels["owner-rv"] + if !ok { + log.Info("Fail to find owner-rv in statefulset's labels; skip checking resource version") + return true + } + largerRV, err := strconv.Atoi(instance.ResourceVersion) + if err != nil { + log.Info("Fail to convert %s to integer; skip checking resource version", instance.ResourceVersion) + return true + } + smallerRV, err := strconv.Atoi(labeledRV) + if err != nil { + log.Info("Fail to convert %s to integer; skip checking resource version", labeledRV) + return true + } + if largerRV < smallerRV { + return false + } + return true +} + func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1beta1.ZookeeperCluster) (err error) { // we cannot upgrade if cluster is in UpgradeFailed @@ -201,6 +223,8 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be r.log.Info("Creating a new Zookeeper StatefulSet", "StatefulSet.Namespace", sts.Namespace, "StatefulSet.Name", sts.Name) + // label the RV of the zookeeperCluster when creating the sts + sts.Labels["owner-rv"] = instance.ResourceVersion err = r.client.Create(context.TODO(), sts) if err != nil { return err @@ -230,6 +254,10 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be r.log.Info("Updating Cluster Size.", "New Data:", data, "Version", version) r.zkClient.UpdateNode(path, data, version) } + // check whether zookeeperCluster is updated before updating the sts + if ! r.zookeeperClusterUpdated(instance, sts) { + return fmt.Errorf("Staleness: cr.ResourceVersion %s is smaller than labeledRV %s", instance.ResourceVersion, sts.Labels["owner-rv"]) + } err = r.updateStatefulSet(instance, foundSts, sts) if err != nil { return err From 8e62f0a17cda1a4259cf7f689e498066aca91e59 Mon Sep 17 00:00:00 2001 From: srteam2020 Date: Sun, 9 May 2021 19:46:03 -0500 Subject: [PATCH 2/5] move the checking earlier --- .../zookeepercluster/zookeepercluster_controller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller.go b/pkg/controller/zookeepercluster/zookeepercluster_controller.go index 34f4e6706..345d05d7e 100644 --- a/pkg/controller/zookeepercluster/zookeepercluster_controller.go +++ b/pkg/controller/zookeepercluster/zookeepercluster_controller.go @@ -233,6 +233,10 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be } else if err != nil { return err } else { + // check whether zookeeperCluster is updated before updating the sts + if ! r.zookeeperClusterUpdated(instance, sts) { + return fmt.Errorf("Staleness: cr.ResourceVersion %s is smaller than labeledRV %s", instance.ResourceVersion, sts.Labels["owner-rv"]) + } foundSTSSize := *foundSts.Spec.Replicas newSTSSize := *sts.Spec.Replicas if newSTSSize != foundSTSSize { @@ -254,10 +258,6 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be r.log.Info("Updating Cluster Size.", "New Data:", data, "Version", version) r.zkClient.UpdateNode(path, data, version) } - // check whether zookeeperCluster is updated before updating the sts - if ! r.zookeeperClusterUpdated(instance, sts) { - return fmt.Errorf("Staleness: cr.ResourceVersion %s is smaller than labeledRV %s", instance.ResourceVersion, sts.Labels["owner-rv"]) - } err = r.updateStatefulSet(instance, foundSts, sts) if err != nil { return err From 78718d88dd09b2a2a102e7d9f2a7c518b33f11b9 Mon Sep 17 00:00:00 2001 From: srteam2020 Date: Sun, 9 May 2021 23:04:25 -0500 Subject: [PATCH 3/5] go fmt --- pkg/controller/zookeepercluster/zookeepercluster_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller.go b/pkg/controller/zookeepercluster/zookeepercluster_controller.go index 345d05d7e..83b4ed58d 100644 --- a/pkg/controller/zookeepercluster/zookeepercluster_controller.go +++ b/pkg/controller/zookeepercluster/zookeepercluster_controller.go @@ -234,7 +234,7 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be return err } else { // check whether zookeeperCluster is updated before updating the sts - if ! r.zookeeperClusterUpdated(instance, sts) { + if !r.zookeeperClusterUpdated(instance, sts) { return fmt.Errorf("Staleness: cr.ResourceVersion %s is smaller than labeledRV %s", instance.ResourceVersion, sts.Labels["owner-rv"]) } foundSTSSize := *foundSts.Spec.Replicas From 0f84c8c2a1f278392b7441705339c62cf72ed2a3 Mon Sep 17 00:00:00 2001 From: srteam2020 Date: Sat, 5 Jun 2021 11:55:36 -0500 Subject: [PATCH 4/5] rename the function and add unit test --- .../zookeepercluster_controller.go | 4 +-- .../zookeepercluster_controller_test.go | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller.go b/pkg/controller/zookeepercluster/zookeepercluster_controller.go index 83b4ed58d..67d4a3540 100644 --- a/pkg/controller/zookeepercluster/zookeepercluster_controller.go +++ b/pkg/controller/zookeepercluster/zookeepercluster_controller.go @@ -164,7 +164,7 @@ func (r *ReconcileZookeeperCluster) Reconcile(request reconcile.Request) (reconc return reconcile.Result{RequeueAfter: ReconcileTime}, nil } -func (r *ReconcileZookeeperCluster) zookeeperClusterUpdated(instance *zookeeperv1beta1.ZookeeperCluster, sts *appsv1.StatefulSet) bool { +func zookeeperClusterFresherThanSts(instance *zookeeperv1beta1.ZookeeperCluster, sts *appsv1.StatefulSet) bool { labeledRV, ok := sts.Labels["owner-rv"] if !ok { log.Info("Fail to find owner-rv in statefulset's labels; skip checking resource version") @@ -234,7 +234,7 @@ func (r *ReconcileZookeeperCluster) reconcileStatefulSet(instance *zookeeperv1be return err } else { // check whether zookeeperCluster is updated before updating the sts - if !r.zookeeperClusterUpdated(instance, sts) { + if !zookeeperClusterFresherThanSts(instance, sts) { return fmt.Errorf("Staleness: cr.ResourceVersion %s is smaller than labeledRV %s", instance.ResourceVersion, sts.Labels["owner-rv"]) } foundSTSSize := *foundSts.Spec.Replicas diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go b/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go index 13364fd2b..267d04b7d 100644 --- a/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go +++ b/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go @@ -563,5 +563,36 @@ var _ = Describe("ZookeeperCluster Controller", func() { Ω(err).To(BeNil()) }) }) + + Context("Checking resource version", func(){ + var ( + sts *appsv1.StatefulSet + ) + + BeforeEach(func() { + z.WithDefaults() + z.ResourceVersion = "100" + sts = &appsv1.StatefulSet{} + sts.Labels = make(map[string]string) + }) + + It("should return true as 99 < 100", func() { + sts.Labels["owner-rv"] = "99" + updated := zookeeperClusterFresherThanSts(z, sts) + Ω(updated).To(BeTrue()) + }) + + It("should return true as 100 == 100", func() { + sts.Labels["owner-rv"] = "100" + updated := zookeeperClusterFresherThanSts(z, sts) + Ω(updated).To(BeTrue()) + }) + + It("should return false as 101 > 100", func() { + sts.Labels["owner-rv"] = "101" + updated := zookeeperClusterFresherThanSts(z, sts) + Ω(updated).To(BeFalse()) + }) + }) }) }) From 66409fd194e043860e7489458beb00d60bc5ecb7 Mon Sep 17 00:00:00 2001 From: srteam2020 Date: Sat, 5 Jun 2021 12:01:22 -0500 Subject: [PATCH 5/5] go fmt --- .../zookeepercluster/zookeepercluster_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go b/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go index 267d04b7d..cea195d10 100644 --- a/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go +++ b/pkg/controller/zookeepercluster/zookeepercluster_controller_test.go @@ -564,7 +564,7 @@ var _ = Describe("ZookeeperCluster Controller", func() { }) }) - Context("Checking resource version", func(){ + Context("Checking resource version", func() { var ( sts *appsv1.StatefulSet )