diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs.go b/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs.go index 74aacd614fb..ba629671140 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "io" + "regexp" "sync" "time" @@ -107,6 +108,7 @@ type LogsOptions struct { ContainerNameSpecified bool Selector string MaxFollowConcurrency int + Prefix bool Object runtime.Object GetPodTimeout time.Duration @@ -116,6 +118,8 @@ type LogsOptions struct { genericclioptions.IOStreams TailSpecified bool + + containerNameFromRefSpecRegexp *regexp.Regexp } func NewLogsOptions(streams genericclioptions.IOStreams, allContainers bool) *LogsOptions { @@ -124,6 +128,8 @@ func NewLogsOptions(streams genericclioptions.IOStreams, allContainers bool) *Lo AllContainers: allContainers, Tail: -1, MaxFollowConcurrency: 5, + + containerNameFromRefSpecRegexp: regexp.MustCompile(`spec\.(?:initContainers|containers|ephemeralContainers){(.+)}`), } } @@ -156,6 +162,7 @@ func NewCmdLogs(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C cmdutil.AddPodRunningTimeoutFlag(cmd, defaultPodLogsTimeout) cmd.Flags().StringVarP(&o.Selector, "selector", "l", o.Selector, "Selector (label query) to filter on.") cmd.Flags().IntVar(&o.MaxFollowConcurrency, "max-log-requests", o.MaxFollowConcurrency, "Specify maximum number of concurrent logs to follow when using by a selector. Defaults to 5.") + cmd.Flags().BoolVar(&o.Prefix, "prefix", o.Prefix, "Prefix each log line with the log source (pod name and container name)") return cmd } @@ -314,14 +321,15 @@ func (o LogsOptions) RunLogs() error { return o.sequentialConsumeRequest(requests) } -func (o LogsOptions) parallelConsumeRequest(requests []rest.ResponseWrapper) error { +func (o LogsOptions) parallelConsumeRequest(requests map[corev1.ObjectReference]rest.ResponseWrapper) error { reader, writer := io.Pipe() wg := &sync.WaitGroup{} wg.Add(len(requests)) - for _, request := range requests { - go func(request rest.ResponseWrapper) { + for objRef, request := range requests { + go func(objRef corev1.ObjectReference, request rest.ResponseWrapper) { defer wg.Done() - if err := o.ConsumeRequestFn(request, writer); err != nil { + out := o.addPrefixIfNeeded(objRef, writer) + if err := o.ConsumeRequestFn(request, out); err != nil { if !o.IgnoreLogErrors { writer.CloseWithError(err) @@ -332,7 +340,7 @@ func (o LogsOptions) parallelConsumeRequest(requests []rest.ResponseWrapper) err fmt.Fprintf(writer, "error: %v\n", err) } - }(request) + }(objRef, request) } go func() { @@ -344,9 +352,10 @@ func (o LogsOptions) parallelConsumeRequest(requests []rest.ResponseWrapper) err return err } -func (o LogsOptions) sequentialConsumeRequest(requests []rest.ResponseWrapper) error { - for _, request := range requests { - if err := o.ConsumeRequestFn(request, o.Out); err != nil { +func (o LogsOptions) sequentialConsumeRequest(requests map[corev1.ObjectReference]rest.ResponseWrapper) error { + for objRef, request := range requests { + out := o.addPrefixIfNeeded(objRef, o.Out) + if err := o.ConsumeRequestFn(request, out); err != nil { return err } } @@ -354,6 +363,27 @@ func (o LogsOptions) sequentialConsumeRequest(requests []rest.ResponseWrapper) e return nil } +func (o LogsOptions) addPrefixIfNeeded(ref corev1.ObjectReference, writer io.Writer) io.Writer { + if !o.Prefix || ref.FieldPath == "" || ref.Name == "" { + return writer + } + + // We rely on ref.FieldPath to contain a reference to a container + // including a container name (not an index) so we can get a container name + // without making an extra API request. + var containerName string + containerNameMatches := o.containerNameFromRefSpecRegexp.FindStringSubmatch(ref.FieldPath) + if len(containerNameMatches) == 2 { + containerName = containerNameMatches[1] + } + + prefix := fmt.Sprintf("[pod/%s/%s] ", ref.Name, containerName) + return &prefixingWriter{ + prefix: []byte(prefix), + writer: writer, + } +} + // DefaultConsumeRequest reads the data from request and writes into // the out writer. It buffers data from requests until the newline or io.EOF // occurs in the data, so it doesn't interleave logs sub-line @@ -384,3 +414,25 @@ func DefaultConsumeRequest(request rest.ResponseWrapper, out io.Writer) error { } } } + +type prefixingWriter struct { + prefix []byte + writer io.Writer +} + +func (pw *prefixingWriter) Write(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } + + // Perform an "atomic" write of a prefix and p to make sure that it doesn't interleave + // sub-line when used concurrently with io.PipeWrite. + n, err := pw.writer.Write(append(pw.prefix, p...)) + if n > len(p) { + // To comply with the io.Writer interface requirements we must + // return a number of bytes written from p (0 <= n <= len(p)), + // so we are ignoring the length of the prefix here. + return len(p), err + } + return n, err +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs_test.go index c783a06cb2d..cab646de570 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/logs/logs_test.go @@ -47,8 +47,12 @@ func TestLog(t *testing.T) { name: "v1 - pod log", opts: func(streams genericclioptions.IOStreams) *LogsOptions { mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{ - &responseWrapperMock{data: strings.NewReader("test log content\n")}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "some-pod", + FieldPath: "spec.containers{some-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, }, } @@ -60,14 +64,92 @@ func TestLog(t *testing.T) { }, expectedOutSubstrings: []string{"test log content\n"}, }, + { + name: "pod logs with prefix", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + mock := &logTestMock{ + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod", + FieldPath: "spec.containers{test-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, + }, + } + + o := NewLogsOptions(streams, false) + o.LogsForObject = mock.mockLogsForObject + o.ConsumeRequestFn = mock.mockConsumeRequest + o.Prefix = true + + return o + }, + expectedOutSubstrings: []string{"[pod/test-pod/test-container] test log content\n"}, + }, + { + name: "pod logs with prefix: init container", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + mock := &logTestMock{ + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod", + FieldPath: "spec.initContainers{test-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, + }, + } + + o := NewLogsOptions(streams, false) + o.LogsForObject = mock.mockLogsForObject + o.ConsumeRequestFn = mock.mockConsumeRequest + o.Prefix = true + + return o + }, + expectedOutSubstrings: []string{"[pod/test-pod/test-container] test log content\n"}, + }, + { + name: "pod logs with prefix: ephemeral container", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + mock := &logTestMock{ + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod", + FieldPath: "spec.ephemeralContainers{test-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, + }, + } + + o := NewLogsOptions(streams, false) + o.LogsForObject = mock.mockLogsForObject + o.ConsumeRequestFn = mock.mockConsumeRequest + o.Prefix = true + + return o + }, + expectedOutSubstrings: []string{"[pod/test-pod/test-container] test log content\n"}, + }, { name: "get logs from multiple requests sequentially", opts: func(streams genericclioptions.IOStreams) *LogsOptions { mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{ - &responseWrapperMock{data: strings.NewReader("test log content from source 1\n")}, - &responseWrapperMock{data: strings.NewReader("test log content from source 2\n")}, - &responseWrapperMock{data: strings.NewReader("test log content from source 3\n")}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "some-pod-1", + FieldPath: "spec.containers{some-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 1\n")}, + { + Kind: "Pod", + Name: "some-pod-2", + FieldPath: "spec.containers{some-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 2\n")}, + { + Kind: "Pod", + Name: "some-pod-3", + FieldPath: "spec.containers{some-container}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 3\n")}, }, } @@ -77,8 +159,9 @@ func TestLog(t *testing.T) { return o }, expectedOutSubstrings: []string{ - // Order in this case must always be the same, because we read requests sequentially - "test log content from source 1\ntest log content from source 2\ntest log content from source 3\n", + "test log content from source 1\n", + "test log content from source 2\n", + "test log content from source 3\n", }, }, { @@ -86,10 +169,22 @@ func TestLog(t *testing.T) { opts: func(streams genericclioptions.IOStreams) *LogsOptions { wg := &sync.WaitGroup{} mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{ - &responseWrapperMock{data: strings.NewReader("test log content from source 1\n")}, - &responseWrapperMock{data: strings.NewReader("test log content from source 2\n")}, - &responseWrapperMock{data: strings.NewReader("test log content from source 3\n")}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "some-pod-1", + FieldPath: "spec.containers{some-container-1}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 1\n")}, + { + Kind: "Pod", + Name: "some-pod-2", + FieldPath: "spec.containers{some-container-2}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 2\n")}, + { + Kind: "Pod", + Name: "some-pod-3", + FieldPath: "spec.containers{some-container-3}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 3\n")}, }, wg: wg, } @@ -112,10 +207,22 @@ func TestLog(t *testing.T) { opts: func(streams genericclioptions.IOStreams) *LogsOptions { wg := &sync.WaitGroup{} mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{ - &responseWrapperMock{data: strings.NewReader("test log content\n")}, - &responseWrapperMock{data: strings.NewReader("test log content\n")}, - &responseWrapperMock{data: strings.NewReader("test log content\n")}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod-1", + FieldPath: "spec.containers{test-container-1}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, + { + Kind: "Pod", + Name: "test-pod-2", + FieldPath: "spec.containers{test-container-2}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, + { + Kind: "Pod", + Name: "test-pod-3", + FieldPath: "spec.containers{test-container-3}", + }: &responseWrapperMock{data: strings.NewReader("test log content\n")}, }, wg: wg, } @@ -134,7 +241,7 @@ func TestLog(t *testing.T) { name: "fail if LogsForObject fails", opts: func(streams genericclioptions.IOStreams) *LogsOptions { o := NewLogsOptions(streams, false) - o.LogsForObject = func(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]restclient.ResponseWrapper, error) { + o.LogsForObject = func(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) (map[corev1.ObjectReference]restclient.ResponseWrapper, error) { return nil, errors.New("Error from the LogsForObject") } return o @@ -145,9 +252,17 @@ func TestLog(t *testing.T) { name: "fail to get logs, if ConsumeRequestFn fails", opts: func(streams genericclioptions.IOStreams) *LogsOptions { mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{ - &responseWrapperMock{}, - &responseWrapperMock{}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod-1", + FieldPath: "spec.containers{test-container-1}", + }: &responseWrapperMock{}, + { + Kind: "Pod", + Name: "test-pod-2", + FieldPath: "spec.containers{test-container-1}", + }: &responseWrapperMock{}, }, } @@ -160,15 +275,66 @@ func TestLog(t *testing.T) { }, expectedErr: "Error from the ConsumeRequestFn", }, + { + name: "follow logs from multiple requests concurrently with prefix", + opts: func(streams genericclioptions.IOStreams) *LogsOptions { + wg := &sync.WaitGroup{} + mock := &logTestMock{ + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod-1", + FieldPath: "spec.containers{test-container-1}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 1\n")}, + { + Kind: "Pod", + Name: "test-pod-2", + FieldPath: "spec.containers{test-container-2}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 2\n")}, + { + Kind: "Pod", + Name: "test-pod-3", + FieldPath: "spec.containers{test-container-3}", + }: &responseWrapperMock{data: strings.NewReader("test log content from source 3\n")}, + }, + wg: wg, + } + wg.Add(3) + + o := NewLogsOptions(streams, false) + o.LogsForObject = mock.mockLogsForObject + o.ConsumeRequestFn = mock.mockConsumeRequest + o.Follow = true + o.Prefix = true + return o + }, + expectedOutSubstrings: []string{ + "[pod/test-pod-1/test-container-1] test log content from source 1\n", + "[pod/test-pod-2/test-container-2] test log content from source 2\n", + "[pod/test-pod-3/test-container-3] test log content from source 3\n", + }, + }, { name: "fail to follow logs from multiple requests, if ConsumeRequestFn fails", opts: func(streams genericclioptions.IOStreams) *LogsOptions { wg := &sync.WaitGroup{} mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{ - &responseWrapperMock{}, - &responseWrapperMock{}, - &responseWrapperMock{}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod-1", + FieldPath: "spec.containers{test-container-1}", + }: &responseWrapperMock{}, + { + Kind: "Pod", + Name: "test-pod-2", + FieldPath: "spec.containers{test-container-2}", + }: &responseWrapperMock{}, + { + Kind: "Pod", + Name: "test-pod-3", + FieldPath: "spec.containers{test-container-3}", + }: &responseWrapperMock{}, }, wg: wg, } @@ -188,7 +354,13 @@ func TestLog(t *testing.T) { name: "fail to follow logs, if ConsumeRequestFn fails", opts: func(streams genericclioptions.IOStreams) *LogsOptions { mock := &logTestMock{ - logsForObjectRequests: []restclient.ResponseWrapper{&responseWrapperMock{}}, + logsForObjectRequests: map[corev1.ObjectReference]restclient.ResponseWrapper{ + { + Kind: "Pod", + Name: "test-pod-1", + FieldPath: "spec.containers{test-container-1}", + }: &responseWrapperMock{}, + }, } o := NewLogsOptions(streams, false) @@ -505,7 +677,7 @@ func (r *responseWrapperMock) Stream() (io.ReadCloser, error) { } type logTestMock struct { - logsForObjectRequests []restclient.ResponseWrapper + logsForObjectRequests map[corev1.ObjectReference]restclient.ResponseWrapper // We need a WaitGroup in some test cases to make sure that we fetch logs concurrently. // These test cases will finish successfully without the WaitGroup, but the WaitGroup @@ -530,7 +702,7 @@ func (l *logTestMock) mockConsumeRequest(request restclient.ResponseWrapper, out return err } -func (l *logTestMock) mockLogsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]restclient.ResponseWrapper, error) { +func (l *logTestMock) mockLogsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) (map[corev1.ObjectReference]restclient.ResponseWrapper, error) { switch object.(type) { case *corev1.Pod: _, ok := options.(*corev1.PodLogOptions) diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/BUILD b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/BUILD index 028bba601a9..1d604e6e5fb 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/BUILD +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/BUILD @@ -49,6 +49,7 @@ go_library( "//staging/src/k8s.io/client-go/kubernetes/typed/apps/v1: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", + "//staging/src/k8s.io/client-go/tools/reference:go_default_library", "//staging/src/k8s.io/client-go/tools/watch:go_default_library", "//staging/src/k8s.io/kubectl/pkg/apps:go_default_library", "//staging/src/k8s.io/kubectl/pkg/describe/versioned:go_default_library", diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/interface.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/interface.go index 425d68f5b4a..83e13714f0c 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/interface.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/interface.go @@ -28,7 +28,7 @@ import ( ) // LogsForObjectFunc is a function type that can tell you how to get logs for a runtime.object -type LogsForObjectFunc func(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]rest.ResponseWrapper, error) +type LogsForObjectFunc func(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) (map[v1.ObjectReference]rest.ResponseWrapper, error) // LogsForObjectFn gives a way to easily override the function for unit testing if needed. var LogsForObjectFn LogsForObjectFunc = logsForObject diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go index e0fbb97578c..4e55dc263e7 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go @@ -21,6 +21,7 @@ import ( "fmt" "os" "sort" + "strings" "time" corev1 "k8s.io/api/core/v1" @@ -28,10 +29,12 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" + "k8s.io/client-go/tools/reference" + "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/podutils" ) -func logsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]rest.ResponseWrapper, error) { +func logsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) (map[corev1.ObjectReference]rest.ResponseWrapper, error) { clientConfig, err := restClientGetter.ToRESTConfig() if err != nil { return nil, err @@ -44,9 +47,8 @@ func logsForObject(restClientGetter genericclioptions.RESTClientGetter, object, 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 corev1client.CoreV1Interface, object, options runtime.Object, timeout time.Duration, allContainers bool) ([]rest.ResponseWrapper, error) { +func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, options runtime.Object, timeout time.Duration, allContainers bool) (map[corev1.ObjectReference]rest.ResponseWrapper, error) { opts, ok := options.(*corev1.PodLogOptions) if !ok { return nil, errors.New("provided options object is not a PodLogOptions") @@ -54,23 +56,59 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt switch t := object.(type) { case *corev1.PodList: - ret := []rest.ResponseWrapper{} + ret := make(map[corev1.ObjectReference]rest.ResponseWrapper) for i := range t.Items { currRet, err := logsForObjectWithClient(clientset, &t.Items[i], options, timeout, allContainers) if err != nil { return nil, err } - ret = append(ret, currRet...) + for k, v := range currRet { + ret[k] = v + } } return ret, nil 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.ResponseWrapper{clientset.Pods(t.Namespace).GetLogs(t.Name, opts)}, nil + var containerName string + if opts == nil || len(opts.Container) == 0 { + // We don't know container name. In this case we expect only one container to be present in the pod (ignoring InitContainers). + // If there is more than one container we should return an error showing all container names. + if len(t.Spec.Containers) != 1 { + containerNames := getContainerNames(t.Spec.Containers) + initContainerNames := getContainerNames(t.Spec.InitContainers) + ephemeralContainerNames := getContainerNames(ephemeralContainersToContainers(t.Spec.EphemeralContainers)) + err := fmt.Sprintf("a container name must be specified for pod %s, choose one of: [%s]", t.Name, containerNames) + if len(initContainerNames) > 0 { + err += fmt.Sprintf(" or one of the init containers: [%s]", initContainerNames) + } + if len(ephemeralContainerNames) > 0 { + err += fmt.Sprintf(" or one of the ephemeral containers: [%s]", ephemeralContainerNames) + } + + return nil, errors.New(err) + } + containerName = t.Spec.Containers[0].Name + } else { + containerName = opts.Container + } + + container, fieldPath := findContainerByName(t, containerName) + if container == nil { + return nil, fmt.Errorf("container %s is not valid for pod %s", opts.Container, t.Name) + } + ref, err := reference.GetPartialReference(scheme.Scheme, t, fieldPath) + if err != nil { + return nil, fmt.Errorf("Unable to construct reference to '%#v': %v", t, err) + } + + ret := make(map[corev1.ObjectReference]rest.ResponseWrapper, 1) + ret[*ref] = clientset.Pods(t.Namespace).GetLogs(t.Name, opts) + return ret, nil } - ret := []rest.ResponseWrapper{} + ret := make(map[corev1.ObjectReference]rest.ResponseWrapper) for _, c := range t.Spec.InitContainers { currOpts := opts.DeepCopy() currOpts.Container = c.Name @@ -78,7 +116,9 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt if err != nil { return nil, err } - ret = append(ret, currRet...) + for k, v := range currRet { + ret[k] = v + } } for _, c := range t.Spec.Containers { currOpts := opts.DeepCopy() @@ -87,7 +127,9 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt if err != nil { return nil, err } - ret = append(ret, currRet...) + for k, v := range currRet { + ret[k] = v + } } for _, c := range t.Spec.EphemeralContainers { currOpts := opts.DeepCopy() @@ -96,7 +138,9 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt if err != nil { return nil, err } - ret = append(ret, currRet...) + for k, v := range currRet { + ret[k] = v + } } return ret, nil @@ -118,3 +162,42 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt return logsForObjectWithClient(clientset, pod, options, timeout, allContainers) } + +// findContainerByName searches for a container by name amongst all containers in a pod. +// Returns a pointer to a container and a field path. +func findContainerByName(pod *corev1.Pod, name string) (container *corev1.Container, fieldPath string) { + for _, c := range pod.Spec.InitContainers { + if c.Name == name { + return &c, fmt.Sprintf("spec.initContainers{%s}", c.Name) + } + } + for _, c := range pod.Spec.Containers { + if c.Name == name { + return &c, fmt.Sprintf("spec.containers{%s}", c.Name) + } + } + for _, c := range pod.Spec.EphemeralContainers { + if c.Name == name { + containerCommon := corev1.Container(c.EphemeralContainerCommon) + return &containerCommon, fmt.Sprintf("spec.ephemeralContainers{%s}", containerCommon.Name) + } + } + return nil, "" +} + +// getContainerNames returns a formatted string containing the container names +func getContainerNames(containers []corev1.Container) string { + names := []string{} + for _, c := range containers { + names = append(names, c.Name) + } + return strings.Join(names, " ") +} + +func ephemeralContainersToContainers(containers []corev1.EphemeralContainer) []corev1.Container { + var ec []corev1.Container + for i := range containers { + ec = append(ec, corev1.Container(containers[i].EphemeralContainerCommon)) + } + return ec +} diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go index 7403ec784da..3ac89ee9796 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go @@ -17,6 +17,7 @@ limitations under the License. package polymorphichelpers import ( + "fmt" "reflect" "testing" "time" @@ -44,102 +45,149 @@ func TestLogsForObject(t *testing.T) { obj runtime.Object opts *corev1.PodLogOptions allContainers bool - pods []runtime.Object + clientsetPods []runtime.Object actions []testclient.Action + + expectedErr string + expectedSources []corev1.ObjectReference }{ { name: "pod logs", - obj: &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, - }, - pods: []runtime.Object{testPod()}, + obj: testPodWithOneContainers(), actions: []testclient.Action{ getLogsAction("test", nil), }, - }, - { - name: "pod logs: all containers", - obj: &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, - Spec: corev1.PodSpec{ - InitContainers: []corev1.Container{ - {Name: "initc1"}, - {Name: "initc2"}, - }, - Containers: []corev1.Container{ - {Name: "c1"}, - {Name: "c2"}, - }, - EphemeralContainers: []corev1.EphemeralContainer{ - { - EphemeralContainerCommon: corev1.EphemeralContainerCommon{Name: "e1"}, - }, - }, + expectedSources: []corev1.ObjectReference{ + { + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), }, }, + }, + { + name: "pod logs: all containers", + obj: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers(), opts: &corev1.PodLogOptions{}, allContainers: true, - pods: []runtime.Object{testPod()}, actions: []testclient.Action{ - getLogsAction("test", &corev1.PodLogOptions{Container: "initc1"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "initc2"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "c2"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "e1"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-and-1-initc1"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-and-1-initc2"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-and-1-c1"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-and-1-c2"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-and-1-e1"}), }, + expectedSources: []corev1.ObjectReference{ + { + Kind: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Namespace, + FieldPath: fmt.Sprintf("spec.initContainers{%s}", testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Spec.InitContainers[0].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Namespace, + FieldPath: fmt.Sprintf("spec.initContainers{%s}", testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Spec.InitContainers[1].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Spec.Containers[0].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Spec.Containers[1].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Namespace, + FieldPath: fmt.Sprintf("spec.ephemeralContainers{%s}", testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Spec.EphemeralContainers[0].Name), + }, + }, + }, + { + name: "pod logs: error - must provide container name", + obj: testPodWithTwoContainers(), + expectedErr: "a container name must be specified for pod foo-two-containers, choose one of: [foo-2-c1 foo-2-c2]", }, { name: "pods list logs", obj: &corev1.PodList{ - Items: []corev1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, - Spec: corev1.PodSpec{ - InitContainers: []corev1.Container{ - {Name: "initc1"}, - {Name: "initc2"}, - }, - Containers: []corev1.Container{ - {Name: "c1"}, - {Name: "c2"}, - }, - }, - }, - }, + Items: []corev1.Pod{*testPodWithOneContainers()}, }, - pods: []runtime.Object{testPod()}, actions: []testclient.Action{ getLogsAction("test", nil), }, + expectedSources: []corev1.ObjectReference{{ + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), + }}, }, { name: "pods list logs: all containers", obj: &corev1.PodList{ - Items: []corev1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{Name: "hello", Namespace: "test"}, - Spec: corev1.PodSpec{ - InitContainers: []corev1.Container{ - {Name: "initc1"}, - {Name: "initc2"}, - }, - Containers: []corev1.Container{ - {Name: "c1"}, - {Name: "c2"}, - }, - }, - }, - }, + Items: []corev1.Pod{*testPodWithTwoContainersAndTwoInitContainers()}, }, opts: &corev1.PodLogOptions{}, allContainers: true, - pods: []runtime.Object{testPod()}, actions: []testclient.Action{ - getLogsAction("test", &corev1.PodLogOptions{Container: "initc1"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "initc2"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "c1"}), - getLogsAction("test", &corev1.PodLogOptions{Container: "c2"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-initc1"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-initc2"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-c1"}), + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-c2"}), }, + expectedSources: []corev1.ObjectReference{ + { + Kind: testPodWithTwoContainersAndTwoInitContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitContainers().Namespace, + FieldPath: fmt.Sprintf("spec.initContainers{%s}", testPodWithTwoContainersAndTwoInitContainers().Spec.InitContainers[0].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitContainers().Namespace, + FieldPath: fmt.Sprintf("spec.initContainers{%s}", testPodWithTwoContainersAndTwoInitContainers().Spec.InitContainers[1].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithTwoContainersAndTwoInitContainers().Spec.Containers[0].Name), + }, + { + Kind: testPodWithTwoContainersAndTwoInitContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithTwoContainersAndTwoInitContainers().Spec.Containers[1].Name), + }, + }, + }, + { + name: "pods list logs: error - must provide container name", + obj: &corev1.PodList{ + Items: []corev1.Pod{*testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers()}, + }, + expectedErr: "a container name must be specified for pod foo-two-containers-and-two-init-containers, choose one of: [foo-2-and-2-and-1-c1 foo-2-and-2-and-1-c2] or one of the init containers: [foo-2-and-2-and-1-initc1 foo-2-and-2-and-1-initc2] or one of the ephemeral containers: [foo-2-and-2-and-1-e1]", }, { name: "replication controller logs", @@ -149,11 +197,18 @@ func TestLogsForObject(t *testing.T) { Selector: map[string]string{"foo": "bar"}, }, }, - pods: []runtime.Object{testPod()}, + clientsetPods: []runtime.Object{testPodWithOneContainers()}, actions: []testclient.Action{ testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}), getLogsAction("test", nil), }, + expectedSources: []corev1.ObjectReference{{ + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), + }}, }, { name: "replica set logs", @@ -163,11 +218,18 @@ func TestLogsForObject(t *testing.T) { Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, }, }, - pods: []runtime.Object{testPod()}, + clientsetPods: []runtime.Object{testPodWithOneContainers()}, actions: []testclient.Action{ testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}), getLogsAction("test", nil), }, + expectedSources: []corev1.ObjectReference{{ + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), + }}, }, { name: "deployment logs", @@ -177,11 +239,18 @@ func TestLogsForObject(t *testing.T) { Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, }, }, - pods: []runtime.Object{testPod()}, + clientsetPods: []runtime.Object{testPodWithOneContainers()}, actions: []testclient.Action{ testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}), getLogsAction("test", nil), }, + expectedSources: []corev1.ObjectReference{{ + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), + }}, }, { name: "job logs", @@ -191,11 +260,18 @@ func TestLogsForObject(t *testing.T) { Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, }, }, - pods: []runtime.Object{testPod()}, + clientsetPods: []runtime.Object{testPodWithOneContainers()}, actions: []testclient.Action{ testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}), getLogsAction("test", nil), }, + expectedSources: []corev1.ObjectReference{{ + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), + }}, }, { name: "stateful set logs", @@ -205,22 +281,50 @@ func TestLogsForObject(t *testing.T) { Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}, }, }, - pods: []runtime.Object{testPod()}, + clientsetPods: []runtime.Object{testPodWithOneContainers()}, actions: []testclient.Action{ testclient.NewListAction(podsResource, podsKind, "test", metav1.ListOptions{LabelSelector: "foo=bar"}), getLogsAction("test", nil), }, + expectedSources: []corev1.ObjectReference{{ + Kind: testPodWithOneContainers().Kind, + APIVersion: testPodWithOneContainers().APIVersion, + Name: testPodWithOneContainers().Name, + Namespace: testPodWithOneContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithOneContainers().Spec.Containers[0].Name), + }}, }, } for _, test := range tests { - fakeClientset := fakeexternal.NewSimpleClientset(test.pods...) - _, err := logsForObjectWithClient(fakeClientset.CoreV1(), test.obj, test.opts, 20*time.Second, test.allContainers) - if err != nil { + fakeClientset := fakeexternal.NewSimpleClientset(test.clientsetPods...) + responses, err := logsForObjectWithClient(fakeClientset.CoreV1(), test.obj, test.opts, 20*time.Second, test.allContainers) + if test.expectedErr == "" && err != nil { t.Errorf("%s: unexpected error: %v", test.name, err) continue } + if err != nil && test.expectedErr != err.Error() { + t.Errorf("%s: expected error: %v, got: %v", test.name, test.expectedErr, err) + continue + } + + if len(test.expectedSources) != len(responses) { + t.Errorf( + "%s: the number of expected sources doesn't match the number of responses: %v, got: %v", + test.name, + len(test.expectedSources), + len(responses), + ) + continue + } + + for _, ref := range test.expectedSources { + if _, ok := responses[ref]; !ok { + t.Errorf("%s: didn't find expected log source object reference: %#v", test.name, ref) + } + } + var i int for i = range test.actions { if len(fakeClientset.Actions()) < i { @@ -240,8 +344,12 @@ func TestLogsForObject(t *testing.T) { } } -func testPod() runtime.Object { +func testPodWithOneContainers() *corev1.Pod { return &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "pod", + APIVersion: "v1", + }, ObjectMeta: metav1.ObjectMeta{ Name: "foo", Namespace: "test", @@ -252,7 +360,79 @@ func testPod() runtime.Object { DNSPolicy: corev1.DNSClusterFirst, Containers: []corev1.Container{ {Name: "c1"}, - {Name: "c2"}, + }, + }, + } +} + +func testPodWithTwoContainers() *corev1.Pod { + return &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "pod", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-two-containers", + Namespace: "test", + Labels: map[string]string{"foo": "bar"}, + }, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + DNSPolicy: corev1.DNSClusterFirst, + Containers: []corev1.Container{ + {Name: "foo-2-c1"}, + {Name: "foo-2-c2"}, + }, + }, + } +} + +func testPodWithTwoContainersAndTwoInitContainers() *corev1.Pod { + return &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "pod", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-two-containers-and-two-init-containers", + Namespace: "test", + }, + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "foo-2-and-2-initc1"}, + {Name: "foo-2-and-2-initc2"}, + }, + Containers: []corev1.Container{ + {Name: "foo-2-and-2-c1"}, + {Name: "foo-2-and-2-c2"}, + }, + }, + } +} + +func testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers() *corev1.Pod { + return &corev1.Pod{ + TypeMeta: metav1.TypeMeta{ + Kind: "pod", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo-two-containers-and-two-init-containers", + Namespace: "test", + }, + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "foo-2-and-2-and-1-initc1"}, + {Name: "foo-2-and-2-and-1-initc2"}, + }, + Containers: []corev1.Container{ + {Name: "foo-2-and-2-and-1-c1"}, + {Name: "foo-2-and-2-and-1-c2"}, + }, + EphemeralContainers: []corev1.EphemeralContainer{ + { + EphemeralContainerCommon: corev1.EphemeralContainerCommon{Name: "foo-2-and-2-and-1-e1"}, + }, }, }, }