Merge pull request #97099 from pacoxu/fix/96986

default container behavior with annotation `kubectl.kubernetes.io/default-container`

staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go

@@ -323,16 +323,7 @@ func (p *ExecOptions) Run() error {
 		return fmt.Errorf("cannot exec into a container in a completed pod; current phase is %s", pod.Status.Phase)
 	}
 
-	containerName := p.ContainerName
+	containerName := getDefaultContainerName(p, pod)
-	if len(containerName) == 0 {
-		if len(pod.Spec.Containers) > 1 {
-			fmt.Fprintf(p.ErrOut, "Defaulting container name to %s.\n", pod.Spec.Containers[0].Name)
-			if p.EnableSuggestedCmdUsage {
-				fmt.Fprintf(p.ErrOut, "Use '%s describe pod/%s -n %s' to see all of the containers in this pod.\n", p.ParentCommandName, pod.Name, p.Namespace)
-			}
-		}
-		containerName = pod.Spec.Containers[0].Name
-	}
 
 	// ensure we can recover the terminal while attached
 	t := p.SetupTTY()
@@ -377,3 +368,12 @@ func (p *ExecOptions) Run() error {
 
 	return nil
 }
+
+func getDefaultContainerName(p *ExecOptions, pod *corev1.Pod) string {
+	containerName := p.ContainerName
+	if len(containerName) != 0 {
+		return containerName
+	}
+
+	return cmdutil.GetDefaultContainerName(pod, p.EnableSuggestedCmdUsage, p.ErrOut)
+}

staging/src/k8s.io/kubectl/pkg/cmd/util/BUILD

@@ -36,6 +36,7 @@ go_library(
         "//staging/src/k8s.io/kubectl/pkg/scheme:go_default_library",
         "//staging/src/k8s.io/kubectl/pkg/util/openapi:go_default_library",
         "//staging/src/k8s.io/kubectl/pkg/util/openapi/validation:go_default_library",
+        "//staging/src/k8s.io/kubectl/pkg/util/podutils:go_default_library",
         "//staging/src/k8s.io/kubectl/pkg/util/templates:go_default_library",
         "//staging/src/k8s.io/kubectl/pkg/validation:go_default_library",
         "//vendor/github.com/evanphx/json-patch:go_default_library",

staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go

@@ -30,6 +30,7 @@ import (
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -44,6 +45,7 @@ import (
 	"k8s.io/client-go/scale"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/klog/v2"
+	"k8s.io/kubectl/pkg/util/podutils"
 	utilexec "k8s.io/utils/exec"
 )
 
@@ -744,3 +746,27 @@ func Warning(cmdErr io.Writer, newGeneratorName, oldGeneratorName string) {
 		oldGeneratorName,
 	)
 }
+
+// GetDefaultContainerName returns the default container name for a pod.
+// it checks the annotation kubectl.kubernetes.io/default-container at first and then the first container name.
+func GetDefaultContainerName(pod *corev1.Pod, enableSuggestedCmdUsage bool, w io.Writer) string {
+	if len(pod.Spec.Containers) > 1 {
+		// in case the "kubectl.kubernetes.io/default-container" annotation is present, we preset the opts.Containers to default to selected
+		// container. This gives users ability to preselect the most interesting container in pod.
+		if annotations := pod.Annotations; annotations != nil && len(annotations[podutils.DefaultContainerAnnotationName]) > 0 {
+			containerName := annotations[podutils.DefaultContainerAnnotationName]
+			if exists, _ := podutils.FindContainerByName(pod, containerName); exists != nil {
+				return containerName
+			} else {
+				fmt.Fprintf(w, "Default container name %q in annotation not found in a pod\n", containerName)
+			}
+		}
+		fmt.Fprintf(w, "Defaulting container name to first container %s.\n", pod.Spec.Containers[0].Name)
+
+		if enableSuggestedCmdUsage {
+			fmt.Fprintf(w, "Use 'kubectl describe pod/%s -n %s' to see all of the containers in this pod.\n", pod.Name, pod.Namespace)
+		}
+	}
+
+	return pod.Spec.Containers[0].Name
+}

staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go

@@ -35,7 +35,7 @@ import (
 )
 
 // defaultLogsContainerAnnotationName is an annotation name that can be used to preselect the interesting container
-// from a pod when running kubectl logs.
+// from a pod when running kubectl logs. It is deprecated and will be remove in 1.25.
 const defaultLogsContainerAnnotationName = "kubectl.kubernetes.io/default-logs-container"
 
 func logsForObject(restClientGetter genericclioptions.RESTClientGetter, object, options runtime.Object, timeout time.Duration, allContainers bool) (map[corev1.ObjectReference]rest.ResponseWrapper, error) {
@@ -73,14 +73,22 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
 		return ret, nil
 
 	case *corev1.Pod:
-		// in case the "kubectl.kubernetes.io/default-logs-container" annotation is present, we preset the opts.Containers to default to selected
+		// in case the "kubectl.kubernetes.io/default-container" annotation is present, we preset the opts.Containers to default to selected
 		// container. This gives users ability to preselect the most interesting container in pod.
-		if annotations := t.GetAnnotations(); annotations != nil && len(opts.Container) == 0 && len(annotations[defaultLogsContainerAnnotationName]) > 0 {
+		if annotations := t.GetAnnotations(); annotations != nil && len(opts.Container) == 0 {
-			containerName := annotations[defaultLogsContainerAnnotationName]
+			var containerName string
-			if exists, _ := findContainerByName(t, containerName); exists != nil {
+			if len(annotations[defaultLogsContainerAnnotationName]) > 0 {
-				opts.Container = containerName
+				containerName = annotations[defaultLogsContainerAnnotationName]
-			} else {
+				fmt.Fprintf(os.Stderr, "Using deprecated annotation `kubectl.kubernetes.io/default-logs-container` in pod/%v. Please use `kubectl.kubernetes.io/default-container` instead\n", t.Name)
-				fmt.Fprintf(os.Stderr, "Default container name %q not found in a pod\n", containerName)
+			} else if len(annotations[podutils.DefaultContainerAnnotationName]) > 0 {
+				containerName = annotations[podutils.DefaultContainerAnnotationName]
+			}
+			if len(containerName) > 0 {
+				if exists, _ := podutils.FindContainerByName(t, containerName); exists != nil {
+					opts.Container = containerName
+				} else {
+					fmt.Fprintf(os.Stderr, "Default container name %q not found in a pod\n", containerName)
+				}
 			}
 		}
 		// if allContainers is true, then we're going to locate all containers and then iterate through them. At that point, "allContainers" is false
@@ -108,7 +116,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt
 				containerName = opts.Container
 			}
 
-			container, fieldPath := findContainerByName(t, containerName)
+			container, fieldPath := podutils.FindContainerByName(t, containerName)
 			if container == nil {
 				return nil, fmt.Errorf("container %s is not valid for pod %s", opts.Container, t.Name)
 			}
@@ -177,28 +185,6 @@ 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{}

staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/diff"
 	fakeexternal "k8s.io/client-go/kubernetes/fake"
 	testclient "k8s.io/client-go/testing"
+	"k8s.io/kubectl/pkg/util/podutils"
 )
 
 var (
@@ -429,7 +430,7 @@ func TestLogsForObjectWithClient(t *testing.T) {
 			name: "two container pod with default container selected",
 			podFn: func() *corev1.Pod {
 				pod := testPodWithTwoContainers()
-				pod.Annotations = map[string]string{defaultLogsContainerAnnotationName: "foo-2-c1"}
+				pod.Annotations = map[string]string{podutils.DefaultContainerAnnotationName: "foo-2-c1"}
 				return pod
 			},
 			podLogOptions:     &corev1.PodLogOptions{},
@@ -439,7 +440,7 @@ func TestLogsForObjectWithClient(t *testing.T) {
 			name: "two container pod with default container selected but also container set explicitly",
 			podFn: func() *corev1.Pod {
 				pod := testPodWithTwoContainers()
-				pod.Annotations = map[string]string{defaultLogsContainerAnnotationName: "foo-2-c1"}
+				pod.Annotations = map[string]string{podutils.DefaultContainerAnnotationName: "foo-2-c1"}
 				return pod
 			},
 			podLogOptions: &corev1.PodLogOptions{
@@ -451,7 +452,7 @@ func TestLogsForObjectWithClient(t *testing.T) {
 			name: "two container pod with non-existing default container selected",
 			podFn: func() *corev1.Pod {
 				pod := testPodWithTwoContainers()
-				pod.Annotations = map[string]string{defaultLogsContainerAnnotationName: "non-existing"}
+				pod.Annotations = map[string]string{podutils.DefaultContainerAnnotationName: "non-existing"}
 				return pod
 			},
 			podLogOptions: &corev1.PodLogOptions{},
@@ -461,7 +462,7 @@ func TestLogsForObjectWithClient(t *testing.T) {
 			name: "two container pod with default container set, but allContainers also set",
 			podFn: func() *corev1.Pod {
 				pod := testPodWithTwoContainers()
-				pod.Annotations = map[string]string{defaultLogsContainerAnnotationName: "foo-2-c1"}
+				pod.Annotations = map[string]string{podutils.DefaultContainerAnnotationName: "foo-2-c1"}
 				return pod
 			},
 			allContainers:     true,

staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go

@@ -17,6 +17,7 @@ limitations under the License.
 package podutils
 
 import (
+	"fmt"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -24,6 +25,10 @@ import (
 	"k8s.io/utils/integer"
 )
 
+// DefaultContainerAnnotationName is an annotation name that can be used to preselect the interesting container
+// from a pod when running kubectl.
+const DefaultContainerAnnotationName = "kubectl.kubernetes.io/default-container"
+
 // IsPodAvailable returns true if a pod is available; false otherwise.
 // Precondition for an available pod is that it must be ready. On top
 // of that, there are two cases when a pod can be considered available:
@@ -186,3 +191,25 @@ func maxContainerRestarts(pod *corev1.Pod) int {
 	}
 	return maxRestarts
 }
+
+// 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, ""
+}