github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-25 12:43:23 +00:00
Code Issues Projects Releases Wiki Activity

cmd/kubectl: make 'kubectl logs' default to the first container when default container cannot be determined or found by annotations (#105964)

Browse Source 
* cmd/kubectl: make 'kubectl logs' default to the first container.

While running 'kubectl logs <pod>', If '-c' is omited and the pod has more than one container, and no default container can be determined from annotations, this command shows an error message and exits. With this fix, it defaults to the first container in such scenarios and show its logs. This aligns behavior with what 'kubectl exec' does currently, and is more in line with KEP SIG-CLI 2227 design.

* fix unit test(forgotten)

* fix spelling typo
This commit is contained in:
Jian Li 2022-01-07 09:40:41 +08:00 committed by GitHub
parent 3bebe8f6b5
commit 0977a5d7cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go
View File

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

38
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go
View File

@ -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
}

46
staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go
View File

@ -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",