Merge pull request #40365 from shiywang/attach

Automatic merge from submit-queue (batch tested with PRs 41145, 38771, 41003, 41089, 40365)

Add `kubectl attach` support for multiple types

To address this issue: https://github.com/kubernetes/kubernetes/issues/24857
the new `kubectl attach` will contain three scenarios depend on args:
1. `kubectl attach POD` :   if only one argument provided, we assume it's a pod name
2. `kubectl attach TYPE NAME` : if two arguments provided, we assume first one is resource we [supported](4770162fd3/pkg/kubectl/cmd/util/factory_object_mapping.go (L285)), the second resource's name.
3. `kubectl attach TYPE/NAME` : one argument provided and arg[0] must contain `/`, ditto

Is there any other scenarios I haven't consider in ?

for now the first scenario is compatible with changed before, also `make test` pass ✅ 
will write some unit test to test second and third scenario, if you guys think i'm doing the right way.
@pwittrock @kargakis @fabianofranz @ymqytw @AdoHe

pkg/kubectl/cmd/attach.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/kubernetes/pkg/client/unversioned/remotecommand"
 	"k8s.io/kubernetes/pkg/kubectl/cmd/templates"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
+	"k8s.io/kubernetes/pkg/kubectl/resource"
 	remotecommandserver "k8s.io/kubernetes/pkg/kubelet/server/remotecommand"
 	"k8s.io/kubernetes/pkg/util/i18n"
 	"k8s.io/kubernetes/pkg/util/term"
@@ -47,7 +48,11 @@ var (
 
 		# Switch to raw terminal mode, sends stdin to 'bash' in ruby-container from pod 123456-7890
 		# and sends stdout/stderr from 'bash' back to the client
-		kubectl attach 123456-7890 -c ruby-container -i -t`)
+		kubectl attach 123456-7890 -c ruby-container -i -t
+
+		# Get output from the first pod of a ReplicaSet named nginx
+		kubectl attach rs/nginx
+		`)
 )
 
 func NewCmdAttach(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer) *cobra.Command {
@@ -61,7 +66,7 @@ func NewCmdAttach(f cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer)
 		Attach: &DefaultRemoteAttach{},
 	}
 	cmd := &cobra.Command{
-		Use:     "attach POD -c CONTAINER",
+		Use:     "attach (POD | TYPE/NAME) -c CONTAINER",
 		Short:   i18n.T("Attach to a running container"),
 		Long:    "Attach to a process that is already running inside an existing container.",
 		Example: attach_example,
@@ -117,17 +122,39 @@ type AttachOptions struct {
 // Complete verifies command line arguments and loads data from the command environment
 func (p *AttachOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, argsIn []string) error {
 	if len(argsIn) == 0 {
-		return cmdutil.UsageError(cmd, "POD is required for attach")
+		return cmdutil.UsageError(cmd, "at least one argument is required for attach")
 	}
-	if len(argsIn) > 1 {
+	if len(argsIn) > 2 {
-		return cmdutil.UsageError(cmd, fmt.Sprintf("expected a single argument: POD, saw %d: %s", len(argsIn), argsIn))
+		return cmdutil.UsageError(cmd, fmt.Sprintf("expected fewer than three arguments: POD or TYPE/NAME or TYPE NAME, saw %d: %s", len(argsIn), argsIn))
 	}
-	p.PodName = argsIn[0]
 
 	namespace, _, err := f.DefaultNamespace()
 	if err != nil {
 		return err
 	}
+
+	mapper, typer := f.Object()
+	builder := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.ClientForMapping), f.Decoder(true)).
+		NamespaceParam(namespace).DefaultNamespace()
+
+	switch len(argsIn) {
+	case 1:
+		builder.ResourceNames("pods", argsIn[0])
+	case 2:
+		builder.ResourceNames(argsIn[0], argsIn[1])
+	}
+
+	obj, err := builder.Do().Object()
+	if err != nil {
+		return err
+	}
+
+	attachablePod, err := f.AttachablePodForObject(obj)
+	if err != nil {
+		return err
+	}
+
+	p.PodName = attachablePod.Name
 	p.Namespace = namespace
 
 	config, err := f.ClientConfig()

pkg/kubectl/cmd/attach_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/spf13/cobra"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/rest/fake"
@@ -56,6 +57,7 @@ func TestPodAndContainerAttach(t *testing.T) {
 		expectError       bool
 		expectedPod       string
 		expectedContainer string
+		obj               runtime.Object
 	}{
 		{
 			p:           &AttachOptions{},
@@ -64,7 +66,7 @@ func TestPodAndContainerAttach(t *testing.T) {
 		},
 		{
 			p:           &AttachOptions{},
-			args:        []string{"foo", "bar"},
+			args:        []string{"one", "two", "three"},
 			expectError: true,
 			name:        "too many args",
 		},
@@ -73,6 +75,7 @@ func TestPodAndContainerAttach(t *testing.T) {
 			args:        []string{"foo"},
 			expectedPod: "foo",
 			name:        "no container, no flags",
+			obj:         attachPod(),
 		},
 		{
 			p:                 &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}},
@@ -80,6 +83,7 @@ func TestPodAndContainerAttach(t *testing.T) {
 			expectedPod:       "foo",
 			expectedContainer: "bar",
 			name:              "container in flag",
+			obj:               attachPod(),
 		},
 		{
 			p:                 &AttachOptions{StreamOptions: StreamOptions{ContainerName: "initfoo"}},
@@ -87,21 +91,42 @@ func TestPodAndContainerAttach(t *testing.T) {
 			expectedPod:       "foo",
 			expectedContainer: "initfoo",
 			name:              "init container in flag",
+			obj:               attachPod(),
 		},
 		{
 			p:           &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}},
 			args:        []string{"foo", "-c", "wrong"},
 			expectError: true,
 			name:        "non-existing container in flag",
+			obj:         attachPod(),
+		},
+		{
+			p:           &AttachOptions{},
+			args:        []string{"pods", "foo"},
+			expectedPod: "foo",
+			name:        "no container, no flags, pods and name",
+			obj:         attachPod(),
+		},
+		{
+			p:           &AttachOptions{},
+			args:        []string{"pod/foo"},
+			expectedPod: "foo",
+			name:        "no container, no flags, pod/name",
+			obj:         attachPod(),
 		},
 	}
 
 	for _, test := range tests {
-		f, tf, _, ns := cmdtesting.NewAPIFactory()
+		f, tf, codec, ns := cmdtesting.NewAPIFactory()
 		tf.Client = &fake.RESTClient{
 			APIRegistry:          api.Registry,
 			NegotiatedSerializer: ns,
-			Client:               fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { return nil, nil }),
+			Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+				if test.obj != nil {
+					return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, test.obj)}, nil
+				}
+				return nil, nil
+			}),
 		}
 		tf.Namespace = "test"
 		tf.ClientConfig = defaultClientConfig()
@@ -130,23 +155,25 @@ func TestPodAndContainerAttach(t *testing.T) {
 func TestAttach(t *testing.T) {
 	version := api.Registry.GroupOrDie(api.GroupName).GroupVersion.Version
 	tests := []struct {
-		name, version, podPath, attachPath, container string
+		name, version, podPath, fetchPodPath, attachPath, container string
-		pod                                           *api.Pod
+		pod                                                         *api.Pod
-		remoteAttachErr                               bool
+		remoteAttachErr                                             bool
-		exepctedErr                                   string
+		exepctedErr                                                 string
 	}{
 		{
-			name:       "pod attach",
+			name:         "pod attach",
-			version:    version,
+			version:      version,
-			podPath:    "/api/" + version + "/namespaces/test/pods/foo",
+			podPath:      "/api/" + version + "/namespaces/test/pods/foo",
-			attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach",
+			fetchPodPath: "/namespaces/test/pods/foo",
-			pod:        attachPod(),
+			attachPath:   "/api/" + version + "/namespaces/test/pods/foo/attach",
-			container:  "bar",
+			pod:          attachPod(),
+			container:    "bar",
 		},
 		{
 			name:            "pod attach error",
 			version:         version,
 			podPath:         "/api/" + version + "/namespaces/test/pods/foo",
+			fetchPodPath:    "/namespaces/test/pods/foo",
 			attachPath:      "/api/" + version + "/namespaces/test/pods/foo/attach",
 			pod:             attachPod(),
 			remoteAttachErr: true,
@@ -154,13 +181,14 @@ func TestAttach(t *testing.T) {
 			exepctedErr:     "attach error",
 		},
 		{
-			name:        "container not found error",
+			name:         "container not found error",
-			version:     version,
+			version:      version,
-			podPath:     "/api/" + version + "/namespaces/test/pods/foo",
+			podPath:      "/api/" + version + "/namespaces/test/pods/foo",
-			attachPath:  "/api/" + version + "/namespaces/test/pods/foo/attach",
+			fetchPodPath: "/namespaces/test/pods/foo",
-			pod:         attachPod(),
+			attachPath:   "/api/" + version + "/namespaces/test/pods/foo/attach",
-			container:   "foo",
+			pod:          attachPod(),
-			exepctedErr: "cannot attach to the container: container not found (foo)",
+			container:    "foo",
+			exepctedErr:  "cannot attach to the container: container not found (foo)",
 		},
 	}
 	for _, test := range tests {
@@ -173,9 +201,12 @@ func TestAttach(t *testing.T) {
 				case p == test.podPath && m == "GET":
 					body := objBody(codec, test.pod)
 					return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil
+				case p == test.fetchPodPath && m == "GET":
+					body := objBody(codec, test.pod)
+					return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil
 				default:
 					// Ensures no GET is performed when deleting by name
-					t.Errorf("%s: unexpected request: %s %#v\n%#v", test.name, req.Method, req.URL, req)
+					t.Errorf("%s: unexpected request: %s %#v\n%#v", p, req.Method, req.URL, req)
 					return nil, fmt.Errorf("unexpected request")
 				}
 			}),
@@ -230,18 +261,19 @@ func TestAttach(t *testing.T) {
 func TestAttachWarnings(t *testing.T) {
 	version := api.Registry.GroupOrDie(api.GroupName).GroupVersion.Version
 	tests := []struct {
-		name, container, version, podPath, expectedErr, expectedOut string
+		name, container, version, podPath, fetchPodPath, expectedErr, expectedOut string
-		pod                                                         *api.Pod
+		pod                                                                       *api.Pod
-		stdin, tty                                                  bool
+		stdin, tty                                                                bool
 	}{
 		{
-			name:        "fallback tty if not supported",
+			name:         "fallback tty if not supported",
-			version:     version,
+			version:      version,
-			podPath:     "/api/" + version + "/namespaces/test/pods/foo",
+			podPath:      "/api/" + version + "/namespaces/test/pods/foo",
-			pod:         attachPod(),
+			fetchPodPath: "/namespaces/test/pods/foo",
-			stdin:       true,
+			pod:          attachPod(),
-			tty:         true,
+			stdin:        true,
-			expectedErr: "Unable to use a TTY - container bar did not allocate one",
+			tty:          true,
+			expectedErr:  "Unable to use a TTY - container bar did not allocate one",
 		},
 	}
 	for _, test := range tests {
@@ -254,6 +286,9 @@ func TestAttachWarnings(t *testing.T) {
 				case p == test.podPath && m == "GET":
 					body := objBody(codec, test.pod)
 					return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil
+				case p == test.fetchPodPath && m == "GET":
+					body := objBody(codec, test.pod)
+					return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil
 				default:
 					t.Errorf("%s: unexpected request: %s %#v\n%#v", test.name, req.Method, req.URL, req)
 					return nil, fmt.Errorf("unexpected request")

pkg/kubectl/cmd/testing/fake.go

@@ -609,6 +609,19 @@ func (f *fakeAPIFactory) LogsForObject(object, options runtime.Object) (*restcli
 	}
 }
 
+func (f *fakeAPIFactory) AttachablePodForObject(object runtime.Object) (*api.Pod, error) {
+	switch t := object.(type) {
+	case *api.Pod:
+		return t, nil
+	default:
+		gvks, _, err := api.Scheme.ObjectKinds(object)
+		if err != nil {
+			return nil, err
+		}
+		return nil, fmt.Errorf("cannot attach to %v: not implemented", gvks[0])
+	}
+}
+
 func (f *fakeAPIFactory) Validator(validate bool, cacheDir string) (validation.Schema, error) {
 	return f.tf.Validator, f.tf.Err
 }

pkg/kubectl/cmd/util/factory_object_mapping.go

@@ -41,6 +41,7 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/api/validation"
+	"k8s.io/kubernetes/pkg/apis/apps"
 	"k8s.io/kubernetes/pkg/apis/batch"
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
@@ -324,28 +325,33 @@ func (f *ring1Factory) AttachablePodForObject(object runtime.Object) (*api.Pod,
 	if err != nil {
 		return nil, err
 	}
+	var selector labels.Selector
+	var namespace string
 	switch t := object.(type) {
+	case *extensions.ReplicaSet:
+		namespace = t.Namespace
+		selector = labels.SelectorFromSet(t.Spec.Selector.MatchLabels)
 	case *api.ReplicationController:
-		selector := labels.SelectorFromSet(t.Spec.Selector)
+		namespace = t.Namespace
-		sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }
+		selector = labels.SelectorFromSet(t.Spec.Selector)
-		pod, _, err := GetFirstPod(clientset.Core(), t.Namespace, selector, 1*time.Minute, sortBy)
+	case *apps.StatefulSet:
-		return pod, err
+		namespace = t.Namespace
+		selector, err = metav1.LabelSelectorAsSelector(t.Spec.Selector)
+		if err != nil {
+			return nil, fmt.Errorf("invalid label selector: %v", err)
+		}
 	case *extensions.Deployment:
-		selector, err := metav1.LabelSelectorAsSelector(t.Spec.Selector)
+		namespace = t.Namespace
+		selector, err = metav1.LabelSelectorAsSelector(t.Spec.Selector)
 		if err != nil {
 			return nil, fmt.Errorf("invalid label selector: %v", err)
 		}
-		sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }
-		pod, _, err := GetFirstPod(clientset.Core(), t.Namespace, selector, 1*time.Minute, sortBy)
-		return pod, err
 	case *batch.Job:
-		selector, err := metav1.LabelSelectorAsSelector(t.Spec.Selector)
+		namespace = t.Namespace
+		selector, err = metav1.LabelSelectorAsSelector(t.Spec.Selector)
 		if err != nil {
 			return nil, fmt.Errorf("invalid label selector: %v", err)
 		}
-		sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }
-		pod, _, err := GetFirstPod(clientset.Core(), t.Namespace, selector, 1*time.Minute, sortBy)
-		return pod, err
 	case *api.Pod:
 		return t, nil
 	default:
@@ -355,6 +361,9 @@ func (f *ring1Factory) AttachablePodForObject(object runtime.Object) (*api.Pod,
 		}
 		return nil, fmt.Errorf("cannot attach to %v: not implemented", gvks[0])
 	}
+	sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }
+	pod, _, err := GetFirstPod(clientset.Core(), namespace, selector, 1*time.Minute, sortBy)
+	return pod, err
 }
 
 func (f *ring1Factory) PrinterForMapping(cmd *cobra.Command, mapping *meta.RESTMapping, withNamespace bool) (kubectl.ResourcePrinter, error) {