Add imageChangeStatus message (#2) (#120)

This is prep for online upgrade. This adds a new status message
that we will use to report the phase that the upgrade is in.
This also has a fix that will retry status changes for any
transient error. You may have seen the error I'm talking about
-- it usually complains about an update failing because we are
not using the latest copy of the object.

api/v1beta1/verticadb_types.go

@@ -521,6 +521,11 @@ type VerticaDBStatus struct {
 	// +operator-sdk:csv:customresourcedefinitions:type=status
 	// Conditions for VerticaDB
 	Conditions []VerticaDBCondition `json:"conditions,omitempty"`
+
+	// +operator-sdk:csv:customresourcedefinitions:type=status
+	// Status message for the current running image change.   If no image change
+	// is occurring, this message remains blank.
+	ImageChangeStatus string `json:"imageChangeStatus"`
 }
 
 // VerticaDBConditionType defines type for VerticaDBCondition

pkg/controllers/imagechange_reconcile.go

@@ -110,6 +110,10 @@ func (u *ImageChangeReconciler) startImageChange(ctx context.Context) (ctrl.Resu
 
 // finishImageChange handles condition status and event recording for the end of an image change
 func (u *ImageChangeReconciler) finishImageChange(ctx context.Context) (ctrl.Result, error) {
+	if err := u.setImageChangeStatus(ctx, ""); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	if err := u.toggleImageChangeInProgress(ctx, corev1.ConditionFalse); err != nil {
 		return ctrl.Result{}, err
 	}
@@ -182,6 +186,10 @@ func (u *ImageChangeReconciler) stopCluster(ctx context.Context) (ctrl.Result, e
 		return ctrl.Result{}, err
 	}
 
+	if err := u.setImageChangeStatus(ctx, "Starting cluster shutdown"); err != nil {
+		return ctrl.Result{}, err
+	}
+
 	start := time.Now()
 	u.VRec.EVRec.Event(u.Vdb, corev1.EventTypeNormal, events.ClusterShutdownStarted,
 		"Calling 'admintools -t stop_db'")
@@ -217,6 +225,10 @@ func (u *ImageChangeReconciler) updateImageInStatefulSets(ctx context.Context) (
 		// Skip the statefulset if it already has the proper image.
 		if sts.Spec.Template.Spec.Containers[names.ServerContainerIndex].Image != u.Vdb.Spec.Image {
 			u.Log.Info("Updating image in old statefulset", "name", sts.ObjectMeta.Name)
+			err = u.setImageChangeStatus(ctx, "Rescheduling pods with new image name")
+			if err != nil {
+				return ctrl.Result{}, err
+			}
 			sts.Spec.Template.Spec.Containers[names.ServerContainerIndex].Image = u.Vdb.Spec.Image
 			// We change the update strategy to OnDelete.  We don't want the k8s
 			// sts controller to interphere and do a rolling update after the
@@ -290,6 +302,9 @@ func (u *ImageChangeReconciler) checkForNewPods(ctx context.Context) (ctrl.Resul
 // been recreated.  Once the cluster is back up, then the image change is considered complete.
 func (u *ImageChangeReconciler) restartCluster(ctx context.Context) (ctrl.Result, error) {
 	u.Log.Info("Starting restart phase of image change for this reconcile iteration")
+	if err := u.setImageChangeStatus(ctx, "Restarting cluster"); err != nil {
+		return ctrl.Result{}, err
+	}
 	// The restart reconciler is called after this reconciler.  But we call the
 	// restart reconciler here so that we restart while the status condition is set.
 	r := MakeRestartReconciler(u.VRec, u.Log, u.Vdb, u.PRunner, u.PFacts)
@@ -318,3 +333,8 @@ func (u *ImageChangeReconciler) anyPodsRunningWithOldImage(ctx context.Context)
 	}
 	return false, nil
 }
+
+// setImageChangeStatus is a helper to set the imageChangeStatus message.
+func (u *ImageChangeReconciler) setImageChangeStatus(ctx context.Context, msg string) error {
+	return status.UpdateImageChangeStatus(ctx, u.VRec.Client, u.Vdb, msg)
+}

pkg/status/status.go

@@ -24,33 +24,38 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+// Update will set status fields in the VerticaDB.  It handles retry for
+// transient errors like when update fails because another client updated the
+// VerticaDB.
 func Update(ctx context.Context, clnt client.Client, vdb *vapi.VerticaDB, updateFunc func(*vapi.VerticaDB) error) error {
-	// Always fetch the latest to minimize the chance of getting a conflict error.
-	nm := types.NamespacedName{Namespace: vdb.Namespace, Name: vdb.Name}
-	if err := clnt.Get(ctx, nm, vdb); err != nil {
-		return err
-	}
+	return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		// Always fetch the latest to minimize the chance of getting a conflict error.
+		nm := types.NamespacedName{Namespace: vdb.Namespace, Name: vdb.Name}
+		if err := clnt.Get(ctx, nm, vdb); err != nil {
+			return err
+		}
 
-	// We will calculate the status for the vdb object. This update is done in
-	// place. If anything differs from the copy then we will do a single update.
-	vdbChg := vdb.DeepCopy()
+		// We will calculate the status for the vdb object. This update is done in
+		// place. If anything differs from the copy then we will do a single update.
+		vdbChg := vdb.DeepCopy()
 
-	// Refresh the status using the users provided function
-	if err := updateFunc(vdbChg); err != nil {
-		return err
-	}
-
-	if !reflect.DeepEqual(vdb.Status, vdbChg.Status) {
-		vdbChg.Status.DeepCopyInto(&vdb.Status)
-		if err := clnt.Status().Update(ctx, vdb); err != nil {
-			return fmt.Errorf("failed to update status of vdb %w", err)
+		// Refresh the status using the users provided function
+		if err := updateFunc(vdbChg); err != nil {
+			return err
 		}
-	}
 
-	return nil
+		if !reflect.DeepEqual(vdb.Status, vdbChg.Status) {
+			vdbChg.Status.DeepCopyInto(&vdb.Status)
+			if err := clnt.Status().Update(ctx, vdb); err != nil {
+				return err
+			}
+		}
+		return nil
+	})
 }
 
 // UpdateCondition will update a condition status
@@ -85,3 +90,12 @@ func UpdateCondition(ctx context.Context, clnt client.Client, vdb *vapi.VerticaD
 
 	return Update(ctx, clnt, vdb, refreshConditionInPlace)
 }
+
+// UpdateImageChangeStatus will update the image change status message.  The
+// input vdb will be updated with the status message.
+func UpdateImageChangeStatus(ctx context.Context, clnt client.Client, vdb *vapi.VerticaDB, msg string) error {
+	return Update(ctx, clnt, vdb, func(vdb *vapi.VerticaDB) error {
+		vdb.Status.ImageChangeStatus = msg
+		return nil
+	})
+}

tests/e2e/upgrade-vertica-ks-0/20-assert.yaml

@@ -27,9 +27,9 @@ spec:
   containers:
     - image: vertica/vertica-k8s:11.0.1-0-minimal
 ---
-apiVersion: apps/v1
-kind: StatefulSet
+apiVersion: vertica.com/v1beta1
+kind: VerticaDB
 metadata:
-  name: v-upgrade-vertica-defsc
+  name: v-upgrade-vertica
 status:
-  readyReplicas: 2
+  imageChangeStatus: "Restarting cluster"

tests/e2e/upgrade-vertica-ks-0/23-assert.yaml

@@ -0,0 +1,19 @@
+# (c) Copyright [2021] Micro Focus or one of its affiliates.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: v-upgrade-vertica-defsc
+status:
+  readyReplicas: 2

tests/e2e/upgrade-vertica-ks-0/23-wait-for-upgrade.yaml

@@ -0,0 +1 @@
+# Intentionally empty to give this step a name in kuttl