From 75700d32bff0e28c7093ef384de78dbdd0db61b2 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 11 Feb 2021 11:48:43 -0500 Subject: [PATCH 1/2] kubectl: Properly respect --quiet in exec/attach --quiet means no informational output for the human that could be confused with the output of the shell / command on the other side. --- staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go | 7 ++++--- staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go | 11 ++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go b/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go index ba63cae65f5..85e7ca9e430 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go @@ -115,6 +115,7 @@ func NewCmdAttach(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra cmd.Flags().StringVarP(&o.ContainerName, "container", "c", o.ContainerName, "Container name. If omitted, the first container in the pod will be chosen") cmd.Flags().BoolVarP(&o.Stdin, "stdin", "i", o.Stdin, "Pass stdin to the container") cmd.Flags().BoolVarP(&o.TTY, "tty", "t", o.TTY, "Stdin is a TTY") + cmd.Flags().BoolVarP(&o.Quiet, "quiet", "q", o.Quiet, "Only print output from the remote session") return cmd } @@ -257,8 +258,8 @@ func (o *AttachOptions) Run() error { } if o.TTY && !containerToAttach.TTY { o.TTY = false - if o.ErrOut != nil { - fmt.Fprintf(o.ErrOut, "Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name) + if !o.Quiet && o.ErrOut != nil { + fmt.Fprintf(o.ErrOut, "error: Unable to use a TTY - container %s did not allocate one\n", containerToAttach.Name) } } else if !o.TTY && containerToAttach.TTY { // the container was launched with a TTY, so we have to force a TTY here, otherwise you'll get @@ -292,7 +293,7 @@ func (o *AttachOptions) Run() error { return err } - if o.Stdin && t.Raw && o.Pod.Spec.RestartPolicy == corev1.RestartPolicyAlways { + if !o.Quiet && o.Stdin && t.Raw && o.Pod.Spec.RestartPolicy == corev1.RestartPolicyAlways { fmt.Fprintf(o.Out, "Session ended, resume using '%s %s -c %s -i -t' command when the pod is running\n", o.CommandName, o.Pod.Name, containerToAttach.Name) } return nil diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go b/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go index 6b874fc68bf..e49ad86ebe0 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go @@ -100,6 +100,7 @@ func NewCmdExec(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.C cmd.Flags().StringVarP(&options.ContainerName, "container", "c", options.ContainerName, "Container name. If omitted, the first container in the pod will be chosen") cmd.Flags().BoolVarP(&options.Stdin, "stdin", "i", options.Stdin, "Pass stdin to the container") cmd.Flags().BoolVarP(&options.TTY, "tty", "t", options.TTY, "Stdin is a TTY") + cmd.Flags().BoolVarP(&options.Quiet, "quiet", "q", options.Quiet, "Only print output from the remote session") return cmd } @@ -174,10 +175,14 @@ func (p *ExecOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, argsIn []s if argsLenAtDash > -1 { p.Command = argsIn[argsLenAtDash:] } else if len(argsIn) > 1 { - fmt.Fprint(p.ErrOut, "kubectl exec [POD] [COMMAND] is DEPRECATED and will be removed in a future version. Use kubectl exec [POD] -- [COMMAND] instead.\n") + if !p.Quiet { + fmt.Fprint(p.ErrOut, "kubectl exec [POD] [COMMAND] is DEPRECATED and will be removed in a future version. Use kubectl exec [POD] -- [COMMAND] instead.\n") + } p.Command = argsIn[1:] } else if len(argsIn) > 0 && len(p.FilenameOptions.Filenames) != 0 { - fmt.Fprint(p.ErrOut, "kubectl exec [POD] [COMMAND] is DEPRECATED and will be removed in a future version. Use kubectl exec [POD] -- [COMMAND] instead.\n") + if !p.Quiet { + fmt.Fprint(p.ErrOut, "kubectl exec [POD] [COMMAND] is DEPRECATED and will be removed in a future version. Use kubectl exec [POD] -- [COMMAND] instead.\n") + } p.Command = argsIn[0:] p.ResourceName = "" } @@ -260,7 +265,7 @@ func (o *StreamOptions) SetupTTY() term.TTY { if !o.isTerminalIn(t) { o.TTY = false - if o.ErrOut != nil { + if !o.Quiet && o.ErrOut != nil { fmt.Fprintln(o.ErrOut, "Unable to use a TTY - input is not a terminal or the right kind of file") } From 43e8ebbbcd3f57d18d8151efb6242f88a763b06d Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Thu, 11 Feb 2021 11:40:25 -0500 Subject: [PATCH 2/2] kubectl: Inline the containers for the user in attach/exec The behavior of the container defaulting in attach/exec is inconsistent and should be unified. As a user, when we default the vast majority of pods will have a small number of containers and so printing the container names inline (as kubectl logs did) is more appropriate. The debug message we printed about using describe was already longer than 99% of all pod container names, so we were wasting user time. Unify container selection for exec and attach to be consistent with old behavior. Properly handle the --quiet flag (should not print in that case) for both commands. Remove EnableCmdSuggestion and the machinery it needs. The message now prints: > Defaulted container "etcdctl" out of: etcdctl, etcd, etcd-metrics, etcd-ensure-env-vars (init), etcd-resources-copy (init) --- .../k8s.io/kubectl/pkg/cmd/attach/attach.go | 41 +------ .../kubectl/pkg/cmd/attach/attach_test.go | 55 ++++++++- .../src/k8s.io/kubectl/pkg/cmd/exec/exec.go | 30 ++--- .../k8s.io/kubectl/pkg/cmd/util/helpers.go | 27 ----- .../kubectl/pkg/cmd/util/podcmd/podcmd.go | 104 ++++++++++++++++++ .../pkg/polymorphichelpers/logsforobject.go | 9 +- .../polymorphichelpers/logsforobject_test.go | 10 +- .../kubectl/pkg/util/podutils/podutils.go | 27 ----- vendor/modules.txt | 1 + 9 files changed, 179 insertions(+), 125 deletions(-) create mode 100644 staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go b/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go index 85e7ca9e430..cd33392dd5b 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach.go @@ -23,7 +23,6 @@ import ( "time" "github.com/spf13/cobra" - "k8s.io/klog/v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -33,6 +32,7 @@ import ( "k8s.io/client-go/tools/remotecommand" "k8s.io/kubectl/pkg/cmd/exec" cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/cmd/util/podcmd" "k8s.io/kubectl/pkg/polymorphichelpers" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/i18n" @@ -68,9 +68,7 @@ type AttachOptions struct { // whether to disable use of standard error when streaming output from tty DisableStderr bool - CommandName string - ParentCommandName string - EnableSuggestedCmdUsage bool + CommandName string Pod *corev1.Pod @@ -185,14 +183,6 @@ func (o *AttachOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []s o.Resources = args o.restClientGetter = f - cmdParent := cmd.Parent() - if cmdParent != nil { - o.ParentCommandName = cmdParent.CommandPath() - } - if len(o.ParentCommandName) > 0 && cmdutil.IsSiblingCommandExists(cmd, "describe") { - o.EnableSuggestedCmdUsage = true - } - config, err := f.ToRESTConfig() if err != nil { return err @@ -312,32 +302,7 @@ func (o *AttachOptions) findAttachablePod(obj runtime.Object) (*corev1.Pod, erro // containerToAttach returns a reference to the container to attach to, given // by name or the first container if name is empty. func (o *AttachOptions) containerToAttachTo(pod *corev1.Pod) (*corev1.Container, error) { - if len(o.ContainerName) > 0 { - for i := range pod.Spec.Containers { - if pod.Spec.Containers[i].Name == o.ContainerName { - return &pod.Spec.Containers[i], nil - } - } - for i := range pod.Spec.InitContainers { - if pod.Spec.InitContainers[i].Name == o.ContainerName { - return &pod.Spec.InitContainers[i], nil - } - } - for i := range pod.Spec.EphemeralContainers { - if pod.Spec.EphemeralContainers[i].Name == o.ContainerName { - return (*corev1.Container)(&pod.Spec.EphemeralContainers[i].EphemeralContainerCommon), nil - } - } - return nil, fmt.Errorf("container not found (%s)", o.ContainerName) - } - - if o.EnableSuggestedCmdUsage { - fmt.Fprintf(o.ErrOut, "Defaulting container name to %s.\n", pod.Spec.Containers[0].Name) - fmt.Fprintf(o.ErrOut, "Use '%s describe pod/%s -n %s' to see all of the containers in this pod.\n", o.ParentCommandName, o.PodName, o.Namespace) - } - - klog.V(4).Infof("defaulting container name to %s", pod.Spec.Containers[0].Name) - return &pod.Spec.Containers[0], nil + return podcmd.FindOrDefaultContainerByName(pod, o.ContainerName, o.Quiet, o.ErrOut) } // GetContainerName returns the name of the container to attach to, with a fallback. diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach_test.go index eaeeae02b8b..c9b809a441a 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/attach/attach_test.go @@ -17,6 +17,7 @@ limitations under the License. package attach import ( + "bytes" "fmt" "io" "net/http" @@ -35,6 +36,7 @@ import ( "k8s.io/client-go/tools/remotecommand" "k8s.io/kubectl/pkg/cmd/exec" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" + "k8s.io/kubectl/pkg/cmd/util/podcmd" "k8s.io/kubectl/pkg/polymorphichelpers" "k8s.io/kubectl/pkg/scheme" ) @@ -65,6 +67,7 @@ func TestPodAndContainerAttach(t *testing.T) { expectError string expectedPodName string expectedContainerName string + expectOut string obj *corev1.Pod }{ { @@ -85,6 +88,25 @@ func TestPodAndContainerAttach(t *testing.T) { expectedPodName: "foo", expectedContainerName: "bar", obj: attachPod(), + expectOut: `Defaulted container "bar" out of: bar, debugger (ephem), initfoo (init)`, + }, + { + name: "no container, no flags, sets default expected container as annotation", + options: &AttachOptions{GetPodTimeout: defaultPodLogsTimeout}, + args: []string{"foo"}, + expectedPodName: "foo", + expectedContainerName: "bar", + obj: setDefaultContainer(attachPod(), "initfoo"), + expectOut: ``, + }, + { + name: "no container, no flags, sets default missing container as annotation", + options: &AttachOptions{GetPodTimeout: defaultPodLogsTimeout}, + args: []string{"foo"}, + expectedPodName: "foo", + expectedContainerName: "bar", + obj: setDefaultContainer(attachPod(), "does-not-exist"), + expectOut: `Defaulted container "bar" out of: bar, debugger (ephem), initfoo (init)`, }, { name: "container in flag", @@ -115,7 +137,7 @@ func TestPodAndContainerAttach(t *testing.T) { options: &AttachOptions{StreamOptions: exec.StreamOptions{ContainerName: "wrong"}, GetPodTimeout: 10}, args: []string{"foo"}, expectedPodName: "foo", - expectError: "container not found", + expectError: "container wrong not found in pod foo", obj: attachPod(), }, { @@ -153,11 +175,23 @@ func TestPodAndContainerAttach(t *testing.T) { pod, err := test.options.findAttachablePod(&corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "test-pod", Namespace: "test"}, Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "initfoo", + }, + }, Containers: []corev1.Container{ { Name: "foobar", }, }, + EphemeralContainers: []corev1.EphemeralContainer{ + { + EphemeralContainerCommon: corev1.EphemeralContainerCommon{ + Name: "ephemfoo", + }, + }, + }, }, }) if err != nil { @@ -171,10 +205,17 @@ func TestPodAndContainerAttach(t *testing.T) { t.Errorf("unexpected pod name: expected %q, got %q", test.expectedContainerName, pod.Name) } + var buf bytes.Buffer + test.options.ErrOut = &buf container, err := test.options.containerToAttachTo(attachPod()) + + if len(test.expectOut) > 0 && !strings.Contains(buf.String(), test.expectOut) { + t.Errorf("unexpected output: output did not contain %q\n---\n%s", test.expectOut, buf.String()) + } + if err != nil { if test.expectError == "" || !strings.Contains(err.Error(), test.expectError) { - t.Errorf("unexpected error: expected %q, got %q", err, test.expectError) + t.Errorf("unexpected error: expected %q, got %q", test.expectError, err) } return } @@ -230,7 +271,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 not found (foo)", + expectedErr: "cannot attach to the container: container foo not found in pod foo", }, } for _, test := range tests { @@ -435,3 +476,11 @@ func attachPod() *corev1.Pod { }, } } + +func setDefaultContainer(pod *corev1.Pod, name string) *corev1.Pod { + if pod.Annotations == nil { + pod.Annotations = make(map[string]string) + } + pod.Annotations[podcmd.DefaultContainerAnnotationName] = name + return pod +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go b/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go index e49ad86ebe0..17f5fa08944 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/exec/exec.go @@ -34,6 +34,7 @@ import ( "k8s.io/client-go/tools/remotecommand" cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/cmd/util/podcmd" "k8s.io/kubectl/pkg/polymorphichelpers" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/i18n" @@ -153,9 +154,6 @@ type ExecOptions struct { Command []string EnforceNamespace bool - ParentCommandName string - EnableSuggestedCmdUsage bool - Builder func() *resource.Builder ExecutablePodFn polymorphichelpers.AttachablePodForObjectFunc restClientGetter genericclioptions.RESTClientGetter @@ -203,14 +201,6 @@ func (p *ExecOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, argsIn []s p.Builder = f.NewBuilder p.restClientGetter = f - cmdParent := cmd.Parent() - if cmdParent != nil { - p.ParentCommandName = cmdParent.CommandPath() - } - if len(p.ParentCommandName) > 0 && cmdutil.IsSiblingCommandExists(cmd, "describe") { - p.EnableSuggestedCmdUsage = true - } - p.Config, err = f.ToRESTConfig() if err != nil { return err @@ -328,7 +318,14 @@ 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 := getDefaultContainerName(p, pod) + containerName := p.ContainerName + if len(containerName) == 0 { + container, err := podcmd.FindOrDefaultContainerByName(pod, containerName, p.Quiet, p.ErrOut) + if err != nil { + return err + } + containerName = container.Name + } // ensure we can recover the terminal while attached t := p.SetupTTY() @@ -373,12 +370,3 @@ 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) -} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go b/staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go index a4a75fefe31..a6bfdd80afd 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go @@ -30,7 +30,6 @@ 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" @@ -45,7 +44,6 @@ 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" ) @@ -746,28 +744,3 @@ 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 { - fmt.Fprintf(w, "Defaulting container name to container %s.\n", containerName) - 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 -} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go new file mode 100644 index 00000000000..1c464419364 --- /dev/null +++ b/staging/src/k8s.io/kubectl/pkg/cmd/util/podcmd/podcmd.go @@ -0,0 +1,104 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package podcmd + +import ( + "fmt" + "io" + "strings" + + v1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" +) + +// 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" + +// FindContainerByName selects the named container from the spec of +// the provided pod or return nil if no such container exists. +func FindContainerByName(pod *v1.Pod, name string) (*v1.Container, string) { + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name == name { + return &pod.Spec.Containers[i], fmt.Sprintf("spec.containers{%s}", name) + } + } + for i := range pod.Spec.InitContainers { + if pod.Spec.InitContainers[i].Name == name { + return &pod.Spec.InitContainers[i], fmt.Sprintf("spec.initContainers{%s}", name) + } + } + for i := range pod.Spec.EphemeralContainers { + if pod.Spec.EphemeralContainers[i].Name == name { + return (*v1.Container)(&pod.Spec.EphemeralContainers[i].EphemeralContainerCommon), fmt.Sprintf("spec.ephemeralContainers{%s}", name) + } + } + return nil, "" +} + +// FindOrDefaultContainerByName defaults a container for a pod to the first container if any +// 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 + // 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 + } + 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)) + } + + klog.V(4).Infof("Defaulting container name to %s", container.Name) + return &pod.Spec.Containers[0], nil +} + +func allContainerNames(pod *v1.Pod) string { + var containers []string + for _, container := range pod.Spec.Containers { + containers = append(containers, container.Name) + } + for _, container := range pod.Spec.EphemeralContainers { + containers = append(containers, fmt.Sprintf("%s (ephem)", container.Name)) + } + for _, container := range pod.Spec.InitContainers { + containers = append(containers, fmt.Sprintf("%s (init)", container.Name)) + } + return strings.Join(containers, ", ") +} diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go index f6fb2400923..5395596fc34 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject.go @@ -30,6 +30,7 @@ import ( corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "k8s.io/client-go/tools/reference" + "k8s.io/kubectl/pkg/cmd/util/podcmd" "k8s.io/kubectl/pkg/scheme" "k8s.io/kubectl/pkg/util/podutils" ) @@ -79,8 +80,8 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt // container. This gives users ability to preselect the most interesting container in pod. if annotations := t.GetAnnotations(); annotations != nil && len(opts.Container) == 0 { var containerName string - if len(annotations[podutils.DefaultContainerAnnotationName]) > 0 { - containerName = annotations[podutils.DefaultContainerAnnotationName] + if len(annotations[podcmd.DefaultContainerAnnotationName]) > 0 { + containerName = annotations[podcmd.DefaultContainerAnnotationName] } else if len(annotations[defaultLogsContainerAnnotationName]) > 0 { // Only log deprecation if we have only the old annotation. This allows users to // set both to support multiple versions of kubectl; if they are setting both @@ -90,7 +91,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt 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) } if len(containerName) > 0 { - if exists, _ := podutils.FindContainerByName(t, containerName); exists != nil { + if exists, _ := podcmd.FindContainerByName(t, containerName); exists != nil { opts.Container = containerName } else { fmt.Fprintf(os.Stderr, "Default container name %q not found in a pod\n", containerName) @@ -121,7 +122,7 @@ func logsForObjectWithClient(clientset corev1client.CoreV1Interface, object, opt containerName = opts.Container } - container, fieldPath := podutils.FindContainerByName(t, containerName) + container, fieldPath := podcmd.FindContainerByName(t, containerName) if container == nil { return nil, fmt.Errorf("container %s is not valid for pod %s", opts.Container, t.Name) } diff --git a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go index 60c340f73ca..928df2480d0 100644 --- a/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go +++ b/staging/src/k8s.io/kubectl/pkg/polymorphichelpers/logsforobject_test.go @@ -32,7 +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" + "k8s.io/kubectl/pkg/cmd/util/podcmd" ) var ( @@ -430,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{podutils.DefaultContainerAnnotationName: "foo-2-c1"} + pod.Annotations = map[string]string{podcmd.DefaultContainerAnnotationName: "foo-2-c1"} return pod }, podLogOptions: &corev1.PodLogOptions{}, @@ -440,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{podutils.DefaultContainerAnnotationName: "foo-2-c1"} + pod.Annotations = map[string]string{podcmd.DefaultContainerAnnotationName: "foo-2-c1"} return pod }, podLogOptions: &corev1.PodLogOptions{ @@ -452,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{podutils.DefaultContainerAnnotationName: "non-existing"} + pod.Annotations = map[string]string{podcmd.DefaultContainerAnnotationName: "non-existing"} return pod }, podLogOptions: &corev1.PodLogOptions{}, @@ -462,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{podutils.DefaultContainerAnnotationName: "foo-2-c1"} + pod.Annotations = map[string]string{podcmd.DefaultContainerAnnotationName: "foo-2-c1"} return pod }, allContainers: true, diff --git a/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go b/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go index 580f3706e49..847eb7e88b4 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go +++ b/staging/src/k8s.io/kubectl/pkg/util/podutils/podutils.go @@ -17,7 +17,6 @@ limitations under the License. package podutils import ( - "fmt" "time" corev1 "k8s.io/api/core/v1" @@ -25,10 +24,6 @@ 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: @@ -191,25 +186,3 @@ 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, "" -} diff --git a/vendor/modules.txt b/vendor/modules.txt index 7f0438b86d8..e4565c32ff9 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2403,6 +2403,7 @@ k8s.io/kubectl/pkg/cmd/top k8s.io/kubectl/pkg/cmd/util k8s.io/kubectl/pkg/cmd/util/editor k8s.io/kubectl/pkg/cmd/util/editor/crlf +k8s.io/kubectl/pkg/cmd/util/podcmd k8s.io/kubectl/pkg/cmd/util/sanity k8s.io/kubectl/pkg/cmd/version k8s.io/kubectl/pkg/cmd/wait