From 919cb1970650fb872bafe0dfa2e07a750dc17188 Mon Sep 17 00:00:00 2001 From: shiywang Date: Tue, 24 Jan 2017 22:28:52 +0800 Subject: [PATCH] kubectl attach support for multiple types hot fix add unit test and statefulSet update example remove package change to ResourceNames remove some code remove strings add fake testing func for AttachablePodForObject minor change add test.obj nil check update testfile gofmt update add fallthough revert back --- pkg/kubectl/cmd/attach.go | 39 ++++++-- pkg/kubectl/cmd/attach_test.go | 97 +++++++++++++------ pkg/kubectl/cmd/testing/fake.go | 13 +++ .../cmd/util/factory_object_mapping.go | 33 ++++--- 4 files changed, 133 insertions(+), 49 deletions(-) diff --git a/pkg/kubectl/cmd/attach.go b/pkg/kubectl/cmd/attach.go index 5e6820cef1f..0c9f181be87 100644 --- a/pkg/kubectl/cmd/attach.go +++ b/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 { - return cmdutil.UsageError(cmd, fmt.Sprintf("expected a single argument: POD, saw %d: %s", len(argsIn), argsIn)) + if len(argsIn) > 2 { + 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() diff --git a/pkg/kubectl/cmd/attach_test.go b/pkg/kubectl/cmd/attach_test.go index a4bb7db5de2..c79b9972906 100644 --- a/pkg/kubectl/cmd/attach_test.go +++ b/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 - pod *api.Pod - remoteAttachErr bool - exepctedErr string + name, version, podPath, fetchPodPath, attachPath, container string + pod *api.Pod + remoteAttachErr bool + exepctedErr string }{ { - name: "pod attach", - version: version, - podPath: "/api/" + version + "/namespaces/test/pods/foo", - attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach", - pod: attachPod(), - container: "bar", + name: "pod attach", + version: version, + podPath: "/api/" + version + "/namespaces/test/pods/foo", + fetchPodPath: "/namespaces/test/pods/foo", + attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach", + 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", - version: version, - podPath: "/api/" + version + "/namespaces/test/pods/foo", - attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach", - pod: attachPod(), - container: "foo", - exepctedErr: "cannot attach to the container: container not found (foo)", + name: "container not found 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(), + 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 - pod *api.Pod - stdin, tty bool + name, container, version, podPath, fetchPodPath, expectedErr, expectedOut string + pod *api.Pod + stdin, tty bool }{ { - name: "fallback tty if not supported", - version: version, - podPath: "/api/" + version + "/namespaces/test/pods/foo", - pod: attachPod(), - stdin: true, - tty: true, - expectedErr: "Unable to use a TTY - container bar did not allocate one", + name: "fallback tty if not supported", + version: version, + podPath: "/api/" + version + "/namespaces/test/pods/foo", + fetchPodPath: "/namespaces/test/pods/foo", + pod: attachPod(), + stdin: true, + 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") diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 8a42326cffa..67e692f3d77 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -591,6 +591,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 } diff --git a/pkg/kubectl/cmd/util/factory_object_mapping.go b/pkg/kubectl/cmd/util/factory_object_mapping.go index 9bdb07b0a63..9c0dcd79cf9 100644 --- a/pkg/kubectl/cmd/util/factory_object_mapping.go +++ b/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) - 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 + namespace = t.Namespace + selector = labels.SelectorFromSet(t.Spec.Selector) + case *apps.StatefulSet: + 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) {