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

WIP: add --config-file and --validate to get cluster command #3352

Closed

Conversation

neha-viswanathan
Copy link
Contributor

@neha-viswanathan neha-viswanathan commented Feb 25, 2021

Related to #1126

Description

This MR adds --config-file and --validate flags to get cluster command

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@neha-viswanathan neha-viswanathan marked this pull request as draft February 25, 2021 04:21
@neha-viswanathan
Copy link
Contributor Author

👋🏽 @Callisto13 can you please review and see if this is what's expected?

@Callisto13
Copy link
Contributor

Hey @neha-viswanathan 👋 , thanks for handling this 🎉 .

This doesn't seem to be working for me and I am seeing all the clusters. On top of the flag being added the config needs to be processed. You can check out other commands which have config loaders (loading code lives in pkg/ctl/cmdutils).

A config file for get cluster will be an alternative to the --name and --region flags.

ie, when I do:

eksctl get cluster -f config.yaml

The config file should be processed and I see:

2021-02-25 10:31:45 [ℹ]  eksctl version 0.40.0-dev+da899ad2.2021-02-25T10:29:56Z
2021-02-25 10:31:45 [ℹ]  using region us-west-2
NAME            VERSION STATUS  CREATED                 VPC                     SUBNETS                                               SECURITYGROUPS
cb-test2        1.18    ACTIVE  2021-01-29T12:13:54Z    vpc-foo   subnet-1,subnet-2,subnet-3,subnet-4,subnet-5,subnet-6  sg-1

When I don't provide a config file or --name and/or --region I see all clusters in the (default or flagged) region, and the config file is not parsed.

For completeness, if I do the following:

eksctl get cluster -f config.yaml -A

The config file should be ignored and all clusters across all regions returned. (You could even have it so that those flags cannot be used together.)

@Callisto13
Copy link
Contributor

Also (just remembered) the behaviour for get cluster <cluster name> and get cluster (no arg) should stay the same (ie config file should not be required).

@Callisto13
Copy link
Contributor

Hey @neha-viswanathan! Just checking in on the status of this, do you plan to finish or would you like it taken over? Thanks!

@neha-viswanathan
Copy link
Contributor Author

Hey @neha-viswanathan! Just checking in on the status of this, do you plan to finish or would you like it taken over? Thanks!

Hi @Callisto13 I will try to get this done in the next few days. Thanks!

@Skarlso
Copy link
Contributor

Skarlso commented Nov 16, 2021

Hi @neha-viswanathan.

Are you still planning on finishing this PR? If not, please ping me so I can wrap it up. :)

Cheers!

@neha-viswanathan
Copy link
Contributor Author

Hello @Skarlso, please feel free to pick it up. Thanks!

@Skarlso
Copy link
Contributor

Skarlso commented Nov 16, 2021

Thank you @neha-viswanathan!

@Skarlso Skarlso closed this Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants