Skip to content

Commit 5c5862e

Browse files
committed
address review comments
1 parent bcda023 commit 5c5862e

File tree

4 files changed

+12
-32
lines changed

4 files changed

+12
-32
lines changed

api/v1beta2/conditions_consts.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ const (
110110
)
111111

112112
const (
113-
// ImageNotReadyReason used when the image is in a queued state.
113+
// ImageNotReadyReason used when the image is not ready.
114114
ImageNotReadyReason = "ImageNotReady"
115115

116116
// ImageImportFailedReason used when the image import is failed.
@@ -119,7 +119,7 @@ const (
119119
// ImageReconciliationFailedReason used when an error occurs during VPC Custom Image reconciliation.
120120
ImageReconciliationFailedReason = "ImageReconciliationFailed"
121121

122-
// ImageQueuedReason used when the image import is failed.
122+
// ImageQueuedReason used when the image is in queued state.
123123
ImageQueuedReason = "ImageQueued"
124124
)
125125

@@ -130,7 +130,7 @@ const (
130130
// ImageImportedCondition reports on current status of the image import job. Ready indicates the import job is finished.
131131
ImageImportedCondition clusterv1beta1.ConditionType = "ImageImported"
132132

133-
// IBMPowerVSImageDeletingV1Beta2Reason surfaces when the image controller by IBMPowerVSImage is deleting.
133+
// IBMPowerVSImageDeletingV1Beta2Reason surfaces when the image is in deleting state.
134134
IBMPowerVSImageDeletingV1Beta2Reason = clusterv1beta1.DeletingV1Beta2Reason
135135
)
136136

@@ -333,8 +333,7 @@ const (
333333
// IBMPowerVSImageReadyCondition is true if the IBMPowerVSImage's deletionTimestamp is not set, IBMPowerVSImage's.
334334
IBMPowerVSImageReadyCondition = clusterv1beta1.ReadyV1Beta2Condition
335335

336-
// IBMPowerVSImageReadyV1Beta2Condition documents the status of the image that is controlled
337-
// by the IBMPowerVSImage.
336+
// IBMPowerVSImageReadyV1Beta2Condition documents the Ready status of the image.
338337
IBMPowerVSImageReadyV1Beta2Condition = "ImageReady"
339338

340339
// IBMPowerVSImageReadyV1Beta2Reason surfaces when the IBMPowerVSImage readiness criteria is met.

api/v1beta2/types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ var (
5656
// PowerVSImageStateACTIVE is the string representing an image in a active state.
5757
PowerVSImageStateACTIVE = PowerVSImageState("active")
5858

59-
// PowerVSImageStateQue is the string representing an image in a queued state.
60-
PowerVSImageStateQue = PowerVSImageState("queued")
59+
// PowerVSImageStateQueued is the string representing an image in a queued state.
60+
PowerVSImageStateQueued = PowerVSImageState("queued")
6161

6262
// PowerVSImageStateFailed is the string representing an image in a failed state.
6363
PowerVSImageStateFailed = PowerVSImageState("failed")

controllers/ibmpowervsimage_controller.go

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,10 @@ func (r *IBMPowerVSImageReconciler) reconcile(ctx context.Context, cluster *infr
187187
Reason: infrav1.ImageImportFailedReason,
188188
})
189189
return ctrl.Result{RequeueAfter: 2 * time.Minute}, fmt.Errorf("failed to import image, message: %s", job.Status.Message)
190-
case infrav1.PowerVSImageStateQue:
190+
case infrav1.PowerVSImageStateQueued:
191191
imageScope.SetNotReady()
192-
imageScope.SetImageState(string(infrav1.PowerVSImageStateQue))
193-
v1beta1conditions.MarkFalse(imageScope.IBMPowerVSImage, infrav1.ImageImportedCondition, string(infrav1.PowerVSImageStateQue), clusterv1beta1.ConditionSeverityInfo, "%s", job.Status.Message)
192+
imageScope.SetImageState(string(infrav1.PowerVSImageStateQueued))
193+
v1beta1conditions.MarkFalse(imageScope.IBMPowerVSImage, infrav1.ImageImportedCondition, string(infrav1.PowerVSImageStateQueued), clusterv1beta1.ConditionSeverityInfo, "%s", job.Status.Message)
194194
v1beta2conditions.Set(imageScope.IBMPowerVSImage, metav1.Condition{
195195
Type: infrav1.IBMPowerVSImageReadyV1Beta2Condition,
196196
Status: metav1.ConditionFalse,
@@ -237,7 +237,7 @@ func reconcileImage(ctx context.Context, img *models.ImageReference, imageScope
237237
log.Info("ImageState", image.State)
238238

239239
switch imageScope.GetImageState() {
240-
case infrav1.PowerVSImageStateQue:
240+
case infrav1.PowerVSImageStateQueued:
241241
log.Info("Image is in queued state")
242242
imageScope.SetNotReady()
243243
v1beta1conditions.MarkFalse(imageScope.IBMPowerVSImage, infrav1.ImageReadyCondition, infrav1.ImageNotReadyReason, clusterv1beta1.ConditionSeverityWarning, "")
@@ -342,25 +342,6 @@ func (r *IBMPowerVSImageReconciler) SetupWithManager(mgr ctrl.Manager) error {
342342
}
343343

344344
func patchIBMPowerVSImage(ctx context.Context, patchHelper *v1beta1patch.Helper, ibmPowerVSImage *infrav1.IBMPowerVSImage) error {
345-
// Before computing ready condition, make sure that ImageReady is always set.
346-
// NOTE: This is required because v1beta2 conditions comply to guideline requiring conditions to be set at the
347-
// first reconcile.
348-
if c := v1beta2conditions.Get(ibmPowerVSImage, infrav1.IBMPowerVSImageReadyV1Beta2Condition); c == nil {
349-
if ibmPowerVSImage.Status.Ready {
350-
v1beta2conditions.Set(ibmPowerVSImage, metav1.Condition{
351-
Type: infrav1.IBMPowerVSImageReadyV1Beta2Condition,
352-
Status: metav1.ConditionTrue,
353-
Reason: infrav1.IBMPowerVSImageReadyV1Beta2Reason,
354-
})
355-
} else {
356-
v1beta2conditions.Set(ibmPowerVSImage, metav1.Condition{
357-
Type: infrav1.IBMPowerVSImageReadyV1Beta2Condition,
358-
Status: metav1.ConditionFalse,
359-
Reason: infrav1.IBMPowerVSImageNotReadyV1Beta2Reason,
360-
})
361-
}
362-
}
363-
364345
// always update the readyCondition.
365346
v1beta1conditions.SetSummary(ibmPowerVSImage,
366347
v1beta1conditions.WithConditions(

controllers/ibmpowervsimage_controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ func TestIBMPowerVSImageReconciler_reconcile(t *testing.T) {
230230
g.Expect(err).To(BeNil())
231231
g.Expect(imageScope.IBMPowerVSImage.Finalizers).To(ContainElement(infrav1.IBMPowerVSImageFinalizer))
232232
g.Expect(imageScope.IBMPowerVSImage.Status.Ready).To(Equal(false))
233-
g.Expect(imageScope.IBMPowerVSImage.Status.ImageState).To(BeEquivalentTo(infrav1.PowerVSImageStateQue))
234-
expectConditionsImage(g, imageScope.IBMPowerVSImage, []conditionAssertion{{infrav1.ImageImportedCondition, corev1.ConditionFalse, clusterv1beta1.ConditionSeverityInfo, string(infrav1.PowerVSImageStateQue)}})
233+
g.Expect(imageScope.IBMPowerVSImage.Status.ImageState).To(BeEquivalentTo(infrav1.PowerVSImageStateQueued))
234+
expectConditionsImage(g, imageScope.IBMPowerVSImage, []conditionAssertion{{infrav1.ImageImportedCondition, corev1.ConditionFalse, clusterv1beta1.ConditionSeverityInfo, string(infrav1.PowerVSImageStateQueued)}})
235235
g.Expect(result.RequeueAfter).To(Not(BeZero()))
236236
})
237237
t.Run("When importing image is still in progress", func(_ *testing.T) {

0 commit comments

Comments
 (0)