Merge pull request #5 from pokearu/bm-events-conditions

api/v1alpha1/baseboardmanagement_types.go

@@ -30,6 +30,9 @@ type BootDevice string
 // BaseboardManagementConditionType represents the condition of the BaseboardManagement.
 type BaseboardManagementConditionType string
 
+// BaseboardManagementConditionStatus represents the status of a BaseboardManagementCondition.
+type BaseboardManagementConditionStatus string
+
 const (
 	On  PowerState = "on"
 	Off PowerState = "off"
@@ -44,8 +47,13 @@ const (
 )
 
 const (
-	// ConnectionError represents failure to connect to the BaseboardManagement.
-	ConnectionError BaseboardManagementConditionType = "ConnectionError"
+	// Contactable defines that a connection can be made to the BaseboardManagement.
+	Contactable BaseboardManagementConditionType = "Contactable"
+)
+
+const (
+	BaseboardManagementConditionTrue  BaseboardManagementConditionStatus = "True"
+	BaseboardManagementConditionFalse BaseboardManagementConditionStatus = "False"
 )
 
 // BaseboardManagementSpec defines the desired state of BaseboardManagement
@@ -92,11 +100,55 @@ type BaseboardManagementCondition struct {
 	// Type of the BaseboardManagement condition.
 	Type BaseboardManagementConditionType `json:"type"`
 
+	// Status is the status of the BaseboardManagement condition.
+	// Can be True or False.
+	Status BaseboardManagementConditionStatus `json:"status"`
+
+	// Last time the BaseboardManagement condition was updated.
+	LastUpdateTime metav1.Time `json:"lastUpdateTime"`
+
 	// Message represents human readable message indicating details about last transition.
 	// +optional
 	Message string `json:"message,omitempty"`
 }
 
+// +kubebuilder:object:generate=false
+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) {
+	var condition *BaseboardManagementCondition
+
+	// Check if there's an existing condition.
+	for i, c := range bm.Status.Conditions {
+		if c.Type == cType {
+			condition = &bm.Status.Conditions[i]
+			break
+		}
+	}
+
+	// We didn't find an existing condition so create a new one and append it.
+	if condition == nil {
+		bm.Status.Conditions = append(bm.Status.Conditions, BaseboardManagementCondition{
+			Type: cType,
+		})
+		condition = &bm.Status.Conditions[len(bm.Status.Conditions)-1]
+	}
+
+	condition.Status = status
+	condition.LastUpdateTime = metav1.Now()
+	for _, opt := range opts {
+		opt(condition)
+	}
+}
+
+func WithBaseboardManagementConditionMessage(m string) BaseboardManagementSetConditionOption {
+	return func(c *BaseboardManagementCondition) {
+		c.Message = m
+	}
+}
+
 // BaseboardManagementRef defines the reference information to a BaseboardManagement resource.
 type BaseboardManagementRef struct {
 	// Name is unique within a namespace to reference a BaseboardManagement resource.

config/crd/bases/bmc.tinkerbell.org_baseboardmanagements.yaml

@@ -93,14 +93,25 @@ spec:
                   of an object's current state.
                 items:
                   properties:
+                    lastUpdateTime:
+                      description: Last time the BaseboardManagement condition was
+                        updated.
+                      format: date-time
+                      type: string
                     message:
                       description: Message represents human readable message indicating
                         details about last transition.
                       type: string
+                    status:
+                      description: Status is the status of the BaseboardManagement
+                        condition. Can be True or False.
+                      type: string
                     type:
                       description: Type of the BaseboardManagement condition.
                       type: string
                   required:
+                  - lastUpdateTime
+                  - status
                   - type
                   type: object
                 type: array

controllers/baseboardmanagement_controller.go

@@ -27,6 +27,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -55,14 +56,21 @@ type BMCClientFactoryFunc func(ctx context.Context, hostIP, port, username, pass
 // BaseboardManagementReconciler reconciles a BaseboardManagement object
 type BaseboardManagementReconciler struct {
 	client           client.Client
+	recorder         record.EventRecorder
 	bmcClientFactory BMCClientFactoryFunc
 	logger           logr.Logger
 }
 
+const (
+	EventGetPowerStateFailed = "GetPowerStateFailed"
+	EventSetPowerStateFailed = "SetPowerStateFailed"
+)
+
 // NewBaseboardManagementReconciler returns a new BaseboardManagementReconciler
-func NewBaseboardManagementReconciler(client client.Client, bmcClientFactory BMCClientFactoryFunc, logger logr.Logger) *BaseboardManagementReconciler {
+func NewBaseboardManagementReconciler(client client.Client, recorder record.EventRecorder, bmcClientFactory BMCClientFactoryFunc, logger logr.Logger) *BaseboardManagementReconciler {
 	return &BaseboardManagementReconciler{
 		client:           client,
+		recorder:         recorder,
 		bmcClientFactory: bmcClientFactory,
 		logger:           logger,
 	}
@@ -119,12 +127,16 @@ 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)
-		result, setConditionErr := r.setCondition(ctx, bm, bmPatch, bmcv1alpha1.ConnectionError, err.Error())
-		if setConditionErr != nil {
-			return result, utilerrors.NewAggregate([]error{fmt.Errorf("failed to set conditions: %v", setConditionErr), err})
+		bm.SetCondition(bmcv1alpha1.Contactable, bmcv1alpha1.BaseboardManagementConditionFalse, bmcv1alpha1.WithBaseboardManagementConditionMessage(err.Error()))
+		result, patchErr := r.patchStatus(ctx, bm, bmPatch)
+		if patchErr != nil {
+			return result, utilerrors.NewAggregate([]error{patchErr, err})
 		}
+
 		return result, err
 	}
+	// Setting condition Contactable to True.
+	bm.SetCondition(bmcv1alpha1.Contactable, bmcv1alpha1.BaseboardManagementConditionTrue)
 
 	// Close BMC connection after reconcilation
 	defer func() {
@@ -138,20 +150,29 @@ func (r *BaseboardManagementReconciler) reconcile(ctx context.Context, bm *bmcv1
 	fieldReconcilers := []baseboardManagementFieldReconciler{
 		r.reconcilePower,
 	}
+
+	var aggErr utilerrors.Aggregate
 	for _, reconiler := range fieldReconcilers {
 		if err := reconiler(ctx, bm, bmcClient); err != nil {
 			logger.Error(err, "Failed to reconcile BaseboardManagement", "host", bm.Spec.Connection.Host)
+			aggErr = utilerrors.NewAggregate([]error{err, aggErr})
 		}
 	}
 
 	// Patch the status after each reconciliation
-	return r.reconcileStatus(ctx, bm, bmPatch)
+	result, err := r.patchStatus(ctx, bm, bmPatch)
+	if err != nil {
+		aggErr = utilerrors.NewAggregate([]error{err, aggErr})
+	}
+
+	return result, utilerrors.Flatten(aggErr)
 }
 
 // reconcilePower ensures the BaseboardManagement Power is in the desired state.
 func (r *BaseboardManagementReconciler) reconcilePower(ctx context.Context, bm *bmcv1alpha1.BaseboardManagement, bmcClient BMCClient) error {
 	powerStatus, err := bmcClient.GetPowerState(ctx)
 	if err != nil {
+		r.recorder.Eventf(bm, corev1.EventTypeWarning, EventGetPowerStateFailed, "failed to get power state: %v", err)
 		return fmt.Errorf("failed to get power state: %v", err)
 	}
 
@@ -165,6 +186,7 @@ func (r *BaseboardManagementReconciler) reconcilePower(ctx context.Context, bm *
 	// Setting baseboard management to desired power state
 	_, err = bmcClient.SetPowerState(ctx, string(bm.Spec.Power))
 	if err != nil {
+		r.recorder.Eventf(bm, corev1.EventTypeWarning, EventSetPowerStateFailed, "failed to set power state: %v", err)
 		return fmt.Errorf("failed to set power state: %v", err)
 	}
 
@@ -174,42 +196,6 @@ func (r *BaseboardManagementReconciler) reconcilePower(ctx context.Context, bm *
 	return nil
 }
 
-// setCondition updates the status.Condition if the condition type is present.
-// Appends if new condition is found.
-// Patches the BaseboardManagement status.
-func (r *BaseboardManagementReconciler) setCondition(ctx context.Context, bm *bmcv1alpha1.BaseboardManagement, bmPatch client.Patch, cType bmcv1alpha1.BaseboardManagementConditionType, message string) (ctrl.Result, error) {
-	currentConditions := bm.Status.Conditions
-	for i := range currentConditions {
-		// If condition exists, update the message if different
-		if currentConditions[i].Type == cType {
-			if currentConditions[i].Message != message {
-				bm.Status.Conditions[i].Message = message
-				return r.patchStatus(ctx, bm, bmPatch)
-			}
-			return ctrl.Result{}, nil
-		}
-	}
-
-	// Append new condition to Conditions
-	condition := bmcv1alpha1.BaseboardManagementCondition{
-		Type:    cType,
-		Message: message,
-	}
-	bm.Status.Conditions = append(bm.Status.Conditions, condition)
-
-	return r.patchStatus(ctx, bm, bmPatch)
-}
-
-// reconcileStatus updates the Power and Conditions and patches BaseboardManagement status.
-func (r *BaseboardManagementReconciler) reconcileStatus(ctx context.Context, bm *bmcv1alpha1.BaseboardManagement, bmPatch client.Patch) (ctrl.Result, error) {
-	// TODO: (pokearu) modify conditions to model current state.
-	// Add condition Status to represent if object has a condition
-	// insted of clearing the conditions.
-	bm.Status.Conditions = []bmcv1alpha1.BaseboardManagementCondition{}
-
-	return r.patchStatus(ctx, bm, bmPatch)
-}
-
 // patchStatus patches the specifies patch on the BaseboardManagement.
 func (r *BaseboardManagementReconciler) patchStatus(ctx context.Context, bm *bmcv1alpha1.BaseboardManagement, patch client.Patch) (ctrl.Result, error) {
 	err := r.client.Status().Patch(ctx, bm, patch)

controllers/baseboardmanagement_controller_test.go

@@ -12,6 +12,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -37,13 +38,15 @@ func TestReconcileSetPowerSuccess(t *testing.T) {
 
 	clientBuilder := fake.NewClientBuilder()
 	client := clientBuilder.WithScheme(scheme).WithRuntimeObjects(objs...).Build()
+	fakeRecorder := record.NewFakeRecorder(2)
 
 	mockBMCClient.EXPECT().GetPowerState(ctx).Return(string(bmcv1alpha1.Off), nil)
 	mockBMCClient.EXPECT().SetPowerState(ctx, string(bmcv1alpha1.On)).Return(true, nil)
 	mockBMCClient.EXPECT().Close(ctx).Return(nil)
 
 	reconciler := controllers.NewBaseboardManagementReconciler(
 		client,
+		fakeRecorder,
 		newMockBMCClientFactoryFunc(mockBMCClient),
 		testr.New(t),
 	)
@@ -76,12 +79,14 @@ func TestReconcileDesiredPowerStateSuccess(t *testing.T) {
 
 	clientBuilder := fake.NewClientBuilder()
 	client := clientBuilder.WithScheme(scheme).WithRuntimeObjects(objs...).Build()
+	fakeRecorder := record.NewFakeRecorder(2)
 
 	mockBMCClient.EXPECT().GetPowerState(ctx).Return(string(bmcv1alpha1.On), nil)
 	mockBMCClient.EXPECT().Close(ctx).Return(nil)
 
 	reconciler := controllers.NewBaseboardManagementReconciler(
 		client,
+		fakeRecorder,
 		newMockBMCClientFactoryFunc(mockBMCClient),
 		testr.New(t),
 	)
@@ -109,6 +114,7 @@ func TestReconcileSecretReferenceError(t *testing.T) {
 	scheme := runtime.NewScheme()
 	_ = bmcv1alpha1.AddToScheme(scheme)
 	_ = corev1.AddToScheme(scheme)
+	fakeRecorder := record.NewFakeRecorder(2)
 
 	tt := map[string]struct {
 		Secret *corev1.Secret
@@ -147,6 +153,7 @@ func TestReconcileSecretReferenceError(t *testing.T) {
 			client := clientBuilder.WithScheme(scheme).WithRuntimeObjects(objs...).Build()
 			reconciler := controllers.NewBaseboardManagementReconciler(
 				client,
+				fakeRecorder,
 				newMockBMCClientFactoryFunc(mockBMCClient),
 				testr.New(t),
 			)
@@ -180,9 +187,11 @@ func TestReconcileConnectionError(t *testing.T) {
 	_ = corev1.AddToScheme(scheme)
 	clientBuilder := fake.NewClientBuilder()
 	client := clientBuilder.WithScheme(scheme).WithRuntimeObjects(objs...).Build()
+	fakeRecorder := record.NewFakeRecorder(2)
 
 	reconciler := controllers.NewBaseboardManagementReconciler(
 		client,
+		fakeRecorder,
 		newMockBMCClientFactoryFuncError(),
 		testr.New(t),
 	)
@@ -198,6 +207,48 @@ func TestReconcileConnectionError(t *testing.T) {
 	g.Expect(err).To(gomega.HaveOccurred())
 }
 
+func TestReconcileSetPowerError(t *testing.T) {
+	g := gomega.NewWithT(t)
+
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	mockBMCClient := mocks.NewMockBMCClient(ctrl)
+
+	bm := getBaseboardManagement()
+	authSecret := getSecret()
+
+	objs := []runtime.Object{bm, authSecret}
+	scheme := runtime.NewScheme()
+	_ = bmcv1alpha1.AddToScheme(scheme)
+	_ = corev1.AddToScheme(scheme)
+
+	clientBuilder := fake.NewClientBuilder()
+	client := clientBuilder.WithScheme(scheme).WithRuntimeObjects(objs...).Build()
+	fakeRecorder := record.NewFakeRecorder(2)
+
+	mockBMCClient.EXPECT().GetPowerState(ctx).Return(string(bmcv1alpha1.Off), nil)
+	mockBMCClient.EXPECT().SetPowerState(ctx, string(bmcv1alpha1.On)).Return(false, errors.New("this is not allowed"))
+	mockBMCClient.EXPECT().Close(ctx).Return(nil)
+
+	reconciler := controllers.NewBaseboardManagementReconciler(
+		client,
+		fakeRecorder,
+		newMockBMCClientFactoryFunc(mockBMCClient),
+		testr.New(t),
+	)
+
+	req := reconcile.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: "test-namespace",
+			Name:      "test-bm",
+		},
+	}
+
+	_, err := reconciler.Reconcile(ctx, req)
+	g.Expect(err).To(gomega.HaveOccurred())
+	g.Expect(fakeRecorder.Events).NotTo(gomega.BeEmpty())
+}
+
 // newMockBMCClientFactoryFunc returns a new BMCClientFactoryFunc
 func newMockBMCClientFactoryFunc(mockBMCClient *mocks.MockBMCClient) controllers.BMCClientFactoryFunc {
 	return func(ctx context.Context, hostIP, port, username, password string) (controllers.BMCClient, error) {

main.go

@@ -141,6 +141,7 @@ func newClientConfig(kubeAPIServer, kubeconfig string) clientcmd.ClientConfig {
 func setupReconcilers(ctx context.Context, mgr ctrl.Manager) {
 	err := (controllers.NewBaseboardManagementReconciler(
 		mgr.GetClient(),
+		mgr.GetEventRecorderFor("baseboardmanagement-controller"),
 		controllers.NewBMCClientFactoryFunc(ctx),
 		ctrl.Log.WithName("controller").WithName("BaseboardManagement"),
 	)).SetupWithManager(mgr)