[kwokctl] Support Force Delete Cluster #1128

joeyyy09 · 2024-05-31T10:21:31Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces a new --force flag for the kwokctl delete cluster command. This flag allows users to force delete a cluster, even if the runtime is unavailable or the cluster does not exist.

Which issue(s) this PR fixes:

Fixes #1124

Special notes for your reviewer:

The main changes include:
Addition of the Force field in the flagpole struct.
Modification of the deleteCluster function to handle the force flag.
Addition of the --force command-line flag in the NewCommand function.

Does this PR introduce a user-facing change?

- Added `--force` flag to `kwokctl delete cluster` command to forcefully delete a cluster.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2024-05-31T10:21:40Z

Hi @joeyyy09. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

netlify · 2024-05-31T10:21:47Z

✅ Deploy Preview for k8s-kwok canceled.

Name	Link
🔨 Latest commit	`af10de4`
🔍 Latest deploy log	https://app.netlify.com/sites/k8s-kwok/deploys/667510feac0a4c00080d0b06

wzshiming · 2024-05-31T10:59:35Z

/ok-to-test

pkg/kwokctl/cmd/delete/cluster/cluster.go

joeyyy09 · 2024-05-31T18:02:11Z

Hey @wzshiming, I've made the required changes, can you please let me know if i've got to change anything else? Thanks!

wzshiming

diff --git a/pkg/kwokctl/cmd/delete/cluster/cluster.go b/pkg/kwokctl/cmd/delete/cluster/cluster.go
index 7530a260..571dbefd 100644
--- a/pkg/kwokctl/cmd/delete/cluster/cluster.go
+++ b/pkg/kwokctl/cmd/delete/cluster/cluster.go
@@ -35,9 +35,10 @@ import (
 type flagpole struct {
 	Name       string
 	Kubeconfig string
+	Force      bool
 }
 
-// NewCommand returns a new cobra.Command for cluster creation
+// NewCommand returns a new cobra.Command for cluster deletion
 func NewCommand(ctx context.Context) *cobra.Command {
 	flags := &flagpole{}
 	flags.Kubeconfig = path.RelFromHome(kubeconfig.GetRecommendedKubeconfigPath())
@@ -52,6 +53,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 		},
 	}
 	cmd.Flags().StringVar(&flags.Kubeconfig, "kubeconfig", flags.Kubeconfig, "The path to the kubeconfig file that will remove the deleted cluster")
+	cmd.Flags().BoolVar(&flags.Force, "force", false, "Force delete the cluster")
 	return cmd
 }
 
@@ -77,6 +79,13 @@ func runE(ctx context.Context, flags *flagpole) error {
 		return err
 	}
 
+	if err := rt.Available(ctx); err != nil {
+		if !flags.Force {
+			return err
+		}
+		logger.Warn("Unavailable runtime but proceed with force delete", "err", err)
+	}
+
 	// Stop the cluster
 	start := time.Now()
 	logger.Info("Cluster is stopping")

wzshiming · 2024-06-05T07:26:17Z

This needs rebase.

joeyyy09 · 2024-06-05T11:43:44Z

This needs rebase.

I think the changes made in this PR have been made in #1125 and has been merged.

wzshiming · 2024-06-06T01:36:47Z

So now you need to rebase this branch to the main.
refer https://www.google.com/search?q=git+rebase+and+fix+conflicts

joeyyy09 · 2024-06-06T06:24:22Z

So now you need to rebase this branch to the main. refer https://www.google.com/search?q=git+rebase+and+fix+conflicts

Made the changes

wzshiming · 2024-06-06T12:58:27Z

diff --git a/pkg/kwokctl/cmd/delete/cluster/cluster.go b/pkg/kwokctl/cmd/delete/cluster/cluster.go
index 7530a260..571dbefd 100644
--- a/pkg/kwokctl/cmd/delete/cluster/cluster.go
+++ b/pkg/kwokctl/cmd/delete/cluster/cluster.go
@@ -35,9 +35,10 @@ import (
 type flagpole struct {
 	Name       string
 	Kubeconfig string
+	Force      bool
 }
 
-// NewCommand returns a new cobra.Command for cluster creation
+// NewCommand returns a new cobra.Command for cluster deletion
 func NewCommand(ctx context.Context) *cobra.Command {
 	flags := &flagpole{}
 	flags.Kubeconfig = path.RelFromHome(kubeconfig.GetRecommendedKubeconfigPath())
@@ -52,6 +53,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 		},
 	}
 	cmd.Flags().StringVar(&flags.Kubeconfig, "kubeconfig", flags.Kubeconfig, "The path to the kubeconfig file that will remove the deleted cluster")
+	cmd.Flags().BoolVar(&flags.Force, "force", false, "Force delete the cluster")
 	return cmd
 }
 
@@ -77,6 +79,13 @@ func runE(ctx context.Context, flags *flagpole) error {
 		return err
 	}
 
+	if err := rt.Available(ctx); err != nil {
+		if !flags.Force {
+			return err
+		}
+		logger.Warn("Unavailable runtime but proceed with force delete", "err", err)
+	}
+
 	// Stop the cluster
 	start := time.Now()
 	logger.Info("Cluster is stopping")

NOTICE THIS PATCH

joeyyy09 · 2024-06-06T13:06:30Z

diff --git a/pkg/kwokctl/cmd/delete/cluster/cluster.go b/pkg/kwokctl/cmd/delete/cluster/cluster.go
index 7530a260..571dbefd 100644
--- a/pkg/kwokctl/cmd/delete/cluster/cluster.go
+++ b/pkg/kwokctl/cmd/delete/cluster/cluster.go
@@ -35,9 +35,10 @@ import (
 type flagpole struct {
 	Name       string
 	Kubeconfig string
+	Force      bool
 }
 
-// NewCommand returns a new cobra.Command for cluster creation
+// NewCommand returns a new cobra.Command for cluster deletion
 func NewCommand(ctx context.Context) *cobra.Command {
 	flags := &flagpole{}
 	flags.Kubeconfig = path.RelFromHome(kubeconfig.GetRecommendedKubeconfigPath())
@@ -52,6 +53,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 		},
 	}
 	cmd.Flags().StringVar(&flags.Kubeconfig, "kubeconfig", flags.Kubeconfig, "The path to the kubeconfig file that will remove the deleted cluster")
+	cmd.Flags().BoolVar(&flags.Force, "force", false, "Force delete the cluster")
 	return cmd
 }
 
@@ -77,6 +79,13 @@ func runE(ctx context.Context, flags *flagpole) error {
 		return err
 	}
 
+	if err := rt.Available(ctx); err != nil {
+		if !flags.Force {
+			return err
+		}
+		logger.Warn("Unavailable runtime but proceed with force delete", "err", err)
+	}
+
 	// Stop the cluster
 	start := time.Now()
 	logger.Info("Cluster is stopping")

NOTICE THIS PATCH

Yeah, should i modify anything here?

wzshiming · 2024-06-06T13:10:54Z

Your changes are wrong.

joeyyy09 · 2024-06-06T13:12:58Z

Your changes are wrong.

Okay, I'll rethink my approach and will fix this.

wzshiming · 2024-06-06T13:13:02Z

It cannot achieve the effect of not forcing deletion

joeyyy09 · 2024-06-06T14:07:04Z

Normal deletion works normal when the --force flag is not set and the runtime is available. If the runtime is not available and the --force flag is not set, the cluster will not be deleted. If the --force flag is set to true, the cluster will be deleted despite the unavailability of the runtime. Is this correct? I was trying to implement this and i've checked with and without the flags on my local build. Can you please explain me in detail if this understanding of mine is wrong?

wzshiming · 2024-06-09T09:57:50Z

diff --git a/pkg/kwokctl/cmd/delete/cluster/cluster.go b/pkg/kwokctl/cmd/delete/cluster/cluster.go
index 7530a260..571dbefd 100644
--- a/pkg/kwokctl/cmd/delete/cluster/cluster.go
+++ b/pkg/kwokctl/cmd/delete/cluster/cluster.go
@@ -35,9 +35,10 @@ import (
 type flagpole struct {
 	Name       string
 	Kubeconfig string
+	Force      bool
 }
 
-// NewCommand returns a new cobra.Command for cluster creation
+// NewCommand returns a new cobra.Command for cluster deletion
 func NewCommand(ctx context.Context) *cobra.Command {
 	flags := &flagpole{}
 	flags.Kubeconfig = path.RelFromHome(kubeconfig.GetRecommendedKubeconfigPath())
@@ -52,6 +53,7 @@ func NewCommand(ctx context.Context) *cobra.Command {
 		},
 	}
 	cmd.Flags().StringVar(&flags.Kubeconfig, "kubeconfig", flags.Kubeconfig, "The path to the kubeconfig file that will remove the deleted cluster")
+	cmd.Flags().BoolVar(&flags.Force, "force", false, "Force delete the cluster")
 	return cmd
 }
 
@@ -77,6 +79,13 @@ func runE(ctx context.Context, flags *flagpole) error {
 		return err
 	}
 
+	if err := rt.Available(ctx); err != nil {
+		if !flags.Force {
+			return err
+		}
+		logger.Warn("Unavailable runtime but proceed with force delete", "err", err)
+	}
+
 	// Stop the cluster
 	start := time.Now()
 	logger.Info("Cluster is stopping")

NOTICE THIS PATCH

Can't you see this patch?
I've already sent you the changes that need to be made.

wzshiming

There are too many irrelevant changes, please squash all commits

Signed-off-by: Joeyyy09 <[email protected]> Add check for runtime Signed-off-by: Joeyyy09 <[email protected]> Update docs Signed-off-by: Joeyyy09 <[email protected]> Rebase to main Signed-off-by: Joeyyy09 <[email protected]> Add force flag Signed-off-by: Joeyyy09 <[email protected]> Modify according to the patch Signed-off-by: Joeyyy09 <[email protected]>

joeyyy09 · 2024-06-21T05:35:24Z

There are too many irrelevant changes, please squash all commits

Yep, squashed all the commits

wzshiming · 2024-06-24T09:30:07Z

/approve
/lgtm

k8s-ci-robot · 2024-06-24T09:30:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: joeyyy09, wzshiming

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [wzshiming]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from Huang-Wei and yuanchen8911 May 31, 2024 10:21

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 31, 2024

joeyyy09 changed the title ~~Support Force Delete Cluster~~ [kwokctl] Support Force Delete Cluster May 31, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 31, 2024

wzshiming reviewed May 31, 2024

View reviewed changes

pkg/kwokctl/cmd/delete/cluster/cluster.go Show resolved Hide resolved

pkg/kwokctl/cmd/delete/cluster/cluster.go Outdated Show resolved Hide resolved

pkg/kwokctl/cmd/delete/cluster/cluster.go Outdated Show resolved Hide resolved

joeyyy09 force-pushed the force-flag branch from dd4ba85 to 85397c7 Compare May 31, 2024 17:56

wzshiming reviewed Jun 3, 2024

View reviewed changes

wzshiming mentioned this pull request Jun 3, 2024

[kwokctl] Added --all flag to kwokctl delete cluster command #1125

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2024

joeyyy09 force-pushed the force-flag branch from cc8ba6c to d45d2a2 Compare June 6, 2024 06:22

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2024

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 9, 2024

wzshiming reviewed Jun 17, 2024

View reviewed changes

joeyyy09 force-pushed the force-flag branch from d58d0ee to af10de4 Compare June 21, 2024 05:34

k8s-ci-robot assigned wzshiming Jun 24, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2024

k8s-ci-robot merged commit 98f0a81 into kubernetes-sigs:main Jun 24, 2024
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[kwokctl] Support Force Delete Cluster #1128

[kwokctl] Support Force Delete Cluster #1128

joeyyy09 commented May 31, 2024 •

edited

Loading

k8s-ci-robot commented May 31, 2024

netlify bot commented May 31, 2024 •

edited

Loading

wzshiming commented May 31, 2024

joeyyy09 commented May 31, 2024

wzshiming left a comment •

edited

Loading

wzshiming commented Jun 5, 2024 •

edited

Loading

joeyyy09 commented Jun 5, 2024

wzshiming commented Jun 6, 2024 •

edited

Loading

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 6, 2024

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 6, 2024

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 6, 2024

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 9, 2024 •

edited

Loading

wzshiming left a comment

joeyyy09 commented Jun 21, 2024

wzshiming commented Jun 24, 2024

k8s-ci-robot commented Jun 24, 2024

[kwokctl] Support Force Delete Cluster #1128

[kwokctl] Support Force Delete Cluster #1128

Conversation

joeyyy09 commented May 31, 2024 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented May 31, 2024

netlify bot commented May 31, 2024 • edited Loading

✅ Deploy Preview for k8s-kwok canceled.

wzshiming commented May 31, 2024

joeyyy09 commented May 31, 2024

wzshiming left a comment • edited Loading

Choose a reason for hiding this comment

wzshiming commented Jun 5, 2024 • edited Loading

joeyyy09 commented Jun 5, 2024

wzshiming commented Jun 6, 2024 • edited Loading

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 6, 2024

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 6, 2024

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 6, 2024

joeyyy09 commented Jun 6, 2024

wzshiming commented Jun 9, 2024 • edited Loading

wzshiming left a comment

Choose a reason for hiding this comment

joeyyy09 commented Jun 21, 2024

wzshiming commented Jun 24, 2024

k8s-ci-robot commented Jun 24, 2024

joeyyy09 commented May 31, 2024 •

edited

Loading

netlify bot commented May 31, 2024 •

edited

Loading

wzshiming left a comment •

edited

Loading

wzshiming commented Jun 5, 2024 •

edited

Loading

wzshiming commented Jun 6, 2024 •

edited

Loading

wzshiming commented Jun 9, 2024 •

edited

Loading