Merge branch 'master' into issue-410-fix-reconcile

.github/workflows/ci.yaml

@@ -18,10 +18,10 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - name: Set up Go 1.17
+    - name: Set up Go 1.21
       uses: actions/setup-go@v2
       with:
-        go-version: 1.17
+        go-version: "1.21"
       id: go
     - name: Set up Go for root
       run: |

Dockerfile

@@ -1,7 +1,7 @@
 ARG DOCKER_REGISTRY
 ARG DISTROLESS_DOCKER_REGISTRY
-ARG ALPINE_VERSION=3.17
-FROM ${DOCKER_REGISTRY:+$DOCKER_REGISTRY/}golang:1.19-alpine${ALPINE_VERSION} as go-builder
+ARG ALPINE_VERSION=3.18
+FROM ${DOCKER_REGISTRY:+$DOCKER_REGISTRY/}golang:1.21-alpine${ALPINE_VERSION} as go-builder
 
 ARG PROJECT_NAME=zookeeper-operator
 ARG REPO_PATH=github.com/pravega/$PROJECT_NAME

api/v1beta1/zookeepercluster_types.go

@@ -26,7 +26,7 @@ const (
 
 	// DefaultZkContainerVersion is the default tag used for for the zookeeper
 	// container
-	DefaultZkContainerVersion = "0.2.14"
+	DefaultZkContainerVersion = "0.2.15"
 
 	// DefaultZkContainerPolicy is the default container pull policy used
 	DefaultZkContainerPolicy = "Always"

api/v1beta1/zookeepercluster_types_test.go

@@ -79,7 +79,7 @@ var _ = Describe("ZookeeperCluster Types", func() {
 			})
 
 			It("Checking tostring() function", func() {
-				Ω(z.Spec.Image.ToString()).To(Equal("pravega/zookeeper:0.2.14"))
+				Ω(z.Spec.Image.ToString()).To(Equal("pravega/zookeeper:0.2.15"))
 			})
 
 		})

charts/zookeeper-operator/Chart.yaml

@@ -1,8 +1,8 @@
 apiVersion: v2
 name: zookeeper-operator
 description: Zookeeper Operator Helm chart for Kubernetes
-version: 0.2.14
-appVersion: 0.2.14
+version: 0.2.15
+appVersion: 0.2.15
 keywords:
 - zookeeper
 - storage

charts/zookeeper-operator/README.md

@@ -55,7 +55,7 @@ The following table lists the configurable parameters of the zookeeper-operator
 | `hooks.image.tag` | Image tag for batch jobs | `"v1.16.10"` |
 | `image.pullPolicy` | Image pull policy | `IfNotPresent` |
 | `image.repository` | Image repository | `pravega/zookeeper-operator` |
-| `image.tag` | Image tag | `0.2.14` |
+| `image.tag` | Image tag | `0.2.15` |
 | `labels` | Operator pod labels | `{}` |
 | `nodeSelector` | Map of key-value pairs to be present as labels in the node in which the pod should run | `{}` |
 | `rbac.create` | Create RBAC resources | `true` |

charts/zookeeper-operator/values.yaml

@@ -9,7 +9,7 @@ global:
 
 image:
   repository: pravega/zookeeper-operator
-  tag: 0.2.14
+  tag: 0.2.15
   pullPolicy: IfNotPresent
 
 securityContext: {}

charts/zookeeper/Chart.yaml

@@ -1,8 +1,8 @@
 apiVersion: v2
 name: zookeeper
 description: Zookeeper Helm chart for Kubernetes
-version: 0.2.14
-appVersion: 0.2.14
+version: 0.2.15
+appVersion: 0.2.15
 keywords:
 - zookeeper
 - storage

charts/zookeeper/README.md

@@ -57,7 +57,7 @@ The following table lists the configurable parameters of the zookeeper chart and
 | `maxUnavailableReplicas` | Max unavailable replicas in pdb | `1` |
 | `triggerRollingRestart` | If true, the zookeeper cluster is restarted. After the restart is triggered, this value is auto-reverted to false. | `false` |
 | `image.repository` | Image repository | `pravega/zookeeper` |
-| `image.tag` | Image tag | `0.2.14` |
+| `image.tag` | Image tag | `0.2.15` |
 | `image.pullPolicy` | Image pull policy | `IfNotPresent` |
 | `domainName` | External host name appended for dns annotation | |
 | `kubernetesClusterDomain` | Domain of the kubernetes cluster | `cluster.local` |

charts/zookeeper/values.yaml

@@ -3,7 +3,7 @@ maxUnavailableReplicas:
 
 image:
   repository: pravega/zookeeper
-  tag: 0.2.14
+  tag: 0.2.15
   pullPolicy: IfNotPresent
 
 triggerRollingRestart: false

config/manager/manager.yaml

@@ -16,7 +16,7 @@ spec:
       containers:
         - name: zookeeper-operator
           # Replace this with the built image name
-          image: pravega/zookeeper-operator:0.2.14
+          image: pravega/zookeeper-operator:0.2.15
           ports:
           - containerPort: 60000
             name: metrics

config/samples/ECS/zookeeper_v1beta1_zookeepercluster_cr.yaml

@@ -6,7 +6,7 @@ spec:
   replicas: 3
   image:
     repository: pravega/zookeeper
-    tag: 0.2.14
+    tag: 0.2.15
   storageType: persistence
   persistence:
     reclaimPolicy: Retain

config/samples/pravega/zookeeper_v1beta1_zookeepercluster_cr.yaml

@@ -6,7 +6,7 @@ spec:
   replicas: 3
   image:
     repository: pravega/zookeeper
-    tag: 0.2.14
+    tag: 0.2.15
   storageType: persistence
   persistence:
     reclaimPolicy: Delete

controllers/zookeepercluster_controller.go

@@ -135,9 +135,9 @@ func getRollingRestartAnnotation() (string, string) {
 // compareResourceVersion compare resoure versions for the supplied ZookeeperCluster and StatefulSet
 // resources
 // Returns:
-//  0 if versions are equal
+// 0 if versions are equal
 // -1 if ZookeeperCluster version is less than StatefulSet version
-//  1 if ZookeeperCluster version is greater than StatefulSet version
+// 1 if ZookeeperCluster version is greater than StatefulSet version
 func compareResourceVersion(zk *zookeeperv1beta1.ZookeeperCluster, sts *appsv1.StatefulSet) int {
 
 	zkResourceVersion, zkErr := strconv.Atoi(zk.ResourceVersion)
@@ -170,6 +170,33 @@ func compareResourceVersion(zk *zookeeperv1beta1.ZookeeperCluster, sts *appsv1.S
 func (r *ZookeeperClusterReconciler) reconcileStatefulSet(instance *zookeeperv1beta1.ZookeeperCluster) (err error) {
 
 	// we cannot upgrade if cluster is in UpgradeFailed
+	if instance.Status.IsClusterInUpgradeFailedState() {
+		sts := zk.MakeStatefulSet(instance)
+		if err = controllerutil.SetControllerReference(instance, sts, r.Scheme); err != nil {
+			return err
+		}
+		foundSts := &appsv1.StatefulSet{}
+		err = r.Client.Get(context.TODO(), types.NamespacedName{
+			Name:      sts.Name,
+			Namespace: sts.Namespace,
+		}, foundSts)
+		if err == nil {
+			err = r.Client.Update(context.TODO(), foundSts)
+			if err != nil {
+				return err
+			}
+			if foundSts.Status.Replicas == foundSts.Status.ReadyReplicas && foundSts.Status.CurrentRevision == foundSts.Status.UpdateRevision {
+				r.Log.Info("failed upgrade completed", "upgrade from:", instance.Status.CurrentVersion, "upgrade to:", instance.Status.TargetVersion)
+				instance.Status.CurrentVersion = instance.Status.TargetVersion
+				instance.Status.SetErrorConditionFalse()
+				return r.clearUpgradeStatus(instance)
+			} else {
+				r.Log.Info("Unable to recover failed upgrade, make sure all nodes are running the target version")
+			}
+
+		}
+	}
+
 	if instance.Status.IsClusterInUpgradeFailedState() {
 		return nil
 	}
@@ -604,7 +631,7 @@ func YAMLExporterReconciler(zookeepercluster *zookeeperv1beta1.ZookeeperCluster)
 	var scheme = scheme.Scheme
 	scheme.AddKnownTypes(zookeeperv1beta1.GroupVersion, zookeepercluster)
 	return &ZookeeperClusterReconciler{
-		Client:   fake.NewFakeClient(zookeepercluster),
+		Client:   fake.NewClientBuilder().WithRuntimeObjects(zookeepercluster).Build(),
 		Scheme:   scheme,
 		ZkClient: new(zk.DefaultZookeeperClient),
 	}

controllers/zookeepercluster_controller_test.go

@@ -106,7 +106,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			)
 
 			BeforeEach(func() {
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).WithStatusSubresource(z).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -135,7 +135,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 
 			BeforeEach(func() {
 				z.WithDefaults()
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).WithStatusSubresource(z).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -206,7 +206,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next := z.DeepCopy()
 				st := zk.MakeStatefulSet(z)
 				next.Spec.Replicas = 6
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -234,7 +234,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				z.Status.Init()
 				next := z.DeepCopy()
 				st := zk.MakeStatefulSet(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -265,7 +265,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				z.Status.Init()
 				next = z.DeepCopy()
 				sa = zk.MakeServiceAccount(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, sa).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, sa).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -282,7 +282,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			})
 			It("should update the service account", func() {
 				next.Spec.Pod.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "test-pull-secret"}}
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, sa).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, sa).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				_, err := r.Reconcile(context.TODO(), req)
 				Ω(err).To(BeNil())
@@ -307,7 +307,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next.Status.CurrentVersion = "0.2.6"
 				next.Status.SetPodsReadyConditionTrue()
 				st := zk.MakeStatefulSet(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).WithStatusSubresource(next).Build()
 				st = &appsv1.StatefulSet{}
 				err = cl.Get(context.TODO(), req.NamespacedName, st)
 				// changing the Revision value to simulate the upgrade scenario
@@ -367,7 +367,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next.Status.TargetVersion = "0.2.7"
 				next.Status.SetUpgradingConditionTrue(" ", " ")
 				st := zk.MakeStatefulSet(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).WithStatusSubresource(next).Build()
 				st = &appsv1.StatefulSet{}
 				err = cl.Get(context.TODO(), req.NamespacedName, st)
 				// changing the Revision value to simulate the upgrade scenario completion
@@ -414,7 +414,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next.Status.SetUpgradingConditionTrue(" ", "1")
 				next.Status.TargetVersion = "0.2.7"
 				st := zk.MakeStatefulSet(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).WithStatusSubresource(next).Build()
 				st = &appsv1.StatefulSet{}
 				err = cl.Get(context.TODO(), req.NamespacedName, st)
 				// changing the Revision value to simulate the upgrade scenario
@@ -443,6 +443,52 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			})
 		})
 
+		Context("Checking for healing of upgrade failed for zookeepercluster", func() {
+			var (
+				cl  client.Client
+				err error
+			)
+
+			BeforeEach(func() {
+				z.WithDefaults()
+				z.Status.Init()
+				next := z.DeepCopy()
+				next.Status.SetErrorConditionTrue("UpgradeFailed", " ")
+				next.Status.TargetVersion = "0.2.7"
+				next.Status.CurrentVersion = "0.2.6"
+				next.Status.ReadyReplicas = 3
+				next.Spec.Replicas = 3
+				next.Spec.Image.Tag = "0.2.7"
+				st := zk.MakeStatefulSet(z)
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				st = &appsv1.StatefulSet{}
+				err = cl.Get(context.TODO(), req.NamespacedName, st)
+				// changing the Revision value to simulate the upgrade scenario
+				st.Status.CurrentRevision = "updateRevision"
+				st.Status.UpdateRevision = "updateRevision"
+				st.Status.UpdatedReplicas = 2
+				cl.Status().Update(context.TODO(), st)
+				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
+				res, err = r.Reconcile(context.TODO(), req)
+				// sleeping for 3 seconds
+				time.Sleep(3 * time.Second)
+				// checking if more than 2 secs have passed from the last update time
+				err = checkSyncTimeout(next, " ", 1, 2*time.Second)
+
+			})
+
+			It("checking update replicas", func() {
+				foundZookeeper := &v1beta1.ZookeeperCluster{}
+				_ = cl.Get(context.TODO(), req.NamespacedName, foundZookeeper)
+				condition := foundZookeeper.Status.CurrentVersion
+				Ω(condition).To(Equal("0.2.7"))
+			})
+
+			It("should not raise an error", func() {
+				Ω(err).To(BeNil())
+			})
+		})
+
 		Context("Upgrading with Targetversion empty", func() {
 			var (
 				cl  client.Client
@@ -458,7 +504,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next.Status.TargetVersion = ""
 				next.Status.IsClusterInUpgradingState()
 				st := zk.MakeStatefulSet(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, st).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -482,7 +528,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			BeforeEach(func() {
 				z.WithDefaults()
 				z.Status.Init()
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).WithStatusSubresource(z).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				req.NamespacedName.Namespace = "temp"
 				res, err = r.Reconcile(context.TODO(), req)
@@ -503,7 +549,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			BeforeEach(func() {
 				z.WithDefaults()
 				z.Status.Init()
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).WithStatusSubresource(z).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -576,7 +622,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next := z.DeepCopy()
 				next.Spec.Ports[0].ContainerPort = 2182
 				svc := zk.MakeClientService(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 			})
@@ -594,7 +640,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			BeforeEach(func() {
 				z.WithDefaults()
 				z.Spec.Persistence = nil
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).WithStatusSubresource(z).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 				err = r.reconcileFinalizers(z)
@@ -624,7 +670,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 			BeforeEach(func() {
 				z.WithDefaults()
 				z.Spec.Persistence = nil
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(z).WithStatusSubresource(z).Build()
 			})
 			It("should have 1 finalizer, should not raise an error", func() {
 				config.DisableFinalizer = false
@@ -699,7 +745,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				next = z.DeepCopy()
 				next.Spec.TriggerRollingRestart = true
 				svc = zk.MakeClientService(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 				err = cl.Get(context.TODO(), req.NamespacedName, foundZk)
@@ -719,7 +765,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 
 				next.Spec.TriggerRollingRestart = false
 				svc = zk.MakeClientService(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 
@@ -757,7 +803,7 @@ var _ = Describe("ZookeeperCluster Controller", func() {
 				// update the crd instance to trigger rolling restart
 				next.Spec.TriggerRollingRestart = true
 				svc = zk.MakeClientService(z)
-				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).Build()
+				cl = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(next, svc).WithStatusSubresource(next).Build()
 				r = &ZookeeperClusterReconciler{Client: cl, Scheme: s, ZkClient: mockZkClient}
 				res, err = r.Reconcile(context.TODO(), req)
 				err = cl.Get(context.TODO(), req.NamespacedName, foundZk)