From 6a3c6f9a427fcea728c40491d5adf675e5f2d069 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Fri, 30 Oct 2020 08:54:13 +0100 Subject: [PATCH] kubectl debug: add tests for Complete,Validate Kubernetes-commit: 3cfcf3a74fc24e6b4b3f58710fe454d1bc3644cc --- pkg/cmd/debug/debug.go | 72 +++++----- pkg/cmd/debug/debug_test.go | 271 +++++++++++++++++++++++++++++++++++- 2 files changed, 307 insertions(+), 36 deletions(-) diff --git a/pkg/cmd/debug/debug.go b/pkg/cmd/debug/debug.go index 03288dfc..da9ee392 100644 --- a/pkg/cmd/debug/debug.go +++ b/pkg/cmd/debug/debug.go @@ -94,27 +94,26 @@ var nameSuffixFunc = utilrand.String // DebugOptions holds the options for an invocation of kubectl debug. type DebugOptions struct { - Args []string - ArgsOnly bool - Attach bool - Container string - CopyTo string - Replace bool - Env []corev1.EnvVar - Image string - Interactive bool - Namespace string - TargetNames []string - PullPolicy corev1.PullPolicy - Quiet bool - SameNode bool - ShareProcesses bool - Target string - TTY bool + Args []string + ArgsOnly bool + Attach bool + Container string + CopyTo string + Replace bool + Env []corev1.EnvVar + Image string + Interactive bool + Namespace string + TargetNames []string + PullPolicy corev1.PullPolicy + Quiet bool + SameNode bool + ShareProcesses bool + TargetContainer string + TTY bool shareProcessedChanged bool - builder *resource.Builder podClient corev1client.PodsGetter genericclioptions.IOStreams @@ -165,7 +164,7 @@ func addDebugFlags(cmd *cobra.Command, opt *DebugOptions) { cmd.Flags().BoolVar(&opt.Quiet, "quiet", 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.Target, "target", "", i18n.T("When debugging a pod, target processes in this container name.")) + 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.")) } @@ -173,7 +172,6 @@ func addDebugFlags(cmd *cobra.Command, opt *DebugOptions) { func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []string) error { var err error - o.builder = f.NewBuilder() o.PullPolicy = corev1.PullPolicy(cmdutil.GetFlagString(cmd, "image-pull-policy")) // Arguments @@ -205,13 +203,6 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st return err } - // Clientset - clientset, err := f.KubernetesClientSet() - if err != nil { - return fmt.Errorf("internal error getting clientset: %v", err) - } - o.podClient = clientset.CoreV1() - // Share processes o.shareProcessedChanged = cmd.Flags().Changed("share-processes") @@ -220,6 +211,17 @@ func (o *DebugOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st // Validate checks that the provided debug options are specified. func (o *DebugOptions) Validate(cmd *cobra.Command) error { + // CopyTo + // These flags are exclusive to --copy-to + if len(o.CopyTo) == 0 { + switch { + case o.Replace: + return fmt.Errorf("--replace may only be used with --copy-to.") + case o.SameNode: + return fmt.Errorf("--same-node may only be used with --copy-to.") + } + } + // Image if len(o.Image) == 0 { return fmt.Errorf("--image is required") @@ -241,8 +243,8 @@ func (o *DebugOptions) Validate(cmd *cobra.Command) error { return fmt.Errorf("invalid image pull policy: %s", o.PullPolicy) } - // Target - if len(o.Target) > 0 && len(o.CopyTo) > 0 { + // TargetContainer + if len(o.TargetContainer) > 0 && len(o.CopyTo) > 0 { return fmt.Errorf("--target is incompatible with --copy-to. Use --share-processes instead.") } @@ -258,7 +260,13 @@ func (o *DebugOptions) Validate(cmd *cobra.Command) error { func (o *DebugOptions) Run(f cmdutil.Factory, cmd *cobra.Command) error { ctx := context.Background() - r := o.builder. + clientset, err := f.KubernetesClientSet() + if err != nil { + return fmt.Errorf("internal error getting clientset: %v", err) + } + o.podClient = clientset.CoreV1() + + r := f.NewBuilder(). WithScheme(scheme.Scheme, scheme.Scheme.PrioritizedVersionsAllGroups()...). NamespaceParam(o.Namespace).DefaultNamespace().ResourceNames("pods", o.TargetNames...). Do() @@ -266,7 +274,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 @@ -398,7 +406,7 @@ func (o *DebugOptions) generateDebugContainer(pod *corev1.Pod) *corev1.Ephemeral TerminationMessagePolicy: corev1.TerminationMessageReadFile, TTY: o.TTY, }, - TargetContainerName: o.Target, + TargetContainerName: o.TargetContainer, } if o.ArgsOnly { diff --git a/pkg/cmd/debug/debug_test.go b/pkg/cmd/debug/debug_test.go index 3e060920..eb05a295 100644 --- a/pkg/cmd/debug/debug_test.go +++ b/pkg/cmd/debug/debug_test.go @@ -18,13 +18,18 @@ package debug import ( "fmt" + "strings" "testing" "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/cli-runtime/pkg/genericclioptions" + cmdtesting "k8s.io/kubectl/pkg/cmd/testing" "k8s.io/utils/pointer" ) @@ -62,10 +67,10 @@ func TestGenerateDebugContainer(t *testing.T) { { name: "namespace targeting", opts: &DebugOptions{ - Container: "debugger", - Image: "busybox", - PullPolicy: corev1.PullIfNotPresent, - Target: "myapp", + Container: "debugger", + Image: "busybox", + PullPolicy: corev1.PullIfNotPresent, + TargetContainer: "myapp", }, expected: &corev1.EphemeralContainer{ EphemeralContainerCommon: corev1.EphemeralContainerCommon{ @@ -976,3 +981,261 @@ func TestGenerateNodeDebugPod(t *testing.T) { }) } } + +func TestCompleteAndValidate(t *testing.T) { + tf := cmdtesting.NewTestFactory() + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() + cmpFilter := cmp.FilterPath(func(p cmp.Path) bool { + switch p.String() { + // IOStreams contains unexported fields + case "IOStreams": + return true + } + return false + }, cmp.Ignore()) + + tests := []struct { + name, args string + wantOpts *DebugOptions + wantError bool + }{ + { + name: "No targets", + args: "--image=image", + wantError: true, + }, + { + name: "Invalid environment variables", + args: "--image=busybox --env=FOO mypod", + wantError: true, + }, + { + name: "Invalid image name", + args: "--image=image:label@deadbeef mypod", + wantError: true, + }, + { + name: "Invalid pull policy", + args: "--image=image --image-pull-policy=whenever-you-feel-like-it", + wantError: true, + }, + { + name: "TTY without stdin", + args: "--image=image --tty", + wantError: true, + }, + { + name: "Set image pull policy", + args: "--image=busybox --image-pull-policy=Always mypod", + wantOpts: &DebugOptions{ + Args: []string{}, + Image: "busybox", + Namespace: "default", + PullPolicy: corev1.PullPolicy("Always"), + ShareProcesses: true, + TargetNames: []string{"mypod"}, + }, + }, + { + name: "Multiple targets", + args: "--image=busybox mypod1 mypod2", + wantOpts: &DebugOptions{ + Args: []string{}, + Image: "busybox", + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod1", "mypod2"}, + }, + }, + { + name: "Arguments with dash", + args: "--image=busybox mypod1 mypod2 -- echo 1 2", + wantOpts: &DebugOptions{ + Args: []string{"echo", "1", "2"}, + Image: "busybox", + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod1", "mypod2"}, + }, + }, + { + name: "Interactive no attach", + args: "-ti --image=busybox --attach=false mypod", + wantOpts: &DebugOptions{ + Args: []string{}, + Attach: false, + Image: "busybox", + Interactive: true, + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod"}, + TTY: true, + }, + }, + { + name: "Set environment variables", + args: "--image=busybox --env=FOO=BAR,BAZ=BAZ mypod", + wantOpts: &DebugOptions{ + Args: []string{}, + Env: []v1.EnvVar{ + {Name: "FOO", Value: "BAR"}, + {Name: "BAZ", Value: "BAZ"}, + }, + Image: "busybox", + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod"}, + }, + }, + { + name: "Ephemeral container: interactive session minimal args", + args: "mypod -it --image=busybox", + wantOpts: &DebugOptions{ + Args: []string{}, + Attach: true, + Image: "busybox", + Interactive: true, + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod"}, + TTY: true, + }, + }, + { + name: "Ephemeral container: non-interactive debugger with image and name", + args: "--image=myproj/debug-tools --image-pull-policy=Always -c debugger mypod", + wantOpts: &DebugOptions{ + Args: []string{}, + Container: "debugger", + Image: "myproj/debug-tools", + Namespace: "default", + PullPolicy: corev1.PullPolicy("Always"), + ShareProcesses: true, + TargetNames: []string{"mypod"}, + }, + }, + { + name: "Ephemeral container: image not specified", + args: "mypod", + wantError: true, + }, + { + name: "Ephemeral container: replace not allowed", + args: "--replace --image=busybox mypod", + wantError: true, + }, + { + name: "Ephemeral container: same-node not allowed", + args: "--same-node --image=busybox mypod", + wantError: true, + }, + { + name: "Pod copy: interactive debug container minimal args", + args: "mypod -it --image=busybox --copy-to=my-debugger", + wantOpts: &DebugOptions{ + Args: []string{}, + Attach: true, + CopyTo: "my-debugger", + Image: "busybox", + Interactive: true, + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod"}, + TTY: true, + }, + }, + { + name: "Pod copy: non-interactive with debug container, image name and command", + args: "mypod --image=busybox --container=my-container --copy-to=my-debugger -- sleep 1d", + wantOpts: &DebugOptions{ + Args: []string{"sleep", "1d"}, + Container: "my-container", + CopyTo: "my-debugger", + Image: "busybox", + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"mypod"}, + }, + }, + { + name: "Pod copy: no image specified", + args: "mypod -it --copy-to=my-debugger", + wantError: true, + }, + { + name: "Pod copy: --target not allowed", + args: "mypod --target --image=busybox --copy-to=my-debugger", + wantError: true, + }, + { + name: "Node: interactive session minimal args", + args: "node/mynode -it --image=busybox", + wantOpts: &DebugOptions{ + Args: []string{}, + Attach: true, + Image: "busybox", + Interactive: true, + Namespace: "default", + ShareProcesses: true, + TargetNames: []string{"node/mynode"}, + TTY: true, + }, + }, + { + name: "Node: no image specified", + args: "node/mynode -it", + wantError: true, + }, + { + name: "Node: replace not allowed", + args: "--image=busybox --replace node/mynode", + wantError: true, + }, + { + name: "Node: same-node not allowed", + args: "--image=busybox --same-node node/mynode", + wantError: true, + }, + { + name: "Node: --target not allowed", + args: "node/mynode --target --image=busybox", + wantError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + opts := NewDebugOptions(ioStreams) + var gotError error + + cmd := &cobra.Command{ + Run: func(cmd *cobra.Command, args []string) { + gotError = opts.Complete(tf, cmd, args) + if gotError != nil { + return + } + gotError = opts.Validate(cmd) + }, + } + cmd.SetArgs(strings.Split(tc.args, " ")) + addDebugFlags(cmd, opts) + + cmdError := cmd.Execute() + + if tc.wantError { + if cmdError != nil || gotError != nil { + return + } + t.Fatalf("CompleteAndValidate got nil errors but wantError: %v", tc.wantError) + } else if cmdError != nil { + t.Fatalf("cmd.Execute got error '%v' executing test cobra.Command, wantError: %v", cmdError, tc.wantError) + } else if gotError != nil { + t.Fatalf("CompleteAndValidate got error: '%v', wantError: %v", gotError, tc.wantError) + } + + if diff := cmp.Diff(tc.wantOpts, opts, cmpFilter, cmpopts.IgnoreUnexported(DebugOptions{})); diff != "" { + t.Error("CompleteAndValidate unexpected diff in generated object: (-want +got):\n", diff) + } + }) + } +}