Running invalid command causes kind CLI to exit with 0

[vainu-lauri]

What happened:
For example kind load typo will exit with 0

What you expected to happen:
Kind to exit with non-zero (Very useful when used in ci or script etc)

How to reproduce it (as minimally and precisely as possible):

kind load typo
echo $?

Anything else we need to know?:

This is due to cobra (spf13/cobra#1156) and affects all commands that have sub-commands in kind (build, completion, create, delete, export, get, load)

The specified NoArgsor any of the cobra validators don't seem to have any effect unless Run or RunE has been defined for the command. One fix might be to just specify something like:

+               Run: func(cmd *cobra.Command, args []string) {
+                       cmd.Help()
+                       os.Exit(1)
+               },

to every command (or also have them return 0 in case called without arguments).
I implemented this locally and it seemed to work as it should. Although it's very far from an elegant solution since that would need to duplicated to all of the commands that have sub-commands.

If my (probably rather bad fix) seems reasonable I'm happy to create a PR, but I think this need a more elegant solution.

Environment:

• kind version: (use kind version): kind v0.10.0 go1.15.7 darwin/amd64
• Kubernetes version: (use kubectl version):
• Docker version: (use docker info):
• OS (e.g. from /etc/os-release):

[BenTheElder]

thanks for the report, this is a nice catch.

One fix might be to just specify something like:

I'd prefer we continue to leave exit calls in app.Main as we have users that import the cli cobra commands (and we should return an error instead of exiting their binary), but we should ensure these return a non-nil error, so perhaps the equivalent RunE on all commands instead and return an error.

The general (if not necessarily elegant) idea of putting an explicit cmd.Help run on all commands makes sense.

[vainu-lauri]

Thanks.

Good point about calling the os.exit() in the subcommand.

I'll open up a PR with the proposed changes and some tests.

Although I'm not sure if the calling of cmd.Help() will cause problems to the users who have imported these commands (that's my main concern).

Then there's the philosophical question whether we should consider incomplete but not wrong command (like kind delete) as failure or not (my current implementation considers it as a failure and exits with 0).