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

feat: add upstream module with k8s-addons example #698

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

Zvikan
Copy link
Contributor

@Zvikan Zvikan commented Jun 27, 2022

What does this PR do?

  • Adds new example using eks upstream module("terraform-aws-modules/eks/aws") with blueprints addons module only

Motivation

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 created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

@Zvikan Zvikan marked this pull request as ready for review June 27, 2022 19:53
@Zvikan Zvikan temporarily deployed to EKS Blueprints Test June 27, 2022 19:53 Inactive
Comment on lines +89 to +92
eks_cluster_id = module.eks.cluster_id
eks_cluster_endpoint = module.eks.cluster_endpoint
eks_oidc_provider = module.eks.oidc_provider
eks_cluster_version = module.eks.cluster_version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is really the most important part of this example, should we call this out in the README/add a comment here?

# EKS Cluster with terraform-aws-eks module
#---------------------------------------------------------------

module "eks" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to keep the example as simple as possible, if you believe we can further remove more pieces here lmk.

@bryantbiggs
Copy link
Contributor

looks good - general question, is upstream well known or should we just call it something like terraform-aws-eks?

@Zvikan
Copy link
Contributor Author

Zvikan commented Jun 27, 2022

looks good - general question, is upstream well known or should we just call it something like terraform-aws-eks?

Agree and forgot to call it out, but I first tried to call it terraform-aws-eks... but having long name example leads to issues with IRSA/IAM resources names that may be limited to 64 chars :(

@Zvikan Zvikan temporarily deployed to EKS Blueprints Test June 27, 2022 20:51 Inactive
vpc_id = module.vpc.vpc_id
subnet_ids = module.vpc.private_subnets

eks_managed_node_group_defaults = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this example should be an upstream module rather than a blueprint.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vara-bonthu customers want to see how kubernetes-addons module can be used with upstream.

Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

LGTM!

vpc_id = module.vpc.vpc_id
subnet_ids = module.vpc.private_subnets

eks_managed_node_group_defaults = {
Copy link
Contributor

Choose a reason for hiding this comment

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

@vara-bonthu customers want to see how kubernetes-addons module can be used with upstream.

@askulkarni2 askulkarni2 merged commit be536b2 into main Jun 28, 2022
@askulkarni2 askulkarni2 deleted the issue-689/upstream-with-blueprints-addons branch June 28, 2022 18:04
allamand pushed a commit to allamand/terraform-aws-eks-blueprints that referenced this pull request Dec 15, 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants