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

Add RoleARN to NodeGroupSummary #1500

Merged
merged 2 commits into from
Nov 13, 2019
Merged

Conversation

t0rr3sp3dr0
Copy link
Contributor

@t0rr3sp3dr0 t0rr3sp3dr0 commented Oct 29, 2019

Description

Add possibility to get Instance Role ARN from eksctl get nodegroup on YAML and JSON outputs.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, and examples directory)
  • Manually tested

@t0rr3sp3dr0
Copy link
Contributor Author

@errordeveloper, do I have to do anything else to get this pull request merged into master?

@t0rr3sp3dr0
Copy link
Contributor Author

@martina-if could you review this pull request?

@martina-if
Copy link
Contributor

Hi @t0rr3sp3dr0 , Thanks for your contribution! Adding the instance role makes sense but the instance profile is not something I've seen requested that much. You could also get it by knowing the instance role. Can you remove that part?

By the way, we also have this issue if you are interested: #585 From there eksctl could expose more details from the stacks and in a more organized way.

@t0rr3sp3dr0 t0rr3sp3dr0 changed the title Add RoleARN and ProfileARN to NodeGroupSummary Add RoleARN to NodeGroupSummary Nov 12, 2019
Signed-off-by: Pedro Tôrres <[email protected]>
@t0rr3sp3dr0
Copy link
Contributor Author

@martina-if I've removed instance profile from the code and its tests.

About the issue you mentioned, I'll take a look on that!

@martina-if martina-if merged commit 83f35be into eksctl-io:master Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants