address review comments

Author: arshadd-b

cloud/scope/powervs_image.go

@@ -39,6 +39,11 @@ import (
 // BucketAccess indicates if the bucket has public or private access public access.
 const BucketAccess = "public"
 
+var (
+	// ErrServiceInsanceNotInActiveState indicates error if serviceInstance is inactive.
+	ErrServiceInsanceNotInActiveState = errors.New("service instance is not in active state")
+)
+
 // PowerVSImageScopeParams defines the input parameters used to create a new PowerVSImageScope.
 type PowerVSImageScopeParams struct {
 	Client          client.Client
@@ -106,8 +111,7 @@ func NewPowerVSImageScope(ctx context.Context, params PowerVSImageScopeParams) (
 			return nil, fmt.Errorf("service instance %s is not yet created", name)
 		}
 		if *serviceInstance.State != string(infrav1.ServiceInstanceStateActive) {
-			err = errors.New("service instance is not in active state")
-			return nil, err
+			return scope, ErrServiceInsanceNotInActiveState
 		}
 		serviceInstanceID = *serviceInstance.GUID
 	}

controllers/ibmpowervsimage_controller.go

@@ -98,6 +98,19 @@ func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		scopeParams.Zone = cluster.Spec.Zone
 	}
 
+	// Create the scope
+	imageScope, err := scope.NewPowerVSImageScope(ctx, scopeParams)
+	if err != nil {
+		if errors.Is(err, scope.ErrServiceInsanceNotInActiveState) {
+			v1beta2conditions.Set(imageScope.IBMPowerVSImage, metav1.Condition{
+				Type:   infrav1.WorkspaceReadyV1Beta2Condition,
+				Status: metav1.ConditionFalse,
+				Reason: infrav1.WorkspaceNotReadyV1Beta2Reason,
+			})
+		}
+		return ctrl.Result{}, fmt.Errorf("failed to create scope: %w", err)
+	}
+
 	// Initialize the patch helper
 	patchHelper, err := v1beta1patch.NewHelper(ibmPowerVSImage, r.Client)
 	if err != nil {
@@ -111,19 +124,6 @@ func (r *IBMPowerVSImageReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		}
 	}()
 
-	// Create the scope
-	imageScope, err := scope.NewPowerVSImageScope(ctx, scopeParams)
-	if err != nil {
-		if err == errors.New("service instance is not in active state") {
-			v1beta2conditions.Set(imageScope.IBMPowerVSImage, metav1.Condition{
-				Type:   infrav1.WorkspaceReadyV1Beta2Condition,
-				Status: metav1.ConditionFalse,
-				Reason: infrav1.WorkspaceNotReadyV1Beta2Reason,
-			})
-		}
-		return ctrl.Result{}, fmt.Errorf("failed to create scope: %w", err)
-	}
-
 	// Handle deleted clusters.
 	if !ibmPowerVSImage.DeletionTimestamp.IsZero() {
 		return r.reconcileDelete(ctx, imageScope)
@@ -311,7 +311,6 @@ func (r *IBMPowerVSImageReconciler) reconcileDelete(ctx context.Context, scope *
 
 	if scope.IBMPowerVSImage.Spec.DeletePolicy != string(infrav1.DeletePolicyRetain) {
 		if err := scope.DeleteImage(); err != nil {
-			log.Error(err, "Error deleting IBMPowerVSImage")
 			v1beta1conditions.MarkFalse(scope.IBMPowerVSImage, infrav1.ImageReadyCondition, clusterv1beta1.DeletionFailedReason, clusterv1beta1.ConditionSeverityWarning, "")
 			v1beta2conditions.Set(scope.IBMPowerVSImage, metav1.Condition{
 				Type:    infrav1.IBMPowerVSImageReadyV1Beta2Condition,

controllers/ibmpowervsimage_controller_test.go

@@ -424,7 +424,7 @@ func TestIBMPowerVSImageReconciler_Reconcile_Conditions(t *testing.T) {
 	}{
 
 		{
-			name: "Conditions should be set after reconcile",
+			name: "Conditions should be set after first reconcile",
 			powervsCluster: &infrav1.IBMPowerVSCluster{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "capi-powervs-cluster"},