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.
This commit is contained in:
Lee Verberne 2019-08-20 19:51:15 +00:00
parent 460fdc7f48
commit cc32702e8f
3 changed files with 38 additions and 19 deletions

View File

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

View File

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

View File

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