cluster-api icon indicating copy to clipboard operation
cluster-api copied to clipboard

✨ Add `clusterctl init list-images` command

Open oscr opened this issue 3 years ago • 2 comments

What this PR does / why we need it:

In clusterctl / init the there is a todo to convert clusterctl init --list-images to a sub-command.

Instead of redeclaring the flags needed by list-images I updated some flags in init from initCmd.Flags() -> initCmd.PersistentFlags() to inherit them. Is this an acceptable approach? (Maybe common flags like kube context etc should really be in root?).

The documentation is sparse and more placeholder. Will be replaced later if the general approach is ok.

$ clusterctl init list-images --help
Lists the container images required for initializing the management cluster.

See https://cluster-api.sigs.k8s.io for more details.

Usage:
  clusterctl init list-images [flags]

Examples:
  # Lists the container images required for initializing the management cluster.
  #
  # Note: This command is a dry-run; it won't perform any action other than printing to screen.
  clusterctl init list-images --infrastructure aws

Flags:
  -h, --help   help for list-images

Global Flags:
      --config $HOME/.cluster-api/clusterctl.yaml   Path to clusterctl configuration (default is $HOME/.cluster-api/clusterctl.yaml) or to a remote location (i.e. https://example.com/clusterctl.yaml)
  -i, --infrastructure strings                      Infrastructure providers and versions (e.g. aws:v0.5.0) to add to the management cluster.
      --kubeconfig string                           Path to the kubeconfig for the management cluster. If unspecified, default discovery rules apply.
      --kubeconfig-context string                   Context to be used within the kubeconfig file. If empty, current context will be used.
  -v, --v int                                       Set the log level verbosity. This overrides the CLUSTERCTL_LOG_LEVEL environment variable.

Example usage

$ clusterctl init list-images --infrastructure aws --kubeconfig-context kind-kind
k8s.gcr.io/cluster-api-aws/cluster-api-aws-controller:v1.4.1

oscr avatar Jul 15 '22 14:07 oscr

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

k8s-ci-robot avatar Jul 15 '22 14:07 k8s-ci-robot

Doing a high level review as the PR is still WIP.

In this PR it looks like clusterctl init and clusterctl init list-images both perform actions. We currently don't have precedence in clusterctl where a command and a sub-command both perform actions. For cases where there are sub-commands the parent seems to only be functioning as a grouper.

Although we don't have precedence for this in clusterctl this seems to be an okay pattern in the ecosystem. Example: kubeadm kubeadm init and kubectl init phase <phase-name> both perform actions.

I would like to get @fabriziopandini thoughts on adopting this pattern in clusterctl.

Sounds fine to me. I think if the sub-command logically makes sense there it would be probably awkward to come up with another top-level command just to avoid having a command and sub-command both performing actions

sbueringer avatar Aug 30 '22 11:08 sbueringer

I don't have strong opinions on the subcommand discussion, ready to approve after @ykakarap green lights the changes

fabriziopandini avatar Sep 01 '22 09:09 fabriziopandini

Thank you @ykakarap for all your help with this and other prs :pray:

oscr avatar Sep 02 '22 06:09 oscr

/lgtm /approve

fabriziopandini avatar Sep 02 '22 09:09 fabriziopandini

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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~~ [fabriziopandini]

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

k8s-ci-robot avatar Sep 02 '22 09:09 k8s-ci-robot