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 EKS Cluster Compositions using upbound-aws-provider #201

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

hflobao
Copy link
Contributor

@hflobao hflobao commented Jun 14, 2024

What does this PR do?

This PR adds 2 compositions to create EKS clusters.

  1. eks/cluster-basic: Creates a basic cluster and nodegroup, with only the required pre-requisites to get the cluster deployed and active.
  2. eks/cluster-with-extras: Creates the cluster and nodegroup, but adding features like KMS key for Secrets encryption, Addons for EFS, S3 and EBS CSI, as well as the AWS Load Balancer Controller.

Motivation

#164

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)

  • Yes, I have added a new example under examples to support my PR

  • Yes, I have updated the docs for this feature

  • Yes, I have linked to an issue or feature request (applicable to PRs that solves a bug or a feature request)

Note:

  • Not all the PRs require examples and docs
  • We prefer small, well tested pull requests. Please ensure your pull requests are self-contained, and commits are squashed

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

@candonov
Copy link
Contributor

Thank you for this PR @hflobao!
Can you please add two claims, one per compositions in a folder examples/upbound-aws-provider/composite-resources/eks? Please include a Readme in that folder to explaining what each claim creates and how to use it, include a kustomization.yaml file that links to the compositions like this one: https://github.com/awslabs/crossplane-on-eks/blob/main/examples/upbound-aws-provider/composite-resources/s3-irsa/kustomization.yaml.

@hflobao
Copy link
Contributor Author

hflobao commented Jul 3, 2024

@candonov , I've added claims for both EKS compositions.
Thanks!

@iamahgoub
Copy link
Contributor

@hflobao -- great work! how about creating the networking infrastructure (VPC, subnets, etc.) as part of the composition? I think we should make it easy for users to experiment with this composition; asking them to create the networking infrastructure themselves, then passing the subnet ids in the claim may discourage them from trying the composition. The other option is modify the instructions in the README to patch the claims YAML with the ids of the existing subnets via snippets that users can easily copy/paste. What do you think?

@hflobao
Copy link
Contributor Author

hflobao commented Jul 8, 2024

Hi @iamahgoub
I was thinking in provide a composition that could integrate and be deployed in an existing environment. That way, accepting existing network parameters makes sense for me.
But yes, you have a point regarding someone just trying it out in a completely new environment. Giving that, I'll create a third composition option in addition the ones already provided. I'll start from the 'cluster-with-extras' and add a 'vpc-and-cluster' composition, creating a VPC and public subnets to provision the cluster and nodegroup. How this sounds to you?

@iamahgoub
Copy link
Contributor

iamahgoub commented Jul 16, 2024

Hi @hflobao,
Apologies for the delayed response.
Sounds good! Just not to delay this PR any further, let's target closing this PR with compositions that create EKS clusters in existing VPCs, and have another PR for EKS composition with new VPC. Can you update the instruction in the README so the VPC hosting the management cluster is used for hosting the workload cluster as well? It would be great if the users can copy/paste a snippet (e.g. sed commands) that updates the files with the required VPC/subnet ids rather than manually editing files.

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