Merge pull request #15593 from prazumovsky/retry-pool-status

pool: retry status update on fail

pkg/operator/ceph/pool/controller.go

@@ -198,9 +198,13 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 		return reconcile.Result{}, *cephBlockPool, nil
 	}
 
+	var statusErr error
 	// The CR was just created, initializing status fields
 	if cephBlockPool.Status == nil {
-		r.updateStatus(request.NamespacedName, cephv1.ConditionProgressing, k8sutil.ObservedGenerationNotAvailable)
+		err = r.updateStatus(request.NamespacedName, cephv1.ConditionProgressing, k8sutil.ObservedGenerationNotAvailable)
+		if err != nil {
+			logger.Errorf("failed to update %q status to %q: %v", request.NamespacedName, cephv1.ConditionProgressing, err)
+		}
 	}
 
 	// Make sure a CephCluster is present otherwise do nothing
@@ -318,7 +322,10 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 			logger.Info(opcontroller.OperatorNotInitializedMessage)
 			return opcontroller.WaitForRequeueIfOperatorNotInitialized, *cephBlockPool, nil
 		}
-		r.updateStatus(request.NamespacedName, cephv1.ConditionFailure, k8sutil.ObservedGenerationNotAvailable)
+		statusErr = r.updateStatus(request.NamespacedName, cephv1.ConditionFailure, k8sutil.ObservedGenerationNotAvailable)
+		if statusErr != nil {
+			logger.Errorf("failed to update %q status to %q: %v", request.NamespacedName, cephv1.ConditionFailure, statusErr)
+		}
 		return reconcileResponse, *cephBlockPool, errors.Wrapf(err, "failed to create pool %q.", cephBlockPool.GetName())
 	}
 
@@ -333,7 +340,10 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 		// Always create a bootstrap peer token in case another cluster wants to add us as a peer
 		reconcileResponse, err = opcontroller.CreateBootstrapPeerSecret(r.context, clusterInfo, cephBlockPool, k8sutil.NewOwnerInfo(cephBlockPool, r.scheme))
 		if err != nil {
-			r.updateStatus(request.NamespacedName, cephv1.ConditionFailure, k8sutil.ObservedGenerationNotAvailable)
+			statusErr = r.updateStatus(request.NamespacedName, cephv1.ConditionFailure, k8sutil.ObservedGenerationNotAvailable)
+			if statusErr != nil {
+				logger.Errorf("failed to update %q status to %q: %v", request.NamespacedName, cephv1.ConditionFailure, statusErr)
+			}
 			return reconcileResponse, *cephBlockPool, errors.Wrapf(err, "failed to create rbd-mirror bootstrap peer for pool %q.", cephBlockPool.GetName())
 		}
 
@@ -355,7 +365,7 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 
 		// update ObservedGeneration in status at the end of reconcile
 		// Set Ready status, we are done reconciling
-		r.updateStatus(request.NamespacedName, cephv1.ConditionReady, observedGeneration)
+		statusErr = r.updateStatus(request.NamespacedName, cephv1.ConditionReady, observedGeneration)
 
 		if cephBlockPool.Spec.StatusCheck.Mirror.Disabled {
 			// Stop monitoring the mirroring status of this pool
@@ -388,7 +398,7 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 		}
 		// update ObservedGeneration in status at the end of reconcile
 		// Set Ready status, we are done reconciling
-		r.updateStatus(request.NamespacedName, cephv1.ConditionReady, observedGeneration)
+		statusErr = r.updateStatus(request.NamespacedName, cephv1.ConditionReady, observedGeneration)
 
 		// Stop monitoring the mirroring status of this pool
 		if blockPoolContextsExists && r.blockPoolContexts[blockPoolChannelKey].started {
@@ -398,6 +408,10 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile
 		}
 	}
 
+	if statusErr != nil {
+		return opcontroller.ImmediateRetryResult, *cephBlockPool, errors.Wrapf(statusErr, "failed to update status of pool %q to %q.", cephBlockPool.Name, cephv1.ConditionReady)
+	}
+
 	// Return and do not requeue
 	logger.Debug("done reconciling")
 	return reconcile.Result{}, *cephBlockPool, nil

pkg/operator/ceph/pool/status.go

@@ -18,6 +18,7 @@ limitations under the License.
 package pool
 
 import (
+	"github.com/pkg/errors"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	cephclient "github.com/rook/rook/pkg/daemon/ceph/client"
 	opcontroller "github.com/rook/rook/pkg/operator/ceph/controller"
@@ -28,16 +29,16 @@ import (
 )
 
 // updateStatus updates a pool CR with the given status
-func (r *ReconcileCephBlockPool) updateStatus(poolName types.NamespacedName, status cephv1.ConditionType, observedGeneration int64) {
+func (r *ReconcileCephBlockPool) updateStatus(poolName types.NamespacedName, status cephv1.ConditionType, observedGeneration int64) error {
 	pool := &cephv1.CephBlockPool{}
 	err := r.client.Get(r.opManagerContext, poolName, pool)
 	if err != nil {
 		if kerrors.IsNotFound(err) {
 			logger.Debug("CephBlockPool resource not found. Ignoring since object must be deleted.")
-			return
+			return nil
 		}
 		logger.Warningf("failed to retrieve pool %q to update status to %q. %v", poolName, status, err)
-		return
+		return errors.Wrapf(err, "failed to retrieve pool %q to update status to %q", poolName, status)
 	}
 
 	if pool.Status == nil {
@@ -56,9 +57,10 @@ func (r *ReconcileCephBlockPool) updateStatus(poolName types.NamespacedName, sta
 	}
 	if err := reporting.UpdateStatus(r.client, pool); err != nil {
 		logger.Warningf("failed to set pool %q status to %q. %v", pool.Name, status, err)
-		return
+		return errors.Wrapf(err, "failed to set pool %q status to %q", pool.Name, status)
 	}
 	logger.Debugf("pool %q status updated to %q", poolName, status)
+	return nil
 }
 
 func updateStatusInfo(cephBlockPool *cephv1.CephBlockPool) {