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

Rework API fields for nodegroup SGs #460

Merged
merged 3 commits into from
Jan 25, 2019
Merged

Rework API fields for nodegroup SGs #460

merged 3 commits into from
Jan 25, 2019

Conversation

errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented Jan 24, 2019

Description

Checklist

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All tests passing (i.e. make test)
  • Added/modified documentation as required (such as the README)
  • Added yourself to the humans.txt file

@errordeveloper
Copy link
Contributor Author

@dlespiau this is still WIP, but I'd like to hear your feedback on the API change itself.

@errordeveloper errordeveloper changed the title WIP: Remove API fields for nodegroup SGs WIP: Rework API fields for nodegroup SGs Jan 24, 2019
@errordeveloper
Copy link
Contributor Author

@rndstr FYI

@dlespiau
Copy link
Contributor

I guess it's to allow disabling the default SGs. Seems fine to me.

@errordeveloper
Copy link
Contributor Author

errordeveloper commented Jan 24, 2019 via email

@errordeveloper
Copy link
Contributor Author

Will also need to address #465.

@errordeveloper
Copy link
Contributor Author

I think it'd be good to also amend nodegroup check function to detect clusters that were affected by #465 and give user a warning.

@errordeveloper errordeveloper changed the title WIP: Rework API fields for nodegroup SGs Rework API fields for nodegroup SGs Jan 25, 2019
@errordeveloper errordeveloper merged commit c97f45a into master Jan 25, 2019
@errordeveloper errordeveloper deleted the fix-425 branch January 25, 2019 14:51
torredil pushed a commit to torredil/eksctl that referenced this pull request May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants