De-ducplicate and clean logic in FindOrDefaultContainerByName

Signed-off-by: Maciej Szulik <soltysh@gmail.com>
This commit is contained in:
Maciej Szulik
2026-03-04 11:44:25 +01:00
parent dd0958fece
commit bb744ec97b
2 changed files with 23 additions and 24 deletions

View File

@@ -137,7 +137,7 @@ func TestPodAndContainerAttach(t *testing.T) {
options: &AttachOptions{StreamOptions: exec.StreamOptions{ContainerName: "wrong"}, GetPodTimeout: 10},
args: []string{"foo"},
expectedPodName: "foo",
expectError: "container wrong not found in pod foo",
expectError: "container wrong is not valid for pod foo out of: bar, debugger (ephem), initfoo (init)",
obj: attachPod(),
},
{
@@ -276,7 +276,7 @@ func TestAttach(t *testing.T) {
attachPath: "/api/" + version + "/namespaces/test/pods/foo/attach",
pod: attachPod(),
container: "foo",
expectedErr: "cannot attach to the container: container foo not found in pod foo",
expectedErr: "cannot attach to the container: container foo is not valid for pod foo out of: bar, debugger (ephem), initfoo (init)",
},
}
for _, test := range tests {

View File

@@ -54,41 +54,40 @@ func FindContainerByName(pod *v1.Pod, name string) (*v1.Container, string) {
// exists, or returns an error. It will print a message to the user indicating a default was
// selected if there was more than one container.
func FindOrDefaultContainerByName(pod *v1.Pod, name string, quiet bool, warn io.Writer) (*v1.Container, error) {
var container *v1.Container
if len(name) > 0 {
container, _ = FindContainerByName(pod, name)
if container == nil {
return nil, fmt.Errorf("container %s not found in pod %s", name, pod.Name)
}
return container, nil
}
// this should never happen, but just in case
if len(pod.Spec.Containers) == 0 {
return nil, fmt.Errorf("pod %s/%s does not have any containers", pod.Namespace, pod.Name)
}
// read the default container the annotation as per
// if no container name was specified read the default container from the annotation as per
// https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/2227-kubectl-default-container
if name := pod.Annotations[DefaultContainerAnnotationName]; len(name) > 0 {
if container, _ = FindContainerByName(pod, name); container != nil {
klog.V(4).Infof("Defaulting container name from annotation %s", container.Name)
return container, nil
if len(name) == 0 {
name = pod.Annotations[DefaultContainerAnnotationName]
if len(name) > 0 {
klog.V(4).Infof("Attempting default container name from annotation %s", name)
}
klog.V(4).Infof("Default container name from annotation %s was not found in the pod", name)
}
// 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))
if len(name) > 0 {
container, _ := FindContainerByName(pod, name)
if container == nil {
return nil, fmt.Errorf("container %s is not valid for pod %s out of: %s", name, pod.Name, AllContainerNames(pod))
}
return container, nil
}
klog.V(4).Infof("Defaulting container name to %s", container.Name)
return &pod.Spec.Containers[0], nil
// fallback to pick the first container to maintain existing behavior
container := &pod.Spec.Containers[0]
if !quiet {
fmt.Fprintf(warn, "Defaulted container %q out of: %s\n", container.Name, AllContainerNames(pod)) // nolint:errcheck
}
klog.V(4).Infof("Using the first container in a pod: %s", container.Name)
return container, nil
}
// AllContainerNames returns a comma-separated list of all container names in the pod,
// including regular containers, ephemeral containers (marked with "ephem"), and init
// containers (marked with "init") in that order.
func AllContainerNames(pod *v1.Pod) string {
var containers []string
for _, container := range pod.Spec.Containers {