Merge pull request #11970 from johngmyers/complete-delete

Implement completion for delete commands

cmd/kops/create_cluster.go

@@ -866,11 +866,15 @@ func completeKubernetesVersion(cmd *cobra.Command, args []string, toComplete str
 	}
 
 	// Remove pre-release versions that have a subsequent stable version.
+	// Also remove the non-useful -rc.0 versions.
 	for _, version := range versions.UnsortedList() {
 		split := strings.Split(version, "-")
 		if len(split) > 1 && versions.Has(split[0]) {
 			versions.Delete(version)
 		}
+		if strings.HasSuffix(version, "-rc.0") {
+			versions.Delete(version)
+		}
 	}
 
 	return versions.List(), cobra.ShellCompDirectiveNoFileComp

cmd/kops/delete.go

@@ -30,7 +30,6 @@ import (
 	"k8s.io/kops/pkg/sshcredentials"
 	"k8s.io/kops/util/pkg/text"
 	"k8s.io/kops/util/pkg/vfs"
-	cmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/templates"
 )
@@ -41,59 +40,40 @@ type DeleteOptions struct {
 }
 
 var (
-	deleteLong = templates.LongDesc(i18n.T(`
-	Delete Kubernetes clusters, instancegroups, instances, and secrets, or a combination of the before mentioned.
-	`))
-
 	deleteExample = templates.Examples(i18n.T(`
-		# Delete an instance
-		kops delete instance i-0a5ed581b862d3425
-
 		# Delete a cluster using a manifest file
 		kops delete -f my-cluster.yaml
 
 		# Delete a cluster using a pasted manifest file from stdin.
 		pbpaste | kops delete -f -
-
-		# Delete a cluster in AWS.
-		kops delete cluster --name=k8s.example.com --state=s3://my-state-store
-
-		# Delete an instancegroup for the k8s-cluster.example.com cluster.
-		# The --yes option runs the command immediately.
-		kops delete ig --name=k8s-cluster.example.com node-example --yes
 	`))
 
-	deleteShort = i18n.T("Delete clusters, instancegroups, instances, or secrets.")
+	deleteShort = i18n.T("Delete clusters, instancegroups, instances, and secrets.")
 )
 
 func NewCmdDelete(f *util.Factory, out io.Writer) *cobra.Command {
 	options := &DeleteOptions{}
 
 	cmd := &cobra.Command{
-		Use:        "delete -f FILENAME [--yes]",
+		Use:        "delete {-f FILENAME}...",
 		Short:      deleteShort,
-		Long:       deleteLong,
 		Example:    deleteExample,
 		SuggestFor: []string{"rm"},
-		Run: func(cmd *cobra.Command, args []string) {
+		Args:       cobra.NoArgs,
-			ctx := context.TODO()
+		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(options.Filenames) == 0 {
+			return RunDelete(context.TODO(), f, out, options)
-				cmd.Help()
-				return
-			}
-			cmdutil.CheckErr(RunDelete(ctx, f, out, options))
 		},
 	}
 
 	cmd.Flags().StringSliceVarP(&options.Filenames, "filename", "f", options.Filenames, "Filename to use to delete the resource")
-	cmd.Flags().BoolVarP(&options.Yes, "yes", "y", options.Yes, "Specify --yes to delete the resource")
+	cmd.Flags().BoolVarP(&options.Yes, "yes", "y", options.Yes, "Specify --yes to immediately delete the resource")
 	cmd.MarkFlagRequired("filename")
 
 	// create subcommands
 	cmd.AddCommand(NewCmdDeleteCluster(f, out))
+	cmd.AddCommand(NewCmdDeleteInstance(f, out))
 	cmd.AddCommand(NewCmdDeleteInstanceGroup(f, out))
 	cmd.AddCommand(NewCmdDeleteSecret(f, out))
-	cmd.AddCommand(NewCmdDeleteInstance(f, out))
 
 	return cmd
 }
@@ -108,12 +88,12 @@ func RunDelete(ctx context.Context, factory *util.Factory, out io.Writer, d *Del
 		if f == "-" {
 			contents, err = ConsumeStdin()
 			if err != nil {
-				return fmt.Errorf("error reading from stdin: %v", err)
+				return fmt.Errorf("reading from stdin: %v", err)
 			}
 		} else {
 			contents, err = vfs.Context.ReadFile(f)
 			if err != nil {
-				return fmt.Errorf("error reading file %q: %v", f, err)
+				return fmt.Errorf("reading file %q: %v", f, err)
 			}
 		}
 
@@ -121,7 +101,7 @@ func RunDelete(ctx context.Context, factory *util.Factory, out io.Writer, d *Del
 		for _, section := range sections {
 			o, gvk, err := kopscodecs.Decode(section, nil)
 			if err != nil {
-				return fmt.Errorf("error parsing file %q: %v", f, err)
+				return fmt.Errorf("parsing file %q: %v", f, err)
 			}
 
 			switch v := o.(type) {
@@ -132,7 +112,7 @@ func RunDelete(ctx context.Context, factory *util.Factory, out io.Writer, d *Del
 				}
 				err = RunDeleteCluster(ctx, factory, out, options)
 				if err != nil {
-					exitWithError(err)
+					return err
 				}
 				deletedClusters.Insert(v.ObjectMeta.Name)
 			case *kopsapi.InstanceGroup:
@@ -150,7 +130,7 @@ func RunDelete(ctx context.Context, factory *util.Factory, out io.Writer, d *Del
 
 				err := RunDeleteInstanceGroup(ctx, factory, out, options)
 				if err != nil {
-					exitWithError(err)
+					return err
 				}
 			case *kopsapi.SSHCredential:
 				fingerprint, err := sshcredentials.Fingerprint(v.Spec.PublicKey)
@@ -167,11 +147,11 @@ func RunDelete(ctx context.Context, factory *util.Factory, out io.Writer, d *Del
 
 				err = RunDeleteSecret(ctx, factory, out, options)
 				if err != nil {
-					exitWithError(err)
+					return err
 				}
 			default:
 				klog.V(2).Infof("Type of object was %T", v)
-				return fmt.Errorf("Unhandled kind %q in %s", gvk, f)
+				return fmt.Errorf("unhandled kind %q in %s", gvk, f)
 			}
 		}
 	}

cmd/kops/delete_cluster.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/kops/cmd/kops/util"
 	kopsapi "k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/commands/commandutils"
 	"k8s.io/kops/pkg/kubeconfig"
 	"k8s.io/kops/pkg/resources"
 	resourceops "k8s.io/kops/pkg/resources/ops"
@@ -48,7 +49,7 @@ type DeleteClusterOptions struct {
 var (
 	deleteClusterLong = templates.LongDesc(i18n.T(`
 	Deletes a Kubernetes cluster and all associated resources.  Resources include instancegroups,
-	secrets and the state store.  There is no "UNDO" for this command.
+	secrets, and the state store.  There is no "UNDO" for this command.
 	`))
 
 	deleteClusterExample = templates.Examples(i18n.T(`
@@ -65,25 +66,14 @@ func NewCmdDeleteCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	options := &DeleteClusterOptions{}
 
 	cmd := &cobra.Command{
-		Use:     "cluster CLUSTERNAME [--yes]",
+		Use:               "cluster [CLUSTER]",
 		Short:             deleteClusterShort,
 		Long:              deleteClusterLong,
 		Example:           deleteClusterExample,
-		Run: func(cmd *cobra.Command, args []string) {
+		Args:              rootCommand.clusterNameArgsNoKubeconfig(&options.ClusterName),
-			ctx := context.TODO()
+		ValidArgsFunction: commandutils.CompleteClusterName(&rootCommand, true),
-
+		RunE: func(cmd *cobra.Command, args []string) error {
-			err := rootCommand.ProcessArgs(args)
+			return RunDeleteCluster(context.TODO(), f, out, options)
-			if err != nil {
-				exitWithError(err)
-			}
-
-			// Note _not_ ClusterName(); we only want the --name flag
-			options.ClusterName = rootCommand.clusterName
-
-			err = RunDeleteCluster(ctx, f, out, options)
-			if err != nil {
-				exitWithError(err)
-			}
 		},
 	}
 
@@ -91,7 +81,9 @@ func NewCmdDeleteCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().BoolVar(&options.Unregister, "unregister", options.Unregister, "Don't delete cloud resources, just unregister the cluster")
 	cmd.Flags().BoolVar(&options.External, "external", options.External, "Delete an external cluster")
 
-	cmd.Flags().StringVar(&options.Region, "region", options.Region, "region")
+	cmd.Flags().StringVar(&options.Region, "region", options.Region, "External cluster's cloud region")
+	cmd.RegisterFlagCompletionFunc("region", completeRegion)
+
 	return cmd
 }
 
@@ -214,3 +206,8 @@ func RunDeleteCluster(ctx context.Context, f *util.Factory, out io.Writer, optio
 	fmt.Fprintf(out, "\nDeleted cluster: %q\n", clusterName)
 	return nil
 }
+
+func completeRegion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+	// TODO call into cloud provider(s) to get list of valid regions
+	return nil, cobra.ShellCompDirectiveNoFileComp
+}

cmd/kops/delete_instance.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -32,6 +33,7 @@ import (
 	"k8s.io/kops/cmd/kops/util"
 	kopsapi "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/cloudinstances"
+	"k8s.io/kops/pkg/commands/commandutils"
 	"k8s.io/kops/pkg/instancegroups"
 	"k8s.io/kops/pkg/validation"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
@@ -40,7 +42,7 @@ import (
 )
 
 // DeleteInstanceOptions is the command Object for an instance deletion.
-type deleteInstanceOptions struct {
+type DeleteInstanceOptions struct {
 	Yes       bool
 	CloudOnly bool
 
@@ -70,7 +72,7 @@ type deleteInstanceOptions struct {
 	Surge bool
 }
 
-func (o *deleteInstanceOptions) initDefaults() {
+func (o *DeleteInstanceOptions) initDefaults() {
 	d := &RollingUpdateOptions{}
 	d.InitDefaults()
 
@@ -102,64 +104,56 @@ func NewCmdDeleteInstance(f *util.Factory, out io.Writer) *cobra.Command {
 		kops delete instance --cloudonly i-0a5ed581b862d3425 --yes
 		`))
 
-	deleteInstanceShort := i18n.T(`Delete an instance`)
+	deleteInstanceShort := i18n.T(`Delete an instance.`)
 
-	var options deleteInstanceOptions
+	var options DeleteInstanceOptions
 	options.initDefaults()
 
 	cmd := &cobra.Command{
-		Use:     "instance",
+		Use:     "instance INSTANCE|NODE",
 		Short:   deleteInstanceShort,
 		Long:    deleteInstanceLong,
 		Example: deleteInstanceExample,
+		Args: func(cmd *cobra.Command, args []string) error {
+			options.ClusterName = rootCommand.ClusterName(true)
+			if options.ClusterName == "" {
+				return fmt.Errorf("--name is required")
 			}
 
-	cmd.Flags().BoolVar(&options.CloudOnly, "cloudonly", options.CloudOnly, "Perform deletion update without confirming progress with k8s")
+			if len(args) == 0 {
+				return fmt.Errorf("must specify ID of instance or name of node to delete")
+			}
+			options.InstanceID = args[0]
+
+			if len(args) != 1 {
+				return fmt.Errorf("can only delete one instance at a time")
+			}
+
+			return nil
+		},
+		ValidArgsFunction: completeInstanceOrNode(&options),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return RunDeleteInstance(context.TODO(), f, out, &options)
+		},
+	}
+
+	cmd.Flags().BoolVar(&options.CloudOnly, "cloudonly", options.CloudOnly, "Perform deletion update without confirming progress with Kubernetes")
 	cmd.Flags().BoolVar(&options.Surge, "surge", options.Surge, "Surge by detaching the node from the ASG before deletion")
 
 	cmd.Flags().DurationVar(&options.ValidationTimeout, "validation-timeout", options.ValidationTimeout, "Maximum time to wait for a cluster to validate")
-	cmd.Flags().Int32Var(&options.ValidateCount, "validate-count", options.ValidateCount, "Amount of times that a cluster needs to be validated after single node update")
+	cmd.Flags().Int32Var(&options.ValidateCount, "validate-count", options.ValidateCount, "Number of times that a cluster needs to be validated after single node update")
 	cmd.Flags().DurationVar(&options.PostDrainDelay, "post-drain-delay", options.PostDrainDelay, "Time to wait after draining each node")
 
-	cmd.Flags().BoolVar(&options.FailOnDrainError, "fail-on-drain-error", true, "The deletion will fail if draining a node fails.")
+	cmd.Flags().BoolVar(&options.FailOnDrainError, "fail-on-drain-error", true, "Fail if draining a node fails")
-	cmd.Flags().BoolVar(&options.FailOnValidate, "fail-on-validate-error", true, "The deletion will fail if the cluster fails to validate.")
+	cmd.Flags().BoolVar(&options.FailOnValidate, "fail-on-validate-error", true, "Fail if the cluster fails to validate")
 
 	cmd.Flags().BoolVarP(&options.Yes, "yes", "y", options.Yes, "Specify --yes to immediately delete the instance")
 
-	cmd.Run = func(cmd *cobra.Command, args []string) {
-		ctx := context.TODO()
-
-		clusterName := rootCommand.ClusterName(true)
-
-		if clusterName == "" {
-			exitWithError(fmt.Errorf("--name is required"))
-			return
-		}
-
-		options.ClusterName = clusterName
-		if len(args) == 0 {
-			exitWithError(fmt.Errorf("specify ID of instance to delete"))
-		}
-		if len(args) != 1 {
-			exitWithError(fmt.Errorf("can only delete one instance at a time"))
-		}
-
-		options.InstanceID = args[0]
-
-		err := RunDeleteInstance(ctx, f, os.Stdout, &options)
-		if err != nil {
-			exitWithError(err)
-			return
-		}
-
-	}
-
 	return cmd
 }
 
-func RunDeleteInstance(ctx context.Context, f *util.Factory, out io.Writer, options *deleteInstanceOptions) error {
+func RunDeleteInstance(ctx context.Context, f *util.Factory, out io.Writer, options *DeleteInstanceOptions) error {
-
+	clientSet, err := f.Clientset()
-	clientset, err := f.Clientset()
 	if err != nil {
 		return err
 	}
@@ -169,36 +163,17 @@ func RunDeleteInstance(ctx context.Context, f *util.Factory, out io.Writer, opti
 		return err
 	}
 
-	contextName := cluster.ObjectMeta.Name
-	clientGetter := genericclioptions.NewConfigFlags(true)
-	clientGetter.Context = &contextName
-
-	config, err := clientGetter.ToRESTConfig()
-	if err != nil {
-		return fmt.Errorf("cannot load kubecfg settings for %q: %v", contextName, err)
-	}
-
 	var nodes []v1.Node
 	var k8sClient kubernetes.Interface
+	var host string
 	if !options.CloudOnly {
-		k8sClient, err = kubernetes.NewForConfig(config)
+		k8sClient, host, nodes, err = getNodes(ctx, cluster, true)
 		if err != nil {
-			return fmt.Errorf("cannot build kube client for %q: %v", contextName, err)
+			return err
-		}
-
-		nodeList, err := k8sClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{})
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "Unable to reach the kubernetes API.\n")
-			fmt.Fprintf(os.Stderr, "Use --cloudonly to do a deletion without confirming progress with the k8s API\n\n")
-			return fmt.Errorf("error listing nodes in cluster: %v", err)
-		}
-
-		if nodeList != nil {
-			nodes = nodeList.Items
 		}
 	}
 
-	list, err := clientset.InstanceGroupsFor(cluster).List(ctx, metav1.ListOptions{})
+	list, err := clientSet.InstanceGroupsFor(cluster).List(ctx, metav1.ListOptions{})
 	if err != nil {
 		return err
 	}
@@ -265,7 +240,7 @@ func RunDeleteInstance(ctx context.Context, f *util.Factory, out io.Writer, opti
 
 	var clusterValidator validation.ClusterValidator
 	if !options.CloudOnly {
-		clusterValidator, err = validation.NewClusterValidator(cluster, cloud, list, config.Host, k8sClient)
+		clusterValidator, err = validation.NewClusterValidator(cluster, cloud, list, host, k8sClient)
 		if err != nil {
 			return fmt.Errorf("cannot create cluster validator: %v", err)
 		}
@@ -275,12 +250,45 @@ func RunDeleteInstance(ctx context.Context, f *util.Factory, out io.Writer, opti
 	return d.UpdateSingleInstance(cloudMember, options.Surge)
 }
 
-func deleteNodeMatch(cloudMember *cloudinstances.CloudInstance, options *deleteInstanceOptions) bool {
+func getNodes(ctx context.Context, cluster *kopsapi.Cluster, verbose bool) (kubernetes.Interface, string, []v1.Node, error) {
+	var nodes []v1.Node
+	var k8sClient kubernetes.Interface
+
+	contextName := cluster.ObjectMeta.Name
+	clientGetter := genericclioptions.NewConfigFlags(true)
+	clientGetter.Context = &contextName
+
+	config, err := clientGetter.ToRESTConfig()
+	if err != nil {
+		return nil, "", nil, fmt.Errorf("cannot load kubecfg settings for %q: %v", contextName, err)
+	}
+
+	k8sClient, err = kubernetes.NewForConfig(config)
+	if err != nil {
+		return nil, "", nil, fmt.Errorf("cannot build kube client for %q: %v", contextName, err)
+	}
+
+	nodeList, err := k8sClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{})
+	if err != nil {
+		if verbose {
+			fmt.Fprintf(os.Stderr, "Unable to reach the kubernetes API.\n")
+			fmt.Fprintf(os.Stderr, "Use --cloudonly to do a deletion without confirming progress with the k8s API\n\n")
+		}
+		return nil, "", nil, fmt.Errorf("listing nodes in cluster: %v", err)
+	}
+
+	if nodeList != nil {
+		nodes = nodeList.Items
+	}
+	return k8sClient, config.Host, nodes, nil
+}
+
+func deleteNodeMatch(cloudMember *cloudinstances.CloudInstance, options *DeleteInstanceOptions) bool {
 	return cloudMember.ID == options.InstanceID ||
 		(!options.CloudOnly && cloudMember.Node != nil && cloudMember.Node.Name == options.InstanceID)
 }
 
-func findDeletionNode(groups map[string]*cloudinstances.CloudInstanceGroup, options *deleteInstanceOptions) *cloudinstances.CloudInstance {
+func findDeletionNode(groups map[string]*cloudinstances.CloudInstanceGroup, options *DeleteInstanceOptions) *cloudinstances.CloudInstance {
 	for _, group := range groups {
 		for _, r := range group.Ready {
 			if deleteNodeMatch(r, options) {
@@ -295,3 +303,80 @@ func findDeletionNode(groups map[string]*cloudinstances.CloudInstanceGroup, opti
 	}
 	return nil
 }
+
+func completeInstanceOrNode(options *DeleteInstanceOptions) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+		if len(args) > 0 {
+			return nil, cobra.ShellCompDirectiveNoFileComp
+		}
+
+		commandutils.ConfigureKlogForCompletion()
+		ctx := context.TODO()
+
+		cluster, clientSet, completions, directive := GetClusterForCompletion(ctx, &rootCommand, nil)
+		if cluster == nil {
+			return completions, directive
+		}
+
+		var nodes []v1.Node
+		var err error
+		if !options.CloudOnly {
+			_, _, nodes, err = getNodes(ctx, cluster, false)
+			if err != nil {
+				cobra.CompErrorln(err.Error())
+			}
+		}
+
+		list, err := clientSet.InstanceGroupsFor(cluster).List(ctx, metav1.ListOptions{})
+		if err != nil {
+			return commandutils.CompletionError("listing instance groups", err)
+		}
+
+		var instanceGroups []*kopsapi.InstanceGroup
+		for i := range list.Items {
+			instanceGroups = append(instanceGroups, &list.Items[i])
+		}
+
+		cloud, err := cloudup.BuildCloud(cluster)
+		if err != nil {
+			return commandutils.CompletionError("initializing cloud", err)
+		}
+
+		groups, err := cloud.GetCloudGroups(cluster, instanceGroups, false, nodes)
+		if err != nil {
+			return commandutils.CompletionError("listing instances", err)
+		}
+
+		completions = nil
+		longestGroup := 0
+		for _, group := range groups {
+			if group.InstanceGroup != nil && longestGroup < len(group.InstanceGroup.Name) {
+				longestGroup = len(group.InstanceGroup.Name)
+			}
+		}
+		for _, group := range groups {
+			for _, instance := range group.Ready {
+				completions = appendInstance(completions, instance, longestGroup)
+			}
+			for _, instance := range group.NeedUpdate {
+				completions = appendInstance(completions, instance, longestGroup)
+			}
+		}
+
+		return completions, cobra.ShellCompDirectiveNoFileComp
+	}
+}
+
+func appendInstance(completions []string, instance *cloudinstances.CloudInstance, longestGroup int) []string {
+	completion := instance.ID
+	if instance.CloudInstanceGroup.InstanceGroup != nil {
+		completion += "\t" + instance.CloudInstanceGroup.InstanceGroup.Name
+
+		if instance.Node != nil {
+			padding := strings.Repeat(" ", longestGroup+1-len(instance.CloudInstanceGroup.InstanceGroup.Name))
+			completion += padding + instance.Node.Name
+			completions = append(completions, instance.Node.Name+"\t"+instance.CloudInstanceGroup.InstanceGroup.Name+padding+instance.ID)
+		}
+	}
+	return append(completions, completion)
+}

cmd/kops/delete_instancegroup.go

@@ -19,6 +19,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"io"
 	"os"
@@ -26,6 +27,7 @@ import (
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/kops/cmd/kops/util"
+	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/instancegroups"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
 	"k8s.io/kops/util/pkg/ui"
@@ -34,12 +36,12 @@ import (
 )
 
 var (
-	deleteIgLong = templates.LongDesc(i18n.T(`
+	deleteInstanceGroupLong = templates.LongDesc(i18n.T(`
-		Delete an instancegroup configuration. kOps has the concept of "instance groups",
+		Delete an instance group configuration. kOps has the concept of "instance groups",
 		which are a group of similar virtual machines. On AWS, they map to an
-		AutoScalingGroup. An ig work either as a Kubernetes master or a node.`))
+		AutoScalingGroup.`))
 
-	deleteIgExample = templates.Examples(i18n.T(`
+	deleteInstanceGroupExample = templates.Examples(i18n.T(`
 
 		# Delete an instancegroup for the k8s-cluster.example.com cluster.
 		# The --yes option runs the command immediately.
@@ -47,7 +49,7 @@ var (
 		kops delete ig --name=k8s-cluster.example.com node-example --yes
 		`))
 
-	deleteIgShort = i18n.T(`Delete instancegroup`)
+	deleteInstanceGroupShort = i18n.T(`Delete instance group.`)
 )
 
 type DeleteInstanceGroupOptions struct {
@@ -60,28 +62,36 @@ func NewCmdDeleteInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {
 	options := &DeleteInstanceGroupOptions{}
 
 	cmd := &cobra.Command{
-		Use:     "instancegroup",
+		Use:     "instancegroup INSTANCE_GROUP",
 		Aliases: []string{"instancegroups", "ig"},
-		Short:   deleteIgShort,
+		Short:   deleteInstanceGroupShort,
-		Long:    deleteIgLong,
+		Long:    deleteInstanceGroupLong,
-		Example: deleteIgExample,
+		Example: deleteInstanceGroupExample,
-		Run: func(cmd *cobra.Command, args []string) {
+		Args: func(cmd *cobra.Command, args []string) error {
-			ctx := context.TODO()
-
-			if len(args) == 0 {
-				exitWithError(fmt.Errorf("Specify name of instance group to delete"))
-			}
-			if len(args) != 1 {
-				exitWithError(fmt.Errorf("Can only edit one instance group at a time!"))
-			}
-
-			groupName := args[0]
-			options.GroupName = groupName
-
 			options.ClusterName = rootCommand.ClusterName(true)
 
+			if options.ClusterName == "" {
+				return fmt.Errorf("--name is required")
+			}
+
+			if len(args) == 0 {
+				return fmt.Errorf("must specify the name of instance group to delete")
+			}
+
+			options.GroupName = args[0]
+
+			if len(args) != 1 {
+				return fmt.Errorf("can only edit one instance group at a time")
+			}
+
+			return nil
+		},
+		ValidArgsFunction: completeInstanceGroup(nil, &[]string{strings.ToLower(string(kops.InstanceGroupRoleMaster))}),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			ctx := context.TODO()
+
 			if !options.Yes {
-				message := fmt.Sprintf("Do you really want to delete instance group %q? This action cannot be undone.", groupName)
+				message := fmt.Sprintf("Do you really want to delete instance group %q? This action cannot be undone.", options.GroupName)
 
 				c := &ui.ConfirmArgs{
 					Out:     out,
@@ -92,7 +102,7 @@ func NewCmdDeleteInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {
 
 				confirmed, err := ui.GetConfirm(c)
 				if err != nil {
-					exitWithError(err)
+					return err
 				}
 				if !confirmed {
 					os.Exit(1)
@@ -101,10 +111,7 @@ func NewCmdDeleteInstanceGroup(f *util.Factory, out io.Writer) *cobra.Command {
 				}
 			}
 
-			err := RunDeleteInstanceGroup(ctx, f, out, options)
+			return RunDeleteInstanceGroup(ctx, f, out, options)
-			if err != nil {
-				exitWithError(err)
-			}
 		},
 	}
 
@@ -123,12 +130,7 @@ func RunDeleteInstanceGroup(ctx context.Context, f *util.Factory, out io.Writer,
 		return fmt.Errorf("GroupName is required")
 	}
 
-	clusterName := options.ClusterName
+	cluster, err := GetCluster(ctx, f, options.ClusterName)
-	if clusterName == "" {
-		return fmt.Errorf("ClusterName is required")
-	}
-
-	cluster, err := GetCluster(ctx, f, clusterName)
 	if err != nil {
 		return err
 	}
@@ -146,18 +148,37 @@ func RunDeleteInstanceGroup(ctx context.Context, f *util.Factory, out io.Writer,
 		return fmt.Errorf("InstanceGroup %q not found", groupName)
 	}
 
-	cloud, err := cloudup.BuildCloud(cluster)
+	fmt.Fprintf(out, "InstanceGroup %q found for deletion\n", groupName)
+
+	if group.Spec.Role == kops.InstanceGroupRoleMaster {
+		groups, err := clientset.InstanceGroupsFor(cluster).List(ctx, metav1.ListOptions{})
 		if err != nil {
-		return err
+			return fmt.Errorf("listing InstanceGroups: %v", err)
 		}
 
-	fmt.Fprintf(out, "InstanceGroup %q found for deletion\n", groupName)
+		onlyMaster := true
+		for _, ig := range groups.Items {
+			if ig.Name != groupName && ig.Spec.Role == kops.InstanceGroupRoleMaster {
+				onlyMaster = false
+				break
+			}
+		}
+
+		if onlyMaster {
+			return fmt.Errorf("cannot delete the only control plane instance group")
+		}
+	}
 
 	if !options.Yes {
 		fmt.Fprintf(out, "\nMust specify --yes to delete instancegroup\n")
 		return nil
 	}
 
+	cloud, err := cloudup.BuildCloud(cluster)
+	if err != nil {
+		return err
+	}
+
 	d := &instancegroups.DeleteInstanceGroup{}
 	d.Cluster = cluster
 	d.Cloud = cloud

cmd/kops/rollingupdate_cluster.go

@@ -190,7 +190,7 @@ func NewCmdRollingUpdateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().DurationVar(&options.PostDrainDelay, "post-drain-delay", options.PostDrainDelay, "Time to wait after draining each node")
 	cmd.Flags().BoolVarP(&options.Interactive, "interactive", "i", options.Interactive, "Prompt to continue after each instance is updated")
 	cmd.Flags().StringSliceVar(&options.InstanceGroups, "instance-group", options.InstanceGroups, "Instance groups to update (defaults to all if not specified)")
-	cmd.RegisterFlagCompletionFunc("instance-group", completeInstanceGroup(&options))
+	cmd.RegisterFlagCompletionFunc("instance-group", completeInstanceGroup(&options.InstanceGroups, &options.InstanceGroupRoles))
 	cmd.Flags().StringSliceVar(&options.InstanceGroupRoles, "instance-group-roles", options.InstanceGroupRoles, "Instance group roles to update ("+strings.Join(allRoles, ",")+")")
 	cmd.RegisterFlagCompletionFunc("instance-group-roles", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		return sets.NewString(allRoles...).Delete(options.InstanceGroupRoles...).List(), cobra.ShellCompDirectiveNoFileComp
@@ -429,7 +429,7 @@ func RunRollingUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer
 	return d.RollingUpdate(groups, list)
 }
 
-func completeInstanceGroup(options *RollingUpdateOptions) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+func completeInstanceGroup(selectedInstanceGroups *[]string, selectedInstanceGroupRoles *[]string) func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 	return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
 		commandutils.ConfigureKlogForCompletion()
 		ctx := context.TODO()
@@ -444,8 +444,14 @@ func completeInstanceGroup(options *RollingUpdateOptions) func(cmd *cobra.Comman
 			return commandutils.CompletionError("listing instance groups", err)
 		}
 
-		alreadySelected := sets.NewString(options.InstanceGroups...)
+		alreadySelected := sets.NewString()
-		alreadySelectedRoles := sets.NewString(options.InstanceGroupRoles...)
+		if selectedInstanceGroups != nil {
+			alreadySelected = alreadySelected.Insert(*selectedInstanceGroups...)
+		}
+		alreadySelectedRoles := sets.NewString()
+		if selectedInstanceGroupRoles != nil {
+			alreadySelectedRoles = alreadySelectedRoles.Insert(*selectedInstanceGroupRoles...)
+		}
 		var igs []string
 		for _, ig := range list.Items {
 			if !alreadySelected.Has(ig.Name) && !alreadySelectedRoles.Has(strings.ToLower(string(ig.Spec.Role))) {

docs/cli/kops.md

@@ -39,7 +39,7 @@ kOps is Kubernetes Operations.
 
 * [kops completion](kops_completion.md)	 - generate the autocompletion script for the specified shell
 * [kops create](kops_create.md)	 - Create a resource by command line, filename or stdin.
-* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, or secrets.
+* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, and secrets.
 * [kops describe](kops_describe.md)	 - Describe a resource.
 * [kops distrust](kops_distrust.md)	 - Distrust keypairs.
 * [kops edit](kops_edit.md)	 - Edit clusters and other resources.

docs/cli/kops_delete.md

@@ -3,34 +3,20 @@
 
 ## kops delete
 
-Delete clusters, instancegroups, instances, or secrets.
+Delete clusters, instancegroups, instances, and secrets.
-
-### Synopsis
-
-Delete Kubernetes clusters, instancegroups, instances, and secrets, or a combination of the before mentioned.
 
 ```
-kops delete -f FILENAME [--yes] [flags]
+kops delete {-f FILENAME}... [flags]
 ```
 
 ### Examples
 
 ```
-  # Delete an instance
-  kops delete instance i-0a5ed581b862d3425
-  
   # Delete a cluster using a manifest file
   kops delete -f my-cluster.yaml
   
   # Delete a cluster using a pasted manifest file from stdin.
   pbpaste | kops delete -f -
-  
-  # Delete a cluster in AWS.
-  kops delete cluster --name=k8s.example.com --state=s3://my-state-store
-  
-  # Delete an instancegroup for the k8s-cluster.example.com cluster.
-  # The --yes option runs the command immediately.
-  kops delete ig --name=k8s-cluster.example.com node-example --yes
 ```
 
 ### Options
@@ -38,7 +24,7 @@ kops delete -f FILENAME [--yes] [flags]
 ```
   -f, --filename strings   Filename to use to delete the resource
   -h, --help               help for delete
-  -y, --yes                Specify --yes to delete the resource
+  -y, --yes                Specify --yes to immediately delete the resource
 ```
 
 ### Options inherited from parent commands
@@ -66,7 +52,7 @@ kops delete -f FILENAME [--yes] [flags]
 
 * [kops](kops.md)	 - kOps is Kubernetes Operations.
 * [kops delete cluster](kops_delete_cluster.md)	 - Delete a cluster.
-* [kops delete instance](kops_delete_instance.md)	 - Delete an instance
+* [kops delete instance](kops_delete_instance.md)	 - Delete an instance.
-* [kops delete instancegroup](kops_delete_instancegroup.md)	 - Delete instancegroup
+* [kops delete instancegroup](kops_delete_instancegroup.md)	 - Delete instance group.
 * [kops delete secret](kops_delete_secret.md)	 - Delete a secret
 

docs/cli/kops_delete_cluster.md

@@ -7,10 +7,10 @@ Delete a cluster.
 
 ### Synopsis
 
-Deletes a Kubernetes cluster and all associated resources.  Resources include instancegroups, secrets and the state store.  There is no "UNDO" for this command.
+Deletes a Kubernetes cluster and all associated resources.  Resources include instancegroups, secrets, and the state store.  There is no "UNDO" for this command.
 
 ```
-kops delete cluster CLUSTERNAME [--yes] [flags]
+kops delete cluster [CLUSTER] [flags]
 ```
 
 ### Examples
@@ -26,7 +26,7 @@ kops delete cluster CLUSTERNAME [--yes] [flags]
 ```
       --external        Delete an external cluster
   -h, --help            help for cluster
-      --region string   region
+      --region string   External cluster's cloud region
       --unregister      Don't delete cloud resources, just unregister the cluster
   -y, --yes             Specify --yes to delete the cluster
 ```
@@ -54,5 +54,5 @@ kops delete cluster CLUSTERNAME [--yes] [flags]
 
 ### SEE ALSO
 
-* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, or secrets.
+* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, and secrets.
 

docs/cli/kops_delete_instance.md

@@ -3,14 +3,14 @@
 
 ## kops delete instance
 
-Delete an instance
+Delete an instance.
 
 ### Synopsis
 
 Delete an instance. By default, it will detach the instance from the instance group, drain it, then terminate it.
 
 ```
-kops delete instance [flags]
+kops delete instance INSTANCE|NODE [flags]
 ```
 
 ### Examples
@@ -30,13 +30,13 @@ kops delete instance [flags]
 ### Options
 
 ```
-      --cloudonly                     Perform deletion update without confirming progress with k8s
+      --cloudonly                     Perform deletion update without confirming progress with Kubernetes
-      --fail-on-drain-error           The deletion will fail if draining a node fails. (default true)
+      --fail-on-drain-error           Fail if draining a node fails (default true)
-      --fail-on-validate-error        The deletion will fail if the cluster fails to validate. (default true)
+      --fail-on-validate-error        Fail if the cluster fails to validate (default true)
   -h, --help                          help for instance
       --post-drain-delay duration     Time to wait after draining each node (default 5s)
       --surge                         Surge by detaching the node from the ASG before deletion (default true)
-      --validate-count int32          Amount of times that a cluster needs to be validated after single node update (default 2)
+      --validate-count int32          Number of times that a cluster needs to be validated after single node update (default 2)
       --validation-timeout duration   Maximum time to wait for a cluster to validate (default 15m0s)
   -y, --yes                           Specify --yes to immediately delete the instance
 ```
@@ -64,5 +64,5 @@ kops delete instance [flags]
 
 ### SEE ALSO
 
-* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, or secrets.
+* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, and secrets.
 

docs/cli/kops_delete_instancegroup.md

@@ -3,14 +3,14 @@
 
 ## kops delete instancegroup
 
-Delete instancegroup
+Delete instance group.
 
 ### Synopsis
 
-Delete an instancegroup configuration. kOps has the concept of "instance groups", which are a group of similar virtual machines. On AWS, they map to an AutoScalingGroup. An ig work either as a Kubernetes master or a node.
+Delete an instance group configuration. kOps has the concept of "instance groups", which are a group of similar virtual machines. On AWS, they map to an AutoScalingGroup.
 
 ```
-kops delete instancegroup [flags]
+kops delete instancegroup INSTANCE_GROUP [flags]
 ```
 
 ### Examples
@@ -52,5 +52,5 @@ kops delete instancegroup [flags]
 
 ### SEE ALSO
 
-* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, or secrets.
+* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, and secrets.
 

docs/cli/kops_delete_secret.md

@@ -50,5 +50,5 @@ kops delete secret [flags]
 
 ### SEE ALSO
 
-* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, or secrets.
+* [kops delete](kops_delete.md)	 - Delete clusters, instancegroups, instances, and secrets.