Generalizing ConditionStatus and adding comments

Signed-off-by: Aravind Ramalingam <[email protected]>

api/v1alpha1/baseboardmanagement_types.go

@@ -30,8 +30,8 @@ type BootDevice string
 // BaseboardManagementConditionType represents the condition of the BaseboardManagement.
 type BaseboardManagementConditionType string
 
-// BaseboardManagementConditionStatus represents the status of a BaseboardManagementCondition.
-type BaseboardManagementConditionStatus string
+// ConditionStatus represents the status of a Condition.
+type ConditionStatus string
 
 const (
 	On  PowerState = "on"
@@ -52,15 +52,13 @@ const (
 )
 
 const (
-	BaseboardManagementConditionTrue  BaseboardManagementConditionStatus = "True"
-	BaseboardManagementConditionFalse BaseboardManagementConditionStatus = "False"
+	ConditionTrue  ConditionStatus = "True"
+	ConditionFalse ConditionStatus = "False"
 )
 
-const (
-	// PausedAnnotation is an annotation that can be applied to BaseboardManagement
-	// object to prevent a controller from processing a resource.
-	pausedAnnotation = "bmc.tinkerbell.org/paused"
-)
+// PausedAnnotation is an annotation that can be applied to BaseboardManagement
+// object to prevent a controller from processing a resource.
+const PausedAnnotation = "bmc.tinkerbell.org/paused"
 
 // BaseboardManagementSpec defines the desired state of BaseboardManagement
 type BaseboardManagementSpec struct {
@@ -108,7 +106,7 @@ type BaseboardManagementCondition struct {
 
 	// Status is the status of the BaseboardManagement condition.
 	// Can be True or False.
-	Status BaseboardManagementConditionStatus `json:"status"`
+	Status ConditionStatus `json:"status"`
 
 	// Last time the BaseboardManagement condition was updated.
 	LastUpdateTime metav1.Time `json:"lastUpdateTime"`
@@ -123,7 +121,7 @@ type BaseboardManagementSetConditionOption func(*BaseboardManagementCondition)
 
 // SetCondition applies the cType condition to bm. If the condition already exists,
 // it is updated.
-func (bm *BaseboardManagement) SetCondition(cType BaseboardManagementConditionType, status BaseboardManagementConditionStatus, opts ...BaseboardManagementSetConditionOption) {
+func (bm *BaseboardManagement) SetCondition(cType BaseboardManagementConditionType, status ConditionStatus, opts ...BaseboardManagementSetConditionOption) {
 	var condition *BaseboardManagementCondition
 
 	// Check if there's an existing condition.
@@ -149,6 +147,7 @@ func (bm *BaseboardManagement) SetCondition(cType BaseboardManagementConditionTy
 	}
 }
 
+// WithBaseboardManagementConditionMessage sets message m to the BaseboardManagementCondition.
 func WithBaseboardManagementConditionMessage(m string) BaseboardManagementSetConditionOption {
 	return func(c *BaseboardManagementCondition) {
 		c.Message = m
@@ -158,27 +157,36 @@ func WithBaseboardManagementConditionMessage(m string) BaseboardManagementSetCon
 // PauseReconcile adds the pausedAnnotation to the BaseboardManagement object.
 func (bm *BaseboardManagement) PauseReconcile() {
 	if bm.Annotations == nil {
-		bm.Annotations = map[string]string{}
+		bm.initAnnotations()
 	}
-	bm.Annotations[pausedAnnotation] = "true"
+	bm.Annotations[PausedAnnotation] = "true"
 }
 
 // ClearPauseAnnotation deletes the pausedAnnotation from the BaseboardManagement object.
 func (bm *BaseboardManagement) ClearPauseAnnotation() {
 	if bm.Annotations != nil {
-		delete(bm.Annotations, pausedAnnotation)
+		delete(bm.Annotations, PausedAnnotation)
 	}
 }
 
 // IsReconcilePaused checks if the pausedAnnotation is present on the BaseboardManagement object.
 func (bm *BaseboardManagement) IsReconcilePaused() bool {
-	if s, ok := bm.Annotations[pausedAnnotation]; ok {
+	if bm.Annotations == nil {
+		return false
+	}
+
+	if s, ok := bm.Annotations[PausedAnnotation]; ok {
 		return s == "true"
 	}
 
 	return false
 }
 
+// initAnnotations initalizes the BaseboardManagement metadata annotations.
+func (bm *BaseboardManagement) initAnnotations() {
+	bm.Annotations = map[string]string{}
+}
+
 // BaseboardManagementRef defines the reference information to a BaseboardManagement resource.
 type BaseboardManagementRef struct {
 	// Name is unique within a namespace to reference a BaseboardManagement resource.

api/v1alpha1/bmcjob_types.go

@@ -46,14 +46,6 @@ const (
 	Status       PowerControl = "status"
 )
 
-// BMCJobConditionStatus represents the status of a BMCJobCondition.
-type BMCJobConditionStatus string
-
-const (
-	BMCJobConditionTrue  BMCJobConditionStatus = "True"
-	BMCJobConditionFalse BMCJobConditionStatus = "False"
-)
-
 // BMCJobSpec defines the desired state of BMCJob
 type BMCJobSpec struct {
 	// BaseboardManagementRef represents the BaseboardManagement resource to execute the job.
@@ -89,7 +81,7 @@ type BMCJobCondition struct {
 
 	// Status is the status of the BMCJob condition.
 	// Can be True or False.
-	Status BMCJobConditionStatus `json:"status"`
+	Status ConditionStatus `json:"status"`
 
 	// Message represents human readable message indicating details about last transition.
 	// +optional
@@ -101,7 +93,7 @@ type BMCJobSetConditionOption func(*BMCJobCondition)
 
 // SetCondition applies the cType condition to bmj. If the condition already exists,
 // it is updated.
-func (bmj *BMCJob) SetCondition(cType BMCJobConditionType, status BMCJobConditionStatus, opts ...BMCJobSetConditionOption) {
+func (bmj *BMCJob) SetCondition(cType BMCJobConditionType, status ConditionStatus, opts ...BMCJobSetConditionOption) {
 	var condition *BMCJobCondition
 
 	// Check if there's an existing condition.
@@ -126,6 +118,7 @@ func (bmj *BMCJob) SetCondition(cType BMCJobConditionType, status BMCJobConditio
 	}
 }
 
+// WithJobConditionMessage sets message m to the BMCJobCondition.
 func WithJobConditionMessage(m string) BMCJobSetConditionOption {
 	return func(c *BMCJobCondition) {
 		c.Message = m

api/v1alpha1/bmctask_types.go

@@ -30,14 +30,6 @@ const (
 	TaskFailed BMCTaskConditionType = "Failed"
 )
 
-// BMCTaskConditionStatus represents the status of a BMCTaskCondition.
-type BMCTaskConditionStatus string
-
-const (
-	BMCTaskConditionTrue  BMCTaskConditionStatus = "True"
-	BMCTaskConditionFalse BMCTaskConditionStatus = "False"
-)
-
 // BMCTaskSpec defines the desired state of BMCTask
 type BMCTaskSpec struct {
 	// Task defines the specific action to be performed.
@@ -96,7 +88,7 @@ type BMCTaskCondition struct {
 
 	// Status is the status of the BMCTask condition.
 	// Can be True or False.
-	Status BMCTaskConditionStatus `json:"status"`
+	Status ConditionStatus `json:"status"`
 
 	// Message represents human readable message indicating details about last transition.
 	// +optional
@@ -108,7 +100,7 @@ type BMCTaskSetConditionOption func(*BMCTaskCondition)
 
 // SetCondition applies the cType condition to bmt. If the condition already exists,
 // it is updated.
-func (bmt *BMCTask) SetCondition(cType BMCTaskConditionType, status BMCTaskConditionStatus, opts ...BMCTaskSetConditionOption) {
+func (bmt *BMCTask) SetCondition(cType BMCTaskConditionType, status ConditionStatus, opts ...BMCTaskSetConditionOption) {
 	var condition *BMCTaskCondition
 
 	// Check if there's an existing condition.
@@ -133,6 +125,7 @@ func (bmt *BMCTask) SetCondition(cType BMCTaskConditionType, status BMCTaskCondi
 	}
 }
 
+// WithTaskConditionMessage sets message m to the BMCTaskCondition.
 func WithTaskConditionMessage(m string) BMCTaskSetConditionOption {
 	return func(c *BMCTaskCondition) {
 		c.Message = m

controllers/baseboardmanagement_controller.go

@@ -133,7 +133,7 @@ func (r *BaseboardManagementReconciler) reconcile(ctx context.Context, bm *bmcv1
 	bmcClient, err := r.bmcClientFactory(ctx, bm.Spec.Connection.Host, strconv.Itoa(bm.Spec.Connection.Port), username, password)
 	if err != nil {
 		logger.Error(err, "BMC connection failed", "host", bm.Spec.Connection.Host)
-		bm.SetCondition(bmcv1alpha1.Contactable, bmcv1alpha1.BaseboardManagementConditionFalse, bmcv1alpha1.WithBaseboardManagementConditionMessage(err.Error()))
+		bm.SetCondition(bmcv1alpha1.Contactable, bmcv1alpha1.ConditionFalse, bmcv1alpha1.WithBaseboardManagementConditionMessage(err.Error()))
 		result, patchErr := r.patchStatus(ctx, bm, bmPatch)
 		if patchErr != nil {
 			return result, utilerrors.NewAggregate([]error{patchErr, err})
@@ -142,7 +142,7 @@ func (r *BaseboardManagementReconciler) reconcile(ctx context.Context, bm *bmcv1
 		return result, err
 	}
 	// Setting condition Contactable to True.
-	bm.SetCondition(bmcv1alpha1.Contactable, bmcv1alpha1.BaseboardManagementConditionTrue)
+	bm.SetCondition(bmcv1alpha1.Contactable, bmcv1alpha1.ConditionTrue)
 
 	// Close BMC connection after reconcilation
 	defer func() {

controllers/bmcjob_controller.go

@@ -109,7 +109,7 @@ func (r *BMCJobReconciler) reconcile(ctx context.Context, bmj *bmcv1alpha1.BMCJo
 	bmcClient, err := r.bmcClientFactory(ctx, baseboardManagement.Spec.Connection.Host, strconv.Itoa(baseboardManagement.Spec.Connection.Port), username, password)
 	if err != nil {
 		logger.Error(err, "BMC connection failed", "host", baseboardManagement.Spec.Connection.Host)
-		bmj.SetCondition(bmcv1alpha1.JobFailed, bmcv1alpha1.BMCJobConditionTrue, bmcv1alpha1.WithJobConditionMessage(fmt.Sprintf("Failed to connect to BMC: %v", err)))
+		bmj.SetCondition(bmcv1alpha1.JobFailed, bmcv1alpha1.ConditionTrue, bmcv1alpha1.WithJobConditionMessage(fmt.Sprintf("Failed to connect to BMC: %v", err)))
 		result, patchErr := r.patchBMCJobStatus(ctx, bmj, bmjPatch)
 		if patchErr != nil {
 			return result, utilerrors.NewAggregate([]error{patchErr, err})
@@ -141,7 +141,7 @@ func (r *BMCJobReconciler) reconcileBMCTasks(ctx context.Context, bmj *bmcv1alph
 	now := metav1.Now()
 	bmj.Status.StartTime = &now
 	// Set the Job to Running
-	bmj.SetCondition(bmcv1alpha1.JobRunning, bmcv1alpha1.BMCJobConditionTrue)
+	bmj.SetCondition(bmcv1alpha1.JobRunning, bmcv1alpha1.ConditionTrue)
 	if result, err := r.patchBMCJobStatus(ctx, bmj, bmjPatch); err != nil {
 		return result, err
 	}
@@ -153,7 +153,7 @@ func (r *BMCJobReconciler) reconcileBMCTasks(ctx context.Context, bmj *bmcv1alph
 		err := r.bmcTaskHandler.CreateBMCTaskWithOwner(ctx, r.client, task, i, bmcTask, bmj)
 		if err != nil {
 			logger.Error(err, "failed to reconcile BMCJob Tasks")
-			bmj.SetCondition(bmcv1alpha1.JobFailed, bmcv1alpha1.BMCJobConditionTrue, bmcv1alpha1.WithJobConditionMessage(fmt.Sprintf("Failed to reconcile BMCJob Tasks: %v", err)))
+			bmj.SetCondition(bmcv1alpha1.JobFailed, bmcv1alpha1.ConditionTrue, bmcv1alpha1.WithJobConditionMessage(fmt.Sprintf("Failed to reconcile BMCJob Tasks: %v", err)))
 			result, patchErr := r.patchBMCJobStatus(ctx, bmj, bmjPatch)
 			if patchErr != nil {
 				return result, utilerrors.NewAggregate([]error{patchErr, err})
@@ -166,7 +166,7 @@ func (r *BMCJobReconciler) reconcileBMCTasks(ctx context.Context, bmj *bmcv1alph
 		err = r.bmcTaskHandler.RunBMCTask(ctx, bmcTask, bmcClient)
 		if err != nil {
 			logger.Error(err, "failed to run BMCJob Task", "Name", bmcTask.Name, "Namespace", bmcTask.Namespace)
-			bmj.SetCondition(bmcv1alpha1.JobFailed, bmcv1alpha1.BMCJobConditionTrue, bmcv1alpha1.WithJobConditionMessage(fmt.Sprintf("Failed to run BMCJob Task %s/%s: %v", bmcTask.Namespace, bmcTask.Name, err)))
+			bmj.SetCondition(bmcv1alpha1.JobFailed, bmcv1alpha1.ConditionTrue, bmcv1alpha1.WithJobConditionMessage(fmt.Sprintf("Failed to run BMCJob Task %s/%s: %v", bmcTask.Namespace, bmcTask.Name, err)))
 			// Patch the BMCTask status
 			if taskPatchError := r.bmcTaskHandler.PatchBMCTaskStatus(ctx, r.client, bmcTask, bmcTaskPatch); taskPatchError != nil {
 				aggErr = utilerrors.NewAggregate([]error{taskPatchError, aggErr})
@@ -189,7 +189,7 @@ func (r *BMCJobReconciler) reconcileBMCTasks(ctx context.Context, bmj *bmcv1alph
 
 	now = metav1.Now()
 	bmj.Status.CompletionTime = &now
-	bmj.SetCondition(bmcv1alpha1.JobCompleted, bmcv1alpha1.BMCJobConditionTrue)
+	bmj.SetCondition(bmcv1alpha1.JobCompleted, bmcv1alpha1.ConditionTrue)
 
 	result, patchErr := r.patchBMCJobStatus(ctx, bmj, bmjPatch)
 	if patchErr != nil {

controllers/bmctaskhandler.go

@@ -71,7 +71,7 @@ func (r *BMCTaskHandler) RunBMCTask(ctx context.Context, bmcTask *bmcv1alpha1.BM
 	if bmcTask.Spec.Task.PowerAction != nil {
 		_, err := bmcClient.SetPowerState(ctx, string(bmcTask.Spec.Task.PowerAction.PowerControl))
 		if err != nil {
-			bmcTask.SetCondition(bmcv1alpha1.TaskFailed, bmcv1alpha1.BMCTaskConditionTrue, bmcv1alpha1.WithTaskConditionMessage(fmt.Sprintf("Failed to perform PowerAction: %v", err)))
+			bmcTask.SetCondition(bmcv1alpha1.TaskFailed, bmcv1alpha1.ConditionTrue, bmcv1alpha1.WithTaskConditionMessage(fmt.Sprintf("Failed to perform PowerAction: %v", err)))
 			return fmt.Errorf("failed to perform PowerAction: %v", err)
 		}
 	}
@@ -81,14 +81,14 @@ func (r *BMCTaskHandler) RunBMCTask(ctx context.Context, bmcTask *bmcv1alpha1.BM
 		// setPersistent is false.
 		_, err := bmcClient.SetBootDevice(ctx, string(bmcTask.Spec.Task.OneTimeBootDeviceAction.Devices[0]), false, bmcTask.Spec.Task.OneTimeBootDeviceAction.EFIBoot)
 		if err != nil {
-			bmcTask.SetCondition(bmcv1alpha1.TaskFailed, bmcv1alpha1.BMCTaskConditionTrue, bmcv1alpha1.WithTaskConditionMessage(fmt.Sprintf("Failed to perform OneTimeBootDeviceAction: %v", err)))
+			bmcTask.SetCondition(bmcv1alpha1.TaskFailed, bmcv1alpha1.ConditionTrue, bmcv1alpha1.WithTaskConditionMessage(fmt.Sprintf("Failed to perform OneTimeBootDeviceAction: %v", err)))
 			return fmt.Errorf("failed to perform OneTimeBootDeviceAction: %v", err)
 		}
 	}
 
 	now = metav1.Now()
 	bmcTask.Status.CompletionTime = &now
-	bmcTask.SetCondition(bmcv1alpha1.TaskCompleted, bmcv1alpha1.BMCTaskConditionTrue)
+	bmcTask.SetCondition(bmcv1alpha1.TaskCompleted, bmcv1alpha1.ConditionTrue)
 
 	return nil
 }