Add ephemeral containers to streamLocation name suggestions

This combines container names into a single list because separating them
into a long, variable length string isn't particularly useful in the
context of an streaming error message.

Author: verb

pkg/registry/core/pod/BUILD

@@ -47,6 +47,7 @@ go_test(
         "//pkg/api/testing:go_default_library",
         "//pkg/apis/core:go_default_library",
         "//pkg/apis/core/install:go_default_library",
+        "//pkg/features:go_default_library",
         "//pkg/kubelet/client:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
@@ -56,7 +57,9 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/client-go/tools/cache:go_default_library",
+        "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
     ],
 )
 

pkg/registry/core/pod/strategy.go

@@ -312,15 +312,6 @@ func ResourceLocation(ctx context.Context, getter ResourceGetter, rt http.RoundT
 	return loc, rt, nil
 }
 
-// getContainerNames returns a formatted string containing the container names
-func getContainerNames(containers []api.Container) string {
-	names := []string{}
-	for _, c := range containers {
-		names = append(names, c.Name)
-	}
-	return strings.Join(names, " ")
-}
-
 // LogLocation returns the log URL for a pod container. If opts.Container is blank
 // and only one container is present in the pod, that container is used.
 func LogLocation(
@@ -553,13 +544,13 @@ func validateContainer(container string, pod *api.Pod) (string, error) {
 		case 0:
 			return "", errors.NewBadRequest(fmt.Sprintf("a container name must be specified for pod %s", pod.Name))
 		default:
-			containerNames := getContainerNames(pod.Spec.Containers)
-			initContainerNames := getContainerNames(pod.Spec.InitContainers)
-			err := fmt.Sprintf("a container name must be specified for pod %s, choose one of: [%s]", pod.Name, containerNames)
-			if len(initContainerNames) > 0 {
-				err += fmt.Sprintf(" or one of the init containers: [%s]", initContainerNames)
-			}
-			return "", errors.NewBadRequest(err)
+			var containerNames []string
+			podutil.VisitContainers(&pod.Spec, func(c *api.Container) bool {
+				containerNames = append(containerNames, c.Name)
+				return true
+			})
+			errStr := fmt.Sprintf("a container name must be specified for pod %s, choose one of: %s", pod.Name, containerNames)
+			return "", errors.NewBadRequest(errStr)
 		}
 	} else {
 		if !podHasContainerWithName(pod, container) {

pkg/registry/core/pod/strategy_test.go

@@ -32,9 +32,12 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/tools/cache"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	apitesting "k8s.io/kubernetes/pkg/api/testing"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/kubelet/client"
 
 	// ensure types are installed
@@ -321,6 +324,7 @@ func (g mockPodGetter) Get(context.Context, string, *metav1.GetOptions) (runtime
 }
 
 func TestCheckLogLocation(t *testing.T) {
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EphemeralContainers, true)()
 	ctx := genericapirequest.NewDefaultContext()
 	fakePodName := "test"
 	tcs := []struct {
@@ -407,7 +411,28 @@ func TestCheckLogLocation(t *testing.T) {
 				Status: api.PodStatus{},
 			},
 			opts:              &api.PodLogOptions{},
-			expectedErr:       errors.NewBadRequest("a container name must be specified for pod test, choose one of: [container1 container2] or one of the init containers: [initcontainer1]"),
+			expectedErr:       errors.NewBadRequest("a container name must be specified for pod test, choose one of: [initcontainer1 container1 container2]"),
+			expectedTransport: nil,
+		},
+		{
+			in: &api.Pod{
+				ObjectMeta: metav1.ObjectMeta{Name: fakePodName},
+				Spec: api.PodSpec{
+					Containers: []api.Container{
+						{Name: "container1"},
+						{Name: "container2"},
+					},
+					InitContainers: []api.Container{
+						{Name: "initcontainer1"},
+					},
+					EphemeralContainers: []api.EphemeralContainer{
+						{EphemeralContainerCommon: api.EphemeralContainerCommon{Name: "debugger"}},
+					},
+				},
+				Status: api.PodStatus{},
+			},
+			opts:              &api.PodLogOptions{},
+			expectedErr:       errors.NewBadRequest("a container name must be specified for pod test, choose one of: [initcontainer1 container1 container2 debugger]"),
 			expectedTransport: nil,
 		},
 		{
@@ -458,10 +483,10 @@ func TestCheckLogLocation(t *testing.T) {
 
 			_, actualTransport, err := LogLocation(ctx, getter, connectionGetter, fakePodName, tc.opts)
 			if !reflect.DeepEqual(err, tc.expectedErr) {
-				t.Errorf("expected %v, got %v", tc.expectedErr, err)
+				t.Errorf("expected %q, got %q", tc.expectedErr, err)
 			}
 			if actualTransport != tc.expectedTransport {
-				t.Errorf("expected %v, got %v", tc.expectedTransport, actualTransport)
+				t.Errorf("expected %q, got %q", tc.expectedTransport, actualTransport)
 			}
 		})
 	}