-
Notifications
You must be signed in to change notification settings - Fork 54
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
Support more than 5 SecurityGroups on one CRD #413
base: master
Are you sure you want to change the base?
Conversation
The default number of security groups per Network Interface is 5. Updating the The recommended way is to define more than 1 security group if you need more than 5 SGs to be specified, as documented in the README. Can you help me understand if any issues with multiple CRDs? We understand this is harder to manage than 1 CRD, but hopefully it is a one-time setup effort for SGP CRDs. |
@sushrk Hi. It seems like a reasonable approach would be to guide users to specify only as many SecurityGroups as they need what do you think? Is there any advantage to enforcing this through maxitems? If I apply it without increasing the qutos, won't it be rejected by the vpc qutos anyway? Or, how about making it possible to dynamically modify maxitems? I can help you that |
I need to check on this.
Yeah, as mentioned before this avoids customers from specifying more than the default number(5) of security groups in the CRD.
Hm, this might be possible if we set maxItems in CRD based on the service quota value. Let me see if it is feasible. |
@sushrk |
We would love this feature also. We have increased service quotas and currently rely on a fork of the VPC RC to get around this |
@sushrk |
Sorry for the late reply. Thinking about this, it makes sense to me to set the We'd need to document this more clearly though, especially when customers want to lower their quota so it does not break existing workloads. I'll need to confirm the customer experience with PM, will post an update tomorrow. @changhyuni are you planning to add the support for this? |
@sushrk |
Can you explain why we are adding a webhook to check the number of SGs? Does this validate the CRD at creation/update/both?
We use aws-go-sdk for EC2 operations, but through a wrapper and helper functions. We haven't yet migrated to v2. |
@sushrk
Right, I think we need to add it to the pod webhook or utils package |
@sushrk |
We need avoid adding upstream service support check in pod mutating webhook. The webhook need to be light and respond quickly. Always calling upstream API will be more and more expensive for most of users who don't have the need to support more than 5 security groups. Since users should be aware that their service quotas before applying the setting to the CRD, I think fail a pod creation and send a pod event as a message should be good enough. |
@haouc |
Issue #, if available:
Modify the maxitems in SecurityGroups.
According to "Amazon VPC Quotas," it should be able to use up to 16.
One CRD must support at least 10 security groups
If I need more than 5 security groups, it's hard to manage
because I need more than one crd. I don't know why one crd doesn't support more than 10 security groups
Description of changes:
We need to use more than 10 security groups.
Increase the maxitems to the maximum number of allowable (16), and modify the descritions and README.
https://docs.aws.amazon.com/vpc/latest/userguide/amazon-vpc-limits.html#vpc-limits-security-groups
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.