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 ListClusters #27

Closed
errordeveloper opened this issue Jun 5, 2018 · 9 comments
Closed

improve ListClusters #27

errordeveloper opened this issue Jun 5, 2018 · 9 comments
Labels
help wanted Extra attention is needed
Milestone

Comments

@errordeveloper
Copy link
Contributor

errordeveloper commented Jun 5, 2018

It will fail if a user has many clusters, so it needs paging.

https://github.com/weaveworks/eksctl/blob/0a39d1a2c1d107e1da90c27889d43bddba42ab3a/pkg/eks/eks.go#L128

It should also use data structure and a formatter instead of using logger.

https://github.com/weaveworks/eksctl/blob/0a39d1a2c1d107e1da90c27889d43bddba42ab3a/pkg/eks/eks.go#L127

@errordeveloper errordeveloper changed the title ListClusters should page through clusters improve ListClusters Jun 5, 2018
@christopherhein
Copy link
Contributor

What kind of design are we looking for for this output? We get a lot of information back from the API for each stack, and for the cluster.

@errordeveloper
Copy link
Contributor Author

I think something similar to kubectl would be nice, but one problem is that I maybe very hard to re-use kubectl printers are very very custom (at least last time I checked), but we should re-check.

@errordeveloper errordeveloper added the help wanted Extra attention is needed label Jun 6, 2018
@errordeveloper
Copy link
Contributor Author

errordeveloper commented Jul 5, 2018

@stefanprodan did you say that you wanted to fix this? It'd be nice to fix before 0.1.0-beta.1.

@errordeveloper errordeveloper modified the milestones: 0.1.0-beta.1, 0.1.0 Jul 5, 2018
@errordeveloper
Copy link
Contributor Author

I'll make it less verbose for beta.1, so it's a bit more useful!

errordeveloper added a commit that referenced this issue Jul 12, 2018
Make `eksctl get clusters` less verbose (#27)
@richardcase
Copy link
Contributor

@errordeveloper i've been looking into this. Paginating through the list of clusters with ListClusters is easy enough. Its a shame that this only returns the cluster name. It would've been great if it returned summary information as well as it could potentially save subsequent calls to DescribeCluster. If only there was a ListClustersWithSummary endpoint!

So the logic is:

  1. Call ListClusters with MaxResults set (this will default to 100 but allow it to be set using a flag)
  2. For each cluster returned by 1) call describe cluster
  3. If log level is greater than 4 (i.e. debug) then for each cluster returned by 1) call ListStackPages and output debug logs
  4. If 1) returned NextToken then repeat steps 1-3 until next token isn't returned

So there could be a lot of calls in a short space of time to the AWS endpoints if this is used with an account that has a lot of EKS clusters. If we have 100 clusters and running in debug log level then it will make 201 requests.

With the data that is returned we'll run it through a printer to format the output of the data. The printers will be based on an interface so in the future we can support different outputs (i.e. table, json, yaml).

Is that what you where thinking?

@errordeveloper
Copy link
Contributor Author

With the data that is returned we'll run it through a printer to format the output of the data. The printers will be based on an interface so in the future we can support different outputs (i.e. table, json, yaml).

Is that what you where thinking?

Yeah, mostly that's what I was thinking indeed... But now I think more, and maybe we could make it even simpler.

And by the way, I wouldn't be too worried about API calls, most people probably won't have more then a few clusters (as default limit is still 3 clusters per account). But whatever we will do now will change when we move to CloudFormation, and many things maybe become easier then as we would have describe stacks with outputs (which supports all sorts of options).

Maybe we could make kubectl get clusters simply print cluster names to start with, so that user can find what clusters they have that they can do something with. They can copy the name and get more info with e.g. kubectl get clusters --name=<name> --output=json, and that's what would trigger deeper recursive calls. I think we should make kubectl get clusters --output=json work too. Let's just keep the default as simple as possible, perhaps calling ListClusters is enough there for now?

We would probably want --output=wide|json|yaml, but for now just one of those would be sufficient, e.g. yaml or json.

@richardcase
Copy link
Contributor

Sounds like a good plan to start with.

@errordeveloper
Copy link
Contributor Author

Closed via #140.

@errordeveloper
Copy link
Contributor Author

🎉 🌮 🎉 🌮 🎉 🌮

torredil pushed a commit to torredil/eksctl that referenced this issue May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants