-
Notifications
You must be signed in to change notification settings - Fork 20
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
chore: Update modules, providers, EKS version, and reorganize examples #53
Conversation
} | ||
kube-proxy = { | ||
most_recent = true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | |
} | |
eks-pod-identity-agent = { | |
most_recent = true | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this PR is required at all.
@@ -2,15 +2,14 @@ provider "aws" { | |||
region = local.region |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't rename the director to patterns. We want to keep it consistent with other terraform AWS modules. Patterns should go in EKS Blueprints repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, are you saying that we should create multiple different ACK deployment options in the EKS Blueprints repository and keep the "complete" "example" here as an end-to-end test?
} | ||
} | ||
|
||
tags = local.tags | ||
|
||
depends_on = [module.vpc] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed as there is an implicit dependency when you provide vpc id and subnets.
# Add-ons | ||
enable_aws_load_balancer_controller = true | ||
enable_metrics_server = true | ||
|
||
tags = local.tags | ||
|
||
depends_on = [module.eks] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed due to implicit dependency via cluster_name, endpoint, etc.
} | ||
|
||
################################################################################ | ||
# ACK Addons | ||
################################################################################ | ||
|
||
module "eks_ack_addons" { | ||
source = "../../" | ||
source = "aws-ia/eks-ack-addons/aws" | ||
version = "2.2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done relatively for a reason so you can test the latest changes. Please leave it as is.
What does this PR do?
🛑 Please open an issue first to discuss any significant work and flesh out details/direction - we would hate for your time to be wasted.
Consult the CONTRIBUTING guide for submitting pull-requests.
This pull request includes several housekeeping and general updates to improve the overall structure, update dependencies, and align with the best practices observed in the EKS blueprints addons.
Motivation
This PR aims to address the improvements discussed in Issue #52, including:
examples
folder topatterns
for consistency with EKS blueprints addons design.source = "../../"
.Test Results
The changes have been tested to ensure:
Additional Notes
These changes will help maintain a clean, updated, and well-structured repository, making it easier for users to follow and implement the modules effectively. The renaming and restructuring align with other EKS-related projects, providing a more consistent experience.