-
Notifications
You must be signed in to change notification settings - Fork 61
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 Terraform AWS EC2 VM Example #583
base: main
Are you sure you want to change the base?
Add Terraform AWS EC2 VM Example #583
Conversation
Signed-off-by: ZhangJianyu <[email protected]> Signed-off-by: lucasmelogithub <[email protected]>
Signed-off-by: lucasmelogithub <[email protected]>
fb7280d
to
a29b11b
Compare
@mkbhanda As promised, a Generic AWS EC2 VM terraform example. I plan to add Azure/GCP VM examples soon too. |
Added @poussa to review since he knows more about terraform |
This example uses the following Terraform module: | ||
|
||
[Terraform Module](https://registry.terraform.io/modules/intel/aws-vm/intel/latest) | ||
|
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.
Why don't we use the standard terraform ec2_instance module instead of Intel one? They seem to be almost the same.
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.
Hey Sakari, thanks for the review. The module on the PR has already been used to showcase end-to-end OPEA of ChatQna and CodeGen automation in AWS/Azure/GCP. Implements system provisioning plus OPEA deployment in minutes, with no human intervention.
Example here: https://github.com/opea-project/GenAIExamples/blob/main/ChatQnA/README.md#-automated-terraform-deployment-using-intel-optimized-cloud-modules-for-terraform
There are plans to support even more examples.
For consistency, this PR is to backport/enable a generic VM creation using the same 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.
My point is that we should use vendor neutral ec2-instance
module from the Terraform community instead of Intel specific clone (ec2-vm
) of the module. Or am I missing something here?
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.
The module being used is also an Open Source Community module.
https://registry.terraform.io/modules/intel/aws-vm/intel/latest
source = "intel/aws-vm/intel" | ||
version = "1.3.3" | ||
key_name = aws_key_pair.TF_key.key_name | ||
instance_type = "c7i.16xlarge" # Modify the instance type as required for your AI needs |
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.
instance type should be variable
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.
It's already a Module variable, other values can be passed as needed.
But I can abstract it out again to variables.tf, I'll make that change.
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.
Added variables. Thx for the review.
} | ||
|
||
# Modify the `vm_count` variable in the variables.tf file to create the required number of EC2 instances | ||
module "ec2-vm" { |
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 should be module ec2_instance
from terraform cummunity.
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.
The module being used is also an Open Source Community module.
https://registry.terraform.io/modules/intel/aws-vm/intel/latest
key_name = aws_key_pair.TF_key.key_name | ||
instance_type = "c7i.16xlarge" # Modify the instance type as required for your AI needs | ||
availability_zone = "us-east-1d" | ||
ami = data.aws_ami.ubuntu-linux-2204.id |
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 should be a variable similar to 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.
Will do.
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.
Added variables. Thx for the review.
|
||
# Size of VM disk in GB | ||
root_block_device = [{ | ||
volume_size = "600" |
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.
volume size should be variable. models take a lot of space.
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.
It's already a Module variable, other values can be passed as needed.
But I can abstract it out again to variables.tf, I'll make that change.
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.
Added variables. Thx for the review.
I'll update the code this and push up this week. |
Signed-off-by: lucasmelogithub <[email protected]>
…hub/GenAIInfra into lmelo-terraform-ec2
Per feedback. |
Description
Issues
N/A
Type of change
List the type of change like below. Please delete options that are not relevant.
Dependencies
Terraform Module:
https://github.com/intel/terraform-intel-aws-vm/tree/main
Tests
Validated
terraform apply
&terraform destroy
works as expected