Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wfernandes opened this issue Jul 29, 2020 · 15 comments
Closed

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

wfernandes opened this issue Jul 29, 2020 · 15 comments
Assignees
Labels
area/clusterctl Issues or PRs related to clusterctl kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@wfernandes
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. area/clusterctl Issues or PRs related to clusterctl labels Jul 29, 2020
@vincepri
Copy link
Member

/milestone v0.3.x

@k8s-ci-robot k8s-ci-robot added this to the v0.3.x milestone Jul 29, 2020
@vincepri
Copy link
Member

vincepri commented Aug 3, 2020

/milestone v0.4.0

@k8s-ci-robot k8s-ci-robot modified the milestones: v0.3.x, v0.4.0 Aug 3, 2020
@prankul88
Copy link
Contributor

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

/assign

@fabriziopandini
Copy link
Member

Thanks @prankul88 !
/lifecycle active

@k8s-ci-robot k8s-ci-robot added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Aug 26, 2020
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. labels Nov 24, 2020
@fabriziopandini
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 24, 2020
@vincepri
Copy link
Member

@prankul88 Are you still interested in tackling this issue?

@prankul88
Copy link
Contributor

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

@prankul88
Copy link
Contributor

@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
Copy link
Contributor Author

@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
Copy link
Contributor

@wfernandes Thanks for the feedback !! :)

@fabriziopandini
Copy link
Member

@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
Copy link
Contributor

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
Copy link
Contributor

/close

@k8s-ci-robot
Copy link
Contributor

@CecileRobertMichon: Closing this issue.

In response to this:

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterctl Issues or PRs related to clusterctl kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

7 participants