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 description how to add the security group to the gpu node pool #173

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChristianBergen
Copy link
Contributor

add description how add the security group to the gpu node pool

Copy link
Member

@schwichti schwichti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem is that you need to add a security group so that gpuexecnodes can talk with execnodes? Why can't we configure this in Terraform? I believe we already have a solution for a customer deployment and IVS node pool. I will get back to you on this.

@ChristianBergen
Copy link
Contributor Author

Correct, this the issue #117. We have now made an additional step in our pipelines, which attaches the SG of the execnodes to the gpuexecnodes launch template. But if you already have a solution for the basic problem, I would suggest that you implement it into the ref-arch. That would be great!

@schwichti
Copy link
Member

I have not found the Terraform code where we did this.

My suggestion is to create an issue with the problem description and your solution until the feature is implemented in Terraform and abandon this PR.

Manual adjustments would be overriden by Terraform in subsequent runs.

@mariogluhakovic
Copy link
Contributor

The removal of Blueprints are in progress on feature branch so we plan to remove blueprint with standalone creation of EKS and also node groups. With this change we will try to add correct security group to all node groups via new terraform code. We already have setup for EKS, and execution and default node groups. Other changes will come in near future.

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.

3 participants