From 9a29dae0cfa4575cc94b3c9d9643e441cea7ef9b Mon Sep 17 00:00:00 2001 From: Erik Schubert Date: Fri, 10 Jan 2025 13:24:03 +0100 Subject: [PATCH] Fix controllerutil.CreateOrPatch() calls (#216) --- api/v1alpha1/zz_generated.deepcopy.go | 2 +- internal/controller/bmc_controller.go | 30 +++----- internal/controller/endpoint_controller.go | 72 ++++++------------- internal/controller/server_controller.go | 71 ++++++------------ internal/controller/serverclaim_controller.go | 38 +++------- 5 files changed, 66 insertions(+), 147 deletions(-) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index f764e86..b923657 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -8,7 +8,7 @@ package v1alpha1 import ( - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) diff --git a/internal/controller/bmc_controller.go b/internal/controller/bmc_controller.go index 2731dbe..4e6ba52 100644 --- a/internal/controller/bmc_controller.go +++ b/internal/controller/bmc_controller.go @@ -16,7 +16,6 @@ import ( "github.com/ironcore-dev/metal-operator/bmc" "github.com/ironcore-dev/metal-operator/internal/bmcutils" v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -161,26 +160,15 @@ func (r *BMCReconciler) discoverServers(ctx context.Context, log logr.Logger, bm return fmt.Errorf("failed to get Servers from BMC: %w", err) } for i, s := range servers { - server := &metalv1alpha1.Server{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "metal.ironcore.dev/v1alpha1", - Kind: "Server", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: bmcutils.GetServerNameFromBMCandIndex(i, bmcObj), - }, - Spec: metalv1alpha1.ServerSpec{ - UUID: strings.ToLower(s.UUID), // always use lower-case uuids - SystemUUID: strings.ToLower(s.UUID), // always use lower-case uuids - BMCRef: &v1.LocalObjectReference{Name: bmcObj.Name}, - }, - } - - if err := controllerutil.SetControllerReference(bmcObj, server, r.Scheme); err != nil { - return fmt.Errorf("failed to set owner reference on Server: %w", err) - } - - opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, server, nil) + server := &metalv1alpha1.Server{} + server.Name = bmcutils.GetServerNameFromBMCandIndex(i, bmcObj) + + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, server, func() error { + server.Spec.UUID = strings.ToLower(s.UUID) + server.Spec.SystemUUID = strings.ToLower(s.UUID) + server.Spec.BMCRef = &v1.LocalObjectReference{Name: bmcObj.Name} + return controllerutil.SetControllerReference(bmcObj, server, r.Scheme) + }) if err != nil { return fmt.Errorf("failed to create or patch Server: %w", err) } diff --git a/internal/controller/endpoint_controller.go b/internal/controller/endpoint_controller.go index 897c4da..41d4d36 100644 --- a/internal/controller/endpoint_controller.go +++ b/internal/controller/endpoint_controller.go @@ -16,7 +16,6 @@ import ( "github.com/ironcore-dev/metal-operator/internal/api/macdb" "github.com/ironcore-dev/metal-operator/internal/bmcutils" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -171,37 +170,22 @@ func (r *EndpointReconciler) getProtocol() string { } 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(), - Kind: "BMC", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: endpoint.Name, - }, - Spec: metalv1alpha1.BMCSpec{ - EndpointRef: &corev1.LocalObjectReference{ - Name: endpoint.Name, - }, - BMCSecretRef: corev1.LocalObjectReference{ - Name: secret.Name, - }, - Protocol: metalv1alpha1.Protocol{ - Name: metalv1alpha1.ProtocolName(m.Protocol), - Port: m.Port, - }, - ConsoleProtocol: &metalv1alpha1.ConsoleProtocol{ - Name: metalv1alpha1.ConsoleProtocolName(m.Console.Type), - Port: m.Console.Port, - }, - }, - } - - if err := controllerutil.SetControllerReference(endpoint, bmcObj, r.Client.Scheme()); err != nil { - return err - } - - opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bmcObj, nil) + bmcObj := &metalv1alpha1.BMC{} + bmcObj.Name = endpoint.Name + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bmcObj, func() error { + spec := &bmcObj.Spec + spec.EndpointRef = &corev1.LocalObjectReference{Name: endpoint.Name} + spec.BMCSecretRef = corev1.LocalObjectReference{Name: secret.Name} + spec.Protocol = metalv1alpha1.Protocol{ + Name: metalv1alpha1.ProtocolName(m.Protocol), + Port: m.Port, + } + spec.ConsoleProtocol = &metalv1alpha1.ConsoleProtocol{ + Name: metalv1alpha1.ConsoleProtocolName(m.Console.Type), + Port: m.Console.Port, + } + return controllerutil.SetControllerReference(endpoint, bmcObj, r.Client.Scheme()) + }) if err != nil { return fmt.Errorf("failed to create or patch BMC: %w", err) } @@ -211,25 +195,15 @@ func (r *EndpointReconciler) applyBMC(ctx context.Context, log logr.Logger, endp } 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(), - Kind: "BMCSecret", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: endpoint.Name, - }, - Data: map[string][]byte{ + bmcSecret := &metalv1alpha1.BMCSecret{} + bmcSecret.Name = endpoint.Name + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bmcSecret, func() error { + bmcSecret.Data = map[string][]byte{ metalv1alpha1.BMCSecretUsernameKeyName: []byte(m.DefaultCredentials[0].Username), metalv1alpha1.BMCSecretPasswordKeyName: []byte(m.DefaultCredentials[0].Password), - }, - } - - if err := controllerutil.SetControllerReference(endpoint, bmcSecret, r.Client.Scheme()); err != nil { - return nil, err - } - - opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bmcSecret, nil) + } + return controllerutil.SetControllerReference(endpoint, bmcSecret, r.Client.Scheme()) + }) if err != nil { return nil, fmt.Errorf("failed to create or patch BMCSecret: %w", err) } diff --git a/internal/controller/server_controller.go b/internal/controller/server_controller.go index dac5733..ed117a4 100644 --- a/internal/controller/server_controller.go +++ b/internal/controller/server_controller.go @@ -467,31 +467,19 @@ func (r *ServerReconciler) updateServerStatus(ctx context.Context, log logr.Logg } func (r *ServerReconciler) applyBootConfigurationAndIgnitionForDiscovery(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server) error { - // apply server boot configuration - bootConfig := &metalv1alpha1.ServerBootConfiguration{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "metal.ironcore.dev/v1alpha1", - Kind: "ServerBootConfiguration", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: server.Name, - Namespace: r.ManagerNamespace, - Annotations: map[string]string{ - InternalAnnotationTypeKeyName: InternalAnnotationTypeValue, - }, - }, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: v1.LocalObjectReference{ - Name: server.Name, - }, - IgnitionSecretRef: &v1.LocalObjectReference{ - Name: server.Name, - }, - Image: r.ProbeOSImage, - }, - } - - opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bootConfig, nil) + bootConfig := &metalv1alpha1.ServerBootConfiguration{} + bootConfig.Name = server.Name + bootConfig.Namespace = r.ManagerNamespace + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, bootConfig, func() error { + if bootConfig.Annotations == nil { + bootConfig.Annotations = make(map[string]string) + } + bootConfig.Annotations[InternalAnnotationTypeKeyName] = InternalAnnotationTypeValue + bootConfig.Spec.ServerRef = v1.LocalObjectReference{Name: server.Name} + bootConfig.Spec.IgnitionSecretRef = &v1.LocalObjectReference{Name: server.Name} + bootConfig.Spec.Image = r.ProbeOSImage + return nil + }) if err != nil { return fmt.Errorf("failed to create or patch ServerBootConfiguration: %w", err) } @@ -500,12 +488,7 @@ func (r *ServerReconciler) applyBootConfigurationAndIgnitionForDiscovery(ctx con if err := r.ensureServerBootConfigRef(ctx, server, bootConfig); err != nil { return err } - - if err := r.applyDefaultIgnitionForServer(ctx, log, server, bootConfig, r.RegistryURL); err != nil { - return err - } - - return nil + return r.applyDefaultIgnitionForServer(ctx, log, server, bootConfig, r.RegistryURL) } func (r *ServerReconciler) applyDefaultIgnitionForServer(ctx context.Context, log logr.Logger, server *metalv1alpha1.Server, bootConfig *metalv1alpha1.ServerBootConfiguration, registryURL string) error { @@ -515,26 +498,16 @@ func (r *ServerReconciler) applyDefaultIgnitionForServer(ctx context.Context, lo return fmt.Errorf("failed to generate default ignitionSecret data: %w", err) } - ignitionSecret := &v1.Secret{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Secret", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: r.ManagerNamespace, - Name: bootConfig.Name, - }, - Data: map[string][]byte{ + ignitionSecret := &v1.Secret{} + ignitionSecret.Name = bootConfig.Name + ignitionSecret.Namespace = r.ManagerNamespace + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, ignitionSecret, func() error { + ignitionSecret.Data = map[string][]byte{ DefaultIgnitionFormatKey: []byte(DefaultIgnitionFormatValue), DefaultIgnitionSecretKeyName: ignitionData, - }, - } - - if err := controllerutil.SetControllerReference(bootConfig, ignitionSecret, r.Client.Scheme()); err != nil { - return fmt.Errorf("failed to set controller reference for default ignitionSecret: %w", err) - } - - opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, ignitionSecret, nil) + } + return controllerutil.SetControllerReference(bootConfig, ignitionSecret, r.Client.Scheme()) + }) if err != nil { return fmt.Errorf("failed to create or patch Ignition Secret: %w", err) } diff --git a/internal/controller/serverclaim_controller.go b/internal/controller/serverclaim_controller.go index fc513b3..2c3eada 100644 --- a/internal/controller/serverclaim_controller.go +++ b/internal/controller/serverclaim_controller.go @@ -246,28 +246,16 @@ func (r *ServerClaimReconciler) patchServerRef(ctx context.Context, claim *metal } 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", - Kind: "ServerBootConfiguration", - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: claim.Namespace, - Name: claim.Name, - }, - Spec: metalv1alpha1.ServerBootConfigurationSpec{ - ServerRef: *claim.Spec.ServerRef, - Image: claim.Spec.Image, - IgnitionSecretRef: claim.Spec.IgnitionSecretRef, - }, - } - - if err := ctrl.SetControllerReference(claim, config, r.Scheme); err != nil { - return fmt.Errorf("failed to set controller reference: %w", err) - } - - // TODO: we might want to add a finalizer on the ignition secret - opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, config, nil) + config := &metalv1alpha1.ServerBootConfiguration{} + config.Name = claim.Name + config.Namespace = claim.Namespace + opResult, err := controllerutil.CreateOrPatch(ctx, r.Client, config, func() error { + // TODO: we might want to add a finalizer on the ignition secret + config.Spec.ServerRef = *claim.Spec.ServerRef + config.Spec.Image = claim.Spec.Image + config.Spec.IgnitionSecretRef = claim.Spec.IgnitionSecretRef + return ctrl.SetControllerReference(claim, config, r.Scheme) + }) if err != nil { return fmt.Errorf("failed to create or patch ServerBootConfiguration: %w", err) } @@ -281,11 +269,7 @@ func (r *ServerClaimReconciler) applyBootConfiguration(ctx context.Context, log APIVersion: "metal.ironcore.dev/v1alpha1", Kind: "ServerBootConfiguration", } - if err := r.Patch(ctx, server, client.MergeFrom(serverBase)); err != nil { - return err - } - - return nil + return r.Patch(ctx, server, client.MergeFrom(serverBase)) } func (r *ServerClaimReconciler) removeClaimRefFromServer(ctx context.Context, server *metalv1alpha1.Server) error {