Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cobra command Kubectl Create does not return error to be handled #1615

Open
oglok opened this issue Jun 27, 2024 · 6 comments
Open

Cobra command Kubectl Create does not return error to be handled #1615

oglok opened this issue Jun 27, 2024 · 6 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@oglok
Copy link

oglok commented Jun 27, 2024

What happened:

I'm writing a function to basically do kubectl create -k /path. However, If the manifests there have already been created, I don't want my program to exit, but just ignore it.

This is a quick snippet:

func createKustomization(kustomization string, kubeconfig string) error {
	klog.Infof("Using create policy with kustomization at %s", kustomization)

	cmds := &cobra.Command{
		Use:   "kubectl",
		Short: "kubectl",
	}
	persistFlags := cmds.PersistentFlags()
	persistFlags.SetNormalizeFunc(cliflag.WarnWordSepNormalizeFunc)
	persistFlags.SetNormalizeFunc(cliflag.WordSepNormalizeFunc)

	kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag()
	kubeConfigFlags.KubeConfig = &kubeconfig
	kubeConfigFlags.AddFlags(persistFlags)

	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)
	matchVersionKubeConfigFlags.AddFlags(persistFlags)

	f := cmdutil.NewFactory(matchVersionKubeConfigFlags)
	ioStreams := genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}
	createCmd := create.NewCmdCreate(f, ioStreams)
	createCmd.SetArgs([]string{"-k", kustomization})
	createCmd.SetOut(ioStreams.Out)
	createCmd.SetErr(ioStreams.ErrOut)
	createCmd.SetIn(ioStreams.In)

	klog.Infof("Executing create command")
	createCmd.SilenceUsage = true
	err := createCmd.Execute()
	if err != nil {
		klog.Infof("Creating kustomization failed: %s", err)
		return nil
	}

	klog.Infof("Kustomization applied successfully")
	return nil

}

createCmd.Execute() just throws an error, but It never gets into klog.Infof("Creating kustomization failed: %s", err)

Am I doing something wrong?

What you expected to happen:

To return an error so I can handle it.

Anything else we need to know?:

Environment:
Library: k8s.io/kubectl v0.30.2

  • Kubernetes client and server versions (use kubectl version):
  • Cloud provider or hardware configuration:
  • OS (e.g: cat /etc/os-release):
@oglok oglok added the kind/bug Categorizes issue or PR as related to a bug. label Jun 27, 2024
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 27, 2024
@brianpursley
Copy link
Member

brianpursley commented Jun 27, 2024

The problem, as you found out, is that inside Cobra's Run() command, we use cmdutil.CheckErr() which by default will os.Exit() when an error occurs, instead of returning it.

Ideally, instead of calling cmd.Execute() you should be able to call o.RunCreate() on CreateOptions without needing to instantiate a Cobra command (or iostreams) at all.

Unfortunately, the create command is written in a way that requires a cobra command, even if you want to call o.RunCreate() directly. It should be refactored to be like other commands, which do not require this.


But, as a workaround, you might be able to make it work by calling RunCreate() and pass it a dummy cobra command, since I don't think you need any of the code paths that use that argument. Something like this...

func createKustomization(kustomization string, kubeconfig string) error {
	klog.Infof("Using create policy with kustomization at %s", kustomization)

	kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag()
	kubeConfigFlags.KubeConfig = &kubeconfig

	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)

	f := cmdutil.NewFactory(matchVersionKubeConfigFlags)
	ioStreams := genericiooptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}

	o := create.NewCreateOptions(ioStreams)
	o.FilenameOptions.Kustomize = kustomization
	dummyCmd := &cobra.Command{}
	err := o.RunCreate(f, dummyCmd)
	if err != nil {
		klog.Errorf("Creating kustomization failed: %s", err)
		return err
	}

	klog.Infof("Kustomization applied successfully")
	return nil
}

Alternatively, you could forget about trying to leverage create altogether and just copy the code you need out of the RunCreate() func. Depending on how specific your needs are, this might make sense as another option to consider.

Also, would apply work for you, instead of create? That might end up being easier to work with.

@ardaguclu
Copy link
Member

/kind support

@k8s-ci-robot k8s-ci-robot added the kind/support Categorizes issue or PR as a support question. label Jun 28, 2024
@oglok
Copy link
Author

oglok commented Jun 28, 2024

Thank you very much @brianpursley ! that makes sense!
your suggestion worked perfectly, I just needed to add one line:
dummyCmd.Flags().Bool("save-config", false, "If true, save the configuration after applying")

Should we open a GH issue for the refactor?

@brianpursley
Copy link
Member

Cool, I'm glad you were able to work around it. The point raised in your issue is definitely valid though.

Should we open a GH issue for the refactor?

There is a somewhat related umbrella issue (#1046), but that is more about separating the options from the flags, which is a slightly different problem than this. It looks like there were a couple of previous attempts to refactor this command as part of that issue, but they were not able to be merged.

@ardaguclu do you have thoughts on whether this should be a separate issue or fall under the umbrella?

@ardaguclu
Copy link
Member

I think, we can manage this specific change in this issue. The other one requires more dramatic changes.

@ardaguclu
Copy link
Member

/remove-kind support
/triage accepted
/priority backlog

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. kind/support Categorizes issue or PR as a support question. labels Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

4 participants