feat(reset): addressed review comments

Signed-off-by: rajaSahil <[email protected]>

pkg/clientutil/service.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2020 The KubeOne Authors.
+Copyright 2025 The KubeOne Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -20,9 +20,10 @@ import (
 	"context"
 	"time"
 
+	"github.com/sirupsen/logrus"
+
 	"k8c.io/kubeone/pkg/fail"
 
-	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -31,17 +32,18 @@ import (
 func CleanupLBs(ctx context.Context, logger logrus.FieldLogger, c client.Client) error {
 	serviceList := &corev1.ServiceList{}
 	if err := c.List(ctx, serviceList); err != nil {
-		return fail.KubeClient(err, "failed to list Service.")
+		return fail.KubeClient(err, "listing services")
 	}
 
 	for _, service := range serviceList.Items {
 		// This service is already in deletion, nothing further needs to happen.
 		if service.DeletionTimestamp != nil {
 			continue
 		}
+		logger.Infof("Cleaning up LoadBalancer Services...")
 		// Only LoadBalancer services incur charges on cloud providers
 		if service.Spec.Type == corev1.ServiceTypeLoadBalancer {
-			logger.Infof("Deleting SVC : %s/%s\n", service.Namespace, service.Name)
+			logger.Debugf("Deleting LoadBalancer Service \"%s/%s\"", service.Namespace, service.Name)
 			if err := DeleteIfExists(ctx, c, &service); err != nil {
 				return err
 			}
@@ -52,11 +54,13 @@ func CleanupLBs(ctx context.Context, logger logrus.FieldLogger, c client.Client)
 }
 
 func WaitCleanupLbs(ctx context.Context, logger logrus.FieldLogger, c client.Client) error {
-	logger.Infoln("Waiting for all load balancer services to get deleted...")
+	logger.Infoln("Waiting for all LoadBalancer Services to get deleted...")
 
 	return wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, false, func(ctx context.Context) (bool, error) {
 		serviceList := &corev1.ServiceList{}
 		if err := c.List(ctx, serviceList); err != nil {
+			logger.Errorf("failed to list services, error: %v", err.Error())
+
 			return false, nil
 		}
 		for _, service := range serviceList.Items {

pkg/clientutil/volumes.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2020 The KubeOne Authors.
+Copyright 2025 The KubeOne Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -21,10 +21,11 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/sirupsen/logrus"
+
 	"k8c.io/kubeone/pkg/fail"
 	"k8c.io/reconciler/pkg/reconciling"
 
-	"github.com/sirupsen/logrus"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"sigs.k8s.io/controller-runtime/pkg/client"

pkg/clientutil/webhook.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2020 The KubeOne Authors.
+Copyright 2025 The KubeOne Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

pkg/cmd/reset.go

@@ -19,26 +19,23 @@ package cmd
 import (
 	"fmt"
 
-	"k8c.io/kubeone/pkg/clientutil"
-	"k8c.io/kubeone/pkg/kubeconfig"
-	"k8c.io/kubeone/pkg/state"
-	"k8c.io/kubeone/pkg/tasks"
-	clusterv1alpha1 "k8c.io/machine-controller/pkg/apis/cluster/v1alpha1"
-
 	"github.com/MakeNowJust/heredoc/v2"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 
-	corev1 "k8s.io/api/core/v1"
+	"k8c.io/kubeone/pkg/kubeconfig"
+	"k8c.io/kubeone/pkg/state"
+	"k8c.io/kubeone/pkg/tasks"
+	clusterv1alpha1 "k8c.io/machine-controller/pkg/apis/cluster/v1alpha1"
 )
 
 type resetOpts struct {
 	globalOptions
-	AutoApprove          bool `longflag:"auto-approve" shortflag:"y"`
-	DestroyWorkers       bool `longflag:"destroy-workers"`
-	CleanupVolumes       bool `longflag:"cleanup-volumes"`
-	CleanupLoadBalancers bool `longflag:"cleanup-load-balancers"`
-	RemoveBinaries       bool `longflag:"remove-binaries"`
+	AutoApprove      bool `longflag:"auto-approve" shortflag:"y"`
+	DestroyWorkers   bool `longflag:"destroy-workers"`
+	RemoveVolumes    bool `longflag:"remove-volumes"`
+	RemoveLBServices bool `longflag:"remove-lb-services"`
+	RemoveBinaries   bool `longflag:"remove-binaries"`
 }
 
 func (opts *resetOpts) BuildState() (*state.State, error) {
@@ -48,8 +45,8 @@ func (opts *resetOpts) BuildState() (*state.State, error) {
 	}
 
 	s.DestroyWorkers = opts.DestroyWorkers
-	s.CleanupVolumes = opts.CleanupVolumes
-	s.CleanupLoadBalancers = opts.CleanupLoadBalancers
+	s.RemoveVolumes = opts.RemoveVolumes
+	s.RemoveLBServices = opts.RemoveLBServices
 	s.RemoveBinaries = opts.RemoveBinaries
 
 	return s, nil
@@ -101,16 +98,16 @@ func resetCmd(rootFlags *pflag.FlagSet) *cobra.Command {
 		"remove kubernetes binaries after resetting the cluster")
 
 	cmd.Flags().BoolVar(
-		&opts.CleanupVolumes,
-		longFlagName(opts, "CleanupVolumes"),
+		&opts.RemoveVolumes,
+		longFlagName(opts, "RemoveVolumes"),
 		true,
-		"cleanup all dynamically provisioned and unretained volumes before resetting the cluster")
+		"remove all dynamically provisioned and unretained volumes before resetting the cluster")
 
 	cmd.Flags().BoolVar(
-		&opts.CleanupLoadBalancers,
-		longFlagName(opts, "CleanupLoadBalancers"),
+		&opts.RemoveLBServices,
+		longFlagName(opts, "RemoveLBServices"),
 		true,
-		"cleanup all load balancers services before resetting the cluster")
+		"remove all load balancers services before resetting the cluster")
 
 	return cmd
 }
@@ -122,15 +119,15 @@ func runReset(opts *resetOpts) error {
 		return err
 	}
 
-	if opts.DestroyWorkers || opts.CleanupVolumes || opts.CleanupLoadBalancers {
+	if opts.DestroyWorkers || opts.RemoveVolumes || opts.RemoveLBServices {
 		if cErr := kubeconfig.BuildKubernetesClientset(s); cErr != nil {
 			s.Logger.Errorln("Failed to build the Kubernetes clientset.")
-			if opts.CleanupLoadBalancers {
+			if opts.RemoveLBServices {
 				s.Logger.Warnln("Unable to list and delete load balancers in the cluster.")
 				s.Logger.Warnln("You can skip this phase by using '--cleanup-load-balancer=false'.")
 				s.Logger.Warnln("If there are load balancers in the cluster, you might have to delete them manually.")
 			}
-			if opts.CleanupVolumes {
+			if opts.RemoveVolumes {
 				s.Logger.Warnln("Unable to list and delete dynamically provisioned and unretained volumes in the cluster.")
 				s.Logger.Warnln("You can skip this phase by using '--cleanup-volumes=false'.")
 				s.Logger.Warnln("If there are unretained volumes in the cluster, you might have to delete them manually.")
@@ -140,65 +137,22 @@ func runReset(opts *resetOpts) error {
 				s.Logger.Warnln("You can skip this phase by using '--destroy-workers=false' flag.")
 				s.Logger.Warnln("If there are worker nodes in the cluster, you might have to delete them manually.")
 			}
-			s.Logger.Warnln("If there are unretained volumes and load balancers in the cluster, you might have to delete them manually.")
 
 			return cErr
 		}
 	}
 
-	if opts.CleanupLoadBalancers {
-		s.Logger.Warnln("cleanup-load-balancers command will PERMANENTLY delete the load balancers from the cluster.")
-		svcList := corev1.ServiceList{}
-		if err = s.DynamicClient.List(s.Context, &svcList); err != nil {
-			s.Logger.Errorln("Failed to list load balancer services.")
-			s.Logger.Warnln("Load balancer Service might not be deleted. If there are services of type load balancer in the cluster, you might have to delete them manually.")
-		}
-		services := []corev1.Service{}
-		for _, svc := range svcList.Items {
-			if svc.Spec.Type == corev1.ServiceTypeLoadBalancer {
-				services = append(services, svc)
-			}
-		}
-		if len(services) > 0 {
-			fmt.Printf("\nThe following load balancer services will be destroyed:\n")
-			for _, svc := range services {
-				fmt.Printf("\t- %s/%s\n", svc.Namespace, svc.Name)
-			}
-		}
+	if opts.RemoveLBServices {
+		s.Logger.Warnln("remove-lb-services command will PERMANENTLY delete the load balancers from the cluster.")
 	}
 
-	if opts.CleanupVolumes {
-		s.Logger.Warnln("cleanup-volumes command will PERMANENTLY delete the unretained volumes from the cluster.")
-
-		pvList := corev1.PersistentVolumeList{}
-		if err = s.DynamicClient.List(s.Context, &pvList); err != nil {
-			s.Logger.Errorln("Failed to list persistent volumes.")
-			s.Logger.Warnln("Volumes might not be deleted. If there are volumes in the cluster, you might have to delete them manually.")
-		}
-		pvs := []corev1.PersistentVolume{}
-		for _, pv := range pvList.Items {
-			if pv.Annotations[clientutil.AnnDynamicallyProvisioned] != "" && pv.Spec.PersistentVolumeReclaimPolicy == corev1.PersistentVolumeReclaimDelete {
-				pvs = append(pvs, pv)
-			}
-		}
-		if len(pvs) > 0 {
-			fmt.Printf("\nThe following volumes will be destroyed:\n")
-			for _, pv := range pvs {
-				fmt.Printf("\t- %s/%s/%s\n", pv.Spec.ClaimRef.Namespace, pv.Spec.ClaimRef.Name, pv.Name)
-			}
-		}
+	if opts.RemoveVolumes {
+		s.Logger.Warnln("remove-volumes command will PERMANENTLY delete the unretained volumes from the cluster.")
 	}
 
 	if opts.DestroyWorkers {
 		s.Logger.Warnln("destroy-workers command will PERMANENTLY destroy the Kubernetes cluster running on the following nodes:")
 
-		for _, node := range s.Cluster.ControlPlane.Hosts {
-			fmt.Printf("\t- reset control plane node %q (%s)\n", node.Hostname, node.PrivateAddress)
-		}
-		for _, node := range s.Cluster.StaticWorkers.Hosts {
-			fmt.Printf("\t- reset static worker nodes %q (%s)\n", node.Hostname, node.PrivateAddress)
-		}
-
 		// Gather information about machine-controller managed nodes
 		machines := clusterv1alpha1.MachineList{}
 		if err = s.DynamicClient.List(s.Context, &machines); err != nil {
@@ -217,6 +171,13 @@ func runReset(opts *resetOpts) error {
 		s.Logger.Warnln("If there are worker nodes in the cluster, you might have to delete them manually.")
 	}
 
+	for _, node := range s.Cluster.ControlPlane.Hosts {
+		fmt.Printf("\t- reset control plane node %q (%s)\n", node.Hostname, node.PrivateAddress)
+	}
+	for _, node := range s.Cluster.StaticWorkers.Hosts {
+		fmt.Printf("\t- reset static worker nodes %q (%s)\n", node.Hostname, node.PrivateAddress)
+	}
+
 	fmt.Printf("\nAfter the command is complete, there's NO way to recover the cluster or its data!\n")
 
 	confirm, err := confirmCommand(opts.AutoApprove)

pkg/state/context.go

@@ -107,8 +107,8 @@ type State struct {
 	Verbose                   bool
 	BackupFile                string
 	DestroyWorkers            bool
-	CleanupVolumes            bool
-	CleanupLoadBalancers      bool
+	RemoveVolumes             bool
+	RemoveLBServices          bool
 	RemoveBinaries            bool
 	ForceUpgrade              bool
 	ForceInstall              bool

pkg/tasks/reset.go

@@ -31,85 +31,53 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 )
 
-// CleanupLoadBalancers Deletes all the load balancers services from the cluster.
-func CleanupLoadBalancers(s *state.State) error {
-	if !s.CleanupLoadBalancers {
+// RemoveLBServices Deletes all the load balancers services from the cluster.
+func RemoveLBServices(s *state.State) error {
+	if !s.RemoveLBServices {
 		return nil
 	}
 	var lastErr error
-	s.Logger.Infoln("Deleting load balancer services...")
-	_ = wait.ExponentialBackoff(defaultRetryBackoff(3), func() (bool, error) {
-		lastErr = clientutil.CleanupLBs(s.Context, s.Logger, s.DynamicClient)
-		if lastErr != nil {
-			s.Logger.Warn("Unable to delete services of type load balancer. Retrying...")
-
-			return false, nil
-		}
-
-		return true, nil
-	})
-
-	_ = wait.ExponentialBackoff(defaultRetryBackoff(3), func() (bool, error) {
-		lastErr = clientutil.WaitCleanupLbs(s.Context, s.Logger, s.DynamicClient)
-		if lastErr != nil {
-			s.Logger.Warn("Waiting for all load balancer services to be deleted...")
+	s.Logger.Infoln("Deleting LoadBalancer Services...")
+	lastErr = clientutil.CleanupLBs(s.Context, s.Logger, s.DynamicClient)
+	if lastErr != nil {
+		s.Logger.Warn("Unable to delete services of type load balancer.")
 
-			return false, nil
-		}
+		return lastErr
+	}
 
-		return true, nil
-	})
+	lastErr = clientutil.WaitCleanupLbs(s.Context, s.Logger, s.DynamicClient)
 	if lastErr != nil {
+		s.Logger.Warn("Waiting for all load balancer services to be deleted...")
+
 		return lastErr
 	}
 
 	return lastErr
 }
 
-// CleanupVolumes Deletes all the dynamically provisioned and unretained volumes from the cluster.
-func CleanupVolumes(s *state.State) error {
-	if !s.CleanupVolumes {
+// RemoveVolumes Deletes all the dynamically provisioned and unretained volumes from the cluster.
+func RemoveVolumes(s *state.State) error {
+	if !s.RemoveVolumes {
 		return nil
 	}
 	var lastErr error
 	s.Logger.Infoln("Deleting dynamically provisioned and unretained volumes...")
-	_ = wait.ExponentialBackoff(defaultRetryBackoff(3), func() (bool, error) {
-		lastErr = clientutil.CleanupUnretainedVolumes(s.Context, s.Logger, s.DynamicClient)
-		if lastErr != nil {
-			s.Logger.Warn("Unable to delete volumes. Retrying...")
-
-			return false, nil
-		}
-
-		return true, nil
-	})
+	lastErr = clientutil.CleanupUnretainedVolumes(s.Context, s.Logger, s.DynamicClient)
 	if lastErr != nil {
+		s.Logger.Warn("Unable to delete volumes.")
 		s.Logger.Infoln("Deleting ValidatingWebhookConfiguration to enable future PV & PVC creation...")
-		_ = wait.ExponentialBackoff(defaultRetryBackoff(3), func() (bool, error) {
-			if err := clientutil.DeletePreventingWebhook(s.Context, s.DynamicClient,
-				"kubernetes-cluster-cleanup-"+strings.Join(clientutil.VolumeResources, "-")); err != nil {
-				s.Logger.Warn("Unable to delete ValidatingWebhookConfiguration. Retrying...")
-
-				return false, nil
-			}
-
-			return true, nil
-		})
+		if err := clientutil.DeletePreventingWebhook(s.Context, s.DynamicClient,
+			"kubernetes-cluster-cleanup-"+strings.Join(clientutil.VolumeResources, "-")); err != nil {
+			s.Logger.Warn("Unable to delete ValidatingWebhookConfiguration.")
+		}
 
 		return lastErr
 	}
 
-	_ = wait.ExponentialBackoff(defaultRetryBackoff(3), func() (bool, error) {
-		lastErr = clientutil.WaitCleanUpVolumes(s.Context, s.Logger, s.DynamicClient)
-		if lastErr != nil {
-			s.Logger.Warn("Waiting for all dynamically provisioned and unretained volumes to be deleted...")
-
-			return false, nil
-		}
-
-		return true, nil
-	})
+	lastErr = clientutil.WaitCleanUpVolumes(s.Context, s.Logger, s.DynamicClient)
 	if lastErr != nil {
+		s.Logger.Warn("Waiting for all dynamically provisioned and unretained volumes to be deleted...")
+
 		return lastErr
 	}
 

pkg/tasks/tasks.go

@@ -391,8 +391,19 @@ func WithUpgrade(t Tasks, followers ...kubeoneapi.HostConfig) Tasks {
 
 func WithReset(t Tasks) Tasks {
 	return t.append(Tasks{
-		{Fn: CleanupLoadBalancers, Operation: "cleanup load balancers"},
-		{Fn: CleanupVolumes, Operation: "cleanup dynamically provisioned and unretained volumes"},
+		{
+			Fn:        RemoveLBServices,
+			Operation: "remove load balancer services",
+			Predicate: func(s *state.State) bool {
+				return s.RemoveLBServices
+			}},
+		{
+			Fn:        RemoveVolumes,
+			Operation: "remove dynamically provisioned and unretained volumes",
+			Predicate: func(s *state.State) bool {
+				return s.RemoveVolumes
+			},
+		},
 		{Fn: destroyWorkers, Operation: "destroying workers"},
 		{Fn: resetAllNodes, Operation: "resetting all nodes"},
 		{Fn: removeBinariesAllNodes, Operation: "removing kubernetes binaries from nodes"},