From 9120557466df086ede14645a1a143b73b5e299fa Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 23 Jul 2018 16:43:50 -0400 Subject: [PATCH 1/2] update attach to use external objs --- pkg/kubectl/cmd/BUILD | 1 + pkg/kubectl/cmd/attach.go | 266 +++++++-------- pkg/kubectl/cmd/attach_test.go | 305 ++++++++++-------- pkg/kubectl/cmd/portforward.go | 9 +- pkg/kubectl/cmd/run.go | 46 +-- pkg/kubectl/conditions.go | 21 ++ pkg/kubectl/polymorphichelpers/BUILD | 5 +- .../attachablepodforobject.go | 17 +- pkg/kubectl/polymorphichelpers/helpers.go | 15 +- .../polymorphichelpers/helpers_test.go | 83 +++-- pkg/kubectl/polymorphichelpers/interface.go | 3 +- .../polymorphichelpers/logsforobject.go | 17 +- .../polymorphichelpers/logsforobject_test.go | 32 +- test/e2e/kubectl/kubectl.go | 2 +- 14 files changed, 440 insertions(+), 382 deletions(-) diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index cdc0a300a9a..2dc32bb870c 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -59,6 +59,7 @@ go_library( "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/certificates:go_default_library", "//pkg/apis/core:go_default_library", + "//pkg/apis/core/v1:go_default_library", "//pkg/apis/core/validation:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/batch/internalversion:go_default_library", diff --git a/pkg/kubectl/cmd/attach.go b/pkg/kubectl/cmd/attach.go index 88ff4f7d415..7f01eafa3ba 100644 --- a/pkg/kubectl/cmd/attach.go +++ b/pkg/kubectl/cmd/attach.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "errors" "fmt" "io" "net/url" @@ -26,17 +25,17 @@ import ( "github.com/golang/glog" "github.com/spf13/cobra" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilerrors "k8s.io/apimachinery/pkg/util/errors" + apiv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/remotecommand" "k8s.io/kubernetes/pkg/api/legacyscheme" - api "k8s.io/kubernetes/pkg/apis/core" - coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" + "k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource" "k8s.io/kubernetes/pkg/kubectl/polymorphichelpers" + "k8s.io/kubernetes/pkg/kubectl/scheme" "k8s.io/kubernetes/pkg/kubectl/util/i18n" ) @@ -62,14 +61,40 @@ const ( defaultPodLogsTimeout = 20 * time.Second ) -func NewCmdAttach(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command { - o := &AttachOptions{ +// AttachOptions declare the arguments accepted by the Exec command +type AttachOptions struct { + StreamOptions + + // whether to disable use of standard error when streaming output from tty + DisableStderr bool + + CommandName string + SuggestedCmdUsage string + + Pod *apiv1.Pod + + AttachFunc func(*AttachOptions, *apiv1.Container, bool, remotecommand.TerminalSizeQueue) func() error + Resources []string + Builder func() *resource.Builder + AttachablePodFn polymorphichelpers.AttachableLogsForObjectFunc + restClientGetter genericclioptions.RESTClientGetter + + Attach RemoteAttach + GetPodTimeout time.Duration + Config *restclient.Config +} + +func NewAttachOptions(streams genericclioptions.IOStreams) *AttachOptions { + return &AttachOptions{ StreamOptions: StreamOptions{ IOStreams: streams, }, - Attach: &DefaultRemoteAttach{}, } +} + +func NewCmdAttach(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command { + o := NewAttachOptions(streams) cmd := &cobra.Command{ Use: "attach (POD | TYPE/NAME) -c CONTAINER", DisableFlagsInUseLine: true, @@ -94,6 +119,29 @@ type RemoteAttach interface { Attach(method string, url *url.URL, config *restclient.Config, stdin io.Reader, stdout, stderr io.Writer, tty bool, terminalSizeQueue remotecommand.TerminalSizeQueue) error } +func defaultAttachFunc(o *AttachOptions, containerToAttach *apiv1.Container, raw bool, sizeQueue remotecommand.TerminalSizeQueue) func() error { + return func() error { + restClient, err := restclient.RESTClientFor(o.Config) + if err != nil { + return err + } + req := restClient.Post(). + Resource("pods"). + Name(o.Pod.Name). + Namespace(o.Pod.Namespace). + SubResource("attach") + req.VersionedParams(&apiv1.PodAttachOptions{ + Container: containerToAttach.Name, + Stdin: o.Stdin, + Stdout: o.Out != nil, + Stderr: !o.DisableStderr, + TTY: raw, + }, legacyscheme.ParameterCodec) + + return o.Attach.Attach("POST", req.URL(), o.Config, o.In, o.Out, o.ErrOut, raw, sizeQueue) + } +} + // DefaultRemoteAttach is the standard implementation of attaching type DefaultRemoteAttach struct{} @@ -111,63 +159,24 @@ func (*DefaultRemoteAttach) Attach(method string, url *url.URL, config *restclie }) } -// AttachOptions declare the arguments accepted by the Exec command -type AttachOptions struct { - StreamOptions - - CommandName string - SuggestedCmdUsage string - - Pod *api.Pod - - Attach RemoteAttach - PodClient coreclient.PodsGetter - GetPodTimeout time.Duration - Config *restclient.Config -} - // 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.UsageErrorf(cmd, "at least 1 argument is required for attach") - } - if len(argsIn) > 2 { - return cmdutil.UsageErrorf(cmd, "expected POD, TYPE/NAME, or TYPE NAME, (at most 2 arguments) saw %d: %v", len(argsIn), argsIn) - } - - namespace, _, err := f.ToRawKubeConfigLoader().Namespace() +func (o *AttachOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { + var err error + o.Namespace, _, err = f.ToRawKubeConfigLoader().Namespace() if err != nil { return err } - p.GetPodTimeout, err = cmdutil.GetPodRunningTimeoutFlag(cmd) + o.AttachablePodFn = polymorphichelpers.AttachablePodForObjectFn + + o.GetPodTimeout, err = cmdutil.GetPodRunningTimeoutFlag(cmd) if err != nil { return cmdutil.UsageErrorf(cmd, err.Error()) } - builder := f.NewBuilder(). - WithScheme(legacyscheme.Scheme). - 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 := polymorphichelpers.AttachablePodForObjectFn(f, obj, p.GetPodTimeout) - if err != nil { - return err - } - - p.PodName = attachablePod.Name - p.Namespace = namespace + o.Builder = f.NewBuilder + o.Resources = args + o.restClientGetter = f fullCmdName := "" cmdParent := cmd.Parent() @@ -175,82 +184,87 @@ func (p *AttachOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, argsIn [ fullCmdName = cmdParent.CommandPath() } if len(fullCmdName) > 0 && cmdutil.IsSiblingCommandExists(cmd, "describe") { - p.SuggestedCmdUsage = fmt.Sprintf("Use '%s describe pod/%s -n %s' to see all of the containers in this pod.", fullCmdName, p.PodName, p.Namespace) + o.SuggestedCmdUsage = fmt.Sprintf("Use '%s describe pod/%s -n %s' to see all of the containers in this pod.", fullCmdName, o.PodName, o.Namespace) } config, err := f.ToRESTConfig() if err != nil { return err } - p.Config = config + o.Config = config - clientset, err := f.ClientSet() - if err != nil { - return err - } + o.AttachFunc = defaultAttachFunc - p.PodClient = clientset.Core() - - if p.CommandName == "" { - p.CommandName = cmd.CommandPath() + if o.CommandName == "" { + o.CommandName = cmd.CommandPath() } return nil } // Validate checks that the provided attach options are specified. -func (p *AttachOptions) Validate() error { - allErrs := []error{} - if len(p.PodName) == 0 { - allErrs = append(allErrs, errors.New("pod name must be specified")) +func (o *AttachOptions) Validate() error { + if len(o.Resources) == 0 { + return fmt.Errorf("at least 1 argument is required for attach") } - if p.Out == nil || p.ErrOut == nil { - allErrs = append(allErrs, errors.New("both output and error output must be provided")) + if len(o.Resources) > 2 { + return fmt.Errorf("expected POD, TYPE/NAME, or TYPE NAME, (at most 2 arguments) saw %d: %v", len(o.Resources), o.Resources) } - if p.Attach == nil || p.PodClient == nil || p.Config == nil { - allErrs = append(allErrs, errors.New("client, client config, and attach must be provided")) + if o.GetPodTimeout <= 0 { + return fmt.Errorf("--pod-running-timeout must be higher than zero") } - return utilerrors.NewAggregate(allErrs) + + return nil } // Run executes a validated remote execution against a pod. -func (p *AttachOptions) Run() error { - if p.Pod == nil { - pod, err := p.PodClient.Pods(p.Namespace).Get(p.PodName, metav1.GetOptions{}) +func (o *AttachOptions) Run() error { + if o.Pod == nil { + b := o.Builder(). + WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...). + NamespaceParam(o.Namespace).DefaultNamespace() + + switch len(o.Resources) { + case 1: + b.ResourceNames("pods", o.Resources[0]) + case 2: + b.ResourceNames(o.Resources[0], o.Resources[1]) + } + + obj, err := b.Do().Object() if err != nil { return err } - if pod.Status.Phase == api.PodSucceeded || pod.Status.Phase == api.PodFailed { - return fmt.Errorf("cannot attach a container in a completed pod; current phase is %s", pod.Status.Phase) + o.Pod, err = o.findAttachablePod(obj) + if err != nil { + return err } - p.Pod = pod + if o.Pod.Status.Phase == apiv1.PodSucceeded || o.Pod.Status.Phase == apiv1.PodFailed { + return fmt.Errorf("cannot attach a container in a completed pod; current phase is %s", o.Pod.Status.Phase) + } // TODO: convert this to a clean "wait" behavior } - pod := p.Pod // check for TTY - containerToAttach, err := p.containerToAttachTo(pod) + containerToAttach, err := o.containerToAttachTo(o.Pod) if err != nil { return fmt.Errorf("cannot attach to the container: %v", err) } - if p.TTY && !containerToAttach.TTY { - p.TTY = false - if p.ErrOut != nil { - fmt.Fprintf(p.ErrOut, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name) + if o.TTY && !containerToAttach.TTY { + o.TTY = false + if o.ErrOut != nil { + fmt.Fprintf(o.ErrOut, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name) } - } else if !p.TTY && containerToAttach.TTY { + } else if !o.TTY && containerToAttach.TTY { // the container was launched with a TTY, so we have to force a TTY here, otherwise you'll get // an error "Unrecognized input header" - p.TTY = true + o.TTY = true } // ensure we can recover the terminal while attached - t := p.setupTTY() - - // save p.Err so we can print the command prompt message below - stderr := p.ErrOut + t := o.setupTTY() var sizeQueue remotecommand.TerminalSizeQueue if t.Raw { @@ -265,66 +279,52 @@ func (p *AttachOptions) Run() error { sizeQueue = t.MonitorSize(&sizePlusOne, size) } - // unset p.Err if it was previously set because both stdout and stderr go over p.Out when tty is - // true - p.ErrOut = nil + o.DisableStderr = true } - fn := func() error { - restClient, err := restclient.RESTClientFor(p.Config) - if err != nil { - return err - } - // TODO: consider abstracting into a client invocation or client helper - req := restClient.Post(). - Resource("pods"). - Name(pod.Name). - Namespace(pod.Namespace). - SubResource("attach") - req.VersionedParams(&api.PodAttachOptions{ - Container: containerToAttach.Name, - Stdin: p.Stdin, - Stdout: p.Out != nil, - Stderr: p.ErrOut != nil, - TTY: t.Raw, - }, legacyscheme.ParameterCodec) - - return p.Attach.Attach("POST", req.URL(), p.Config, p.In, p.Out, p.ErrOut, t.Raw, sizeQueue) + if !o.Quiet { + fmt.Fprintln(o.ErrOut, "If you don't see a command prompt, try pressing enter.") } - - if !p.Quiet && stderr != nil { - fmt.Fprintln(stderr, "If you don't see a command prompt, try pressing enter.") - } - if err := t.Safe(fn); err != nil { + if err := t.Safe(o.AttachFunc(o, containerToAttach, t.Raw, sizeQueue)); err != nil { return err } - if p.Stdin && t.Raw && pod.Spec.RestartPolicy == api.RestartPolicyAlways { - fmt.Fprintf(p.Out, "Session ended, resume using '%s %s -c %s -i -t' command when the pod is running\n", p.CommandName, pod.Name, containerToAttach.Name) + if o.Stdin && t.Raw && o.Pod.Spec.RestartPolicy == apiv1.RestartPolicyAlways { + fmt.Fprintf(o.Out, "Session ended, resume using '%s %s -c %s -i -t' command when the pod is running\n", o.CommandName, o.Pod.Name, containerToAttach.Name) } return nil } +func (o *AttachOptions) findAttachablePod(obj runtime.Object) (*apiv1.Pod, error) { + attachablePod, err := o.AttachablePodFn(o.restClientGetter, obj, o.GetPodTimeout) + if err != nil { + return nil, err + } + + o.StreamOptions.PodName = attachablePod.Name + return attachablePod, nil +} + // containerToAttach returns a reference to the container to attach to, given // by name or the first container if name is empty. -func (p *AttachOptions) containerToAttachTo(pod *api.Pod) (*api.Container, error) { - if len(p.ContainerName) > 0 { +func (o *AttachOptions) containerToAttachTo(pod *apiv1.Pod) (*apiv1.Container, error) { + if len(o.ContainerName) > 0 { for i := range pod.Spec.Containers { - if pod.Spec.Containers[i].Name == p.ContainerName { + if pod.Spec.Containers[i].Name == o.ContainerName { return &pod.Spec.Containers[i], nil } } for i := range pod.Spec.InitContainers { - if pod.Spec.InitContainers[i].Name == p.ContainerName { + if pod.Spec.InitContainers[i].Name == o.ContainerName { return &pod.Spec.InitContainers[i], nil } } - return nil, fmt.Errorf("container not found (%s)", p.ContainerName) + return nil, fmt.Errorf("container not found (%s)", o.ContainerName) } - if len(p.SuggestedCmdUsage) > 0 { - fmt.Fprintf(p.ErrOut, "Defaulting container name to %s.\n", pod.Spec.Containers[0].Name) - fmt.Fprintf(p.ErrOut, "%s\n", p.SuggestedCmdUsage) + if len(o.SuggestedCmdUsage) > 0 { + fmt.Fprintf(o.ErrOut, "Defaulting container name to %s.\n", pod.Spec.Containers[0].Name) + fmt.Fprintf(o.ErrOut, "%s\n", o.SuggestedCmdUsage) } glog.V(4).Infof("defaulting container name to %s", pod.Spec.Containers[0].Name) @@ -332,8 +332,8 @@ func (p *AttachOptions) containerToAttachTo(pod *api.Pod) (*api.Container, error } // GetContainerName returns the name of the container to attach to, with a fallback. -func (p *AttachOptions) GetContainerName(pod *api.Pod) (string, error) { - c, err := p.containerToAttachTo(pod) +func (o *AttachOptions) GetContainerName(pod *apiv1.Pod) (string, error) { + c, err := o.containerToAttachTo(pod) if err != nil { return "", err } diff --git a/pkg/kubectl/cmd/attach_test.go b/pkg/kubectl/cmd/attach_test.go index 007df0dccdf..3b8236c2a1a 100644 --- a/pkg/kubectl/cmd/attach_test.go +++ b/pkg/kubectl/cmd/attach_test.go @@ -25,8 +25,7 @@ import ( "testing" "time" - "github.com/spf13/cobra" - + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -34,10 +33,9 @@ import ( "k8s.io/client-go/rest/fake" "k8s.io/client-go/tools/remotecommand" "k8s.io/kubernetes/pkg/api/legacyscheme" - api "k8s.io/kubernetes/pkg/apis/core" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" - cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" + "k8s.io/kubernetes/pkg/kubectl/polymorphichelpers" "k8s.io/kubernetes/pkg/kubectl/scheme" ) @@ -53,130 +51,136 @@ func (f *fakeRemoteAttach) Attach(method string, url *url.URL, config *restclien return f.err } +func fakeAttachablePodFn(pod *corev1.Pod) polymorphichelpers.AttachableLogsForObjectFunc { + return func(getter genericclioptions.RESTClientGetter, obj runtime.Object, timeout time.Duration) (*corev1.Pod, error) { + return pod, nil + } +} + func TestPodAndContainerAttach(t *testing.T) { tests := []struct { - args []string - p *AttachOptions - name string - expectError bool - expectedPod string - expectedContainer string - timeout time.Duration - obj runtime.Object + name string + args []string + options *AttachOptions + expectError string + expectedPodName string + expectedContainerName string + obj *corev1.Pod }{ { - p: &AttachOptions{}, - expectError: true, name: "empty", - timeout: 1, + options: &AttachOptions{GetPodTimeout: 1}, + expectError: "at least 1 argument is required", }, { - p: &AttachOptions{}, - args: []string{"one", "two", "three"}, - expectError: true, name: "too many args", - timeout: 2, + options: &AttachOptions{GetPodTimeout: 2}, + args: []string{"one", "two", "three"}, + expectError: "at most 2 arguments", }, { - p: &AttachOptions{}, - args: []string{"foo"}, - expectedPod: "foo", - name: "no container, no flags", + name: "no container, no flags", + options: &AttachOptions{GetPodTimeout: defaultPodLogsTimeout}, + args: []string{"foo"}, + expectedPodName: "foo", + expectedContainerName: "bar", + obj: attachPod(), + }, + { + name: "container in flag", + options: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}, GetPodTimeout: 10000000}, + args: []string{"foo"}, + expectedPodName: "foo", + expectedContainerName: "bar", + obj: attachPod(), + }, + { + name: "init container in flag", + options: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "initfoo"}, GetPodTimeout: 30}, + args: []string{"foo"}, + expectedPodName: "foo", + expectedContainerName: "initfoo", + obj: attachPod(), + }, + { + name: "non-existing container", + options: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "wrong"}, GetPodTimeout: 10}, + args: []string{"foo"}, + expectedPodName: "foo", + expectError: "container not found", + obj: attachPod(), + }, + { + name: "no container, no flags, pods and name", + options: &AttachOptions{GetPodTimeout: 10000}, + args: []string{"pods", "foo"}, + expectedPodName: "foo", + expectedContainerName: "bar", + obj: attachPod(), + }, + { + name: "invalid get pod timeout value", + options: &AttachOptions{GetPodTimeout: 0}, + args: []string{"pod/foo"}, + expectedPodName: "foo", + expectedContainerName: "bar", obj: attachPod(), - timeout: defaultPodLogsTimeout, - }, - { - p: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}}, - args: []string{"foo"}, - expectedPod: "foo", - expectedContainer: "bar", - name: "container in flag", - obj: attachPod(), - timeout: 10000000, - }, - { - p: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "initfoo"}}, - args: []string{"foo"}, - expectedPod: "foo", - expectedContainer: "initfoo", - name: "init container in flag", - obj: attachPod(), - timeout: 30, - }, - { - p: &AttachOptions{StreamOptions: StreamOptions{ContainerName: "bar"}}, - args: []string{"foo", "-c", "wrong"}, - expectError: true, - name: "non-existing container in flag", - obj: attachPod(), - timeout: 10, - }, - { - p: &AttachOptions{}, - args: []string{"pods", "foo"}, - expectedPod: "foo", - name: "no container, no flags, pods and name", - obj: attachPod(), - timeout: 10000, - }, - { - p: &AttachOptions{}, - args: []string{"pod/foo"}, - expectedPod: "foo", - name: "no container, no flags, pod/name", - obj: attachPod(), - timeout: 1, - }, - { - p: &AttachOptions{}, - args: []string{"pod/foo"}, - expectedPod: "foo", - name: "invalid get pod timeout value", - obj: attachPod(), - expectError: true, - timeout: 0, + expectError: "must be higher than zero", }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - tf := cmdtesting.NewTestFactory().WithNamespace("test") - defer tf.Cleanup() + // setup opts to fetch our test pod + test.options.AttachablePodFn = fakeAttachablePodFn(test.obj) + test.options.Resources = test.args - codec := legacyscheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) - ns := legacyscheme.Codecs - - tf.Client = &fake.RESTClient{ - GroupVersion: schema.GroupVersion{Group: "", Version: "v1"}, - NegotiatedSerializer: ns, - 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.ClientConfigVal = defaultClientConfig() - - cmd := &cobra.Command{} - options := test.p - cmdutil.AddPodRunningTimeoutFlag(cmd, test.timeout) - - err := options.Complete(tf, cmd, test.args) - if test.expectError && err == nil { - t.Errorf("%s: unexpected non-error", test.name) - } - if !test.expectError && err != nil { - t.Errorf("%s: unexpected error: %v", test.name, err) - } - if err != nil { + if err := test.options.Validate(); err != nil { + if !strings.Contains(err.Error(), test.expectError) { + t.Errorf("unexpected error: expected %q, got %q", test.expectError, err) + } return } - if options.PodName != test.expectedPod { - t.Errorf("%s: expected: %s, got: %s", test.name, test.expectedPod, options.PodName) + + pod, err := test.options.findAttachablePod(&corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pod", Namespace: "test"}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "foobar", + }, + }, + }, + }) + if err != nil { + if !strings.Contains(err.Error(), test.expectError) { + t.Errorf("unexpected error: expected %q, got %q", err, test.expectError) + } + return } - if options.ContainerName != test.expectedContainer { - t.Errorf("%s: expected: %s, got: %s", test.name, test.expectedContainer, options.ContainerName) + + if pod.Name != test.expectedPodName { + t.Errorf("unexpected pod name: expected %q, got %q", test.expectedContainerName, pod.Name) + } + + container, err := test.options.containerToAttachTo(attachPod()) + if err != nil { + if !strings.Contains(err.Error(), test.expectError) { + t.Errorf("unexpected error: expected %q, got %q", err, test.expectError) + } + return + } + + if container.Name != test.expectedContainerName { + t.Errorf("unexpected container name: expected %q, got %q", test.expectedContainerName, container.Name) + } + + if test.options.PodName != test.expectedPodName { + t.Errorf("%s: expected: %s, got: %s", test.name, test.expectedPodName, test.options.PodName) + } + + if len(test.expectError) > 0 { + t.Fatalf("expected error %q, but saw none", test.expectError) } }) } @@ -186,7 +190,7 @@ func TestAttach(t *testing.T) { version := "v1" tests := []struct { name, version, podPath, fetchPodPath, attachPath, container string - pod *api.Pod + pod *corev1.Pod remoteAttachErr bool exepctedErr string }{ @@ -247,11 +251,12 @@ func TestAttach(t *testing.T) { }), } tf.ClientConfigVal = &restclient.Config{APIPath: "/api", ContentConfig: restclient.ContentConfig{NegotiatedSerializer: legacyscheme.Codecs, GroupVersion: &schema.GroupVersion{Version: test.version}}} + remoteAttach := &fakeRemoteAttach{} if test.remoteAttachErr { remoteAttach.err = fmt.Errorf("attach error") } - params := &AttachOptions{ + options := &AttachOptions{ StreamOptions: StreamOptions{ ContainerName: test.container, IOStreams: genericclioptions.NewTestIOStreamsDiscard(), @@ -259,12 +264,24 @@ func TestAttach(t *testing.T) { Attach: remoteAttach, GetPodTimeout: 1000, } - cmd := &cobra.Command{} - cmdutil.AddPodRunningTimeoutFlag(cmd, 1000) - if err := params.Complete(tf, cmd, []string{"foo"}); err != nil { - t.Fatal(err) + + options.restClientGetter = tf + options.Namespace = "test" + options.Resources = []string{"foo"} + options.Builder = tf.NewBuilder + options.AttachablePodFn = fakeAttachablePodFn(test.pod) + options.AttachFunc = func(opts *AttachOptions, containerToAttach *corev1.Container, raw bool, sizeQueue remotecommand.TerminalSizeQueue) func() error { + return func() error { + u, err := url.Parse(fmt.Sprintf("%s?container=%s", test.attachPath, containerToAttach.Name)) + if err != nil { + return err + } + + return options.Attach.Attach("POST", u, nil, nil, nil, nil, raw, sizeQueue) + } } - err := params.Run() + + err := options.Run() if test.exepctedErr != "" && err.Error() != test.exepctedErr { t.Errorf("%s: Unexpected exec error: %v", test.name, err) return @@ -294,7 +311,7 @@ func TestAttachWarnings(t *testing.T) { version := "v1" tests := []struct { name, container, version, podPath, fetchPodPath, expectedErr string - pod *api.Pod + pod *corev1.Pod stdin, tty bool }{ { @@ -313,6 +330,8 @@ func TestAttachWarnings(t *testing.T) { tf := cmdtesting.NewTestFactory().WithNamespace("test") defer tf.Cleanup() + streams, _, _, bufErr := genericclioptions.NewTestIOStreams() + codec := legacyscheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) ns := legacyscheme.Codecs @@ -328,30 +347,42 @@ func TestAttachWarnings(t *testing.T) { 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) + t.Errorf("%s: unexpected request: %s %#v\n%#v", p, req.Method, req.URL, req) return nil, fmt.Errorf("unexpected request") } }), } tf.ClientConfigVal = &restclient.Config{APIPath: "/api", ContentConfig: restclient.ContentConfig{NegotiatedSerializer: legacyscheme.Codecs, GroupVersion: &schema.GroupVersion{Version: test.version}}} - streams, _, _, bufErr := genericclioptions.NewTestIOStreams() - ex := &fakeRemoteAttach{} - params := &AttachOptions{ + + options := &AttachOptions{ StreamOptions: StreamOptions{ - ContainerName: test.container, - IOStreams: streams, Stdin: test.stdin, TTY: test.tty, + ContainerName: test.container, + IOStreams: streams, }, - Attach: ex, + + Attach: &fakeRemoteAttach{}, GetPodTimeout: 1000, } - cmd := &cobra.Command{} - cmdutil.AddPodRunningTimeoutFlag(cmd, 1000) - if err := params.Complete(tf, cmd, []string{"foo"}); err != nil { - t.Fatal(err) + + options.restClientGetter = tf + options.Namespace = "test" + options.Resources = []string{"foo"} + options.Builder = tf.NewBuilder + options.AttachablePodFn = fakeAttachablePodFn(test.pod) + options.AttachFunc = func(opts *AttachOptions, containerToAttach *corev1.Container, raw bool, sizeQueue remotecommand.TerminalSizeQueue) func() error { + return func() error { + u, err := url.Parse("http://foo.bar") + if err != nil { + return err + } + + return options.Attach.Attach("POST", u, nil, nil, nil, nil, raw, sizeQueue) + } } - if err := params.Run(); err != nil { + + if err := options.Run(); err != nil { t.Fatal(err) } @@ -367,25 +398,25 @@ func TestAttachWarnings(t *testing.T) { } } -func attachPod() *api.Pod { - return &api.Pod{ +func attachPod() *corev1.Pod { + return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test", ResourceVersion: "10"}, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyAlways, - DNSPolicy: api.DNSClusterFirst, - Containers: []api.Container{ + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + DNSPolicy: corev1.DNSClusterFirst, + Containers: []corev1.Container{ { Name: "bar", }, }, - InitContainers: []api.Container{ + InitContainers: []corev1.Container{ { Name: "initfoo", }, }, }, - Status: api.PodStatus{ - Phase: api.PodRunning, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, }, } } diff --git a/pkg/kubectl/cmd/portforward.go b/pkg/kubectl/cmd/portforward.go index e32b349acf1..27bb52904bc 100644 --- a/pkg/kubectl/cmd/portforward.go +++ b/pkg/kubectl/cmd/portforward.go @@ -34,6 +34,7 @@ import ( "k8s.io/client-go/transport/spdy" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" + apiv1 "k8s.io/kubernetes/pkg/apis/core/v1" coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -210,7 +211,13 @@ func (o *PortForwardOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg // handle service port mapping to target port if needed switch t := obj.(type) { case *api.Service: - o.Ports, err = translateServicePortToTargetPort(args[1:], *t, *forwardablePod) + // TODO(juanvallejo): remove this once we convert this command to work with externals + internalPod := &api.Pod{} + if err := apiv1.Convert_v1_Pod_To_core_Pod(forwardablePod, internalPod, nil); err != nil { + return err + } + + o.Ports, err = translateServicePortToTargetPort(args[1:], *t, *internalPod) if err != nil { return err } diff --git a/pkg/kubectl/cmd/run.go b/pkg/kubectl/cmd/run.go index 35c062ce3a6..e90c297f787 100644 --- a/pkg/kubectl/cmd/run.go +++ b/pkg/kubectl/cmd/run.go @@ -22,18 +22,20 @@ import ( "github.com/docker/distribution/reference" "github.com/spf13/cobra" - "k8s.io/client-go/dynamic" - "github.com/golang/glog" + + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" + coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" - coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -377,27 +379,27 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e return err } opts.Config = config + opts.AttachFunc = defaultAttachFunc - clientset, err := f.ClientSet() + clientset, err := kubernetes.NewForConfig(config) if err != nil { return err } - opts.PodClient = clientset.Core() attachablePod, err := polymorphichelpers.AttachablePodForObjectFn(f, runObject.Object, opts.GetPodTimeout) if err != nil { return err } - err = handleAttachPod(f, clientset.Core(), attachablePod.Namespace, attachablePod.Name, opts) + err = handleAttachPod(f, clientset.CoreV1(), attachablePod.Namespace, attachablePod.Name, opts) if err != nil { return err } - var pod *api.Pod + var pod *corev1.Pod leaveStdinOpen := o.LeaveStdinOpen waitForExitCode := !leaveStdinOpen && restartPolicy == api.RestartPolicyNever if waitForExitCode { - pod, err = waitForPod(clientset.Core(), attachablePod.Namespace, attachablePod.Name, kubectl.PodCompleted) + pod, err = waitForPod(clientset.CoreV1(), attachablePod.Namespace, attachablePod.Name, kubectl.PodCompleted) if err != nil { return err } @@ -409,9 +411,9 @@ func (o *RunOptions) Run(f cmdutil.Factory, cmd *cobra.Command, args []string) e } switch pod.Status.Phase { - case api.PodSucceeded: + case corev1.PodSucceeded: return nil - case api.PodFailed: + case corev1.PodFailed: unknownRcErr := fmt.Errorf("pod %s/%s failed with unknown exit code", pod.Namespace, pod.Name) if len(pod.Status.ContainerStatuses) == 0 || pod.Status.ContainerStatuses[0].State.Terminated == nil { return unknownRcErr @@ -466,20 +468,20 @@ func (o *RunOptions) removeCreatedObjects(f cmdutil.Factory, createdObjects []*R } // waitForPod watches the given pod until the exitCondition is true -func waitForPod(podClient coreclient.PodsGetter, ns, name string, exitCondition watch.ConditionFunc) (*api.Pod, error) { +func waitForPod(podClient coreclientv1.PodsGetter, ns, name string, exitCondition watch.ConditionFunc) (*corev1.Pod, error) { w, err := podClient.Pods(ns).Watch(metav1.SingleObject(metav1.ObjectMeta{Name: name})) if err != nil { return nil, err } intr := interrupt.New(nil, w.Stop) - var result *api.Pod + var result *corev1.Pod err = intr.Run(func() error { ev, err := watch.Until(0, w, func(ev watch.Event) (bool, error) { return exitCondition(ev) }) if ev != nil { - result = ev.Object.(*api.Pod) + result = ev.Object.(*corev1.Pod) } return err }) @@ -492,37 +494,39 @@ func waitForPod(podClient coreclient.PodsGetter, ns, name string, exitCondition return result, err } -func handleAttachPod(f cmdutil.Factory, podClient coreclient.PodsGetter, ns, name string, opts *AttachOptions) error { +func handleAttachPod(f cmdutil.Factory, podClient coreclientv1.PodsGetter, ns, name string, opts *AttachOptions) error { pod, err := waitForPod(podClient, ns, name, kubectl.PodRunningAndReady) if err != nil && err != kubectl.ErrPodCompleted { return err } - if pod.Status.Phase == api.PodSucceeded || pod.Status.Phase == api.PodFailed { + if pod.Status.Phase == corev1.PodSucceeded || pod.Status.Phase == corev1.PodFailed { return logOpts(f, pod, opts) } - opts.PodClient = podClient + opts.Pod = pod opts.PodName = name opts.Namespace = ns - // TODO: opts.Run sets opts.Err to nil, we need to find a better way - stderr := opts.ErrOut + if opts.AttachFunc == nil { + opts.AttachFunc = defaultAttachFunc + } + if err := opts.Run(); err != nil { - fmt.Fprintf(stderr, "Error attaching, falling back to logs: %v\n", err) + fmt.Fprintf(opts.ErrOut, "Error attaching, falling back to logs: %v\n", err) return logOpts(f, pod, opts) } return nil } // logOpts logs output from opts to the pods log. -func logOpts(restClientGetter genericclioptions.RESTClientGetter, pod *api.Pod, opts *AttachOptions) error { +func logOpts(restClientGetter genericclioptions.RESTClientGetter, pod *corev1.Pod, opts *AttachOptions) error { ctrName, err := opts.GetContainerName(pod) if err != nil { return err } - requests, err := polymorphichelpers.LogsForObjectFn(restClientGetter, pod, &api.PodLogOptions{Container: ctrName}, opts.GetPodTimeout, false) + requests, err := polymorphichelpers.LogsForObjectFn(restClientGetter, pod, &corev1.PodLogOptions{Container: ctrName}, opts.GetPodTimeout, false) if err != nil { return err } diff --git a/pkg/kubectl/conditions.go b/pkg/kubectl/conditions.go index 771ccbc4fb8..d28b74ed53f 100644 --- a/pkg/kubectl/conditions.go +++ b/pkg/kubectl/conditions.go @@ -19,12 +19,14 @@ package kubectl import ( "fmt" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" "k8s.io/kubernetes/pkg/api/pod" + podv1 "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/apis/apps" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/extensions" @@ -140,6 +142,13 @@ func PodRunning(event watch.Event) (bool, error) { case api.PodFailed, api.PodSucceeded: return false, ErrPodCompleted } + case *corev1.Pod: + switch t.Status.Phase { + case corev1.PodRunning: + return true, nil + case corev1.PodFailed, corev1.PodSucceeded: + return false, ErrPodCompleted + } } return false, nil } @@ -157,6 +166,11 @@ func PodCompleted(event watch.Event) (bool, error) { case api.PodFailed, api.PodSucceeded: return true, nil } + case *corev1.Pod: + switch t.Status.Phase { + case corev1.PodFailed, corev1.PodSucceeded: + return true, nil + } } return false, nil } @@ -177,6 +191,13 @@ func PodRunningAndReady(event watch.Event) (bool, error) { case api.PodRunning: return pod.IsPodReady(t), nil } + case *corev1.Pod: + switch t.Status.Phase { + case corev1.PodFailed, corev1.PodSucceeded: + return false, ErrPodCompleted + case corev1.PodRunning: + return podv1.IsPodReady(t), nil + } } return false, nil } diff --git a/pkg/kubectl/polymorphichelpers/BUILD b/pkg/kubectl/polymorphichelpers/BUILD index 9771a8b7cd4..7cd53f52e52 100644 --- a/pkg/kubectl/polymorphichelpers/BUILD +++ b/pkg/kubectl/polymorphichelpers/BUILD @@ -28,8 +28,6 @@ go_library( "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", "//pkg/apis/extensions:go_default_library", - "//pkg/client/clientset_generated/internalclientset:go_default_library", - "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", "//pkg/controller:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/genericclioptions:go_default_library", @@ -49,6 +47,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", ], ) @@ -73,7 +72,6 @@ go_test( "//pkg/apis/batch:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/extensions:go_default_library", - "//pkg/client/clientset_generated/internalclientset/fake:go_default_library", "//pkg/controller:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", "//staging/src/k8s.io/api/apps/v1beta1:go_default_library", @@ -90,6 +88,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", ], ) diff --git a/pkg/kubectl/polymorphichelpers/attachablepodforobject.go b/pkg/kubectl/polymorphichelpers/attachablepodforobject.go index 5e8b7f1850e..0e1cdbdc48d 100644 --- a/pkg/kubectl/polymorphichelpers/attachablepodforobject.go +++ b/pkg/kubectl/polymorphichelpers/attachablepodforobject.go @@ -24,30 +24,29 @@ import ( "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" api "k8s.io/kubernetes/pkg/apis/core" apiv1 "k8s.io/kubernetes/pkg/apis/core/v1" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" ) // attachablePodForObject returns the pod to which to attach given an object. -func attachablePodForObject(restClientGetter genericclioptions.RESTClientGetter, object runtime.Object, timeout time.Duration) (*api.Pod, error) { +func attachablePodForObject(restClientGetter genericclioptions.RESTClientGetter, object runtime.Object, timeout time.Duration) (*corev1.Pod, error) { switch t := object.(type) { case *api.Pod: - return t, nil + externalPod := &corev1.Pod{} + err := apiv1.Convert_core_Pod_To_v1_Pod(t, externalPod, nil) + return externalPod, err case *corev1.Pod: - internalPod := &api.Pod{} - err := apiv1.Convert_v1_Pod_To_core_Pod(t, internalPod, nil) - return internalPod, err - + return t, nil } clientConfig, err := restClientGetter.ToRESTConfig() if err != nil { return nil, err } - clientset, err := internalclientset.NewForConfig(clientConfig) + clientset, err := corev1client.NewForConfig(clientConfig) if err != nil { return nil, err } @@ -57,6 +56,6 @@ func attachablePodForObject(restClientGetter genericclioptions.RESTClientGetter, return nil, fmt.Errorf("cannot attach to %T: %v", object, err) } sortBy := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) } - pod, _, err := GetFirstPod(clientset.Core(), namespace, selector.String(), timeout, sortBy) + pod, _, err := GetFirstPod(clientset, namespace, selector.String(), timeout, sortBy) return pod, err } diff --git a/pkg/kubectl/polymorphichelpers/helpers.go b/pkg/kubectl/polymorphichelpers/helpers.go index 5d9f0c3336f..3e1ba59cceb 100644 --- a/pkg/kubectl/polymorphichelpers/helpers.go +++ b/pkg/kubectl/polymorphichelpers/helpers.go @@ -32,17 +32,16 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" + coreclient "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/kubernetes/pkg/apis/apps" "k8s.io/kubernetes/pkg/apis/batch" api "k8s.io/kubernetes/pkg/apis/core" - apiv1 "k8s.io/kubernetes/pkg/apis/core/v1" "k8s.io/kubernetes/pkg/apis/extensions" - coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" ) // GetFirstPod returns a pod matching the namespace and label selector // and the number of all pods that match the label selector. -func GetFirstPod(client coreclient.PodsGetter, namespace string, selector string, timeout time.Duration, sortBy func([]*v1.Pod) sort.Interface) (*api.Pod, int, error) { +func GetFirstPod(client coreclient.PodsGetter, namespace string, selector string, timeout time.Duration, sortBy func([]*v1.Pod) sort.Interface) (*v1.Pod, int, error) { options := metav1.ListOptions{LabelSelector: selector} podList, err := client.Pods(namespace).List(options) @@ -52,15 +51,11 @@ func GetFirstPod(client coreclient.PodsGetter, namespace string, selector string pods := []*v1.Pod{} for i := range podList.Items { pod := podList.Items[i] - externalPod := &v1.Pod{} - apiv1.Convert_core_Pod_To_v1_Pod(&pod, externalPod, nil) - pods = append(pods, externalPod) + pods = append(pods, &pod) } if len(pods) > 0 { sort.Sort(sortBy(pods)) - internalPod := &api.Pod{} - apiv1.Convert_v1_Pod_To_core_Pod(pods[0], internalPod, nil) - return internalPod, len(podList.Items), nil + return pods[0], len(podList.Items), nil } // Watch until we observe a pod @@ -78,7 +73,7 @@ func GetFirstPod(client coreclient.PodsGetter, namespace string, selector string if err != nil { return nil, 0, err } - pod, ok := event.Object.(*api.Pod) + pod, ok := event.Object.(*v1.Pod) if !ok { return nil, 0, fmt.Errorf("%#v is not a pod event", event) } diff --git a/pkg/kubectl/polymorphichelpers/helpers_test.go b/pkg/kubectl/polymorphichelpers/helpers_test.go index 98a06d5f467..21f8af20d16 100644 --- a/pkg/kubectl/polymorphichelpers/helpers_test.go +++ b/pkg/kubectl/polymorphichelpers/helpers_test.go @@ -22,15 +22,14 @@ import ( "testing" "time" - "k8s.io/api/core/v1" - + corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/watch" + fakeexternal "k8s.io/client-go/kubernetes/fake" testcore "k8s.io/client-go/testing" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/controller" ) @@ -39,30 +38,30 @@ func TestGetFirstPod(t *testing.T) { tests := []struct { name string - podList *api.PodList + podList *corev1.PodList watching []watch.Event - sortBy func([]*v1.Pod) sort.Interface + sortBy func([]*corev1.Pod) sort.Interface - expected *api.Pod + expected *corev1.Pod expectedNum int expectedErr bool }{ { name: "kubectl logs - two ready pods", podList: newPodList(2, -1, -1, labelSet), - sortBy: func(pods []*v1.Pod) sort.Interface { return controller.ByLogging(pods) }, - expected: &api.Pod{ + sortBy: func(pods []*corev1.Pod) sort.Interface { return controller.ByLogging(pods) }, + expected: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-1", Namespace: metav1.NamespaceDefault, CreationTimestamp: metav1.Date(2016, time.April, 1, 1, 0, 0, 0, time.UTC), Labels: map[string]string{"test": "selector"}, }, - Status: api.PodStatus{ - Conditions: []api.PodCondition{ + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ { - Status: api.ConditionTrue, - Type: api.PodReady, + Status: corev1.ConditionTrue, + Type: corev1.PodReady, }, }, }, @@ -72,22 +71,22 @@ func TestGetFirstPod(t *testing.T) { { name: "kubectl logs - one unhealthy, one healthy", podList: newPodList(2, -1, 1, labelSet), - sortBy: func(pods []*v1.Pod) sort.Interface { return controller.ByLogging(pods) }, - expected: &api.Pod{ + sortBy: func(pods []*corev1.Pod) sort.Interface { return controller.ByLogging(pods) }, + expected: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-2", Namespace: metav1.NamespaceDefault, CreationTimestamp: metav1.Date(2016, time.April, 1, 1, 0, 1, 0, time.UTC), Labels: map[string]string{"test": "selector"}, }, - Status: api.PodStatus{ - Conditions: []api.PodCondition{ + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ { - Status: api.ConditionTrue, - Type: api.PodReady, + Status: corev1.ConditionTrue, + Type: corev1.PodReady, }, }, - ContainerStatuses: []api.ContainerStatus{{RestartCount: 5}}, + ContainerStatuses: []corev1.ContainerStatus{{RestartCount: 5}}, }, }, expectedNum: 2, @@ -95,19 +94,19 @@ func TestGetFirstPod(t *testing.T) { { name: "kubectl attach - two ready pods", podList: newPodList(2, -1, -1, labelSet), - sortBy: func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }, - expected: &api.Pod{ + sortBy: func(pods []*corev1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }, + expected: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-1", Namespace: metav1.NamespaceDefault, CreationTimestamp: metav1.Date(2016, time.April, 1, 1, 0, 0, 0, time.UTC), Labels: map[string]string{"test": "selector"}, }, - Status: api.PodStatus{ - Conditions: []api.PodCondition{ + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ { - Status: api.ConditionTrue, - Type: api.PodReady, + Status: corev1.ConditionTrue, + Type: corev1.PodReady, }, }, }, @@ -138,19 +137,19 @@ func TestGetFirstPod(t *testing.T) { }, }, }, - sortBy: func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }, - expected: &api.Pod{ + sortBy: func(pods []*corev1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) }, + expected: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-1", Namespace: metav1.NamespaceDefault, CreationTimestamp: metav1.Date(2016, time.April, 1, 1, 0, 0, 0, time.UTC), Labels: map[string]string{"test": "selector"}, }, - Status: api.PodStatus{ - Conditions: []api.PodCondition{ + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ { - Status: api.ConditionTrue, - Type: api.PodReady, + Status: corev1.ConditionTrue, + Type: corev1.PodReady, }, }, }, @@ -161,7 +160,7 @@ func TestGetFirstPod(t *testing.T) { for i := range tests { test := tests[i] - fake := fake.NewSimpleClientset(test.podList) + fake := fakeexternal.NewSimpleClientset(test.podList) if len(test.watching) > 0 { watcher := watch.NewFake() for _, event := range test.watching { @@ -196,21 +195,21 @@ func TestGetFirstPod(t *testing.T) { } } -func newPodList(count, isUnready, isUnhealthy int, labels map[string]string) *api.PodList { - pods := []api.Pod{} +func newPodList(count, isUnready, isUnhealthy int, labels map[string]string) *corev1.PodList { + pods := []corev1.Pod{} for i := 0; i < count; i++ { - newPod := api.Pod{ + newPod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("pod-%d", i+1), Namespace: metav1.NamespaceDefault, CreationTimestamp: metav1.Date(2016, time.April, 1, 1, 0, i, 0, time.UTC), Labels: labels, }, - Status: api.PodStatus{ - Conditions: []api.PodCondition{ + Status: corev1.PodStatus{ + Conditions: []corev1.PodCondition{ { - Status: api.ConditionTrue, - Type: api.PodReady, + Status: corev1.ConditionTrue, + Type: corev1.PodReady, }, }, }, @@ -218,12 +217,12 @@ func newPodList(count, isUnready, isUnhealthy int, labels map[string]string) *ap pods = append(pods, newPod) } if isUnready > -1 && isUnready < count { - pods[isUnready].Status.Conditions[0].Status = api.ConditionFalse + pods[isUnready].Status.Conditions[0].Status = corev1.ConditionFalse } if isUnhealthy > -1 && isUnhealthy < count { - pods[isUnhealthy].Status.ContainerStatuses = []api.ContainerStatus{{RestartCount: 5}} + pods[isUnhealthy].Status.ContainerStatuses = []corev1.ContainerStatus{{RestartCount: 5}} } - return &api.PodList{ + return &corev1.PodList{ Items: pods, } } diff --git a/pkg/kubectl/polymorphichelpers/interface.go b/pkg/kubectl/polymorphichelpers/interface.go index 7904616a4f7..0cb2e1725ad 100644 --- a/pkg/kubectl/polymorphichelpers/interface.go +++ b/pkg/kubectl/polymorphichelpers/interface.go @@ -24,7 +24,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" - api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" ) @@ -36,7 +35,7 @@ type LogsForObjectFunc func(restClientGetter genericclioptions.RESTClientGetter, var LogsForObjectFn LogsForObjectFunc = logsForObject // AttachableLogsForObjectFunc is a function type that can tell you how to get the pod for which to attach a given object -type AttachableLogsForObjectFunc func(restClientGetter genericclioptions.RESTClientGetter, object runtime.Object, timeout time.Duration) (*api.Pod, error) +type AttachableLogsForObjectFunc func(restClientGetter genericclioptions.RESTClientGetter, object runtime.Object, timeout time.Duration) (*v1.Pod, error) // AttachablePodForObjectFn gives a way to easily override the function for unit testing if needed. var AttachablePodForObjectFn AttachableLogsForObjectFunc = attachablePodForObject diff --git a/pkg/kubectl/polymorphichelpers/logsforobject.go b/pkg/kubectl/polymorphichelpers/logsforobject.go index 0cb68526516..4b543711d27 100644 --- a/pkg/kubectl/polymorphichelpers/logsforobject.go +++ b/pkg/kubectl/polymorphichelpers/logsforobject.go @@ -26,9 +26,9 @@ import ( "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" coreinternal "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" ) @@ -38,16 +38,18 @@ func logsForObject(restClientGetter genericclioptions.RESTClientGetter, object, if err != nil { return nil, err } - clientset, err := internalclientset.NewForConfig(clientConfig) + + clientset, err := corev1client.NewForConfig(clientConfig) if err != nil { return nil, err } return logsForObjectWithClient(clientset, object, options, timeout, allContainers) } +// TODO: remove internal clientset once all callers use external versions // this is split for easy test-ability -func logsForObjectWithClient(clientset internalclientset.Interface, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]*rest.Request, error) { - opts, ok := options.(*coreinternal.PodLogOptions) +func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]*rest.Request, error) { + opts, ok := options.(*corev1.PodLogOptions) if !ok { return nil, errors.New("provided options object is not a PodLogOptions") } @@ -78,7 +80,7 @@ func logsForObjectWithClient(clientset internalclientset.Interface, object, opti case *coreinternal.Pod: // if allContainers is true, then we're going to locate all containers and then iterate through them. At that point, "allContainers" is false if !allContainers { - return []*rest.Request{clientset.Core().Pods(t.Namespace).GetLogs(t.Name, opts)}, nil + return []*rest.Request{clientset.Pods(t.Namespace).GetLogs(t.Name, opts)}, nil } ret := []*rest.Request{} @@ -106,7 +108,7 @@ func logsForObjectWithClient(clientset internalclientset.Interface, object, opti case *corev1.Pod: // if allContainers is true, then we're going to locate all containers and then iterate through them. At that point, "allContainers" is false if !allContainers { - return []*rest.Request{clientset.Core().Pods(t.Namespace).GetLogs(t.Name, opts)}, nil + return []*rest.Request{clientset.Pods(t.Namespace).GetLogs(t.Name, opts)}, nil } ret := []*rest.Request{} @@ -136,8 +138,9 @@ func logsForObjectWithClient(clientset internalclientset.Interface, object, opti if err != nil { return nil, fmt.Errorf("cannot get the logs from %T: %v", object, err) } + sortBy := func(pods []*v1.Pod) sort.Interface { return controller.ByLogging(pods) } - pod, numPods, err := GetFirstPod(clientset.Core(), namespace, selector.String(), timeout, sortBy) + pod, numPods, err := GetFirstPod(clientset, namespace, selector.String(), timeout, sortBy) if err != nil { return nil, err } diff --git a/pkg/kubectl/polymorphichelpers/logsforobject_test.go b/pkg/kubectl/polymorphichelpers/logsforobject_test.go index 3f87f710b53..960cd567c31 100644 --- a/pkg/kubectl/polymorphichelpers/logsforobject_test.go +++ b/pkg/kubectl/polymorphichelpers/logsforobject_test.go @@ -21,34 +21,34 @@ import ( "testing" "time" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/diff" + fakeexternal "k8s.io/client-go/kubernetes/fake" testclient "k8s.io/client-go/testing" "k8s.io/kubernetes/pkg/apis/apps" "k8s.io/kubernetes/pkg/apis/batch" - api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/extensions" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" ) var ( - podsResource = schema.GroupVersionResource{Resource: "pods"} - podsKind = schema.GroupVersionKind{Kind: "Pod"} + podsResource = schema.GroupVersionResource{Version: "v1", Resource: "pods"} + podsKind = schema.GroupVersionKind{Version: "v1", Kind: "Pod"} ) func TestLogsForObject(t *testing.T) { tests := []struct { name string obj runtime.Object - opts *api.PodLogOptions + opts *corev1.PodLogOptions pods []runtime.Object actions []testclient.Action }{ { name: "pod logs", - obj: &api.Pod{ + obj: &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, }, pods: []runtime.Object{testPod()}, @@ -58,9 +58,9 @@ func TestLogsForObject(t *testing.T) { }, { name: "replication controller logs", - obj: &api.ReplicationController{ + obj: &corev1.ReplicationController{ ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, - Spec: api.ReplicationControllerSpec{ + Spec: corev1.ReplicationControllerSpec{ Selector: map[string]string{"foo": "bar"}, }, }, @@ -129,8 +129,8 @@ func TestLogsForObject(t *testing.T) { } for _, test := range tests { - fakeClientset := fake.NewSimpleClientset(test.pods...) - _, err := logsForObjectWithClient(fakeClientset, test.obj, test.opts, 20*time.Second, false) + fakeClientset := fakeexternal.NewSimpleClientset(test.pods...) + _, err := logsForObjectWithClient(fakeClientset.CoreV1(), test.obj, test.opts, 20*time.Second, false) if err != nil { t.Errorf("%s: unexpected error: %v", test.name, err) continue @@ -151,21 +151,21 @@ func TestLogsForObject(t *testing.T) { } func testPod() runtime.Object { - return &api.Pod{ + return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", Labels: map[string]string{"foo": "bar"}, }, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyAlways, - DNSPolicy: api.DNSClusterFirst, - Containers: []api.Container{{Name: "c1"}}, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + DNSPolicy: corev1.DNSClusterFirst, + Containers: []corev1.Container{{Name: "c1"}}, }, } } -func getLogsAction(namespace string, opts *api.PodLogOptions) testclient.Action { +func getLogsAction(namespace string, opts *corev1.PodLogOptions) testclient.Action { action := testclient.GenericActionImpl{} action.Verb = "get" action.Namespace = namespace diff --git a/test/e2e/kubectl/kubectl.go b/test/e2e/kubectl/kubectl.go index 17589e3f902..dfbd184d3de 100644 --- a/test/e2e/kubectl/kubectl.go +++ b/test/e2e/kubectl/kubectl.go @@ -564,7 +564,7 @@ var _ = SIGDescribe("Kubectl client", func() { ExecOrDie() Expect(runOutput).ToNot(ContainSubstring("stdin closed")) g := func(pods []*v1.Pod) sort.Interface { return sort.Reverse(controller.ActivePods(pods)) } - runTestPod, _, err := polymorphichelpers.GetFirstPod(f.InternalClientset.Core(), ns, "run=run-test-3", 1*time.Minute, g) + runTestPod, _, err := polymorphichelpers.GetFirstPod(f.ClientSet.CoreV1(), ns, "run=run-test-3", 1*time.Minute, g) if err != nil { os.Exit(1) } From 3a3633a32ec83675b63c126932d00390165d3d54 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 30 Jul 2018 15:26:41 -0400 Subject: [PATCH 2/2] update logs --- pkg/kubectl/cmd/BUILD | 2 - pkg/kubectl/cmd/logs.go | 170 ++++++++++++++-------- pkg/kubectl/cmd/logs_test.go | 274 +++++++++++++++++++++-------------- 3 files changed, 274 insertions(+), 172 deletions(-) diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index 2dc32bb870c..eb7721e31c4 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -60,7 +60,6 @@ go_library( "//pkg/apis/certificates:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/v1:go_default_library", - "//pkg/apis/core/validation:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/batch/internalversion:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", @@ -197,7 +196,6 @@ go_test( "//pkg/apis/batch:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/extensions:go_default_library", - "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/kubectl/cmd/create:go_default_library", "//pkg/kubectl/cmd/testing:go_default_library", "//pkg/kubectl/cmd/util:go_default_library", diff --git a/pkg/kubectl/cmd/logs.go b/pkg/kubectl/cmd/logs.go index 79acc9af55e..69821f2a217 100644 --- a/pkg/kubectl/cmd/logs.go +++ b/pkg/kubectl/cmd/logs.go @@ -25,11 +25,10 @@ import ( "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" - api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" @@ -80,6 +79,24 @@ type LogsOptions struct { ResourceArg string AllContainers bool Options runtime.Object + Resources []string + + ConsumeRequestFn func(*rest.Request, io.Writer) error + + // PodLogOptions + SinceTime string + SinceSeconds time.Duration + Follow bool + Previous bool + Timestamps bool + LimitBytes int64 + Tail int64 + Container string + + // whether or not a container name was given via --container + ContainerNameSpecified bool + Interactive bool + Selector string Object runtime.Object GetPodTimeout time.Duration @@ -93,6 +110,7 @@ func NewLogsOptions(streams genericclioptions.IOStreams, allContainers bool) *Lo return &LogsOptions{ IOStreams: streams, AllContainers: allContainers, + Tail: -1, } } @@ -119,40 +137,74 @@ func NewCmdLogs(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C Aliases: []string{"log"}, } cmd.Flags().BoolVar(&o.AllContainers, "all-containers", o.AllContainers, "Get all containers's logs in the pod(s).") - cmd.Flags().BoolP("follow", "f", false, "Specify if the logs should be streamed.") - cmd.Flags().Bool("timestamps", false, "Include timestamps on each line in the log output") - cmd.Flags().Int64("limit-bytes", 0, "Maximum bytes of logs to return. Defaults to no limit.") - cmd.Flags().BoolP("previous", "p", false, "If true, print the logs for the previous instance of the container in a pod if it exists.") - cmd.Flags().Int64("tail", -1, "Lines of recent log file to display. Defaults to -1 with no selector, showing all log lines otherwise 10, if a selector is provided.") - cmd.Flags().String("since-time", "", i18n.T("Only return logs after a specific date (RFC3339). Defaults to all logs. Only one of since-time / since may be used.")) - cmd.Flags().Duration("since", 0, "Only return logs newer than a relative duration like 5s, 2m, or 3h. Defaults to all logs. Only one of since-time / since may be used.") - cmd.Flags().StringP("container", "c", "", "Print the logs of this container") - cmd.Flags().Bool("interactive", false, "If true, prompt the user for input when required.") + cmd.Flags().BoolVarP(&o.Follow, "follow", "f", o.Follow, "Specify if the logs should be streamed.") + cmd.Flags().BoolVar(&o.Timestamps, "timestamps", o.Timestamps, "Include timestamps on each line in the log output") + cmd.Flags().Int64Var(&o.LimitBytes, "limit-bytes", o.LimitBytes, "Maximum bytes of logs to return. Defaults to no limit.") + cmd.Flags().BoolVarP(&o.Previous, "previous", "p", o.Previous, "If true, print the logs for the previous instance of the container in a pod if it exists.") + cmd.Flags().Int64Var(&o.Tail, "tail", o.Tail, "Lines of recent log file to display. Defaults to -1 with no selector, showing all log lines otherwise 10, if a selector is provided.") + cmd.Flags().StringVar(&o.SinceTime, "since-time", o.SinceTime, i18n.T("Only return logs after a specific date (RFC3339). Defaults to all logs. Only one of since-time / since may be used.")) + cmd.Flags().DurationVar(&o.SinceSeconds, "since", o.SinceSeconds, "Only return logs newer than a relative duration like 5s, 2m, or 3h. Defaults to all logs. Only one of since-time / since may be used.") + cmd.Flags().StringVarP(&o.Container, "container", "c", o.Container, "Print the logs of this container") + cmd.Flags().BoolVar(&o.Interactive, "interactive", o.Interactive, "If true, prompt the user for input when required.") cmd.Flags().MarkDeprecated("interactive", "This flag is no longer respected and there is no replacement.") cmdutil.AddPodRunningTimeoutFlag(cmd, defaultPodLogsTimeout) - cmd.Flags().StringP("selector", "l", "", "Selector (label query) to filter on.") + cmd.Flags().StringVarP(&o.Selector, "selector", "l", o.Selector, "Selector (label query) to filter on.") return cmd } +func (o *LogsOptions) ToLogOptions() (*corev1.PodLogOptions, error) { + logOptions := &corev1.PodLogOptions{ + Container: o.Container, + Follow: o.Follow, + Previous: o.Previous, + Timestamps: o.Timestamps, + } + + if len(o.SinceTime) > 0 { + t, err := util.ParseRFC3339(o.SinceTime, metav1.Now) + if err != nil { + return nil, err + } + + logOptions.SinceTime = &t + } + + if o.LimitBytes != 0 { + logOptions.LimitBytes = &o.LimitBytes + } + + if o.SinceSeconds != 0 { + // round up to the nearest second + sec := int64(o.SinceSeconds.Round(time.Second).Seconds()) + logOptions.SinceSeconds = &sec + } + + if len(o.Selector) > 0 && o.Tail != -1 { + logOptions.TailLines = &selectorTail + } else if o.Tail != -1 { + logOptions.TailLines = &o.Tail + } + + return logOptions, nil +} + func (o *LogsOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { - containerName := cmdutil.GetFlagString(cmd, "container") - selector := cmdutil.GetFlagString(cmd, "selector") + o.ContainerNameSpecified = cmd.Flag("container").Changed + o.Resources = args + switch len(args) { case 0: - if len(selector) == 0 { + if len(o.Selector) == 0 { return cmdutil.UsageErrorf(cmd, "%s", logsUsageStr) } case 1: o.ResourceArg = args[0] - if len(selector) != 0 { + if len(o.Selector) != 0 { return cmdutil.UsageErrorf(cmd, "only a selector (-l) or a POD name is allowed") } case 2: - if cmd.Flag("container").Changed { - return cmdutil.UsageErrorf(cmd, "only one of -c or an inline [CONTAINER] arg is allowed") - } o.ResourceArg = args[0] - containerName = args[1] + o.Container = args[1] default: return cmdutil.UsageErrorf(cmd, "%s", logsUsageStr) } @@ -162,48 +214,21 @@ func (o *LogsOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []str return err } - logOptions := &api.PodLogOptions{ - Container: containerName, - Follow: cmdutil.GetFlagBool(cmd, "follow"), - Previous: cmdutil.GetFlagBool(cmd, "previous"), - Timestamps: cmdutil.GetFlagBool(cmd, "timestamps"), - } - if sinceTime := cmdutil.GetFlagString(cmd, "since-time"); len(sinceTime) > 0 { - t, err := util.ParseRFC3339(sinceTime, metav1.Now) - if err != nil { - return err - } - logOptions.SinceTime = &t - } - if limit := cmdutil.GetFlagInt64(cmd, "limit-bytes"); limit != 0 { - logOptions.LimitBytes = &limit - } - tail := cmdutil.GetFlagInt64(cmd, "tail") - if tail != -1 { - logOptions.TailLines = &tail - } - if sinceSeconds := cmdutil.GetFlagDuration(cmd, "since"); sinceSeconds != 0 { - // round up to the nearest second - sec := int64(sinceSeconds.Round(time.Second).Seconds()) - logOptions.SinceSeconds = &sec - } + o.ConsumeRequestFn = consumeRequest + o.GetPodTimeout, err = cmdutil.GetPodRunningTimeoutFlag(cmd) if err != nil { return err } - o.Options = logOptions + + o.Options, err = o.ToLogOptions() + if err != nil { + return err + } + o.RESTClientGetter = f o.LogsForObject = polymorphichelpers.LogsForObjectFn - if len(selector) != 0 { - if logOptions.Follow { - return cmdutil.UsageErrorf(cmd, "only one of follow (-f) or selector (-l) is allowed") - } - if logOptions.TailLines == nil && tail != -1 { - logOptions.TailLines = &selectorTail - } - } - if o.Object == nil { builder := f.NewBuilder(). WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...). @@ -212,14 +237,14 @@ func (o *LogsOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []str if o.ResourceArg != "" { builder.ResourceNames("pods", o.ResourceArg) } - if selector != "" { - builder.ResourceTypes("pods").LabelSelectorParam(selector) + if o.Selector != "" { + builder.ResourceTypes("pods").LabelSelectorParam(o.Selector) } infos, err := builder.Do().Infos() if err != nil { return err } - if selector == "" && len(infos) != 1 { + if o.Selector == "" && len(infos) != 1 { return errors.New("expected a resource") } o.Object = infos[0].Object @@ -229,15 +254,36 @@ func (o *LogsOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []str } func (o LogsOptions) Validate() error { - logsOptions, ok := o.Options.(*api.PodLogOptions) + if o.Follow && len(o.Selector) > 0 { + return fmt.Errorf("only one of follow (-f) or selector (-l) is allowed") + } + + if len(o.SinceTime) > 0 && o.SinceSeconds != 0 { + return fmt.Errorf("at most one of `sinceTime` or `sinceSeconds` may be specified") + } + + logsOptions, ok := o.Options.(*corev1.PodLogOptions) if !ok { return errors.New("unexpected logs options object") } if o.AllContainers && len(logsOptions.Container) > 0 { return fmt.Errorf("--all-containers=true should not be specified with container name %s", logsOptions.Container) } - if errs := validation.ValidatePodLogOptions(logsOptions); len(errs) > 0 { - return errs.ToAggregate() + + if o.ContainerNameSpecified && len(o.Resources) == 2 { + return fmt.Errorf("only one of -c or an inline [CONTAINER] arg is allowed") + } + + if o.LimitBytes < 0 { + return fmt.Errorf("--limit-bytes must be greater than 0") + } + + if logsOptions.SinceSeconds != nil && *logsOptions.SinceSeconds < int64(0) { + return fmt.Errorf("--since must be greater than 0") + } + + if logsOptions.TailLines != nil && *logsOptions.TailLines < 0 { + return fmt.Errorf("TailLines must be greater than or equal to 0") } return nil @@ -251,7 +297,7 @@ func (o LogsOptions) RunLogs() error { } for _, request := range requests { - if err := consumeRequest(request, o.Out); err != nil { + if err := o.ConsumeRequestFn(request, o.Out); err != nil { return err } } diff --git a/pkg/kubectl/cmd/logs_test.go b/pkg/kubectl/cmd/logs_test.go index 03d2dd22a24..934fa5ee8e0 100644 --- a/pkg/kubectl/cmd/logs_test.go +++ b/pkg/kubectl/cmd/logs_test.go @@ -17,29 +17,20 @@ limitations under the License. package cmd import ( - "bytes" "errors" - "io/ioutil" - "net/http" + "fmt" + "io" "strings" "testing" "time" - "github.com/spf13/cobra" - - "fmt" - + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" restclient "k8s.io/client-go/rest" - "k8s.io/client-go/rest/fake" - "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" "k8s.io/kubernetes/pkg/kubectl/genericclioptions" - "k8s.io/kubernetes/pkg/kubectl/polymorphichelpers" - "k8s.io/kubernetes/pkg/kubectl/scheme" ) func TestLog(t *testing.T) { @@ -48,11 +39,8 @@ func TestLog(t *testing.T) { pod *api.Pod }{ { - name: "v1 - pod log", - version: "v1", - podPath: "/namespaces/test/pods/foo", - logPath: "/api/v1/namespaces/test/pods/foo/log", - pod: testPod(), + name: "v1 - pod log", + pod: testPod(), }, } for _, test := range tests { @@ -61,41 +49,19 @@ func TestLog(t *testing.T) { tf := cmdtesting.NewTestFactory().WithNamespace("test") defer tf.Cleanup() - codec := legacyscheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) - ns := legacyscheme.Codecs - - tf.Client = &fake.RESTClient{ - NegotiatedSerializer: ns, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - switch p, m := req.URL.Path, req.Method; { - case p == test.podPath && m == "GET": - body := objBody(codec, test.pod) - return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil - case p == test.logPath && m == "GET": - body := ioutil.NopCloser(bytes.NewBufferString(logContent)) - return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil - default: - t.Errorf("%s: unexpected request: %#v\n%#v", test.name, req.URL, req) - return nil, nil - } - }), - } - tf.ClientConfigVal = defaultClientConfig() - oldLogFn := polymorphichelpers.LogsForObjectFn - defer func() { - polymorphichelpers.LogsForObjectFn = oldLogFn - }() - clientset, err := tf.ClientSet() - if err != nil { - t.Fatal(err) - } - polymorphichelpers.LogsForObjectFn = logTestMock{client: clientset}.logsForObject - streams, _, buf, _ := genericclioptions.NewTestIOStreams() - cmd := NewCmdLogs(tf, streams) - cmd.Flags().Set("namespace", "test") - cmd.Run(cmd, []string{"foo"}) + mock := &logTestMock{ + logsContent: logContent, + } + + opts := NewLogsOptions(streams, false) + opts.Namespace = "test" + opts.Object = test.pod + opts.Options = &corev1.PodLogOptions{} + opts.LogsForObject = mock.mockLogsForObject + opts.ConsumeRequestFn = mock.mockConsumeRequest + opts.RunLogs() if buf.String() != logContent { t.Errorf("%s: did not get expected log content. Got: %s", test.name, buf.String()) @@ -119,66 +85,152 @@ func testPod() *api.Pod { } } -func TestValidateLogFlags(t *testing.T) { +func TestValidateLogOptions(t *testing.T) { f := cmdtesting.NewTestFactory() defer f.Cleanup() f.WithNamespace("") tests := []struct { name string - flags map[string]string args []string + opts func(genericclioptions.IOStreams) *LogsOptions expected string }{ { - name: "since & since-time", - flags: map[string]string{"since": "1h", "since-time": "2006-01-02T15:04:05Z"}, + name: "since & since-time", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.SinceSeconds = time.Hour + o.SinceTime = "2006-01-02T15:04:05Z" + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, args: []string{"foo"}, expected: "at most one of `sinceTime` or `sinceSeconds` may be specified", }, { - name: "negative since-time", - flags: map[string]string{"since": "-1s"}, + name: "negative since-time", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.SinceSeconds = -1 * time.Second + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, args: []string{"foo"}, expected: "must be greater than 0", }, { - name: "negative limit-bytes", - flags: map[string]string{"limit-bytes": "-100"}, + name: "negative limit-bytes", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.LimitBytes = -100 + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, args: []string{"foo"}, expected: "must be greater than 0", }, { - name: "negative tail", - flags: map[string]string{"tail": "-100"}, + name: "negative tail", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.Tail = -100 + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, args: []string{"foo"}, expected: "must be greater than or equal to 0", }, { - name: "container name combined with --all-containers", - flags: map[string]string{"all-containers": "true"}, + name: "container name combined with --all-containers", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, true) + o.Container = "my-container" + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, args: []string{"my-pod", "my-container"}, expected: "--all-containers=true should not be specified with container", }, + { + name: "container name combined with second argument", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.Container = "my-container" + o.ContainerNameSpecified = true + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, + args: []string{"my-pod", "my-container"}, + expected: "only one of -c or an inline", + }, + { + name: "follow and selector conflict", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.Selector = "foo" + o.Follow = true + + var err error + o.Options, err = o.ToLogOptions() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return o + }, + expected: "only one of follow (-f) or selector (-l) is allowed", + }, } for _, test := range tests { streams := genericclioptions.NewTestIOStreamsDiscard() - cmd := NewCmdLogs(f, streams) - out := "" - for flag, value := range test.flags { - cmd.Flags().Set(flag, value) - } - // checkErr breaks tests in case of errors, plus we just - // need to check errors returned by the command validation - o := NewLogsOptions(streams, test.flags["all-containers"] == "true") - cmd.Run = func(cmd *cobra.Command, args []string) { - o.Complete(f, cmd, args) - out = o.Validate().Error() - } - cmd.Run(cmd, test.args) - if !strings.Contains(out, test.expected) { - t.Errorf("%s: expected to find:\n\t%s\nfound:\n\t%s\n", test.name, test.expected, out) + o := test.opts(streams) + o.Resources = test.args + + err := o.Validate() + if err == nil { + t.Fatalf("expected error %q, got none", test.expected) + } + + if !strings.Contains(err.Error(), test.expected) { + t.Errorf("%s: expected to find:\n\t%s\nfound:\n\t%s\n", test.name, test.expected, err.Error()) } } } @@ -190,49 +242,49 @@ func TestLogComplete(t *testing.T) { tests := []struct { name string args []string - flags map[string]string + opts func(genericclioptions.IOStreams) *LogsOptions expected string }{ { - name: "No args case", - flags: map[string]string{"selector": ""}, + name: "No args case", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + return NewLogsOptions(streams, false) + }, expected: "'logs (POD | TYPE/NAME) [CONTAINER_NAME]'.\nPOD or TYPE/NAME is a required argument for the logs command", }, { - name: "One args case", - args: []string{"foo"}, - flags: map[string]string{"selector": "foo"}, + name: "One args case", + args: []string{"foo"}, + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.Selector = "foo" + return o + }, expected: "only a selector (-l) or a POD name is allowed", }, { - name: "Two args case", - args: []string{"foo", "foo1"}, - flags: map[string]string{"container": "foo1"}, - expected: "only one of -c or an inline [CONTAINER] arg is allowed", - }, - { - name: "More than two args case", - args: []string{"foo", "foo1", "foo2"}, - flags: map[string]string{"tail": "1"}, + name: "More than two args case", + args: []string{"foo", "foo1", "foo2"}, + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + o := NewLogsOptions(streams, false) + o.Tail = 1 + return o + }, expected: "'logs (POD | TYPE/NAME) [CONTAINER_NAME]'.\nPOD or TYPE/NAME is a required argument for the logs command", }, - { - name: "follow and selecter conflict", - flags: map[string]string{"selector": "foo", "follow": "true"}, - expected: "only one of follow (-f) or selector (-l) is allowed", - }, } for _, test := range tests { cmd := NewCmdLogs(f, genericclioptions.NewTestIOStreamsDiscard()) - var err error out := "" - for flag, value := range test.flags { - cmd.Flags().Set(flag, value) - } + // checkErr breaks tests in case of errors, plus we just // need to check errors returned by the command validation - o := NewLogsOptions(genericclioptions.NewTestIOStreamsDiscard(), false) - err = o.Complete(f, cmd, test.args) + o := test.opts(genericclioptions.NewTestIOStreamsDiscard()) + err := o.Complete(f, cmd, test.args) + if err == nil { + t.Fatalf("expected error %q, got none", test.expected) + } + out = err.Error() if !strings.Contains(out, test.expected) { t.Errorf("%s: expected to find:\n\t%s\nfound:\n\t%s\n", test.name, test.expected, out) @@ -241,17 +293,23 @@ func TestLogComplete(t *testing.T) { } type logTestMock struct { - client internalclientset.Interface + logsContent string } -func (m logTestMock) logsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]*restclient.Request, error) { - switch t := object.(type) { +func (l *logTestMock) mockConsumeRequest(req *restclient.Request, out io.Writer) error { + fmt.Fprintf(out, l.logsContent) + return nil +} + +func (l *logTestMock) mockLogsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]*restclient.Request, error) { + switch object.(type) { case *api.Pod: - opts, ok := options.(*api.PodLogOptions) + _, ok := options.(*corev1.PodLogOptions) if !ok { return nil, errors.New("provided options object is not a PodLogOptions") } - return []*restclient.Request{m.client.Core().Pods(t.Namespace).GetLogs(t.Name, opts)}, nil + + return []*restclient.Request{{}}, nil default: return nil, fmt.Errorf("cannot get the logs from %T", object) }