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 Terraform AWS EC2 VM Example #583

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

Conversation

lucasmelogithub
Copy link

Description

  • Add Terraform Example for AWS EC2 VM
  • Enhance .gitignore for Terraform files

Issues

N/A

Type of change

List the type of change like below. Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would break existing design and interface)

Dependencies

Terraform Module:
https://github.com/intel/terraform-intel-aws-vm/tree/main

Tests

Validated terraform apply & terraform destroy works as expected

NeoZhangJianyu and others added 2 commits November 18, 2024 10:41
Signed-off-by: ZhangJianyu <[email protected]>
Signed-off-by: lucasmelogithub <[email protected]>
Signed-off-by: lucasmelogithub <[email protected]>
@lucasmelogithub
Copy link
Author

@mkbhanda As promised, a Generic AWS EC2 VM terraform example.

I plan to add Azure/GCP VM examples soon too.

@lianhao lianhao added this to the v1.1 milestone Nov 20, 2024
@lianhao lianhao requested review from poussa and removed request for daisy-ycguo November 20, 2024 00:59
@lianhao
Copy link
Collaborator

lianhao commented Nov 20, 2024

Added @poussa to review since he knows more about terraform

@lianhao lianhao removed this from the v1.1 milestone Nov 20, 2024
This example uses the following Terraform module:

[Terraform Module](https://registry.terraform.io/modules/intel/aws-vm/intel/latest)

Copy link
Collaborator

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.

Copy link
Author

@lucasmelogithub lucasmelogithub Nov 22, 2024

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.

Copy link
Collaborator

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?

Copy link
Author

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
Copy link
Collaborator

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

Copy link
Author

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.

Copy link
Author

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" {
Copy link
Collaborator

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.

Copy link
Author

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
Copy link
Collaborator

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.

Copy link
Author

@lucasmelogithub lucasmelogithub Dec 3, 2024

Choose a reason for hiding this comment

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

Will do.

Copy link
Author

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"
Copy link
Collaborator

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.

Copy link
Author

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.

Copy link
Author

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.

@lucasmelogithub
Copy link
Author

I'll update the code this and push up this week.

@lucasmelogithub
Copy link
Author

Per feedback.
Added variables for availability zone, instancetype and volume size.

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.

4 participants