⚠️ clusterctl completion zsh: Use native zsh completion

[superbrothers]

What this PR does / why we need it:

ref/ #4090

Due to this change, we will not able to load the zsh completion code
with the following method:

source <(clusterctl completion zsh)

Instead, we execute the following command once:

clusterctl completion zsh >"${fpath[1]}/_clusterctl"

Then we need to start a new shell for this setup to take effect.

---

In addition, github.com/spf13/cobra is updated to v1.1.1 in order to use
native zsh completion feature.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Closes #4090

[k8s-ci-robot]

Hi @superbrothers. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[prankul88]

/assign

Thanks for this PR. I can review this to check functionality.

[prankul88]

I am getting a permission error when running this command,

zsh: permission denied: /usr/local/share/zsh/site-functions/_clusterctl

Not sure if only it is system-specific.

[superbrothers]

It depends on the system.

[wfernandes]

@superbrothers Thanks for this update! I haven't been able to test the zsh completion but it mostly lgtm just some nits and clarifications regarding some files.

[wfernandes]

/ok-to-test

[CecileRobertMichon]

➜  cluster-api git:(pr/4113) bin/clusterctl 

alpha       -- Commands for features in alpha.
completion  -- Output shell completion code for the specified shell (bash or
config      -- Display provider configuration and templates to create worklo
delete      -- Delete one or more providers from the management cluster.
describe    -- Describe workload clusters.
generate    -- Generate yaml using clusterctl yaml processor.
get         -- Get info from a management or a workload cluster
help        -- Help about any command
init        -- Initialize a management cluster.
move        -- Move Cluster API objects and all dependencies between managem
upgrade     -- Upgrade core and provider components in a management cluster.
version     -- Print clusterctl version.

Works for me!

/lgtm

[wfernandes]

/lgtm
/approve

[CecileRobertMichon]

Just one question: why is the change marked as breaking ⚠️ ?

[wfernandes]

@CecileRobertMichon It seems that the loading of the zsh logic (from the changes in the docs) has changed a bit.

[CecileRobertMichon]

@CecileRobertMichon It seems that the loading of the zsh logic (from the changes in the docs) has changed a bit.

Ok, just wanted to make sure there wasn't another breaking change I had missed

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, wfernandes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [CecileRobertMichon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wfernandes]

/test pull-cluster-api-test-main

[marckhouzam]

Just one question: why is the change marked as breaking ⚠️ ?

Due to this change, we will not able to load the zsh completion code with the following method:

source <(clusterctl completion zsh)

Zsh recommends putting the completion file on fpath instead of sourcing it, which is why Cobra does not support sourcing the completion file. This is better for performance as it enables zsh to do some sort of caching of the script.

It is however possible to support sourcing if you feel it is better for your users. To do that you have to add the line compdef _clusterctl clusterctl to the completion script (at the end of the script is safe). This is what the Helm project has chosen to do to avoid a "breaking" change: https://github.com/helm/helm/blob/037a43c98bc6331ffe7027d89e57fb1334cc1189/cmd/helm/completion.go#L168

[wfernandes]

@marckhouzam We have this in our documentation https://master.cluster-api.sigs.k8s.io/clusterctl/commands/completion.html#zsh which seems like it adds it to the fpath.

Also the output of clusterctl completion zsh has compdef _clusterctl clusterctl as the first line.

So I guess this isn't a breaking change. Yay!

[marckhouzam]

@marckhouzam We have this in our documentation https://master.cluster-api.sigs.k8s.io/clusterctl/commands/completion.html#zsh which seems like it adds it to the fpath.

Well, it tells users to add it themselves 😁. This is great and may be what you prefer to stick with. I just wanted to mention there is a way to enable sourcing of you feel it is important.

Also the output of clusterctl completion zsh has compdef _clusterctl clusterctl as the first line.

I believe the first line starts with a #, which makes the line a directive for zsh when the script is on fpath. You would need a second one without the # for sourcing to work.

But I'm not arguing in favor, I just wanted to inform as it is something that has come up a couple of times in Cobra Issues.

[superbrothers]

It is however possible to support sourcing if you feel it is better for your users. To do that you have to add the line compdef _clusterctl clusterctl to the completion script (at the end of the script is safe). This is what the Helm project has chosen to do to avoid a "breaking" change:

This is a nice technique. We should use this technique to avoid a "breaking change".