Merge pull request kubernetes#110668 from brianpursley/k-108630

Remove unused flags from kubectl run

Author: k8s-ci-robot

staging/src/k8s.io/kubectl/pkg/cmd/run/run.go

@@ -36,7 +36,6 @@ import (
 	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/cli-runtime/pkg/resource"
-	"k8s.io/client-go/kubernetes"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/tools/cache"
 	watchtools "k8s.io/client-go/tools/watch"
@@ -94,6 +93,8 @@ const (
 
 var metadataAccessor = meta.NewAccessor()
 
+var attachFunc = attach.DefaultAttachFunc
+
 type RunObject struct {
 	Object  runtime.Object
 	Mapping *meta.RESTMapping
@@ -105,7 +106,6 @@ type RunOptions struct {
 	PrintFlags  *genericclioptions.PrintFlags
 	RecordFlags *genericclioptions.RecordFlags
 
-	DeleteFlags   *cmddelete.DeleteFlags
 	DeleteOptions *cmddelete.DeleteOptions
 
 	DryRunStrategy cmdutil.DryRunStrategy
@@ -135,7 +135,6 @@ type RunOptions struct {
 func NewRunOptions(streams genericclioptions.IOStreams) *RunOptions {
 	return &RunOptions{
 		PrintFlags:  genericclioptions.NewPrintFlags("created").WithTypeSetter(scheme.Scheme),
-		DeleteFlags: cmddelete.NewDeleteFlags("to use to replace the resource."),
 		RecordFlags: genericclioptions.NewRecordFlags(),
 
 		Recorder: genericclioptions.NoopRecorder{},
@@ -159,7 +158,6 @@ func NewCmdRun(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Co
 		},
 	}
 
-	o.DeleteFlags.AddFlags(cmd)
 	o.PrintFlags.AddFlags(cmd)
 	o.RecordFlags.AddFlags(cmd)
 
@@ -226,18 +224,17 @@ func (o *RunOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 		return printer.PrintObj(obj, o.Out)
 	}
 
-	deleteOpts, err := o.DeleteFlags.ToOptions(dynamicClient, o.IOStreams)
-	if err != nil {
-		return err
+	o.DeleteOptions = &cmddelete.DeleteOptions{
+		CascadingStrategy: metav1.DeletePropagationBackground,
+		DynamicClient:     dynamicClient,
+		GracePeriod:       -1,
+		IgnoreNotFound:    true,
+		IOStreams:         o.IOStreams,
+		Quiet:             o.Quiet,
+		Timeout:           time.Duration(0),
+		WaitForDeletion:   false,
 	}
 
-	deleteOpts.IgnoreNotFound = true
-	deleteOpts.WaitForDeletion = false
-	deleteOpts.GracePeriod = -1
-	deleteOpts.Quiet = o.Quiet
-
-	o.DeleteOptions = deleteOpts
-
 	return nil
 }
 
@@ -325,7 +322,11 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
 
 	if o.Attach {
 		if remove {
-			defer o.removeCreatedObjects(f, createdObjects)
+			defer func() {
+				if err := o.removeCreatedObjects(f, createdObjects); err != nil {
+					fmt.Fprintf(o.ErrOut, "Delete failed: %v\n", err)
+				}
+			}()
 		}
 
 		opts := &attach.AttachOptions{
@@ -345,9 +346,9 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e
 			return err
 		}
 		opts.Config = config
-		opts.AttachFunc = attach.DefaultAttachFunc
+		opts.AttachFunc = attachFunc
 
-		clientset, err := kubernetes.NewForConfig(config)
+		clientset, err := f.KubernetesClientSet()
 		if err != nil {
 			return err
 		}

staging/src/k8s.io/kubectl/pkg/cmd/run/run_test.go

@@ -33,10 +33,13 @@ import (
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/rest/fake"
+	"k8s.io/client-go/tools/remotecommand"
+	"k8s.io/kubectl/pkg/cmd/attach"
 	"k8s.io/kubectl/pkg/cmd/delete"
 	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
 	cmdutil "k8s.io/kubectl/pkg/cmd/util"
@@ -640,6 +643,193 @@ func TestExpose(t *testing.T) {
 	}
 }
 
+func TestRunAttach(t *testing.T) {
+	tests := []struct {
+		name                string
+		rm                  bool
+		quiet               bool
+		deleteErrorMessage  string
+		expectedDeleteCount int
+		expectedOut         string
+		expectedErrOut      string
+	}{
+		{
+			name:                "test attach",
+			rm:                  false,
+			quiet:               false,
+			expectedDeleteCount: 0,
+			expectedOut:         "",
+			expectedErrOut:      "If you don't see a command prompt, try pressing enter.\n",
+		},
+		{
+			name:                "test attach with quiet",
+			rm:                  false,
+			quiet:               true,
+			expectedDeleteCount: 0,
+			expectedOut:         "",
+			expectedErrOut:      "",
+		},
+		{
+			name:                "test attach with rm",
+			rm:                  true,
+			quiet:               false,
+			expectedDeleteCount: 1,
+			expectedOut:         "pod \"foo\" deleted\n",
+			expectedErrOut:      "If you don't see a command prompt, try pressing enter.\n",
+		},
+		{
+			name:                "test attach with rm should not print message if quiet is specified",
+			rm:                  true,
+			quiet:               true,
+			expectedDeleteCount: 1,
+			expectedOut:         "",
+			expectedErrOut:      "",
+		},
+		{
+			name:                "error should be displayed if delete fails",
+			rm:                  true,
+			quiet:               false,
+			deleteErrorMessage:  "delete error message",
+			expectedDeleteCount: 1,
+			expectedOut:         "",
+			expectedErrOut:      "If you don't see a command prompt, try pressing enter.\nDelete failed: delete error message\n",
+		},
+	}
+
+	fakePod := &corev1.Pod{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "v1",
+			Kind:       "Pod",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "foo",
+			Namespace: "default",
+		},
+		Spec: corev1.PodSpec{
+			Containers: []corev1.Container{
+				{
+					Name: "bar",
+				},
+			},
+		},
+		Status: corev1.PodStatus{
+			Phase: corev1.PodRunning,
+			Conditions: []corev1.PodCondition{
+				{
+					Type:   corev1.PodReady,
+					Status: corev1.ConditionTrue,
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(tt *testing.T) {
+			postCount := 0
+			attachCount := 0
+			deleteCount := 0
+
+			attachFunc = func(o *attach.AttachOptions, containerToAttach *corev1.Container, raw bool, sizeQueue remotecommand.TerminalSizeQueue) func() error {
+				if containerToAttach.Name != "bar" {
+					tt.Fatalf("expected attach to container name \"bar\", but got %q", containerToAttach.Name)
+				}
+				return func() error {
+					attachCount++
+					return nil
+				}
+			}
+
+			tf := cmdtesting.NewTestFactory().WithNamespace("test")
+			defer tf.Cleanup()
+
+			codec := scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
+			ns := scheme.Codecs.WithoutConversion()
+			tf.Client = &fake.RESTClient{
+				GroupVersion:         schema.GroupVersion{Version: ""},
+				NegotiatedSerializer: ns,
+				Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+					switch p, m := req.URL.Path, req.Method; {
+					case m == "POST" && p == "/namespaces/test/pods":
+						postCount++
+						body := cmdtesting.ObjBody(codec, fakePod)
+						return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
+					case m == "GET" && p == "/api/v1/namespaces/default/pods":
+						event := &metav1.WatchEvent{
+							Type:   "ADDED",
+							Object: runtime.RawExtension{Object: fakePod},
+						}
+						body := cmdtesting.ObjBody(codec, event)
+						return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
+					case m == "GET" && p == "/namespaces/default/pods/foo":
+						body := cmdtesting.ObjBody(codec, fakePod)
+						return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
+					case m == "DELETE" && p == "/namespaces/default/pods/foo":
+						deleteCount++
+						if test.deleteErrorMessage != "" {
+							body := cmdtesting.ObjBody(codec, &metav1.Status{
+								Status:  metav1.StatusFailure,
+								Message: test.deleteErrorMessage,
+							})
+							return &http.Response{StatusCode: http.StatusInternalServerError, Header: cmdtesting.DefaultHeader(), Body: body}, nil
+						} else {
+							body := cmdtesting.ObjBody(codec, fakePod)
+							return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: body}, nil
+						}
+					default:
+						tt.Errorf("unexpected request: %s %#v\n%#v", req.Method, req.URL, req)
+						return nil, fmt.Errorf("unexpected request")
+					}
+				}),
+			}
+
+			tf.ClientConfigVal = &restclient.Config{
+				ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Version: ""}, NegotiatedSerializer: ns},
+			}
+
+			streams, _, bufOut, bufErr := genericclioptions.NewTestIOStreams()
+			cmdutil.BehaviorOnFatal(func(str string, code int) {
+				bufErr.Write([]byte(str))
+			})
+
+			cmd := NewCmdRun(tf, streams)
+			cmd.Flags().Set("image", "test-image")
+			cmd.Flags().Set("attach", "true")
+			if test.rm {
+				cmd.Flags().Set("rm", "true")
+			}
+			if test.quiet {
+				cmd.Flags().Set("quiet", "true")
+			}
+
+			parentCmd := cobra.Command{}
+			parentCmd.AddCommand(cmd)
+
+			cmd.Run(cmd, []string{"test-pod"})
+
+			if postCount != 1 {
+				tt.Fatalf("expected 1 post request, but got %d", postCount)
+			}
+
+			if attachCount != 1 {
+				tt.Fatalf("expected 1 attach call, but got %d", attachCount)
+			}
+
+			if deleteCount != test.expectedDeleteCount {
+				tt.Fatalf("expected %d delete requests, but got %d", test.expectedDeleteCount, deleteCount)
+			}
+
+			if bufErr.String() != test.expectedErrOut {
+				tt.Fatalf("unexpected error. got: %q, expected: %q", bufErr.String(), test.expectedErrOut)
+			}
+
+			if bufOut.String() != test.expectedOut {
+				tt.Fatalf("unexpected output. got: %q, expected: %q", bufOut.String(), test.expectedOut)
+			}
+		})
+
+	}
+}
+
 func TestRunOverride(t *testing.T) {
 	tests := []struct {
 		name           string