address review comments

Author: arshadd-b

api/v1beta1/ibmpowervs_conversion.go

@@ -215,6 +215,5 @@ func Convert_v1beta2_IBMPowerVSMachineStatus_To_v1beta1_IBMPowerVSMachineStatus(
 }
 
 func Convert_v1beta2_IBMPowerVSImageStatus_To_v1beta1_IBMPowerVSImageStatus(in *infrav1.IBMPowerVSImageStatus, out *IBMPowerVSImageStatus, s apiconversion.Scope) error {
-	// CAPI V1Beta2 was added in CAPIBM v1beta2
 	return autoConvert_v1beta2_IBMPowerVSImageStatus_To_v1beta1_IBMPowerVSImageStatus(in, out, s)
 }

api/v1beta2/conditions_consts.go

@@ -118,6 +118,9 @@ const (
 
 	// ImageReconciliationFailedReason used when an error occurs during VPC Custom Image reconciliation.
 	ImageReconciliationFailedReason = "ImageReconciliationFailed"
+
+	// ImageQueuedReason used when the image import is failed.
+	ImageQueuedReason = "ImageQueued"
 )
 
 const (
@@ -129,9 +132,6 @@ const (
 
 	// IBMPowerVSImageDeletingV1Beta2Reason surfaces when the image controller by IBMPowerVSImage is deleting.
 	IBMPowerVSImageDeletingV1Beta2Reason = clusterv1beta1.DeletingV1Beta2Reason
-
-	// IBMPowerVSMachineImageDeletingV1Beta2Reason surfaces when the image controller by IBMPowerVSImage is deleting.
-	IBMPowerVSMachineImageDeletingV1Beta2Reason = clusterv1beta1.DeletingV1Beta2Reason
 )
 
 const (
@@ -328,19 +328,19 @@ const (
 	COSInstanceDeletingV1Beta2Reason = clusterv1beta1.DeletingV1Beta2Reason
 )
 
-// IBMPowerVSCluster's Ready condition and corresponding reasons that will be used in v1Beta2 API version.
+// IBMPowerVSImage's Ready condition and corresponding reasons that will be used in v1Beta2 API version.
 const (
-	// IBMPowerVSClusterReadyV1Beta2Condition is true if the IBMPowerVSCluster's deletionTimestamp is not set, IBMPowerVSCluster's
+	// IBMPowerVSImageReadyV1Beta2Condition is true if the IBMPowerVSImage's deletionTimestamp is not set, IBMPowerVSImage's
 	// FailureDomainsReady, VCenterAvailable and ClusterModulesReady conditions are true.
 	IBMPowerVSImageReadyV1Beta2Condition = clusterv1beta1.ReadyV1Beta2Condition
 
-	// IBMPowerVSClusterReadyV1Beta2Reason surfaces when the IBMPowerVSCluster readiness criteria is met.
+	// IBMPowerVSImageReadyV1Beta2Reason surfaces when the IBMPowerVSImage readiness criteria is met.
 	IBMPowerVSImageReadyV1Beta2Reason = clusterv1beta1.ReadyV1Beta2Reason
 
-	// IBMPowerVSClusterNotReadyV1Beta2Reason surfaces when the IBMPowerVSCluster readiness criteria is not met.
+	// IBMPowerVSImageNotReadyV1Beta2Reason surfaces when the IBMPowerVSImage readiness criteria is not met.
 	IBMPowerVSImageNotReadyV1Beta2Reason = clusterv1beta1.NotReadyV1Beta2Reason
 
-	// IBMPowerVSClusterReadyUnknownV1Beta2Reason surfaces when at least one of the IBMPowerVSCluster readiness criteria is unknown
-	// and none of the IBMPowerVSCluster readiness criteria is met.
+	// IBMPowerVSImageReadyUnknownV1Beta2Reason surfaces when at least one of the IBMPowerVSImage readiness criteria is unknown
+	// and none of the IBMPowerVSImage readiness criteria is met.
 	IBMPowerVSImageReadyUnknownV1Beta2Reason = clusterv1beta1.ReadyUnknownV1Beta2Reason
 )

cloud/scope/powervs_image.go

@@ -32,7 +32,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2"
-	infrav1beta2 "sigs.k8s.io/cluster-api-provider-ibmcloud/api/v1beta2"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/powervs"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/cloud/services/resourcecontroller"
 	"sigs.k8s.io/cluster-api-provider-ibmcloud/pkg/endpoints"
@@ -170,12 +169,12 @@ func (i *PowerVSImageScope) CreateImageCOSBucket(ctx context.Context) (*models.I
 		record.Warnf(i.IBMPowerVSImage, "FailedRetrieveImage", "Failed to retrieve image %q", m.Name)
 		return nil, nil, err
 	} else if imageReply != nil {
-		log.Info("Image already exists")
+		log.Info("Image already exists", "imageName", m.Name)
 		return imageReply, nil, nil
 	}
 
 	if lastJob, _ := i.GetImportJob(); lastJob != nil {
-		if *lastJob.Status.State != string(infrav1beta2.PowerVSImageStateCompleted) && *lastJob.Status.State != string(infrav1beta2.PowerVSImageStateFailed) {
+		if *lastJob.Status.State != string(infrav1.PowerVSImageStateCompleted) && *lastJob.Status.State != string(infrav1.PowerVSImageStateFailed) {
 			log.Info("Previous import job not yet finished", "state", *lastJob.Status.State)
 			return nil, nil, nil
 		}
@@ -196,7 +195,7 @@ func (i *PowerVSImageScope) CreateImageCOSBucket(ctx context.Context) (*models.I
 		record.Warnf(i.IBMPowerVSImage, "FailedCreateImageImportJob", "Failed image import job creation - %v", err)
 		return nil, nil, err
 	}
-	log.Info("New import job request created")
+	log.Info("New import job request created", "jobID", *jobRef.ID)
 	record.Eventf(i.IBMPowerVSImage, "SuccessfulCreateImageImportJob", "Created image import job %q", *jobRef.ID)
 	return nil, jobRef, nil
 }

controllers/ibmpowervsimage_controller.go

@@ -151,7 +151,7 @@ func (r *IBMPowerVSImageReconciler) reconcile(ctx context.Context, cluster *infr
 	if jobID := imageScope.GetJobID(); jobID != "" {
 		job, err := imageScope.IBMPowerVSClient.GetJob(jobID)
 		if err != nil {
-			log.Info("Unable to get job details")
+			log.Info("Unable to get job details", "jobID", jobID)
 			return ctrl.Result{RequeueAfter: 2 * time.Minute}, err
 		}
 
@@ -181,7 +181,7 @@ func (r *IBMPowerVSImageReconciler) reconcile(ctx context.Context, cluster *infr
 			v1beta2conditions.Set(imageScope.IBMPowerVSImage, metav1.Condition{
 				Type:   infrav1.IBMPowerVSImageReadyV1Beta2Condition,
 				Status: metav1.ConditionFalse,
-				Reason: infrav1.ImageNotReadyReason,
+				Reason: infrav1.ImageQueuedReason,
 			})
 			return ctrl.Result{RequeueAfter: 2 * time.Minute}, nil
 		default:
@@ -214,7 +214,7 @@ func reconcileImage(ctx context.Context, img *models.ImageReference, imageScope
 	if img != nil {
 		image, err := imageScope.IBMPowerVSClient.GetImage(*img.ImageID)
 		if err != nil {
-			log.Info("Unable to get image details")
+			log.Info("Unable to get image details", "imageID", *img.ImageID)
 			return ctrl.Result{}, err
 		}
 
@@ -258,7 +258,7 @@ func reconcileImage(ctx context.Context, img *models.ImageReference, imageScope
 
 	// Requeue after 1 minute if image is not ready to update status of the image properly.
 	if !imageScope.IsReady() {
-		log.Info("Image is not yet ready")
+		log.Info("Image is not yet ready, requeue", "state", imageScope.GetImageState())
 		return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil
 	}