Merge pull request #7973 from dchen1107/podstatus

Add kubectl log --previous support to view last terminated container log

contrib/completions/bash/kubectl

@@ -399,6 +399,8 @@ _kubectl_log()
     flags+=("--help")
     flags+=("-h")
     flags+=("--interactive")
+    flags+=("--previous")
+    flags+=("-p")
 
     must_have_one_flag=()
     must_have_one_noun=()

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

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
@@ -136,6 +140,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
 

pkg/api/types.go

@@ -1332,6 +1332,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

pkg/api/v1/conversion.go

@@ -1808,6 +1808,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 {
@@ -1816,6 +1817,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 {

pkg/api/v1/types.go

@@ -1322,6 +1322,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

pkg/api/v1beta1/types.go

@@ -1190,6 +1190,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

pkg/api/v1beta2/types.go

@@ -1210,6 +1210,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

pkg/api/v1beta3/conversion_generated.go

@@ -2560,6 +2560,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
 }
 
@@ -2572,6 +2573,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
 }
 

pkg/api/v1beta3/types.go

@@ -1322,6 +1322,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

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

pkg/kubelet/kubelet.go

@@ -1386,21 +1386,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 {
@@ -1410,7 +1420,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.

pkg/kubelet/kubelet_test.go

@@ -3134,6 +3134,9 @@ func TestValidateContainerStatus(t *testing.T) {
 					State: api.ContainerState{
 						Running: &api.ContainerStateRunning{},
 					},
+					LastTerminationState: api.ContainerState{
+						Termination: &api.ContainerStateTerminated{},
+					},
 				},
 			},
 			success: true,
@@ -3165,7 +3168,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)
@@ -3176,9 +3179,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) {

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

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)

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),