search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.58k stars 1.31k forks source link

Improve clusterctl's --list-variables output #3422

Closed wfernandes closed 3 years ago

wfernandes commented 4 years ago

User Story

As a developer/user/operator I would like to see more details when I list the variables of a template yaml using clusterctl so that I know what values will be used and what values I need to specify.

Detailed Description Currently, This is the output of a clusterctl command with --list-variables:

$ clusterctl config cluster foo -i aws:v0.5.4 --list-variables
Using configuration File="/Users/wfernandes/.cluster-api/clusterctl.yaml"
Fetching File="cluster-template.yaml" Provider="infrastructure-aws" Version="v0.5.4"
Variables:
  - AWS_CONTROL_PLANE_MACHINE_TYPE
  - AWS_NODE_MACHINE_TYPE
  - AWS_REGION
  - AWS_SSH_KEY_NAME
  - CLUSTER_NAME
  - CONTROL_PLANE_MACHINE_COUNT
  - KUBERNETES_VERSION
  - WORKER_MACHINE_COUNT

This is great. But now that we have support for default values ${VAR:=defaultValue} it would be nice to also view the default values that may be specified in the template.

Anything else you would like to add: Extra bonus: It would also be nice to know which variables I may need to yet specify. That is, out of these variables, AWS_CONTROL_PLANE_MACHINE_TYPE, AWS_NODE_MACHINE_TYPE, AWS_SSH_KEY_NAME I may only have setup AWS_SSH_KEY_NAME in my clusterctl.yaml config.

A suggestion could be simply to display a table format such that the blank/empty values are the ones that may be required. This way we could gain insight into what default values are being used and also what values are missing that I may still need to configure.

VARIABLE VALUE
AWS_REGION us-east1
AWS_CONTROL_PLANE_MACHINE_TYPE
AWS_NODE_MACHINE_TYPE
AWS_SSH_KEY_NAME my-ssh-key

/kind feature /area clusterctl

vincepri commented 4 years ago

/milestone v0.3.x

vincepri commented 4 years ago

/milestone v0.4.0

prankul88 commented 4 years ago

@wfernandes Hi, I would like to work on this.

/assign

fabriziopandini commented 4 years ago

Thanks @prankul88 ! /lifecycle active

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fabriziopandini commented 3 years ago

/remove-lifecycle stale

vincepri commented 3 years ago

@prankul88 Are you still interested in tackling this issue?

prankul88 commented 3 years ago

@vincepri Yes, had left it due to some issues. Would like to give it a shot once this week.

prankul88 commented 3 years ago

@wfernandes Hi, I wanted a suggestion from you, if you can help. To fetch the default values from the template should I prefer making a separate function (Eg, func DefaultValues) and use it here or rather make Variables a map instead of a string to allow it to store default values.

I am facing some doubt on how to fetch the "Param" from envsubst package after traversing. Working on it..

wfernandes commented 3 years ago

@prankul88 Changing the Variables() []string definition would mean a change in the interface and which is not ideal but it may not be a non-blocker for v0.4.x since the main clusterctl API wouldn't be affected (I think).

I suggest moving towards the first option for now and see what the reviewers think. Some points to also think about is ensuring the list of variables is ordered so it's easier for users to look at. That is, iterating through a map is unordered and it could be difficult for users to see a different ordered list every time they print out the list.

FYI, I'm currently no longer on this project and won't be involved in further PR reviews. I suggest reaching out to other maintainers/reviewers (e.g. @fabriziopandini) to get their opinion.

Again, thanks for working on this 🙂

prankul88 commented 3 years ago

@wfernandes Thanks for the feedback !! :)

fabriziopandini commented 3 years ago

@prankul88 are you working to this issue? @CecileRobertMichon recently filed with an interesting variation of this request. do you mind if I close this issue in favour of the new one?

CecileRobertMichon commented 3 years ago

Ah sorry, I searched for existing issues but didn't see this issue before opening mine. I'm fine with copying my proposal here as a comment too if we want to keep history.

CecileRobertMichon commented 3 years ago

/close

k8s-ci-robot commented 3 years ago

@CecileRobertMichon: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/3422#issuecomment-813638984): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.