From 3ffdee7d2b2afb48d67828ac136f05ba9c6d8619 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 13 Feb 2023 12:36:18 +0300 Subject: [PATCH 1/3] kubectl debug: Standartize add flag function This PR standartize add flag function interface to align with other kubectl commands. --- .../src/k8s.io/kubectl/pkg/cmd/debug/debug.go | 35 ++++++++++--------- .../kubectl/pkg/cmd/debug/debug_test.go | 2 +- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go index 6a473180c35..e85a5aebb4c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go @@ -166,28 +166,29 @@ func NewCmdDebug(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra. }, } - addDebugFlags(cmd, o) - cmdutil.AddJsonFilenameFlag(cmd.Flags(), &o.FilenameOptions.Filenames, "identifying the resource to debug") + o.AddFlags(cmd) return cmd } -func addDebugFlags(cmd *cobra.Command, opt *DebugOptions) { - cmd.Flags().BoolVar(&opt.ArgsOnly, "arguments-only", opt.ArgsOnly, i18n.T("If specified, everything after -- will be passed to the new container as Args instead of Command.")) - cmd.Flags().BoolVar(&opt.Attach, "attach", opt.Attach, i18n.T("If true, wait for the container to start running, and then attach as if 'kubectl attach ...' were called. Default false, unless '-i/--stdin' is set, in which case the default is true.")) - cmd.Flags().StringVarP(&opt.Container, "container", "c", opt.Container, i18n.T("Container name to use for debug container.")) - cmd.Flags().StringVar(&opt.CopyTo, "copy-to", opt.CopyTo, i18n.T("Create a copy of the target Pod with this name.")) - cmd.Flags().BoolVar(&opt.Replace, "replace", opt.Replace, i18n.T("When used with '--copy-to', delete the original Pod.")) +func (o *DebugOptions) AddFlags(cmd *cobra.Command) { + cmdutil.AddJsonFilenameFlag(cmd.Flags(), &o.FilenameOptions.Filenames, "identifying the resource to debug") + + cmd.Flags().BoolVar(&o.ArgsOnly, "arguments-only", o.ArgsOnly, i18n.T("If specified, everything after -- will be passed to the new container as Args instead of Command.")) + cmd.Flags().BoolVar(&o.Attach, "attach", o.Attach, i18n.T("If true, wait for the container to start running, and then attach as if 'kubectl attach ...' were called. Default false, unless '-i/--stdin' is set, in which case the default is true.")) + cmd.Flags().StringVarP(&o.Container, "container", "c", o.Container, i18n.T("Container name to use for debug container.")) + cmd.Flags().StringVar(&o.CopyTo, "copy-to", o.CopyTo, i18n.T("Create a copy of the target Pod with this name.")) + cmd.Flags().BoolVar(&o.Replace, "replace", o.Replace, i18n.T("When used with '--copy-to', delete the original Pod.")) cmd.Flags().StringToString("env", nil, i18n.T("Environment variables to set in the container.")) - cmd.Flags().StringVar(&opt.Image, "image", opt.Image, i18n.T("Container image to use for debug container.")) - cmd.Flags().StringToStringVar(&opt.SetImages, "set-image", opt.SetImages, i18n.T("When used with '--copy-to', a list of name=image pairs for changing container images, similar to how 'kubectl set image' works.")) + cmd.Flags().StringVar(&o.Image, "image", o.Image, i18n.T("Container image to use for debug container.")) + cmd.Flags().StringToStringVar(&o.SetImages, "set-image", o.SetImages, i18n.T("When used with '--copy-to', a list of name=image pairs for changing container images, similar to how 'kubectl set image' works.")) cmd.Flags().String("image-pull-policy", "", i18n.T("The image pull policy for the container. If left empty, this value will not be specified by the client and defaulted by the server.")) - cmd.Flags().BoolVarP(&opt.Interactive, "stdin", "i", opt.Interactive, i18n.T("Keep stdin open on the container(s) in the pod, even if nothing is attached.")) - cmd.Flags().BoolVarP(&opt.Quiet, "quiet", "q", opt.Quiet, i18n.T("If true, suppress informational messages.")) - cmd.Flags().BoolVar(&opt.SameNode, "same-node", opt.SameNode, i18n.T("When used with '--copy-to', schedule the copy of target Pod on the same node.")) - cmd.Flags().BoolVar(&opt.ShareProcesses, "share-processes", opt.ShareProcesses, i18n.T("When used with '--copy-to', enable process namespace sharing in the copy.")) - cmd.Flags().StringVar(&opt.TargetContainer, "target", "", i18n.T("When using an ephemeral container, target processes in this container name.")) - cmd.Flags().BoolVarP(&opt.TTY, "tty", "t", opt.TTY, i18n.T("Allocate a TTY for the debugging container.")) - cmd.Flags().StringVar(&opt.Profile, "profile", ProfileLegacy, i18n.T(`Debugging profile. Options are "legacy", "general", "baseline", or "restricted".`)) + cmd.Flags().BoolVarP(&o.Interactive, "stdin", "i", o.Interactive, i18n.T("Keep stdin open on the container(s) in the pod, even if nothing is attached.")) + cmd.Flags().BoolVarP(&o.Quiet, "quiet", "q", o.Quiet, i18n.T("If true, suppress informational messages.")) + cmd.Flags().BoolVar(&o.SameNode, "same-node", o.SameNode, i18n.T("When used with '--copy-to', schedule the copy of target Pod on the same node.")) + cmd.Flags().BoolVar(&o.ShareProcesses, "share-processes", o.ShareProcesses, i18n.T("When used with '--copy-to', enable process namespace sharing in the copy.")) + cmd.Flags().StringVar(&o.TargetContainer, "target", "", i18n.T("When using an ephemeral container, target processes in this container name.")) + cmd.Flags().BoolVarP(&o.TTY, "tty", "t", o.TTY, i18n.T("Allocate a TTY for the debugging container.")) + cmd.Flags().StringVar(&o.Profile, "profile", ProfileLegacy, i18n.T(`Debugging profile. Options are "legacy", "general", "baseline", or "restricted".`)) } // Complete finishes run-time initialization of debug.DebugOptions. diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go index c4964dadc9b..3096143415c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go @@ -1649,7 +1649,7 @@ func TestCompleteAndValidate(t *testing.T) { }, } cmd.SetArgs(strings.Split(tc.args, " ")) - addDebugFlags(cmd, opts) + opts.AddFlags(cmd) cmdError := cmd.Execute() From d66b339868ea08ef4d3bda09fdf64abacaa3f41e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 13 Feb 2023 13:03:50 +0300 Subject: [PATCH 2/3] kubectl debug: Initialize pod client and builder in complete This PR initializes podclient and builder in complete function instead run function. --- .../src/k8s.io/kubectl/pkg/cmd/debug/debug.go | 26 +++++++++++++------ .../kubectl/pkg/cmd/debug/debug_test.go | 2 +- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go index e85a5aebb4c..2e7dbef15db 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go @@ -40,6 +40,7 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/cli-runtime/pkg/printers" "k8s.io/cli-runtime/pkg/resource" + "k8s.io/client-go/kubernetes" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/cache" watchtools "k8s.io/client-go/tools/watch" @@ -133,6 +134,7 @@ type DebugOptions struct { podClient corev1client.CoreV1Interface + Builder *resource.Builder genericclioptions.IOStreams WarningPrinter *printers.WarningPrinter @@ -243,6 +245,20 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st o.Applier = applier } + clientConfig, err := f.ToRESTConfig() + if err != nil { + return err + } + + client, err := kubernetes.NewForConfig(clientConfig) + if err != nil { + return err + } + + o.podClient = client.CoreV1() + + o.Builder = f.NewBuilder() + return nil } @@ -328,13 +344,7 @@ func (o *DebugOptions) Validate() error { func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { ctx := context.Background() - clientset, err := f.KubernetesClientSet() - if err != nil { - return fmt.Errorf("internal error getting clientset: %v", err) - } - o.podClient = clientset.CoreV1() - - r := f.NewBuilder(). + r := o.Builder. WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...). FilenameParam(o.explicitNamespace, &o.FilenameOptions). NamespaceParam(o.Namespace).DefaultNamespace().ResourceNames("pods", o.TargetNames...). @@ -343,7 +353,7 @@ func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { return err } - err = r.Visit(func(info *resource.Info, err error) error { + err := r.Visit(func(info *resource.Info, err error) error { if err != nil { // TODO(verb): configurable early return return err diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go index 3096143415c..84224670107 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug_test.go @@ -1665,7 +1665,7 @@ func TestCompleteAndValidate(t *testing.T) { } if diff := cmp.Diff(tc.wantOpts, opts, cmpFilter, cmpopts.IgnoreFields(DebugOptions{}, - "attachChanged", "shareProcessedChanged", "podClient", "WarningPrinter", "Applier", "explicitNamespace")); diff != "" { + "attachChanged", "shareProcessedChanged", "podClient", "WarningPrinter", "Applier", "explicitNamespace", "Builder")); diff != "" { t.Error("CompleteAndValidate unexpected diff in generated object: (-want +got):\n", diff) } }) From f5b0d728c59fbcca571e4a21ca2f473149e72f03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 13 Feb 2023 13:16:35 +0300 Subject: [PATCH 3/3] kubectl debug: Use restClientGetter instead cmd.Factory As the move towards using `restClientGetter` interface instead gigantic `cmd.Factory`, this PR does that change. --- .../src/k8s.io/kubectl/pkg/cmd/debug/debug.go | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go index 2e7dbef15db..a95b2f136fe 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/debug/debug.go @@ -152,7 +152,7 @@ func NewDebugOptions(streams genericclioptions.IOStreams) *DebugOptions { } // NewCmdDebug returns a cobra command that runs kubectl debug. -func NewCmdDebug(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra.Command { +func NewCmdDebug(restClientGetter genericclioptions.RESTClientGetter, streams genericclioptions.IOStreams) *cobra.Command { o := NewDebugOptions(streams) cmd := &cobra.Command{ @@ -162,9 +162,9 @@ func NewCmdDebug(f cmdutil.Factory, streams genericclioptions.IOStreams) *cobra. Long: debugLong, Example: debugExample, Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(o.Complete(f, cmd, args)) + cmdutil.CheckErr(o.Complete(restClientGetter, cmd, args)) cmdutil.CheckErr(o.Validate()) - cmdutil.CheckErr(o.Run(f, cmd)) + cmdutil.CheckErr(o.Run(restClientGetter, cmd)) }, } @@ -194,7 +194,7 @@ func (o *DebugOptions) AddFlags(cmd *cobra.Command) { } // Complete finishes run-time initialization of debug.DebugOptions. -func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { +func (o *DebugOptions) Complete(restClientGetter genericclioptions.RESTClientGetter, cmd *cobra.Command, args []string) error { var err error o.PullPolicy = corev1.PullPolicy(cmdutil.GetFlagString(cmd, "image-pull-policy")) @@ -223,7 +223,7 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st } // Namespace - o.Namespace, o.explicitNamespace, err = f.ToRawKubeConfigLoader().Namespace() + o.Namespace, o.explicitNamespace, err = restClientGetter.ToRawKubeConfigLoader().Namespace() if err != nil { return err } @@ -245,7 +245,7 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st o.Applier = applier } - clientConfig, err := f.ToRESTConfig() + clientConfig, err := restClientGetter.ToRESTConfig() if err != nil { return err } @@ -257,7 +257,7 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st o.podClient = client.CoreV1() - o.Builder = f.NewBuilder() + o.Builder = resource.NewBuilder(restClientGetter) return nil } @@ -341,7 +341,7 @@ func (o *DebugOptions) Validate() error { } // Run executes a kubectl debug. -func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { +func (o *DebugOptions) Run(restClientGetter genericclioptions.RESTClientGetter, cmd *cobra.Command) error { ctx := context.Background() r := o.Builder. @@ -388,14 +388,14 @@ func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { Attach: &attach.DefaultRemoteAttach{}, } - config, err := f.ToRESTConfig() + config, err := restClientGetter.ToRESTConfig() if err != nil { return err } opts.Config = config opts.AttachFunc = attach.DefaultAttachFunc - if err := o.handleAttachPod(ctx, f, debugPod.Namespace, debugPod.Name, containerName, opts); err != nil { + if err := o.handleAttachPod(ctx, restClientGetter, debugPod.Namespace, debugPod.Name, containerName, opts); err != nil { return err } } @@ -794,7 +794,7 @@ func (o *DebugOptions) waitForContainer(ctx context.Context, ns, podName, contai return result, err } -func (o *DebugOptions) handleAttachPod(ctx context.Context, f cmdutil.Factory, ns, podName, containerName string, opts *attach.AttachOptions) error { +func (o *DebugOptions) handleAttachPod(ctx context.Context, restClientGetter genericclioptions.RESTClientGetter, ns, podName, containerName string, opts *attach.AttachOptions) error { pod, err := o.waitForContainer(ctx, ns, podName, containerName) if err != nil { return err @@ -815,12 +815,12 @@ func (o *DebugOptions) handleAttachPod(ctx context.Context, f cmdutil.Factory, n } if status.State.Terminated != nil { klog.V(1).Info("Ephemeral container terminated, falling back to logs") - return logOpts(f, pod, opts) + return logOpts(restClientGetter, pod, opts) } if err := opts.Run(); err != nil { fmt.Fprintf(opts.ErrOut, "warning: couldn't attach to pod/%s, falling back to streaming logs: %v\n", podName, err) - return logOpts(f, pod, opts) + return logOpts(restClientGetter, pod, opts) } return nil }