From eae2c424efd9dda8cb0f5ba01ab4c955cdf42cfa Mon Sep 17 00:00:00 2001 From: Brian Pursley Date: Mon, 5 Sep 2022 09:34:48 -0400 Subject: [PATCH] Revert "Remove unused flags from kubectl run" --- staging/src/k8s.io/kubectl/pkg/cmd/run/run.go | 35 ++-- .../k8s.io/kubectl/pkg/cmd/run/run_test.go | 190 ------------------ 2 files changed, 17 insertions(+), 208 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/run/run.go b/staging/src/k8s.io/kubectl/pkg/cmd/run/run.go index c57d2e6b8472..dd154ce24dac 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/run/run.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/run/run.go @@ -36,6 +36,7 @@ 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" @@ -93,8 +94,6 @@ const ( var metadataAccessor = meta.NewAccessor() -var attachFunc = attach.DefaultAttachFunc - type RunObject struct { Object runtime.Object Mapping *meta.RESTMapping @@ -106,6 +105,7 @@ type RunOptions struct { PrintFlags *genericclioptions.PrintFlags RecordFlags *genericclioptions.RecordFlags + DeleteFlags *cmddelete.DeleteFlags DeleteOptions *cmddelete.DeleteOptions DryRunStrategy cmdutil.DryRunStrategy @@ -135,6 +135,7 @@ 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{}, @@ -158,6 +159,7 @@ func NewCmdRun(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Co }, } + o.DeleteFlags.AddFlags(cmd) o.PrintFlags.AddFlags(cmd) o.RecordFlags.AddFlags(cmd) @@ -224,17 +226,18 @@ func (o *RunOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error { return printer.PrintObj(obj, o.Out) } - 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, err := o.DeleteFlags.ToOptions(dynamicClient, o.IOStreams) + if err != nil { + return err } + deleteOpts.IgnoreNotFound = true + deleteOpts.WaitForDeletion = false + deleteOpts.GracePeriod = -1 + deleteOpts.Quiet = o.Quiet + + o.DeleteOptions = deleteOpts + return nil } @@ -322,11 +325,7 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e if o.Attach { if remove { - defer func() { - if err := o.removeCreatedObjects(f, createdObjects); err != nil { - fmt.Fprintf(o.ErrOut, "Delete failed: %v\n", err) - } - }() + defer o.removeCreatedObjects(f, createdObjects) } opts := &attach.AttachOptions{ @@ -346,9 +345,9 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e return err } opts.Config = config - opts.AttachFunc = attachFunc + opts.AttachFunc = attach.DefaultAttachFunc - clientset, err := f.KubernetesClientSet() + clientset, err := kubernetes.NewForConfig(config) if err != nil { return err } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/run/run_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/run/run_test.go index 5ebf6a807121..0d3491a37bb8 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/run/run_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/run/run_test.go @@ -33,13 +33,10 @@ 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" @@ -643,193 +640,6 @@ 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