Harmonize BMCUser reconciler code

Author: afritzler

internal/controller/bmcuser_controller.go

@@ -5,6 +5,7 @@ package controller
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -25,7 +26,7 @@ import (
 )
 
 const (
-	BMCUserFinalizer = "metal.ironcore.dev/bmcuser-finalizer"
+	BMCUserFinalizer = "metal.ironcore.dev/bmcuser"
 )
 
 // BMCUserReconciler reconciles a BMCUser object
@@ -53,24 +54,18 @@ func (r *BMCUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 func (r *BMCUserReconciler) reconcileExists(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) (ctrl.Result, error) {
 	if !user.DeletionTimestamp.IsZero() {
-		err := r.delete(ctx, log, user)
-		if err != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to delete User: %w", err)
-		}
-		return ctrl.Result{}, nil
+		return r.delete(ctx, log, user)
 	}
 	return r.reconcile(ctx, log, user)
 }
 
 func (r *BMCUserReconciler) reconcile(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) (ctrl.Result, error) {
 	if user.Spec.BMCRef == nil {
-		log.Info("No BMC reference set for User, skipping reconciliation", "User", user.Name)
+		log.Info("No BMC reference set for User, skipping reconciliation")
 		return ctrl.Result{}, nil
 	}
 	bmcObj := &metalv1alpha1.BMC{}
-	if err := r.Get(ctx, client.ObjectKey{
-		Name: user.Spec.BMCRef.Name,
-	}, bmcObj); err != nil {
+	if err := r.Get(ctx, client.ObjectKey{Name: user.Spec.BMCRef.Name}, bmcObj); err != nil {
 		return ctrl.Result{}, err
 	}
 	if err := r.updateEffectiveSecret(ctx, log, user, bmcObj); err != nil {
@@ -84,21 +79,18 @@ func (r *BMCUserReconciler) reconcile(ctx context.Context, log logr.Logger, user
 		return ctrl.Result{}, fmt.Errorf("failed to get BMC client: %w", err)
 	}
 	defer bmcClient.Logout()
-	err = r.patchUserStatus(ctx, log, user, bmcClient)
-	if err != nil {
+	if err = r.patchUserStatus(ctx, log, user, bmcClient); err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to update User status: %w", err)
 	}
 
 	if user.Spec.BMCSecretRef == nil {
-		log.Info("No BMCSecret reference set for User, creating a new one", "User", user.Name)
+		log.Info("No BMCSecret reference set for User, creating a new one")
 		if err := r.ensureBMCSecretForUser(ctx, log, bmcClient, user, bmcObj); err != nil {
 			return ctrl.Result{}, fmt.Errorf("failed to handle missing BMCSecret reference: %w", err)
 		}
 	}
 	bmcSecret := &metalv1alpha1.BMCSecret{}
-	if err := r.Get(ctx, client.ObjectKey{
-		Name: user.Spec.BMCSecretRef.Name,
-	}, bmcSecret); err != nil {
+	if err := r.Get(ctx, client.ObjectKey{Name: user.Spec.BMCSecretRef.Name}, bmcSecret); err != nil {
 		return ctrl.Result{}, err
 	}
 
@@ -177,10 +169,7 @@ func (r *BMCUserReconciler) handleRotatingPassword(ctx context.Context, log logr
 		user.Status.LastRotation.Time.Add(user.Spec.RotationPeriod.Duration).After(time.Now()) &&
 		!forceRotation {
 		log.V(1).Info("BMC user password rotation is not needed yet", "User", user.Name)
-		return ctrl.Result{
-			Requeue:      true,
-			RequeueAfter: user.Spec.RotationPeriod.Duration,
-		}, nil
+		return ctrl.Result{RequeueAfter: user.Spec.RotationPeriod.Duration}, nil
 	}
 	log.Info("Rotating BMC user password", "User", user.Name)
 	accountService, err := bmcClient.GetAccountService()
@@ -362,7 +351,8 @@ func (r *BMCUserReconciler) bmcConnectionTest(ctx context.Context, secret *metal
 	}
 	_, err = bmcutils.CreateBMCClient(ctx, r.Client, protocolScheme, bmcObj.Spec.Protocol.Name, address, bmcObj.Spec.Protocol.Port, secret, r.BMCOptions)
 	if err != nil {
-		if httpErr, ok := err.(*common.Error); ok {
+		var httpErr *common.Error
+		if errors.As(err, &httpErr) {
 			if httpErr.HTTPReturnedStatusCode == 401 || httpErr.HTTPReturnedStatusCode == 403 {
 				return true, nil
 			}
@@ -372,31 +362,29 @@ func (r *BMCUserReconciler) bmcConnectionTest(ctx context.Context, secret *metal
 	return false, nil
 }
 
-func (r *BMCUserReconciler) delete(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) error {
+func (r *BMCUserReconciler) delete(ctx context.Context, log logr.Logger, user *metalv1alpha1.BMCUser) (ctrl.Result, error) {
 	if user.Spec.BMCRef == nil {
 		log.Info("No BMC reference set for User, skipping deletion", "User", user.Name)
-		return nil
+		return ctrl.Result{}, nil
 	}
 	bmcObj := &metalv1alpha1.BMC{}
-	if err := r.Get(ctx, client.ObjectKey{
-		Name: user.Spec.BMCRef.Name,
-	}, bmcObj); err != nil {
-		return client.IgnoreNotFound(err)
+	if err := r.Get(ctx, client.ObjectKey{Name: user.Spec.BMCRef.Name}, bmcObj); err != nil {
+		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 	bmcClient, err := r.getBMCClient(ctx, bmcObj)
 	if err != nil {
-		return fmt.Errorf("failed to get BMC client: %w", err)
+		return ctrl.Result{}, fmt.Errorf("failed to get BMC client: %w", err)
 	}
 	defer bmcClient.Logout()
 	log.Info("Deleting BMC account for User", "User", user.Name)
 	if err := bmcClient.DeleteAccount(ctx, user.Spec.UserName, user.Status.ID); err != nil {
-		return fmt.Errorf("failed to delete BMC account: %w", err)
+		return ctrl.Result{}, fmt.Errorf("failed to delete BMC account: %w", err)
 	}
 	log.Info("Successfully deleted BMC account and removed finalizer for User", "User", user.Name)
 	if modified, err := clientutils.PatchEnsureNoFinalizer(ctx, r.Client, user, BMCUserFinalizer); err != nil || modified {
-		return err
+		return ctrl.Result{}, err
 	}
-	return nil
+	return ctrl.Result{}, nil
 }
 
 // SetupWithManager sets up the controller with the Manager.