Merge test to main (#113) #114
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
* root block device increase size to 80 GB * add nvidia runtime
There was a problem hiding this comment.
Pull Request Overview
A Terraform update that removes an explicit EIP dependency, increases the VM root volume size, and adjusts Docker GPU runtime flags.
- Remove explicit
depends_onfor EIP association in the allocator server - Increase root block device
volume_sizefrom 40 to 80 - Add
--runtime=nvidiato Docker run commands alongside--gpus all
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lablink-allocator/main.tf | Removed the depends_on block for the EIP association |
| lablink-allocator/lablink-allocator-service/terraform/main.tf | Updated root volume size and added GPU runtime flag to Docker run |
Comments suppressed due to low confidence (1)
lablink-allocator/main.tf:78
- Removing the explicit depends_on on the EIP association may cause the server instance to be created before the Elastic IP is attached, leading to race conditions. Consider re-adding the depends_on or referencing the EIP in the network interface block to enforce correct resource ordering.
depends_on = [
| @@ -94,7 +94,7 @@ resource "aws_instance" "lablink_vm" { | |||
| vpc_security_group_ids = [aws_security_group.lablink_sg_.id] | |||
| key_name = aws_key_pair.lablink_key_pair.key_name | |||
| root_block_device { | |||
| volume_size = 40 | |||
| volume_size = 80 | |||
There was a problem hiding this comment.
[nitpick] Hardcoding the volume size reduces flexibility; consider parameterizing this value via a Terraform variable or input to allow easier adjustments without changing code.
| volume_size = 80 | |
| volume_size = var.volume_size |
Copilot uses AI. Check for mistakes.
| docker run -dit --runtime=nvidia --gpus all -e ALLOCATOR_HOST=${var.allocator_ip} ${var.image_name} | ||
| else | ||
| echo "Cloning repository: $TUTORIAL_REPO_TO_CLONE" | ||
| docker run -dit --gpus all -e ALLOCATOR_HOST=${var.allocator_ip} -e TUTORIAL_REPO_TO_CLONE=${var.repository} ${var.image_name} | ||
| docker run -dit --runtime=nvidia --gpus all -e ALLOCATOR_HOST=${var.allocator_ip} -e TUTORIAL_REPO_TO_CLONE=${var.repository} ${var.image_name} |
There was a problem hiding this comment.
[nitpick] Using both --runtime=nvidia and --gpus all flags may be redundant depending on Docker version; pick one method to enable GPU support and ensure compatibility.
Copilot uses AI. Check for mistakes.
No description provided.