docker swarm init: fix help printing

[hernandanielg]

fixes #4400

On the setHelpFunc function it tries to execute plugins's helps at first based in the ammount of arguments, the help for docker swarm init --help was getting matched with docker-init plugin, thus printing pluging help to output

To fix this we make sure that even thought we have 1 or more arguments it will check if the command it's first level of command by checking that the root command is the same as the parent command

- What I did

- How I did it

- How to verify it

• compile with docker buildx bake
• execute swarm init command with --help flag ./build/docker swarm init --help

expected output

➜  cli git:(fix-swarm-init-help) ./build/docker swarm init --help

Usage:  docker swarm init [OPTIONS]

Initialize a swarm

Options:
      --advertise-addr string                  Advertised address (format: "<ip|interface>[:port]")
      --autolock                               Enable manager autolocking (requiring an unlock key to start a stopped manager)
      --availability string                    Availability of the node ("active", "pause", "drain") (default "active")
      --cert-expiry duration                   Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
      --data-path-addr string                  Address or interface to use for data path traffic (format: "<ip|interface>")
      --data-path-port uint32                  Port number to use for data path traffic (1024 - 49151). If no value is set or is set to 0, the default port
                                               (4789) is used.
      --default-addr-pool ipNetSlice           default address pool in CIDR format (default [])
      --default-addr-pool-mask-length uint32   default address pool subnet mask length (default 24)
      --dispatcher-heartbeat duration          Dispatcher heartbeat period (ns|us|ms|s|m|h) (default 5s)
      --external-ca external-ca                Specifications of one or more certificate signing endpoints
      --force-new-cluster                      Force create a new cluster from current state
      --listen-addr node-addr                  Listen address (format: "<ip|interface>[:port]") (default 0.0.0.0:2377)
      --max-snapshots uint                     Number of additional Raft snapshots to retain
      --snapshot-interval uint                 Number of log entries between Raft snapshots (default 10000)
      --task-history-limit int                 Task history retention limit (default 5)

- Description for the changelog

fix to prevent wrong help message for commands with same name than docker plugins

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

Merging #4403 (b0cd429) into master (118d6ba) will not change coverage.
The diff coverage is 0.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4403   +/-   ##
=======================================
  Coverage   61.05%   61.05%           
=======================================
  Files         295      295           
  Lines       20647    20647           
=======================================
  Hits        12605    12605           
  Misses       7146     7146           
  Partials      896      896           

[thaJeztah]

Thank you for working on this!!

I'm slightly trying to get my head around the fix; and probably it could need a comment in the code, describing what we're doing here, and a similar comment in the commit message.

Also (not yet fully grasping the fix 😓 , so perhaps silly question); this only takes .Parent() into account, does that affect cases where "nested" commands could run into this?

[hernandanielg]

Hey! 👋🏻 I enjoyed working on this one, even tho it's an easy fix I struggled to make dlv debugger to work in vscode and then to find the issue, but it was fun

So yeah I know, I need to add comments and maybe do it in a better way like create a function that does the check and returns a boolean

The idea is to identify if the command is first level command, in the case of docker swarm init --help

docker: is the root command
swarm : 1st level command
init  : 2nd level command
--help: is a flag

Why to idenfity 1st only? because docker plugins are in the form docker-<plugin name>, for this reason it was taking 2nd command init and making it docker-init

So how do we identify if it's the 1st level? checking parent command and root command, if both are the same then my command it's 1st level command - in the init case, root cmd is docker and parent is swarm so it will fail, no need to check great parent or nested

Yeah definitely will improve the commit, this was my first shot - thanks for the review!

[thaJeztah]

(quick comment, in a call right now)

and maybe do it in a better way like create a function that does the check and returns a boolean

I'm fine with keeping the code in-line. Abstracting things can also hide logic, and sometimes it's fine to just have the logic in plain sight (not "shoved under the rug" to hide it away)

[hernandanielg]

added comments and rewrote commit msg, any help with wording will be highly appreciated 🙏🏻

[hernandanielg]

Hello @thaJeztah changes has been performed, can this be reviewed again? 🙏🏻

[vvoland]

In my case, Parent is always nil, so this check is never true, making it effectively if false {

I haven't fully followed the whole code yet, but it looks to me like we can just explicitly delete that whole branch:

docker <plugin> ... --help will be forwarded to the plugin cli directly so it won't even be handled here.

docker help <plugin> is already handled by setupHelpCommand

Or are there any other ways to "call for help" that I'm not aware of?

[hernandanielg]

I think 2nd condition (the one I added) makes sense only if len(args) is > 1 , situations like docker foo bar so bar won't be considered to check if it's a plugin... but yeah I agree with you, maybe we can remove the whole thing, I'll make some tests and modify this PR (bear with me)