From 2ad4307963c9c02d0eb530554e6a7aeacac31296 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Fri, 24 Jan 2025 12:00:14 +0100 Subject: [PATCH] Improve MachineSet create and delete logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- .../machinedeployment/machinedeployment_controller.go | 5 ++++- .../controllers/machinedeployment/machinedeployment_sync.go | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index 589d284666c3..79780aa4c1d1 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller.go @@ -337,10 +337,13 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error { // else delete owned machinesets. for _, ms := range s.machineSets { if ms.DeletionTimestamp.IsZero() { - log.Info("Deleting MachineSet", "MachineSet", klog.KObj(ms)) + if err := r.Client.Delete(ctx, ms); err != nil && !apierrors.IsNotFound(err) { return errors.Wrapf(err, "failed to delete MachineSet %s", klog.KObj(ms)) } + // Note: We intentionally log after Delete because we want this log line to show up only after DeletionTimestamp has been set. + // Also, setting DeletionTimestamp doesn't mean the MachineSet is actually deleted (deletion takes some time). + log.Info("Deleting MachineSet (MachineDeployment deleted)", "MachineSet", klog.KObj(ms)) } } diff --git a/internal/controllers/machinedeployment/machinedeployment_sync.go b/internal/controllers/machinedeployment/machinedeployment_sync.go index 5adf52516c46..729656b61b32 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync.go @@ -175,14 +175,12 @@ func (r *Reconciler) createMachineSetAndWait(ctx context.Context, deployment *cl log = log.WithValues("MachineSet", klog.KObj(newMS)) ctx = ctrl.LoggerInto(ctx, log) - log.Info(fmt.Sprintf("Creating new MachineSet, %s", createReason)) - // Create the MachineSet. if err := ssa.Patch(ctx, r.Client, machineDeploymentManagerName, newMS); err != nil { r.recorder.Eventf(deployment, corev1.EventTypeWarning, "FailedCreate", "Failed to create MachineSet %s: %v", klog.KObj(newMS), err) return nil, errors.Wrapf(err, "failed to create new MachineSet %s", klog.KObj(newMS)) } - log.V(4).Info("Created new MachineSet") + log.Info(fmt.Sprintf("MachineSet created (%s)", createReason)) r.recorder.Eventf(deployment, corev1.EventTypeNormal, "SuccessfulCreate", "Created MachineSet %s", klog.KObj(newMS)) // Keep trying to get the MachineSet. This will force the cache to update and prevent any future reconciliation of @@ -647,6 +645,8 @@ func (r *Reconciler) cleanupDeployment(ctx context.Context, oldMSs []*clusterv1. r.recorder.Eventf(deployment, corev1.EventTypeWarning, "FailedDelete", "Failed to delete MachineSet %q: %v", ms.Name, err) return err } + // Note: We intentionally log after Delete because we want this log line to show up only after DeletionTimestamp has been set. + log.Info("Deleting MachineSet (cleanup of old MachineSet)", "MachineSet", klog.KObj(ms)) r.recorder.Eventf(deployment, corev1.EventTypeNormal, "SuccessfulDelete", "Deleted MachineSet %q", ms.Name) }