Merge pull request #10707 from kargakis/logs-with-resource-builder

docs/man/man1/kubectl-logs.1

@@ -19,15 +19,15 @@ Print the logs for a container in a pod. If the pod has only one container, the
 .SH OPTIONS
 .PP
 \fB\-c\fP, \fB\-\-container\fP=""
-    Container name
+    Print the logs of this container
 
 .PP
 \fB\-f\fP, \fB\-\-follow\fP=false
     Specify if the logs should be streamed.
 
 .PP
-\fB\-\-interactive\fP=true
-    If true, prompt the user for input when required. Default true.
+\fB\-\-interactive\fP=false
+    If true, prompt the user for input when required.
 
 .PP
 \fB\-\-limit\-bytes\fP=0

docs/user-guide/kubectl/kubectl_logs.md

@@ -66,7 +66,7 @@ $ kubectl logs --since=1h nginx
 ### Options
 
 ```
-  -c, --container="": Container name
+  -c, --container="": Print the logs of this container
   -f, --follow[=false]: Specify if the logs should be streamed.
       --limit-bytes=0: Maximum bytes of logs to return. Defaults to no limit.
   -p, --previous[=false]: If true, print the logs for the previous instance of the container in a pod if it exists.
@@ -108,7 +108,7 @@ $ kubectl logs --since=1h nginx
 
 * [kubectl](kubectl.md)	 - kubectl controls the Kubernetes cluster manager
 
-###### Auto generated by spf13/cobra on 14-Oct-2015
+###### Auto generated by spf13/cobra on 28-Oct-2015
 
 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_logs.md?pixel)]()

pkg/client/unversioned/pods.go

@@ -38,6 +38,7 @@ type PodInterface interface {
 	Watch(label labels.Selector, field fields.Selector, opts api.ListOptions) (watch.Interface, error)
 	Bind(binding *api.Binding) error
 	UpdateStatus(pod *api.Pod) (*api.Pod, error)
+	GetLogs(name string, opts *api.PodLogOptions) *Request
 }
 
 // pods implements PodsNamespacer interface
@@ -118,3 +119,8 @@ func (c *pods) UpdateStatus(pod *api.Pod) (result *api.Pod, err error) {
 	err = c.r.Put().Namespace(c.ns).Resource("pods").Name(pod.Name).SubResource("status").Body(pod).Do().Into(result)
 	return
 }
+
+// Get constructs a request for getting the logs for a pod
+func (c *pods) GetLogs(name string, opts *api.PodLogOptions) *Request {
+	return c.r.Get().Namespace(c.ns).Name(name).Resource("pods").SubResource("log").VersionedParams(opts, api.Scheme)
+}

pkg/client/unversioned/pods_test.go

@@ -187,3 +187,29 @@ func TestUpdatePod(t *testing.T) {
 	receivedPod, err := c.Setup(t).Pods(ns).Update(requestPod)
 	c.Validate(t, receivedPod, err)
 }
+
+func TestPodGetLogs(t *testing.T) {
+	ns := api.NamespaceDefault
+	opts := &api.PodLogOptions{
+		Follow:     true,
+		Timestamps: true,
+	}
+	c := &testClient{}
+
+	request := c.Setup(t).Pods(ns).GetLogs("podName", opts)
+	if request.verb != "GET" {
+		t.Fatalf("unexpected verb %q, expected %q", request.verb, "GET")
+	}
+	if request.resource != "pods" {
+		t.Fatalf("unexpected resource %q, expected %q", request.subresource, "pods")
+	}
+	if request.subresource != "log" {
+		t.Fatalf("unexpected subresource %q, expected %q", request.subresource, "log")
+	}
+	expected := map[string]string{"container": "", "follow": "true", "previous": "false", "timestamps": "true"}
+	for gotKey, gotValue := range request.params {
+		if gotValue[0] != expected[gotKey] {
+			t.Fatalf("unexpected key-value %s=%s, expected %s=%s", gotKey, gotValue[0], gotKey, expected[gotKey])
+		}
+	}
+}

pkg/client/unversioned/testclient/fake_pods.go

@@ -18,6 +18,7 @@ package testclient
 
 import (
 	"k8s.io/kubernetes/pkg/api"
+	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/fields"
 	"k8s.io/kubernetes/pkg/labels"
 	"k8s.io/kubernetes/pkg/watch"
@@ -99,3 +100,15 @@ func (c *FakePods) UpdateStatus(pod *api.Pod) (*api.Pod, error) {
 
 	return obj.(*api.Pod), err
 }
+
+func (c *FakePods) GetLogs(name string, opts *api.PodLogOptions) *client.Request {
+	action := GenericActionImpl{}
+	action.Verb = "get"
+	action.Namespace = c.Namespace
+	action.Resource = "pod"
+	action.Subresource = "logs"
+	action.Value = opts
+
+	_, _ = c.Fake.Invokes(action, &api.Pod{})
+	return &client.Request{}
+}

pkg/kubectl/cmd/cmd_test.go

@@ -232,6 +232,26 @@ func NewAPIFactory() (*cmdutil.Factory, *testFactory, runtime.Codec) {
 			generator, ok := generators[name]
 			return generator, ok
 		},
+		LogsForObject: func(object, options runtime.Object) (*client.Request, error) {
+			fakeClient := t.Client.(*fake.RESTClient)
+			c := client.NewOrDie(t.ClientConfig)
+			c.Client = fakeClient.Client
+
+			switch t := object.(type) {
+			case *api.Pod:
+				opts, ok := options.(*api.PodLogOptions)
+				if !ok {
+					return nil, errors.New("provided options object is not a PodLogOptions")
+				}
+				return c.Pods(t.Namespace).GetLogs(t.Name, opts), nil
+			default:
+				_, kind, err := api.Scheme.ObjectVersionAndKind(object)
+				if err != nil {
+					return nil, err
+				}
+				return nil, fmt.Errorf("cannot get the logs from %s", kind)
+			}
+		},
 	}
 	rf := cmdutil.NewFactory(nil)
 	f.PodSelectorForObject = rf.PodSelectorForObject

pkg/kubectl/cmd/log_test.go

@@ -20,8 +20,12 @@ import (
 	"bytes"
 	"io/ioutil"
 	"net/http"
+	"os"
+	"strings"
 	"testing"
 
+	"github.com/spf13/cobra"
+
 	"k8s.io/kubernetes/pkg/api"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/client/unversioned/fake"
@@ -35,7 +39,7 @@ func TestLog(t *testing.T) {
 		{
 			name:    "v1 - pod log",
 			version: "v1",
-			podPath: "/api/v1/namespaces/test/pods/foo",
+			podPath: "/namespaces/test/pods/foo",
 			logPath: "/api/v1/namespaces/test/pods/foo/log",
 			pod:     testPod(),
 		},
@@ -88,3 +92,49 @@ func testPod() *api.Pod {
 		},
 	}
 }
+
+func TestValidateLogFlags(t *testing.T) {
+	f, _, _ := NewAPIFactory()
+
+	tests := []struct {
+		name     string
+		flags    map[string]string
+		expected string
+	}{
+		{
+			name:     "since & since-time",
+			flags:    map[string]string{"since": "1h", "since-time": "2006-01-02T15:04:05Z"},
+			expected: "only one of sinceTime or sinceSeconds can be provided",
+		},
+		{
+			name:     "negative limit-bytes",
+			flags:    map[string]string{"limit-bytes": "-100"},
+			expected: "limitBytes must be a positive integer or nil",
+		},
+		{
+			name:     "negative tail",
+			flags:    map[string]string{"tail": "-100"},
+			expected: "tailLines must be a non-negative integer or nil",
+		},
+	}
+	for _, test := range tests {
+		cmd := NewCmdLog(f, bytes.NewBuffer([]byte{}))
+		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 := &LogsOptions{}
+		cmd.Run = func(cmd *cobra.Command, args []string) {
+			o.Complete(f, os.Stdout, cmd, args)
+			out = o.Validate().Error()
+			o.RunLog()
+		}
+		cmd.Run(cmd, []string{"foo"})
+
+		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)
+		}
+	}
+}

pkg/kubectl/cmd/run.go

@@ -206,7 +206,7 @@ func Run(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cob
 		if err != nil {
 			return err
 		}
-		return handleAttachPod(client, attachablePod, opts)
+		return handleAttachPod(f, client, attachablePod, opts)
 	}
 
 	outputFormat := cmdutil.GetFlagString(cmd, "output")
@@ -245,20 +245,40 @@ func waitForPodRunning(c *client.Client, pod *api.Pod, out io.Writer) (status ap
 	}
 }
 
-func handleAttachPod(c *client.Client, pod *api.Pod, opts *AttachOptions) error {
+func handleAttachPod(f *cmdutil.Factory, c *client.Client, pod *api.Pod, opts *AttachOptions) error {
 	status, err := waitForPodRunning(c, pod, opts.Out)
 	if err != nil {
 		return err
 	}
 	if status == api.PodSucceeded || status == api.PodFailed {
-		return handleLog(c, pod.Namespace, pod.Name, &api.PodLogOptions{Container: opts.GetContainerName(pod)}, opts.Out)
+		req, err := f.LogsForObject(pod, &api.PodLogOptions{Container: opts.GetContainerName(pod)})
+		if err != nil {
+			return err
+		}
+		readCloser, err := req.Stream()
+		if err != nil {
+			return err
+		}
+		defer readCloser.Close()
+		_, err = io.Copy(opts.Out, readCloser)
+		return err
 	}
 	opts.Client = c
 	opts.PodName = pod.Name
 	opts.Namespace = pod.Namespace
 	if err := opts.Run(); err != nil {
 		fmt.Fprintf(opts.Out, "Error attaching, falling back to logs: %v\n", err)
-		return handleLog(c, pod.Namespace, pod.Name, &api.PodLogOptions{Container: opts.GetContainerName(pod)}, opts.Out)
+		req, err := f.LogsForObject(pod, &api.PodLogOptions{Container: opts.GetContainerName(pod)})
+		if err != nil {
+			return err
+		}
+		readCloser, err := req.Stream()
+		if err != nil {
+			return err
+		}
+		defer readCloser.Close()
+		_, err = io.Copy(opts.Out, readCloser)
+		return err
 	}
 	return nil
 }

pkg/kubectl/cmd/util/factory.go

@@ -83,6 +83,8 @@ type Factory struct {
 	PortsForObject func(object runtime.Object) ([]string, error)
 	// LabelsForObject returns the labels associated with the provided object
 	LabelsForObject func(object runtime.Object) (map[string]string, error)
+	// LogsForObject returns a request for the logs associated with the provided object
+	LogsForObject func(object, options runtime.Object) (*client.Request, error)
 	// Returns a schema that can validate objects stored on disk.
 	Validator func(validate bool, cacheDir string) (validation.Schema, error)
 	// Returns the default namespace to use in cases where no
@@ -218,6 +220,27 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 		LabelsForObject: func(object runtime.Object) (map[string]string, error) {
 			return meta.NewAccessor().Labels(object)
 		},
+		LogsForObject: func(object, options runtime.Object) (*client.Request, error) {
+			c, err := clients.ClientForVersion("")
+			if err != nil {
+				return nil, err
+			}
+
+			switch t := object.(type) {
+			case *api.Pod:
+				opts, ok := options.(*api.PodLogOptions)
+				if !ok {
+					return nil, errors.New("provided options object is not a PodLogOptions")
+				}
+				return c.Pods(t.Namespace).GetLogs(t.Name, opts), nil
+			default:
+				_, kind, err := api.Scheme.ObjectVersionAndKind(object)
+				if err != nil {
+					return nil, err
+				}
+				return nil, fmt.Errorf("cannot get the logs from %s", kind)
+			}
+		},
 		Scaler: func(mapping *meta.RESTMapping) (kubectl.Scaler, error) {
 			client, err := clients.ClientForVersion(mapping.APIVersion)
 			if err != nil {