From eb0955f66622a275db69925a5abdc3693632b9d7 Mon Sep 17 00:00:00 2001 From: Enrico Candino Date: Tue, 28 Jul 2026 18:22:25 +0200 Subject: [PATCH] Add `cluster delete` autocompletion (#1080) * Add cluster deletion and completion enhancements - Implemented `--all` flag for deleting all clusters in a namespace. - Updated argument parsing to allow for maximum one cluster name. - Added completion functions for cluster names with namespace filtering. - Enhanced tests for cluster argument resolution and completion functions. * fix docs * Add log message for empty cluster deletion in specified namespace * Refactor context handling in command functions to use `cmd.Context()` --- cli/cmds/cluster_delete.go | 111 +++++++++++++++++++++++------- cli/cmds/cluster_delete_test.go | 61 ++++++++++++++++ cli/cmds/cluster_list.go | 4 +- cli/cmds/completion.go | 44 ++++++++++++ cli/cmds/completion_test.go | 34 +++++++++ cli/cmds/kubeconfig.go | 3 +- cli/cmds/policy_create.go | 2 +- cli/cmds/policy_delete.go | 4 +- cli/cmds/policy_list.go | 4 +- docs/cli/k3kcli.adoc | 1 + docs/cli/k3kcli_cluster_delete.md | 1 + 11 files changed, 232 insertions(+), 37 deletions(-) diff --git a/cli/cmds/cluster_delete.go b/cli/cmds/cluster_delete.go index 273890bc..a4477a96 100644 --- a/cli/cmds/cluster_delete.go +++ b/cli/cmds/cluster_delete.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -19,7 +20,10 @@ import ( k3kcluster "github.com/rancher/k3k/pkg/controller/cluster" ) -var keepData bool +var ( + keepData bool + deleteAll bool +) func NewClusterDeleteCmd(appCtx *AppContext) *cobra.Command { cmd := &cobra.Command{ @@ -27,10 +31,13 @@ func NewClusterDeleteCmd(appCtx *AppContext) *cobra.Command { Short: "Delete an existing cluster.", Example: "k3kcli cluster delete [command options] NAME", RunE: delete(appCtx), - Args: cobra.ExactArgs(1), + Args: cobra.MaximumNArgs(1), } cmd.Flags().BoolVar(&keepData, "keep-data", false, "keeps persistence volumes created for the cluster after deletion") + cmd.Flags().BoolVarP(&deleteAll, "all", "A", false, "delete all the clusters in the namespace") + + cmd.ValidArgsFunction = completeClusterNames CobraFlagNamespace(appCtx, cmd, completeClusterNamespaces) @@ -39,16 +46,50 @@ func NewClusterDeleteCmd(appCtx *AppContext) *cobra.Command { func delete(appCtx *AppContext) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := context.Background() + ctx := cmd.Context() client := appCtx.Client - name := args[0] + + if deleteAll { + if len(args) > 0 { + return errors.New("cannot specify a cluster name together with --all") + } + + if appCtx.namespace == "" { + return errors.New("--all requires a namespace, set it with --namespace/-n") + } + + var clusters v1beta1.ClusterList + if err := client.List(ctx, &clusters, ctrlclient.InNamespace(appCtx.namespace)); err != nil { + return err + } + + if len(clusters.Items) == 0 { + logrus.Infof("No clusters found in namespace '%s'", appCtx.namespace) + return nil + } + + for i := range clusters.Items { + if err := deleteCluster(ctx, client, &clusters.Items[i]); err != nil { + return err + } + } + + return nil + } + + if len(args) != 1 { + return errors.New("expected exactly one cluster name") + } + + namespace, name, err := resolveClusterArg(appCtx, args[0]) + if err != nil { + return err + } if name == k3kcluster.ClusterInvalidName { return errors.New("invalid cluster name") } - namespace := appCtx.Namespace(name) - cluster := v1beta1.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -63,31 +104,51 @@ func delete(appCtx *AppContext) func(cmd *cobra.Command, args []string) error { return err } - logrus.Infof("Deleting '%s' cluster in namespace '%s'", name, namespace) + return deleteCluster(ctx, client, &cluster) + } +} - // keep bootstrap secrets and tokens if --keep-data flag is passed - if keepData { - // skip removing tokenSecret - if err := RemoveOwnerReferenceFromSecret(ctx, k3kcluster.TokenSecretName(cluster.Name), client, cluster); err != nil { - return err - } - } else { - matchingLabels := ctrlclient.MatchingLabels(map[string]string{"cluster": cluster.Name, "role": "server"}) - listOpts := ctrlclient.ListOptions{Namespace: cluster.Namespace} - matchingLabels.ApplyToList(&listOpts) - deleteOpts := &ctrlclient.DeleteAllOfOptions{ListOptions: listOpts} - - if err := client.DeleteAllOf(ctx, &corev1.PersistentVolumeClaim{}, deleteOpts); err != nil { - return ctrlclient.IgnoreNotFound(err) - } +// resolveClusterArg splits an arg that may be in "namespace/name" form. +// A bare "name" falls back to the k3k- convention (respecting the -n flag) via appCtx.Namespace. +// When both the -n flag and an explicit namespace prefix are given and they disagree an error is returned. +func resolveClusterArg(appCtx *AppContext, arg string) (namespace, name string, err error) { + if ns, clusterName, ok := strings.Cut(arg, "/"); ok { + if appCtx.namespace != "" && appCtx.namespace != ns { + return "", "", fmt.Errorf("namespace mismatch: flag --namespace %q conflicts with argument namespace %q", appCtx.namespace, ns) } - if err := client.Delete(ctx, &cluster); err != nil { + return ns, clusterName, nil + } + + return appCtx.Namespace(arg), arg, nil +} + +// deleteCluster removes a cluster and, unless --keep-data is set, its server PersistentVolumeClaims. +func deleteCluster(ctx context.Context, client ctrlclient.Client, cluster *v1beta1.Cluster) error { + logrus.Infof("Deleting '%s' cluster in namespace '%s'", cluster.Name, cluster.Namespace) + + // keep bootstrap secrets and tokens if --keep-data flag is passed + if keepData { + // skip removing tokenSecret + if err := RemoveOwnerReferenceFromSecret(ctx, k3kcluster.TokenSecretName(cluster.Name), client, *cluster); err != nil { + return err + } + } else { + matchingLabels := ctrlclient.MatchingLabels(map[string]string{"cluster": cluster.Name, "role": "server"}) + listOpts := ctrlclient.ListOptions{Namespace: cluster.Namespace} + matchingLabels.ApplyToList(&listOpts) + deleteOpts := &ctrlclient.DeleteAllOfOptions{ListOptions: listOpts} + + if err := client.DeleteAllOf(ctx, &corev1.PersistentVolumeClaim{}, deleteOpts); err != nil { return ctrlclient.IgnoreNotFound(err) } - - return nil } + + if err := client.Delete(ctx, cluster); err != nil { + return ctrlclient.IgnoreNotFound(err) + } + + return nil } func RemoveOwnerReferenceFromSecret(ctx context.Context, name string, cl ctrlclient.Client, cluster v1beta1.Cluster) error { diff --git a/cli/cmds/cluster_delete_test.go b/cli/cmds/cluster_delete_test.go index 298d14bb..441ca813 100644 --- a/cli/cmds/cluster_delete_test.go +++ b/cli/cmds/cluster_delete_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -20,3 +21,63 @@ func TestDeleteMissingCluster(t *testing.T) { require.EqualError(t, err, `cluster "missing" not found in namespace "k3k-missing"`) } + +func Test_resolveClusterArg(t *testing.T) { + tests := []struct { + name string + flagNamespace string + arg string + wantNamespace string + wantName string + wantErr bool + }{ + { + name: "bare name defaults to k3k-", + arg: "mycluster", + wantNamespace: "k3k-mycluster", + wantName: "mycluster", + }, + { + name: "bare name respects the namespace flag", + flagNamespace: "custom", + arg: "mycluster", + wantNamespace: "custom", + wantName: "mycluster", + }, + { + name: "namespace/name form is split", + arg: "k3k-foo/mycluster", + wantNamespace: "k3k-foo", + wantName: "mycluster", + }, + { + name: "namespace/name matching the flag is accepted", + flagNamespace: "k3k-foo", + arg: "k3k-foo/mycluster", + wantNamespace: "k3k-foo", + wantName: "mycluster", + }, + { + name: "namespace/name conflicting with the flag errors", + flagNamespace: "bar", + arg: "k3k-foo/mycluster", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + appCtx := &AppContext{namespace: tt.flagNamespace} + + namespace, name, err := resolveClusterArg(appCtx, tt.arg) + if tt.wantErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.wantNamespace, namespace) + assert.Equal(t, tt.wantName, name) + }) + } +} diff --git a/cli/cmds/cluster_list.go b/cli/cmds/cluster_list.go index 72b3c192..8e217b62 100644 --- a/cli/cmds/cluster_list.go +++ b/cli/cmds/cluster_list.go @@ -1,8 +1,6 @@ package cmds import ( - "context" - "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/types" "k8s.io/cli-runtime/pkg/printers" @@ -29,7 +27,7 @@ func NewClusterListCmd(appCtx *AppContext) *cobra.Command { func list(appCtx *AppContext) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := context.Background() + ctx := cmd.Context() client := appCtx.Client var clusters v1beta1.ClusterList diff --git a/cli/cmds/completion.go b/cli/cmds/completion.go index 5f50650c..beefcb98 100644 --- a/cli/cmds/completion.go +++ b/cli/cmds/completion.go @@ -50,6 +50,29 @@ func completeClusterNamespaces(cmd *cobra.Command, args []string, toComplete str return clusterNamespaceCompletions(cmd.Context(), client) } +// completeClusterNames is a cobra.CompletionFunc that completes the cluster name argument with "namespace/name" values. +// When the "namespace" flag is set the results are filtered to that namespace. +func completeClusterNames(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + // only the first positional argument is a cluster name + if len(args) != 0 { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + // with --all no cluster name is expected, so suppress completions + if all, _ := cmd.Flags().GetBool("all"); all { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + cl, err := completionClient(cmd) + if err != nil { + return nil, cobra.ShellCompDirectiveError + } + + namespace, _ := cmd.Flags().GetString("namespace") + + return clusterNameCompletions(cmd.Context(), cl, namespace) +} + // namespaceCompletions lists every namespace in the host cluster, excluding any // already provided to the command's "namespace" flag. func namespaceCompletions(cmd *cobra.Command, cl client.Client) ([]string, cobra.ShellCompDirective) { @@ -89,6 +112,27 @@ func clusterNamespaceCompletions(ctx context.Context, client client.Client) ([]s return sets.List(names), cobra.ShellCompDirectiveNoFileComp } +// clusterNameCompletions lists the k3k Clusters as "namespace/name" values, optionally filtered to a single namespace. +func clusterNameCompletions(ctx context.Context, cl client.Client, namespace string) ([]string, cobra.ShellCompDirective) { + var opts []client.ListOption + + if namespace != "" { + opts = append(opts, client.InNamespace(namespace)) + } + + var clusters v1beta1.ClusterList + if err := cl.List(ctx, &clusters, opts...); err != nil { + return nil, cobra.ShellCompDirectiveError + } + + names := make([]string, 0, len(clusters.Items)) + for _, cluster := range clusters.Items { + names = append(names, cluster.Namespace+"/"+cluster.Name) + } + + return names, cobra.ShellCompDirectiveNoFileComp +} + // mustRegisterFlagCompletion registers a completion function for a flag and // aborts if the flag does not exist. This only fails on programmer error, so // there is no reason to bubble it up to the caller. diff --git a/cli/cmds/completion_test.go b/cli/cmds/completion_test.go index 68999051..c6742ba3 100644 --- a/cli/cmds/completion_test.go +++ b/cli/cmds/completion_test.go @@ -93,3 +93,37 @@ func Test_completeClusterNamespaces(t *testing.T) { // only namespaces that contain a cluster, deduplicated; "default" is excluded assert.ElementsMatch(t, []string{"k3k-foo", "k3k-bar"}, names) } + +func Test_completeClusterNames(t *testing.T) { + fakeClient := fake.NewClientBuilder(). + WithScheme(completionTestScheme(t)). + WithObjects( + cluster("foo", "k3k-foo"), + cluster("bar", "k3k-bar"), + cluster("bar-2", "k3k-bar"), + ). + Build() + + names, directive := clusterNameCompletions(t.Context(), fakeClient, "") + + assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) + // clusters across all namespaces, in "namespace/name" form + assert.ElementsMatch(t, []string{"k3k-foo/foo", "k3k-bar/bar", "k3k-bar/bar-2"}, names) +} + +func Test_completeClusterNames_filtersNamespace(t *testing.T) { + fakeClient := fake.NewClientBuilder(). + WithScheme(completionTestScheme(t)). + WithObjects( + cluster("foo", "k3k-foo"), + cluster("bar", "k3k-bar"), + cluster("bar-2", "k3k-bar"), + ). + Build() + + names, directive := clusterNameCompletions(t.Context(), fakeClient, "k3k-bar") + + assert.Equal(t, cobra.ShellCompDirectiveNoFileComp, directive) + // only the clusters in the requested namespace + assert.ElementsMatch(t, []string{"k3k-bar/bar", "k3k-bar/bar-2"}, names) +} diff --git a/cli/cmds/kubeconfig.go b/cli/cmds/kubeconfig.go index 38b1099a..4430fe63 100644 --- a/cli/cmds/kubeconfig.go +++ b/cli/cmds/kubeconfig.go @@ -1,7 +1,6 @@ package cmds import ( - "context" "net/url" "os" "path/filepath" @@ -75,7 +74,7 @@ func generateKubeconfigFlags(cmd *cobra.Command, cfg *GenerateKubeconfigConfig) func generate(appCtx *AppContext, cfg *GenerateKubeconfigConfig) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := context.Background() + ctx := cmd.Context() client := appCtx.Client clusterKey := types.NamespacedName{ diff --git a/cli/cmds/policy_create.go b/cli/cmds/policy_create.go index 58e0195c..64489c1b 100644 --- a/cli/cmds/policy_create.go +++ b/cli/cmds/policy_create.go @@ -58,7 +58,7 @@ func NewPolicyCreateCmd(appCtx *AppContext) *cobra.Command { func policyCreateAction(appCtx *AppContext, config *VirtualClusterPolicyCreateConfig) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := context.Background() + ctx := cmd.Context() client := appCtx.Client policyName := args[0] diff --git a/cli/cmds/policy_delete.go b/cli/cmds/policy_delete.go index 2e3f8bbd..71937925 100644 --- a/cli/cmds/policy_delete.go +++ b/cli/cmds/policy_delete.go @@ -1,8 +1,6 @@ package cmds import ( - "context" - "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -23,7 +21,7 @@ func NewPolicyDeleteCmd(appCtx *AppContext) *cobra.Command { func policyDeleteAction(appCtx *AppContext) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := context.Background() + ctx := cmd.Context() client := appCtx.Client name := args[0] diff --git a/cli/cmds/policy_list.go b/cli/cmds/policy_list.go index 2080f788..9fd18ebb 100644 --- a/cli/cmds/policy_list.go +++ b/cli/cmds/policy_list.go @@ -1,8 +1,6 @@ package cmds import ( - "context" - "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/types" "k8s.io/cli-runtime/pkg/printers" @@ -24,7 +22,7 @@ func NewPolicyListCmd(appCtx *AppContext) *cobra.Command { func policyList(appCtx *AppContext) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { - ctx := context.Background() + ctx := cmd.Context() client := appCtx.Client var policies v1beta1.VirtualClusterPolicyList diff --git a/docs/cli/k3kcli.adoc b/docs/cli/k3kcli.adoc index e1f94582..03190dab 100644 --- a/docs/cli/k3kcli.adoc +++ b/docs/cli/k3kcli.adoc @@ -94,6 +94,7 @@ k3kcli cluster delete [command options] NAME === Options ---- + -A, --all delete all the clusters in the namespace -h, --help help for delete --keep-data keeps persistence volumes created for the cluster after deletion -n, --namespace string namespace of the k3k cluster diff --git a/docs/cli/k3kcli_cluster_delete.md b/docs/cli/k3kcli_cluster_delete.md index 4a63d5fe..10e26c9a 100644 --- a/docs/cli/k3kcli_cluster_delete.md +++ b/docs/cli/k3kcli_cluster_delete.md @@ -15,6 +15,7 @@ k3kcli cluster delete [command options] NAME ### Options ``` + -A, --all delete all the clusters in the namespace -h, --help help for delete --keep-data keeps persistence volumes created for the cluster after deletion -n, --namespace string namespace of the k3k cluster