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.

Sign up for GitHub

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

Jump to bottom

(aws-eks): add service ip range setting to cluster class #16541

Closed
2 tasks
Krotki opened this issue Sep 18, 2021 · 4 comments · Fixed by #16957
Closed
2 tasks

(aws-eks): add service ip range setting to cluster class #16541

Krotki opened this issue Sep 18, 2021 · 4 comments · Fixed by #16957
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@Krotki
Copy link

Krotki commented Sep 18, 2021 •
edited
Loading

In console we can specify Service IPv4 range, same goes with CfnCluster class, which contains kubernetesNetworkConfig? property. Unfortunately it is not possible to set it via Cluster class.

Use Case

We planed to migrate our on-premis k8s kluster to aws. To do it we planned to expose pod's k8s network and service k8s network to EKS via VPN and adjust CoreDNS accordingly. Unfortunately by default k8s comes with service ip range of 10.96.0.0/12, EKS comes with 10.100.0.0/16, which overlap. Since our cluster is up and running, we cannot change it. The only option to enable this scenerio is to create new EKS cluster with different service ip range.

Proposed Solution

Actually I'm not sure how Cluster class works ATM, but if it uses CfnCluster class underneath, it is only a matter of exposing kubernetesNetworkConfig? property.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request
@Krotki Krotki added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Sep 18, 2021
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Sep 18, 2021
@otaviomacedo
Copy link
Contributor

Actually I'm not sure how Cluster class works ATM, but if it uses CfnCluster class underneath, it is only a matter of exposing kubernetesNetworkConfig? property.

That's right. But even if the property is not exposed by the high-level construct (Cluster, in this case), you can always use an escape hatch and do it yourself.

@otaviomacedo otaviomacedo added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Sep 24, 2021
@Krotki
Copy link
Author

Krotki commented Sep 27, 2021

I had a brief look as source code and correct me if I'm wrong, but I't seems to me that CfnCluster class is not used. Instead there is ClusterResource which creates CustomResource of type Custom::AWSCDK-EKS-Cluster. There is a custom resource handler which interprets data from CustomResource and creates/updates/deletes resource. I guess only thing missing is to extend handler to handle create (client already supports it) and add error on update. Also extending CustomResource with necessary data is necessary and as a last step adding property to Cluster class. Am I missing anything ?

I might create a PR for this issue, but testing is a bit of an challenge. Let's say I would like to do some manual tests. How can I install local version of cdk-eks locally ? Is there any instructions ?

@otaviomacedo
Copy link
Contributor

Yes, take a look at our contributing guide. In particular, there is a link-all.sh script that creates a link from your test application's node_modules to your local version of the CDK.

@otaviomacedo otaviomacedo removed their assignment Oct 1, 2021
@ayush987goyal ayush987goyal mentioned this issue Oct 13, 2021
Merged
@mergify mergify bot closed this as completed in #16957 Oct 17, 2021
mergify bot pushed a commit that referenced this issue Oct 17, 2021
@ayush987goyal
feat(eks): configure serviceIpv4Cidr on the cluster (#16957)
72102c7 
Refs:
1. https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-eks-cluster-kubernetesnetworkconfig.html
2. https://docs.aws.amazon.com/eks/latest/APIReference/API_KubernetesNetworkConfigRequest.html#AmazonEKS-Type-KubernetesNetworkConfigRequest-serviceIpv4Cidr

Notes:
1. Currently I have not updated the integ tests since the deployed takes a lot of time and it requires inferentia service limit increase. Do you think this change needs an integ tests updating (tried it out locally and it succeeded till auto-scaling)? 
2. Couldn't find a good place in the Readme to add this feature. Would really help if we could come up with a good explanation and place for the same.

Closes #16541 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@kferrone kferrone mentioned this issue Oct 21, 2021
Closed
2 tasks
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
@ayush987goyal @TikiTDO
feat(eks): configure serviceIpv4Cidr on the cluster (aws#16957)
19dce19 
Refs:
1. https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-eks-cluster-kubernetesnetworkconfig.html
2. https://docs.aws.amazon.com/eks/latest/APIReference/API_KubernetesNetworkConfigRequest.html#AmazonEKS-Type-KubernetesNetworkConfigRequest-serviceIpv4Cidr

Notes:
1. Currently I have not updated the integ tests since the deployed takes a lot of time and it requires inferentia service limit increase. Do you think this change needs an integ tests updating (tried it out locally and it succeeded till auto-scaling)? 
2. Couldn't find a good place in the Readme to add this feature. Would really help if we could come up with a good explanation and place for the same.

Closes aws#16541 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(eks): configure serviceIpv4Cidr on the cluster
2 participants
@otaviomacedo @Krotki