-
Notifications
You must be signed in to change notification settings - Fork 240
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: Trino on EKS #437
feat: Trino on EKS #437
Conversation
changed image file extensions from .png to .PNG
Changed NodeSelector for worker to be deployed on spot instances
… 4 or above Using single AZ in Karpenter NodePool to avoid inter-AZ data transfer costs for big data workloads like Trino
Create Glue database, crawler and table in same region where Trino on EKS is deployed
specify region to create Glue database, crawler and table in same region as Trino on EKS deployment
added FT execution example, Final cleanup instructions
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.
@youngjeong46 Great job on this PR 🚀! I've added a few small suggestions for arranging the code snippets, but other than that, it looks ready to be merged.
distributed-databases/trino/main.tf
Outdated
@@ -0,0 +1,114 @@ | |||
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.
Please change the name of this file to eks.tf
. Then, transfer the code of locals.tf
and data.tf
into main.tf
. Once you've moved everything over, you can go ahead and delete the locals.tf and data.tf files.
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.
addressed
@@ -0,0 +1,32 @@ | |||
provider "aws" { |
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.
Move contents from the providers.tf
file into main.tf
, and after that, you can delete the providers.tf
file.
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.
addressed
@@ -0,0 +1,156 @@ | |||
#--------------------------------------------------------------- |
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.
Rename this file from resources.tf
to trino.tf
.
Change the name of the resources.tf file to trino.tf.
Then, transfer all Trino-related resources, including the deployment of the Trino add-on module, into this newly renamed trino.tf
file.
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.
addressed
#--------------------------------------- | ||
# Trino Helm Add-on | ||
#--------------------------------------- | ||
module "trino_addon" { |
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.
Transfer this module code to trino.tf
to consolidate all Trino-related deployment resources into a single file, making management easier.
Additionally, if you have the capacity, consider adding this as a new add-on under the Data Addons TF module.
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.
addressed - will also add this under the data addons tf module as a follow up.
variable "namespace" { | ||
description = "Namespace for Trino" | ||
type = string | ||
default = "trino" | ||
} | ||
|
||
variable "trino_sa" { | ||
description = "Service Account name for Trino" | ||
type = string | ||
default = "trino-sa" | ||
} |
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.
You can use local variables or hard code these within trino.tf
file and remove from variables
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.
addressed
feat: Trino on EKS part 2
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.
Thank you 🙌🏼
Awesome. Long awaited from DoEKS kitchen:) This recipe will help in deploying and scaling cost-efficient Trino clusters in minutes on EKS using Karpenter and Spot instances |
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.
Motivation
More
website/docs
orwebsite/blog
section for this featurepre-commit run -a
with this PR. Link for installing pre-commit locallyFor Moderators
Additional Notes