From 826326c63efb08ec6c36b6f1191ebcabc4b97e95 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Thu, 28 Oct 2021 14:13:12 +0200 Subject: [PATCH] Remove template cleanup func --- controllers/topology/reconcile_state.go | 60 ++++++++----------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/controllers/topology/reconcile_state.go b/controllers/topology/reconcile_state.go index 87403a8bcc05..10914cfee0bf 100644 --- a/controllers/topology/reconcile_state.go +++ b/controllers/topology/reconcile_state.go @@ -24,7 +24,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/storage/names" "sigs.k8s.io/cluster-api/controllers/topology/internal/check" "sigs.k8s.io/cluster-api/controllers/topology/internal/contract" @@ -70,9 +69,6 @@ func (r *ClusterReconciler) reconcileInfrastructureCluster(ctx context.Context, // reconcileControlPlane works to bring the current state of a managed topology in line with the desired state. This involves // updating the cluster where needed. func (r *ClusterReconciler) reconcileControlPlane(ctx context.Context, s *scope.Scope) error { - // Set a default nil return function for the cleanup operation. - cleanup := func() error { return nil } - // If the clusterClass mandates the controlPlane has infrastructureMachines, reconcile it. if s.Blueprint.HasControlPlaneInfrastructureMachine() { ctx, _ := tlog.LoggerFrom(ctx).WithObject(s.Desired.ControlPlane.InfrastructureMachineTemplate).Into(ctx) @@ -83,7 +79,7 @@ func (r *ClusterReconciler) reconcileControlPlane(ctx context.Context, s *scope. } // Create or update the MachineInfrastructureTemplate of the control plane. - cleanup, err = r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + err = r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ ref: cpInfraRef, current: s.Current.ControlPlane.InfrastructureMachineTemplate, desired: s.Desired.ControlPlane.InfrastructureMachineTemplate, @@ -98,25 +94,17 @@ func (r *ClusterReconciler) reconcileControlPlane(ctx context.Context, s *scope. // The controlPlaneObject.Spec.machineTemplate.infrastructureRef has to be updated in the desired object err = contract.ControlPlane().MachineTemplate().InfrastructureRef().Set(s.Desired.ControlPlane.Object, refToUnstructured(cpInfraRef)) if err != nil { - return kerrors.NewAggregate([]error{ - errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object}), - cleanup(), - }) + return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object}) } } // Create or update the ControlPlaneObject for the ControlPlaneState. ctx, _ = tlog.LoggerFrom(ctx).WithObject(s.Desired.ControlPlane.Object).Into(ctx) if err := r.reconcileReferencedObject(ctx, s.Current.ControlPlane.Object, s.Desired.ControlPlane.Object); err != nil { - return kerrors.NewAggregate([]error{ - errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object}), - cleanup(), - }) + return errors.Wrapf(err, "failed to update %s", tlog.KObj{Obj: s.Desired.ControlPlane.Object}) } - // At this point we've updated the ControlPlane object and, where required, the ControlPlane InfrastructureMachineTemplate - // without error. Run the cleanup in order to delete the old InfrastructureMachineTemplate if template rotation was done during update. - return cleanup() + return nil } // reconcileCluster reconciles the desired state of the Cluster object. @@ -180,14 +168,14 @@ func (r *ClusterReconciler) createMachineDeployment(ctx context.Context, md *sco log := tlog.LoggerFrom(ctx).WithMachineDeployment(md.Object) ctx, _ = log.WithObject(md.InfrastructureMachineTemplate).Into(ctx) - if _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + if err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ desired: md.InfrastructureMachineTemplate, }); err != nil { return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: md.Object}) } ctx, _ = log.WithObject(md.BootstrapTemplate).Into(ctx) - if _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + if err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ desired: md.BootstrapTemplate, }); err != nil { return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: md.Object}) @@ -206,7 +194,7 @@ func (r *ClusterReconciler) updateMachineDeployment(ctx context.Context, cluster log := tlog.LoggerFrom(ctx).WithMachineDeployment(desiredMD.Object) ctx, _ = log.WithObject(desiredMD.InfrastructureMachineTemplate).Into(ctx) - if _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + if err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ ref: &desiredMD.Object.Spec.Template.Spec.InfrastructureRef, current: currentMD.InfrastructureMachineTemplate, desired: desiredMD.InfrastructureMachineTemplate, @@ -217,7 +205,7 @@ func (r *ClusterReconciler) updateMachineDeployment(ctx context.Context, cluster } ctx, _ = log.WithObject(desiredMD.BootstrapTemplate).Into(ctx) - if _, err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ + if err := r.reconcileReferencedTemplate(ctx, reconcileReferencedTemplateInput{ ref: desiredMD.Object.Spec.Template.Spec.Bootstrap.ConfigRef, current: currentMD.BootstrapTemplate, desired: desiredMD.BootstrapTemplate, @@ -337,39 +325,37 @@ type reconcileReferencedTemplateInput struct { // This function specifically takes care of the first step and updates the reference locally. So the remaining steps // can be executed afterwards. // NOTE: This func has a side effect in case of template rotation, changing both the desired object and the object reference. -func (r *ClusterReconciler) reconcileReferencedTemplate(ctx context.Context, in reconcileReferencedTemplateInput) (func() error, error) { +func (r *ClusterReconciler) reconcileReferencedTemplate(ctx context.Context, in reconcileReferencedTemplateInput) error { log := tlog.LoggerFrom(ctx) - cleanupFunc := func() error { return nil } - // If there is no current object, create the desired object. if in.current == nil { log.Infof("Creating %s", tlog.KObj{Obj: in.desired}) if err := r.Client.Create(ctx, in.desired.DeepCopy()); err != nil { - return nil, errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: in.desired}) + return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: in.desired}) } - return cleanupFunc, nil + return nil } if in.ref == nil { - return nil, errors.Errorf("failed to rotate %s: ref should not be nil", in.desired.GroupVersionKind()) + return errors.Errorf("failed to rotate %s: ref should not be nil", in.desired.GroupVersionKind()) } // Check if the current and desired referenced object are compatible. if err := in.compatibilityChecker(in.current, in.desired); err != nil { - return nil, err + return err } // Check differences between current and desired objects, and if there are changes eventually start the template rotation. patchHelper, err := mergepatch.NewHelper(in.current, in.desired, r.Client) if err != nil { - return nil, errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: in.current}) + return errors.Wrapf(err, "failed to create patch helper for %s", tlog.KObj{Obj: in.current}) } // Return if no changes are detected. if !patchHelper.HasChanges() { log.V(3).Infof("No changes for %s", tlog.KObj{Obj: in.desired}) - return cleanupFunc, nil + return nil } // If there are no changes in the spec, and thus only changes in metadata, instead of doing a full template @@ -377,9 +363,9 @@ func (r *ClusterReconciler) reconcileReferencedTemplate(ctx context.Context, in if !patchHelper.HasSpecChanges() { log.Infof("Patching %s", tlog.KObj{Obj: in.desired}) if err := patchHelper.Patch(ctx); err != nil { - return nil, errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: in.desired}) + return errors.Wrapf(err, "failed to patch %s", tlog.KObj{Obj: in.desired}) } - return cleanupFunc, nil + return nil } // Create the new template. @@ -392,7 +378,7 @@ func (r *ClusterReconciler) reconcileReferencedTemplate(ctx context.Context, in log.Infof("Rotating %s, new name %s", tlog.KObj{Obj: in.current}, newName) log.Infof("Creating %s", tlog.KObj{Obj: in.desired}) if err := r.Client.Create(ctx, in.desired.DeepCopy()); err != nil { - return nil, errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: in.desired}) + return errors.Wrapf(err, "failed to create %s", tlog.KObj{Obj: in.desired}) } // Update the reference with the new name. @@ -400,13 +386,5 @@ func (r *ClusterReconciler) reconcileReferencedTemplate(ctx context.Context, in // TODO: find a way to make side effect more explicit in.ref.Name = newName - // Set up a cleanup func for removing the old template. - // NOTE: This function must be called after updating the object containing the reference to the Template. - return func() error { - log.Infof("Deleting %s", tlog.KObj{Obj: in.current}) - if err := r.Client.Delete(ctx, in.current); err != nil { - return errors.Wrapf(err, "failed to delete %s", tlog.KObj{Obj: in.desired}) - } - return nil - }, nil + return nil }