From ecaf0874b268cdf1f55f9fa5cfd886d2995f9b2d Mon Sep 17 00:00:00 2001 From: Dawn Chen Date: Wed, 22 Apr 2015 15:35:49 -0700 Subject: [PATCH 1/3] Introduce --previous option to kubectl log --- contrib/completions/bash/kubectl | 2 ++ docs/kubectl_log.md | 6 +++++- docs/man/man1/kubectl-log.1 | 7 +++++++ pkg/kubectl/cmd/log.go | 12 +++++++++++- 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl index 4d29416a5c5..cfd9a3374d9 100644 --- a/contrib/completions/bash/kubectl +++ b/contrib/completions/bash/kubectl @@ -404,6 +404,8 @@ _kubectl_log() flags+=("--help") flags+=("-h") flags+=("--interactive") + flags+=("--previous") + flags+=("-p") must_have_one_flag=() must_have_one_noun=() diff --git a/docs/kubectl_log.md b/docs/kubectl_log.md index def67609563..fc5a606a21d 100644 --- a/docs/kubectl_log.md +++ b/docs/kubectl_log.md @@ -8,7 +8,7 @@ Print the logs for a container in a pod. Print the logs for a container in a pod. If the pod has only one container, the container name is optional. ``` -kubectl log [-f] POD [CONTAINER] +kubectl log [-f] [-p] POD [CONTAINER] ``` ### Examples @@ -17,6 +17,9 @@ kubectl log [-f] POD [CONTAINER] // Returns snapshot of ruby-container logs from pod 123456-7890. $ kubectl log 123456-7890 ruby-container +// Returns snapshot of previous terminated ruby-container logs from pod 123456-7890. +$ kubectl log -p 123456-7890 ruby-container + // Starts streaming of ruby-container logs from pod 123456-7890. $ kubectl log -f 123456-7890 ruby-container ``` @@ -27,6 +30,7 @@ $ kubectl log -f 123456-7890 ruby-container -f, --follow=false: Specify if the logs should be streamed. -h, --help=false: help for log --interactive=true: If true, prompt the user for input when required. Default true. + -p, --previous=false: If true, print the logs for the previous instance of the container in a pod if it exists. ``` ### Options inherited from parent commands diff --git a/docs/man/man1/kubectl-log.1 b/docs/man/man1/kubectl-log.1 index 3cea9c32782..0c34ffc6ff8 100644 --- a/docs/man/man1/kubectl-log.1 +++ b/docs/man/man1/kubectl-log.1 @@ -29,6 +29,10 @@ Print the logs for a container in a pod. If the pod has only one container, the \fB\-\-interactive\fP=true If true, prompt the user for input when required. Default true. +.PP +\fB\-p\fP, \fB\-\-previous\fP=false + If true, print the logs for the previous instance of the container in a pod if it exists. + .SH OPTIONS INHERITED FROM PARENT COMMANDS .PP @@ -140,6 +144,9 @@ Print the logs for a container in a pod. If the pod has only one container, the // Returns snapshot of ruby\-container logs from pod 123456\-7890. $ kubectl log 123456\-7890 ruby\-container +// Returns snapshot of previous terminated ruby\-container logs from pod 123456\-7890. +$ kubectl log \-p 123456\-7890 ruby\-container + // Starts streaming of ruby\-container logs from pod 123456\-7890. $ kubectl log \-f 123456\-7890 ruby\-container diff --git a/pkg/kubectl/cmd/log.go b/pkg/kubectl/cmd/log.go index 68a0fffd2b8..9378075b500 100644 --- a/pkg/kubectl/cmd/log.go +++ b/pkg/kubectl/cmd/log.go @@ -31,6 +31,9 @@ const ( log_example = `// Returns snapshot of ruby-container logs from pod 123456-7890. $ kubectl log 123456-7890 ruby-container +// Returns snapshot of previous terminated ruby-container logs from pod 123456-7890. +$ kubectl log -p 123456-7890 ruby-container + // Starts streaming of ruby-container logs from pod 123456-7890. $ kubectl log -f 123456-7890 ruby-container` ) @@ -60,7 +63,7 @@ func selectContainer(pod *api.Pod, in io.Reader, out io.Writer) string { // NewCmdLog creates a new pod log command func NewCmdLog(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd := &cobra.Command{ - Use: "log [-f] POD [CONTAINER]", + Use: "log [-f] [-p] POD [CONTAINER]", Short: "Print the logs for a container in a pod.", Long: "Print the logs for a container in a pod. If the pod has only one container, the container name is optional.", Example: log_example, @@ -71,6 +74,7 @@ func NewCmdLog(f *cmdutil.Factory, out io.Writer) *cobra.Command { } cmd.Flags().BoolP("follow", "f", false, "Specify if the logs should be streamed.") cmd.Flags().Bool("interactive", true, "If true, prompt the user for input when required. Default true.") + cmd.Flags().BoolP("previous", "p", false, "If true, print the logs for the previous instance of the container in a pod if it exists.") return cmd } @@ -115,6 +119,11 @@ func RunLog(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string follow = true } + previous := false + if cmdutil.GetFlagBool(cmd, "previous") { + previous = true + } + readCloser, err := client.RESTClient.Get(). Namespace(namespace). Name(podID). @@ -122,6 +131,7 @@ func RunLog(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []string SubResource("log"). Param("follow", strconv.FormatBool(follow)). Param("container", container). + Param("previous", strconv.FormatBool(previous)). Stream() if err != nil { return err From ffa59470104f5f752f90eba3a587274317a2d6b2 Mon Sep 17 00:00:00 2001 From: Dawn Chen Date: Thu, 7 May 2015 14:10:10 -0700 Subject: [PATCH 2/3] Introduce Previous as PodLogOptions --- pkg/api/types.go | 3 +++ pkg/api/v1/conversion.go | 2 ++ pkg/api/v1/types.go | 3 +++ pkg/api/v1beta1/types.go | 3 +++ pkg/api/v1beta2/types.go | 3 +++ pkg/api/v1beta3/conversion_generated.go | 2 ++ pkg/api/v1beta3/types.go | 3 +++ 7 files changed, 19 insertions(+) diff --git a/pkg/api/types.go b/pkg/api/types.go index c172ca4e8fc..176d64dfd26 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1330,6 +1330,9 @@ type PodLogOptions struct { // If true, follow the logs for the pod Follow bool + + // If true, return previous terminated container logs + Previous bool } // PodExecOptions is the query options to a Pod's remote exec call diff --git a/pkg/api/v1/conversion.go b/pkg/api/v1/conversion.go index a5aa0665569..47d0e3f55c8 100644 --- a/pkg/api/v1/conversion.go +++ b/pkg/api/v1/conversion.go @@ -1802,6 +1802,7 @@ func init() { } out.Container = in.Container out.Follow = in.Follow + out.Previous = in.Previous return nil }, func(in *newer.PodLogOptions, out *PodLogOptions, s conversion.Scope) error { @@ -1810,6 +1811,7 @@ func init() { } out.Container = in.Container out.Follow = in.Follow + out.Previous = in.Previous return nil }, func(in *PodProxyOptions, out *newer.PodProxyOptions, s conversion.Scope) error { diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index ae1c424449f..e1ac6a86ae5 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -1320,6 +1320,9 @@ type PodLogOptions struct { // If true, follow the logs for the pod Follow bool `json:"follow,omitempty" description:"follow the log stream of the pod; defaults to false"` + + // If true, return previous terminated container logs + Previous bool `json:"previous,omitempty" description:"return previous terminated container logs; defaults to false"` } // PodExecOptions is the query options to a Pod's remote exec call diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 951a2f92d73..1f6e1c651dd 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -1188,6 +1188,9 @@ type PodLogOptions struct { // If true, follow the logs for the pod Follow bool `json:"follow,omitempty" description:"follow the log stream of the pod; defaults to false"` + + // If true, return previous terminated container logs + Previous bool `json:"previous,omitempty" description:"return previous terminated container logs; defaults to false"` } // PodExecOptions is the query options to a Pod's remote exec call diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index a1b3d0d31aa..a478e4b58dd 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -1208,6 +1208,9 @@ type PodLogOptions struct { // If true, follow the logs for the pod Follow bool `json:"follow,omitempty" description:"follow the log stream of the pod; defaults to false"` + + // If true, return previous terminated container logs + Previous bool `json:"previous,omitempty" description:"return previous terminated container logs; defaults to false"` } // PodExecOptions is the query options to a Pod's remote exec call diff --git a/pkg/api/v1beta3/conversion_generated.go b/pkg/api/v1beta3/conversion_generated.go index a1701b4bea8..49d9cd5a178 100644 --- a/pkg/api/v1beta3/conversion_generated.go +++ b/pkg/api/v1beta3/conversion_generated.go @@ -2544,6 +2544,7 @@ func convert_v1beta3_PodLogOptions_To_api_PodLogOptions(in *PodLogOptions, out * } out.Container = in.Container out.Follow = in.Follow + out.Previous = in.Previous return nil } @@ -2556,6 +2557,7 @@ func convert_api_PodLogOptions_To_v1beta3_PodLogOptions(in *newer.PodLogOptions, } out.Container = in.Container out.Follow = in.Follow + out.Previous = in.Previous return nil } diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index b0ff3fd8647..ef1c43b469c 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -1320,6 +1320,9 @@ type PodLogOptions struct { // If true, follow the logs for the pod Follow bool `json:"follow,omitempty" description:"follow the log stream of the pod; defaults to false"` + + // If true, return previous terminated container logs + Previous bool `json:"previous,omitempty" description:"return previous terminated container logs; defaults to false"` } // PodExecOptions is the query options to a Pod's remote exec call From 86479cc56c807dab3ecc948c20c8ae16d019cf20 Mon Sep 17 00:00:00 2001 From: Dawn Chen Date: Thu, 7 May 2015 11:34:16 -0700 Subject: [PATCH 3/3] Add support to pull log for last terminated container --- pkg/kubelet/kubelet.go | 22 ++++++++++++++++------ pkg/kubelet/kubelet_test.go | 17 +++++++++++++++-- pkg/kubelet/server.go | 5 +++-- pkg/kubelet/server_test.go | 23 +++++++++++++++-------- pkg/registry/pod/rest.go | 3 +++ 5 files changed, 52 insertions(+), 18 deletions(-) diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index ccf7daa547b..9c0f72b69dc 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1373,21 +1373,31 @@ func (kl *Kubelet) validatePodPhase(podStatus *api.PodStatus) error { return fmt.Errorf("pod is not in 'Running', 'Succeeded' or 'Failed' state - State: %q", podStatus.Phase) } -func (kl *Kubelet) validateContainerStatus(podStatus *api.PodStatus, containerName string) (containerID string, err error) { +func (kl *Kubelet) validateContainerStatus(podStatus *api.PodStatus, containerName string, previous bool) (containerID string, err error) { + var cID string + cStatus, found := api.GetContainerStatus(podStatus.ContainerStatuses, containerName) if !found { return "", fmt.Errorf("container %q not found in pod", containerName) } - if cStatus.State.Waiting != nil { - return "", fmt.Errorf("container %q is in waiting state.", containerName) + if previous { + if cStatus.LastTerminationState.Termination == nil { + return "", fmt.Errorf("previous terminated container %q not found in pod", containerName) + } + cID = cStatus.LastTerminationState.Termination.ContainerID + } else { + if cStatus.State.Waiting != nil { + return "", fmt.Errorf("container %q is in waiting state.", containerName) + } + cID = cStatus.ContainerID } - return kubecontainer.TrimRuntimePrefix(cStatus.ContainerID), nil + return kubecontainer.TrimRuntimePrefix(cID), nil } // GetKubeletContainerLogs returns logs from the container // TODO: this method is returning logs of random container attempts, when it should be returning the most recent attempt // or all of them. -func (kl *Kubelet) GetKubeletContainerLogs(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { +func (kl *Kubelet) GetKubeletContainerLogs(podFullName, containerName, tail string, follow, previous bool, stdout, stderr io.Writer) error { // TODO(vmarmol): Refactor to not need the pod status and verification. podStatus, err := kl.GetPodStatus(podFullName) if err != nil { @@ -1397,7 +1407,7 @@ func (kl *Kubelet) GetKubeletContainerLogs(podFullName, containerName, tail stri // No log is available if pod is not in a "known" phase (e.g. Unknown). return err } - containerID, err := kl.validateContainerStatus(&podStatus, containerName) + containerID, err := kl.validateContainerStatus(&podStatus, containerName, previous) if err != nil { // No log is available if the container status is missing or is in the // waiting state. diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index f02cb66ee6c..a9d599d4731 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -3141,6 +3141,9 @@ func TestValidateContainerStatus(t *testing.T) { State: api.ContainerState{ Running: &api.ContainerStateRunning{}, }, + LastTerminationState: api.ContainerState{ + Termination: &api.ContainerStateTerminated{}, + }, }, }, success: true, @@ -3172,7 +3175,7 @@ func TestValidateContainerStatus(t *testing.T) { for i, tc := range testCases { _, err := kubelet.validateContainerStatus(&api.PodStatus{ ContainerStatuses: tc.statuses, - }, containerName) + }, containerName, false) if tc.success { if err != nil { t.Errorf("[case %d]: unexpected failure - %v", i, err) @@ -3183,9 +3186,19 @@ func TestValidateContainerStatus(t *testing.T) { } if _, err := kubelet.validateContainerStatus(&api.PodStatus{ ContainerStatuses: testCases[0].statuses, - }, "blah"); err == nil { + }, "blah", false); err == nil { t.Errorf("expected error with invalid container name") } + if _, err := kubelet.validateContainerStatus(&api.PodStatus{ + ContainerStatuses: testCases[0].statuses, + }, containerName, true); err != nil { + t.Errorf("unexpected error with for previous terminated container - %v", err) + } + if _, err := kubelet.validateContainerStatus(&api.PodStatus{ + ContainerStatuses: testCases[1].statuses, + }, containerName, true); err == nil { + t.Errorf("expected error with for previous terminated container") + } } func TestUpdateNewNodeStatus(t *testing.T) { diff --git a/pkg/kubelet/server.go b/pkg/kubelet/server.go index 7da16532edb..550e3182374 100644 --- a/pkg/kubelet/server.go +++ b/pkg/kubelet/server.go @@ -107,7 +107,7 @@ type HostInterface interface { GetPodStatus(name string) (api.PodStatus, error) RunInContainer(name string, uid types.UID, container string, cmd []string) ([]byte, error) ExecInContainer(name string, uid types.UID, container string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error - GetKubeletContainerLogs(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error + GetKubeletContainerLogs(podFullName, containerName, tail string, follow, previous bool, stdout, stderr io.Writer) error ServeLogs(w http.ResponseWriter, req *http.Request) PortForward(name string, uid types.UID, port uint16, stream io.ReadWriteCloser) error StreamingConnectionIdleTimeout() time.Duration @@ -230,6 +230,7 @@ func (s *Server) handleContainerLogs(w http.ResponseWriter, req *http.Request) { uriValues := u.Query() follow, _ := strconv.ParseBool(uriValues.Get("follow")) + previous, _ := strconv.ParseBool(uriValues.Get("previous")) tail := uriValues.Get("tail") pod, ok := s.host.GetPodByName(podNamespace, podID) @@ -256,7 +257,7 @@ func (s *Server) handleContainerLogs(w http.ResponseWriter, req *http.Request) { fw := flushwriter.Wrap(w) w.Header().Set("Transfer-Encoding", "chunked") w.WriteHeader(http.StatusOK) - err = s.host.GetKubeletContainerLogs(kubecontainer.GetPodFullName(pod), containerName, tail, follow, fw, fw) + err = s.host.GetKubeletContainerLogs(kubecontainer.GetPodFullName(pod), containerName, tail, follow, previous, fw, fw) if err != nil { s.error(w, err) return diff --git a/pkg/kubelet/server_test.go b/pkg/kubelet/server_test.go index 795040c81c4..e6d46b05805 100644 --- a/pkg/kubelet/server_test.go +++ b/pkg/kubelet/server_test.go @@ -53,7 +53,7 @@ type fakeKubelet struct { containerVersionFunc func() (kubecontainer.Version, error) execFunc func(pod string, uid types.UID, container string, cmd []string, in io.Reader, out, err io.WriteCloser, tty bool) error portForwardFunc func(name string, uid types.UID, port uint16, stream io.ReadWriteCloser) error - containerLogsFunc func(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error + containerLogsFunc func(podFullName, containerName, tail string, follow, pervious bool, stdout, stderr io.Writer) error streamingConnectionIdleTimeoutFunc func() time.Duration hostnameFunc func() string } @@ -90,8 +90,8 @@ func (fk *fakeKubelet) ServeLogs(w http.ResponseWriter, req *http.Request) { fk.logFunc(w, req) } -func (fk *fakeKubelet) GetKubeletContainerLogs(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { - return fk.containerLogsFunc(podFullName, containerName, tail, follow, stdout, stderr) +func (fk *fakeKubelet) GetKubeletContainerLogs(podFullName, containerName, tail string, follow, previous bool, stdout, stderr io.Writer) error { + return fk.containerLogsFunc(podFullName, containerName, tail, follow, previous, stdout, stderr) } func (fk *fakeKubelet) GetHostname() string { @@ -553,8 +553,8 @@ func setPodByNameFunc(fw *serverTestFramework, namespace, pod, container string) } } -func setGetContainerLogsFunc(fw *serverTestFramework, t *testing.T, expectedPodName, expectedContainerName, expectedTail string, expectedFollow bool, output string) { - fw.fakeKubelet.containerLogsFunc = func(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { +func setGetContainerLogsFunc(fw *serverTestFramework, t *testing.T, expectedPodName, expectedContainerName, expectedTail string, expectedFollow, expectedPrevious bool, output string) { + fw.fakeKubelet.containerLogsFunc = func(podFullName, containerName, tail string, follow, previous bool, stdout, stderr io.Writer) error { if podFullName != expectedPodName { t.Errorf("expected %s, got %s", expectedPodName, podFullName) } @@ -567,6 +567,10 @@ func setGetContainerLogsFunc(fw *serverTestFramework, t *testing.T, expectedPodN if follow != expectedFollow { t.Errorf("expected %t, got %t", expectedFollow, follow) } + if previous != expectedPrevious { + t.Errorf("expected %t, got %t", expectedPrevious, previous) + } + io.WriteString(stdout, output) return nil } @@ -581,8 +585,9 @@ func TestContainerLogs(t *testing.T) { expectedContainerName := "baz" expectedTail := "" expectedFollow := false + expectedPrevious := false setPodByNameFunc(fw, podNamespace, podName, expectedContainerName) - setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, output) + setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, expectedPrevious, output) resp, err := http.Get(fw.testHTTPServer.URL + "/containerLogs/" + podNamespace + "/" + podName + "/" + expectedContainerName) if err != nil { t.Errorf("Got error GETing: %v", err) @@ -608,8 +613,9 @@ func TestContainerLogsWithTail(t *testing.T) { expectedContainerName := "baz" expectedTail := "5" expectedFollow := false + expectedPrevious := false setPodByNameFunc(fw, podNamespace, podName, expectedContainerName) - setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, output) + setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, expectedPrevious, output) resp, err := http.Get(fw.testHTTPServer.URL + "/containerLogs/" + podNamespace + "/" + podName + "/" + expectedContainerName + "?tail=5") if err != nil { t.Errorf("Got error GETing: %v", err) @@ -635,8 +641,9 @@ func TestContainerLogsWithFollow(t *testing.T) { expectedContainerName := "baz" expectedTail := "" expectedFollow := true + expectedPrevious := false setPodByNameFunc(fw, podNamespace, podName, expectedContainerName) - setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, output) + setGetContainerLogsFunc(fw, t, expectedPodName, expectedContainerName, expectedTail, expectedFollow, expectedPrevious, output) resp, err := http.Get(fw.testHTTPServer.URL + "/containerLogs/" + podNamespace + "/" + podName + "/" + expectedContainerName + "?follow=1") if err != nil { t.Errorf("Got error GETing: %v", err) diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index 4e2ec94e91c..ff54ff0d0b4 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -212,6 +212,9 @@ func LogLocation(getter ResourceGetter, connInfo client.ConnectionInfoGetter, ct if opts.Follow { params.Add("follow", "true") } + if opts.Previous { + params.Add("previous", "true") + } loc := &url.URL{ Scheme: nodeScheme, Host: fmt.Sprintf("%s:%d", nodeHost, nodePort),