Conversation
This comment was marked as abuse.
This comment was marked as abuse.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Oracle Cloud Infrastructure (OCI) template for provisioning Coder workspaces on Oracle Cloud VMs. The template provides compute instance configuration with flexible CPU and memory options, persistent home volume storage, and automated agent setup via cloud-init. It follows similar patterns established in other cloud provider templates in the registry (Hetzner, Linode).
Key changes:
- OCI VM provisioning with flexible shapes supporting 1-8 OCPUs and 1-32GB memory
- Persistent block volume storage for user home directories with configurable sizes (50-700GB)
- Cloud-init automation for workspace initialization and Coder agent setup
- Multi-region support across 7 OCI regions (US, EU, UK, Australia)
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| registry/Excellencedev/templates/oci-vm/main.tf | Main Terraform configuration defining OCI resources, parameters, and workspace provisioning logic |
| registry/Excellencedev/templates/oci-vm/cloud-init/cloud-config.yaml.tftpl | Cloud-init template for automated VM setup, disk mounting, and Coder agent installation |
| registry/Excellencedev/templates/oci-vm/README.md | Documentation describing prerequisites, authentication requirements, and usage instructions |
| .icons/oracle.svg | Oracle logo icon for template display |
| } | ||
|
|
||
| variable "fingerprint" { | ||
| description = "fingerprint" |
There was a problem hiding this comment.
The description "fingerprint" should be capitalized and more descriptive, similar to other variable descriptions in the file (e.g., "Tenancy OCID", "User OCID"). Consider changing it to "API Key Fingerprint" for consistency and clarity.
| validation { | ||
| min = data.coder_parameter.instance_ocpus.value | ||
| error = "Memory must be at least equal to OCPUs (minimum 1:1 ratio)." | ||
| } | ||
|
|
There was a problem hiding this comment.
The validation only checks that memory is at least equal to OCPUs (minimum 1:1 ratio), but the description states the maximum should be 64x OCPUs (max 1:64 ratio). There's no validation for the upper bound. Users could select incompatible combinations like 1 OCPU with 32 GB, which exceeds OCI's 1:64 ratio constraint. Consider adding a max validation or additional options that enforce the valid memory ranges for each OCPU count.
| variable "ssh_public_key" { | ||
| description = "SSH public key for debugging access" | ||
| type = string | ||
| default = "" | ||
| } |
There was a problem hiding this comment.
The variable "ssh_public_key" is defined but never used in the configuration. If this is intended for debugging access to the VM, it should be added to the instance metadata. If it's not needed, the variable declaration should be removed to avoid confusion.
| attachment_type = "paravirtualized" | ||
| compartment_id = local.compartment_id | ||
| instance_id = oci_core_instance.workspace[0].id | ||
| #device = "/dev/sdb" |
There was a problem hiding this comment.
This commented-out device configuration should either be removed or uncommented if it's needed. Leaving commented code in production templates can cause confusion. If the device path is not required for paravirtualized attachments in OCI, this line should be removed entirely.
| resource "oci_core_volume" "home_volume" { | ||
| availability_domain = data.oci_identity_availability_domains.ads.availability_domains[0].name | ||
| compartment_id = local.compartment_id | ||
| display_name = local.home_volume_label | ||
| size_in_gbs = data.coder_parameter.home_volume_size.value | ||
| } |
There was a problem hiding this comment.
The home volume resource lacks a lifecycle block with ignore_changes to protect the volume from being deleted or recreated when workspace parameters change. Other similar templates (e.g., digitalocean-linux, linode-vm) use "lifecycle { ignore_changes = all }" on their volume resources to prevent data loss. Without this, changing mutable parameters like OCPUs or memory could trigger a volume replacement, destroying user data.
| - **User OCID** | ||
| - **Fingerprint** | ||
| - **Private Key** | ||
| - **Compartment OCID**(Optional) default to Tenancy OCID if not defined |
There was a problem hiding this comment.
The grammar should be "defaults to Tenancy OCID" instead of "default to Tenancy OCID" to match the subject-verb agreement.
| } | ||
| source_details { | ||
| source_type = "image" | ||
| source_id = data.oci_core_images.ubuntu_image.images[0].id |
There was a problem hiding this comment.
Accessing the first image with index [0] could fail if the data source returns no images. Consider adding error handling or validation to ensure at least one Ubuntu 22.04 image is available for the selected shape and compartment before attempting to access it.
| option { | ||
| name = "70GB" | ||
| value = 700 | ||
| } |
There was a problem hiding this comment.
The value for the "70GB" option is set to 700 instead of 70. This inconsistency means selecting "70GB" will actually provision a 700GB volume, which is 10 times larger than intended. This should be changed to 70 to match the displayed name.
|
|
||
| resource "oci_core_instance" "workspace" { | ||
| count = data.coder_workspace.me.start_count | ||
| availability_domain = data.oci_identity_availability_domains.ads.availability_domains[0].name |
There was a problem hiding this comment.
Accessing the first availability domain with index [0] assumes at least one availability domain exists in the compartment. If none are available, this will fail. Consider adding validation or using a more robust approach to select an availability domain.
| fingerprint = var.fingerprint | ||
| private_key = var.private_key | ||
| region = data.coder_parameter.region.value | ||
| } |
There was a problem hiding this comment.
Missing provider "coder" {} declaration. Other templates in this repository (e.g., hetzner-linux, linode-vm) include an explicit coder provider block. While it may work without it in some cases, it's a best practice to declare all providers explicitly for clarity and to avoid potential issues.
|
From what I can see this is a complete copy and paste of this PR #525 Including the same Video and incorrect icon for the what we would want for this oracle template. |
Description
This PR adds an Oracle Cloud Infrastructure (OCI) template example for provisioning Coder workspaces on Oracle Cloud Infrastructure. It includes compute instances, networking, and storage configuration.
Type of Change
Template Information
Path:
registry/Excellencedev/templates/oci-vmTesting & Validation
bun test)bun fmt)Related Issues
/fixes #201
/claim #201
Demo Video:
511064050-2164a766-7838-415e-bbcc-04ba58322b53.mp4