diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go index 1c464419364..bf760645d4a 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go @@ -82,14 +82,14 @@ func FindOrDefaultContainerByName(pod *v1.Pod, name string, quiet bool, warn io. // pick the first container as per existing behavior container = &pod.Spec.Containers[0] if !quiet && (len(pod.Spec.Containers) > 1 || len(pod.Spec.InitContainers) > 0 || len(pod.Spec.EphemeralContainers) > 0) { - fmt.Fprintf(warn, "Defaulted container %q out of: %s\n", container.Name, allContainerNames(pod)) + fmt.Fprintf(warn, "Defaulted container %q out of: %s\n", container.Name, AllContainerNames(pod)) } klog.V(4).Infof("Defaulting container name to %s", container.Name) return &pod.Spec.Containers[0], nil } -func allContainerNames(pod *v1.Pod) string { +func AllContainerNames(pod *v1.Pod) string { var containers []string for _, container := range pod.Spec.Containers { containers = append(containers, container.Name) diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go index 7f5a275b01a..c4d5d02d500 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go @@ -21,7 +21,6 @@ import ( "fmt" "os" "sort" - "strings" "time" corev1 "k8s.io/api/core/v1" @@ -104,23 +103,11 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt } if currOpts.Container == "" { - // 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) - } + // Default to the first container name(aligning behavior with `kubectl exec'). currOpts.Container = t.Spec.Containers[0].Name + if len(t.Spec.Containers) > 1 || len(t.Spec.InitContainers) > 0 || len(t.Spec.EphemeralContainers) > 0 { + fmt.Fprintf(os.Stderr, "Defaulted container %q out of: %s\n", currOpts.Container, podcmd.AllContainerNames(t)) + } } container, fieldPath := podcmd.FindContainerByName(t, currOpts.Container) @@ -191,20 +178,3 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt return logsForObjectWithClient(clientset, pod, options, timeout, allContainers) } - -// 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 65a0c75d402..0d002dc2905 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go @@ -119,9 +119,20 @@ func TestLogsForObject(t *testing.T) { }, }, { - 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: "pod logs: default to first container", + obj: testPodWithTwoContainers(), + actions: []testclient.Action{ + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-c1"}), + }, + expectedSources: []corev1.ObjectReference{ + { + Kind: testPodWithTwoContainers().Kind, + APIVersion: testPodWithTwoContainers().APIVersion, + Name: testPodWithTwoContainers().Name, + Namespace: testPodWithTwoContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithTwoContainers().Spec.Containers[0].Name), + }, + }, }, { name: "pods list logs", @@ -248,11 +259,22 @@ func TestLogsForObject(t *testing.T) { }, }, { - name: "pods list logs: error - must provide container name", + name: "pods list logs: default to first container", 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]", + actions: []testclient.Action{ + getLogsAction("test", &corev1.PodLogOptions{Container: "foo-2-and-2-and-1-c1"}), + }, + expectedSources: []corev1.ObjectReference{ + { + Kind: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Kind, + APIVersion: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().APIVersion, + Name: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Name, + Namespace: testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Namespace, + FieldPath: fmt.Sprintf("spec.containers{%s}", testPodWithTwoContainersAndTwoInitAndOneEphemeralContainers().Spec.Containers[0].Name), + }, + }, }, { name: "replication controller logs", @@ -485,10 +507,10 @@ func TestLogsForObjectWithClient(t *testing.T) { expectedError string }{ { - name: "two container pod without default container selected", - podFn: testPodWithTwoContainers, - podLogOptions: &corev1.PodLogOptions{}, - expectedError: `a container name must be specified for pod foo-two-containers, choose one of: [foo-2-c1 foo-2-c2]`, + name: "two container pod without default container selected should default to the first one", + podFn: testPodWithTwoContainers, + podLogOptions: &corev1.PodLogOptions{}, + expectedFieldPath: `spec.containers{foo-2-c1}`, }, { name: "two container pod with default container selected", @@ -513,14 +535,14 @@ func TestLogsForObjectWithClient(t *testing.T) { expectedFieldPath: `spec.containers{foo-2-c2}`, }, { - name: "two container pod with non-existing default container selected", + name: "two container pod with non-existing default container selected should default to the first container", podFn: func() *corev1.Pod { pod := testPodWithTwoContainers() pod.Annotations = map[string]string{podcmd.DefaultContainerAnnotationName: "non-existing"} return pod }, - podLogOptions: &corev1.PodLogOptions{}, - expectedError: `a container name must be specified for pod foo-two-containers, choose one of: [foo-2-c1 foo-2-c2]`, + podLogOptions: &corev1.PodLogOptions{}, + expectedFieldPath: `spec.containers{foo-2-c1}`, }, { name: "two container pod with default container set, but allContainers also set",