From 686ecf448ce9be7f20f591f56ae1533fced77d09 Mon Sep 17 00:00:00 2001 From: Andreas Fritzler Date: Thu, 4 Jul 2024 13:27:23 +0200 Subject: [PATCH] Use `CreateOrUpdate` in `Server` creation --- internal/controller/bmc_controller.go | 11 +++++--- internal/controller/endpoint_controller.go | 24 ++++++++++-------- internal/controller/helper.go | 4 --- internal/controller/server_controller.go | 25 +++++++++---------- internal/controller/serverclaim_controller.go | 12 ++++++--- 5 files changed, 41 insertions(+), 35 deletions(-) diff --git a/internal/controller/bmc_controller.go b/internal/controller/bmc_controller.go index c59bea2..19139c7 100644 --- a/internal/controller/bmc_controller.go +++ b/internal/controller/bmc_controller.go @@ -76,7 +76,7 @@ func (r *BMCReconciler) reconcile(ctx context.Context, log logr.Logger, bmcObj * } log.V(1).Info("Updated BMC status") - if err := r.discoverServers(ctx, bmcObj); err != nil && !errors.IsNotFound(err) { + if err := r.discoverServers(ctx, log, bmcObj); err != nil && !errors.IsNotFound(err) { return ctrl.Result{}, fmt.Errorf("failed to discover servers: %w", err) } log.V(1).Info("Discovered servers") @@ -131,7 +131,7 @@ func (r *BMCReconciler) updateBMCStatusDetails(ctx context.Context, log logr.Log return nil } -func (r *BMCReconciler) discoverServers(ctx context.Context, bmcObj *metalv1alpha1.BMC) error { +func (r *BMCReconciler) discoverServers(ctx context.Context, log logr.Logger, bmcObj *metalv1alpha1.BMC) error { bmcClient, err := GetBMCClientFromBMC(ctx, r.Client, bmcObj, r.Insecure) if err != nil { return fmt.Errorf("failed to create BMC client: %w", err) @@ -160,9 +160,12 @@ func (r *BMCReconciler) discoverServers(ctx context.Context, bmcObj *metalv1alph if err := controllerutil.SetControllerReference(bmcObj, server, r.Scheme); err != nil { return fmt.Errorf("failed to set owner reference on Server: %w", err) } - if err := r.Patch(ctx, server, client.Apply, fieldOwner); err != nil { - return fmt.Errorf("failed to apply Server: %w", err) + + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, server, nil) + if err != nil { + return fmt.Errorf("failed to create or patched Server: %w", err) } + log.V(1).Info("Created or patched Server", "Server", server.Name, "Operation", opResult) } return nil diff --git a/internal/controller/endpoint_controller.go b/internal/controller/endpoint_controller.go index adf2b99..59fa5e6 100644 --- a/internal/controller/endpoint_controller.go +++ b/internal/controller/endpoint_controller.go @@ -97,12 +97,12 @@ func (r *EndpointReconciler) reconcile(ctx context.Context, log logr.Logger, end // TODO: ensure that BMC has the correct MACAddress var bmcSecret *metalv1alpha1.BMCSecret - if bmcSecret, err = r.applyBMCSecret(ctx, endpoint, m); err != nil { + if bmcSecret, err = r.applyBMCSecret(ctx, log, endpoint, m); err != nil { return ctrl.Result{}, fmt.Errorf("failed to apply BMCSecret: %w", err) } log.V(1).Info("Applied BMC secret for endpoint") - if err := r.applyBMC(ctx, endpoint, bmcSecret, m); err != nil { + if err := r.applyBMC(ctx, log, endpoint, bmcSecret, m); err != nil { return ctrl.Result{}, fmt.Errorf("failed to apply BMC object: %w", err) } log.V(1).Info("Applied BMC object for endpoint") @@ -116,12 +116,12 @@ func (r *EndpointReconciler) reconcile(ctx context.Context, log logr.Logger, end defer bmcClient.Logout() var bmcSecret *metalv1alpha1.BMCSecret - if bmcSecret, err = r.applyBMCSecret(ctx, endpoint, m); err != nil { + if bmcSecret, err = r.applyBMCSecret(ctx, log, endpoint, m); err != nil { return ctrl.Result{}, fmt.Errorf("failed to apply BMCSecret: %w", err) } log.V(1).Info("Applied local test BMC secret for endpoint") - if err := r.applyBMC(ctx, endpoint, bmcSecret, m); err != nil { + if err := r.applyBMC(ctx, log, endpoint, bmcSecret, m); err != nil { return ctrl.Result{}, fmt.Errorf("failed to apply BMC object: %w", err) } log.V(1).Info("Applied local test BMC object for endpoint") @@ -142,7 +142,7 @@ func (r *EndpointReconciler) getProtocol() string { return protocol } -func (r *EndpointReconciler) applyBMC(ctx context.Context, endpoint *metalv1alpha1.Endpoint, secret *metalv1alpha1.BMCSecret, m macdb.MacPrefix) error { +func (r *EndpointReconciler) applyBMC(ctx context.Context, log logr.Logger, endpoint *metalv1alpha1.Endpoint, secret *metalv1alpha1.BMCSecret, m macdb.MacPrefix) error { bmcObj := &metalv1alpha1.BMC{ TypeMeta: metav1.TypeMeta{ APIVersion: metalv1alpha1.GroupVersion.String(), @@ -174,14 +174,16 @@ func (r *EndpointReconciler) applyBMC(ctx context.Context, endpoint *metalv1alph return err } - if err := r.Patch(ctx, bmcObj, client.Apply, fieldOwner); err != nil { - return err + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bmcObj, nil) + if err != nil { + return fmt.Errorf("failed to create or patch BMC: %w", err) } + log.V(1).Info("Created or patched BMC", "BMC", bmcObj.Name, "Operation", opResult) return nil } -func (r *EndpointReconciler) applyBMCSecret(ctx context.Context, endpoint *metalv1alpha1.Endpoint, m macdb.MacPrefix) (*metalv1alpha1.BMCSecret, error) { +func (r *EndpointReconciler) applyBMCSecret(ctx context.Context, log logr.Logger, endpoint *metalv1alpha1.Endpoint, m macdb.MacPrefix) (*metalv1alpha1.BMCSecret, error) { bmcSecret := &metalv1alpha1.BMCSecret{ TypeMeta: metav1.TypeMeta{ APIVersion: metalv1alpha1.GroupVersion.String(), @@ -200,9 +202,11 @@ func (r *EndpointReconciler) applyBMCSecret(ctx context.Context, endpoint *metal return nil, err } - if err := r.Patch(ctx, bmcSecret, client.Apply, fieldOwner); err != nil { - return nil, err + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bmcSecret, nil) + if err != nil { + return nil, fmt.Errorf("failed to create or patch BMCSecret: %w", err) } + log.V(1).Info("Created or patched BMSecret", "BMCSecret", bmcSecret.Name, "Operation", opResult) return bmcSecret, nil } diff --git a/internal/controller/helper.go b/internal/controller/helper.go index f58744d..8365584 100644 --- a/internal/controller/helper.go +++ b/internal/controller/helper.go @@ -8,10 +8,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const ( - fieldOwner = client.FieldOwner("metal.ironcore.dev/controller-manager") -) - func shouldIgnoreReconciliation(obj client.Object) bool { val, found := obj.GetAnnotations()[metalv1alpha1.OperationAnnotation] if !found { diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go index 2283afd..d244b9b 100644 --- a/internal/controller/server_controller.go +++ b/internal/controller/server_controller.go @@ -184,7 +184,7 @@ func (r *ServerReconciler) ensureServerStateTransition(ctx context.Context, log switch server.Status.State { case metalv1alpha1.ServerStateInitial: // apply boot configuration - if err := r.applyBootConfigurationAndIgnitionForDiscovery(ctx, server); err != nil { + if err := r.applyBootConfigurationAndIgnitionForDiscovery(ctx, log, server); err != nil { return false, fmt.Errorf("failed to apply server boot configuration: %w", err) } log.V(1).Info("Applied Server boot configuration") @@ -315,7 +315,7 @@ func (r *ServerReconciler) updateServerStatus(ctx context.Context, log logr.Logg return nil } -func (r *ServerReconciler) applyBootConfigurationAndIgnitionForDiscovery(ctx context.Context, server *metalv1alpha1.Server) error { +func (r *ServerReconciler) applyBootConfigurationAndIgnitionForDiscovery(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server) error { // apply server boot configuration bootConfig := &metalv1alpha1.ServerBootConfiguration{ TypeMeta: metav1.TypeMeta{ @@ -340,27 +340,24 @@ func (r *ServerReconciler) applyBootConfigurationAndIgnitionForDiscovery(ctx con }, } - if err := r.Patch(ctx, bootConfig, client.Apply, fieldOwner, client.ForceOwnership); err != nil { - return err + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bootConfig, nil) + if err != nil { + return fmt.Errorf("failed to create or patch ServerBootConfiguration: %w", err) } + log.V(1).Info("Created or patched", "ServerBootConfiguration", bootConfig.Name, "Operation", opResult) if err := r.ensureServerBootConfigRef(ctx, server, bootConfig); err != nil { return err } - if err := r.applyDefaultIgnitionForServer(ctx, server, bootConfig, r.RegistryURL); err != nil { + if err := r.applyDefaultIgnitionForServer(ctx, log, server, bootConfig, r.RegistryURL); err != nil { return err } return nil } -func (r *ServerReconciler) applyDefaultIgnitionForServer( - ctx context.Context, - server *metalv1alpha1.Server, - bootConfig *metalv1alpha1.ServerBootConfiguration, - registryURL string, -) error { +func (r *ServerReconciler) applyDefaultIgnitionForServer(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server, bootConfig *metalv1alpha1.ServerBootConfiguration, registryURL string) error { probeFlags := fmt.Sprintf("--registry-url=%s --server-uuid=%s", registryURL, server.Spec.UUID) ignitionData, err := r.generateDefaultIgnitionDataForServer(probeFlags) if err != nil { @@ -385,9 +382,11 @@ func (r *ServerReconciler) applyDefaultIgnitionForServer( return fmt.Errorf("failed to set controller reference for default ignitionSecret: %w", err) } - if err := r.Patch(ctx, ignitionSecret, client.Apply, fieldOwner, client.ForceOwnership); err != nil { - return fmt.Errorf("failed to apply default ignitionSecret: %w", err) + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, ignitionSecret, nil) + if err != nil { + return fmt.Errorf("failed to create or patch Ignition Secret: %w", err) } + log.V(1).Info("Created or patched Ignition Secret", "Secret", ignitionSecret.Name, "Operation", opResult) return nil } diff --git a/internal/controller/serverclaim_controller.go b/internal/controller/serverclaim_controller.go index cfa9289..d81d7ea 100644 --- a/internal/controller/serverclaim_controller.go +++ b/internal/controller/serverclaim_controller.go @@ -7,6 +7,8 @@ import ( "context" "fmt" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/go-logr/logr" "github.com/ironcore-dev/controller-utils/clientutils" metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1" @@ -149,7 +151,7 @@ func (r *ServerClaimReconciler) reconcile(ctx context.Context, log logr.Logger, return ctrl.Result{}, nil } - if err := r.applyBootConfiguration(ctx, server, claim); err != nil { + if err := r.applyBootConfiguration(ctx, log, server, claim); err != nil { return ctrl.Result{}, fmt.Errorf("failed to apply boot configuration: %w", err) } @@ -186,7 +188,7 @@ func (r *ServerClaimReconciler) patchServerClaimPhase(ctx context.Context, claim return true, nil } -func (r *ServerClaimReconciler) applyBootConfiguration(ctx context.Context, server *metalv1alpha1.Server, claim *metalv1alpha1.ServerClaim) error { +func (r *ServerClaimReconciler) applyBootConfiguration(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server, claim *metalv1alpha1.ServerClaim) error { config := &metalv1alpha1.ServerBootConfiguration{ TypeMeta: metav1.TypeMeta{ APIVersion: "metal.ironcore.dev/v1alpha1", @@ -208,9 +210,11 @@ func (r *ServerClaimReconciler) applyBootConfiguration(ctx context.Context, serv } // TODO: we might want to add a finalizer on the ignition secret - if err := r.Patch(ctx, config, client.Apply, fieldOwner, client.ForceOwnership); err != nil { - return fmt.Errorf("failed to apply boot configuration: %w", err) + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, config, nil) + if err != nil { + return fmt.Errorf("failed to create or patch ServerBootConfiguration: %w", err) } + log.V(1).Info("Created or patched ServerBootConfiguration", "ServerBootConfiguration", config.Name, "Operation", opResult) serverBase := server.DeepCopy() server.Spec.BootConfigurationRef = &v1.ObjectReference{