add oracle oci-vm template - #525
Conversation
Signed-off-by: Anis KHALFALLAH <khalfallah.anis@hotmail.com>
Signed-off-by: Anis KHALFALLAH <khalfallah.anis@hotmail.com>
|
@aniskhalfallah Thank you very much for the contribution! I love the thorough video showing absolutely everything. I will go ahead and test this and review it and this should be an easy merge :) |
|
Also @aniskhalfallah I would run |
Signed-off-by: Anis KHALFALLAH <khalfallah.anis@hotmail.com>
|
@DevelopmentCats Thanks for your words! |
|
Can you resolve the merge conflicts, and I will follow up with the guy that I have testing this. |
|
@DevelopmentCats Done! The merge conflict has been resolved |
|
I'm going to go ahead and just set up an account for this so I can test it and we can get this merged. Sorry for the delay on this OCI is tricky haha. @aniskhalfallah |
There was a problem hiding this comment.
You may want to go over the instances and regions in the README since these need to be configured based on the location and available compute available to the user of this template @aniskhalfallah
Also you may want to go over what actually persists when the workspace stops vs when its deleted. I think the behavior is probably ideal, but if its not mentioned people will definitely question this when resources are still existing for stopped workspaces.
Lastly you may want to remove compartment_id from the template entirely, as I get a deprecation notice from OCI when using it explicitly stating "Do Not Use This"
|
I am fairly certain this template will work, but for whatever reason I can not get it to work with my trial account. I think something with trial accounts prevents me from utilizing the API to create compute resources or something, but it does not seem to be related to the terraform configuration. Honestly I think we can probably call this good since ultimately IMHO this looks accurate for a starter template for OCI, and anyone using OCI should be able to configure this on their own in regards to instance types and regions. I would address my above comments though. |
Hi @DevelopmentCats, thanks for the feedback! Regarding That said, I believe the deprecation warning you're seeing is specific to I'll also address your other comments, I'm planning to commit everything by end of this week so I can test it properly before pushing. |
…nt resource and improve README Signed-off-by: Anis KHALFALLAH <khalfallah.anis@hotmail.com>
|
Hi @DevelopmentCats |
I apologize for the delay I was reaching out to someone who has the full OCI setup to test this, but I will just request access so Ic an do it myself and we can get this merged. |
|
This template was never tested by the user, and I have no access to OCI to do so myself. If we want to resubmit this with testing proof we can reassess then |
|
Hi @DevelopmentCats , am I the user ? If so I added the testing proof in the PR when i opened it, you can clearly see the VM created in OCI from coder :) |
Yo uare totally right hwere and this is in your own namespace so I think we can probably just be good here and merge it. |
|
Hey @aniskhalfallah! First of all sorry for the delays (and some of the PR spam) here. We've retooling our registry contribution process. I was able to test this end-to-end and found some bugs. Would you mind reviewing #1062 and seeing if this is OK to merge into your module? If so, I can merge both in today or tomorrow |
…h debug access (#1062) ## Description While testing this template end-to-end against a real OCI tenancy, I found a few things that don't actually work once you get past the reviews: - The home volume never mounts. `disk_setup`/`mounts` run early in cloud-init's boot sequence, before the OCI volume attachment (a separate, async API call) has actually shown up as a device. Confirmed via the instance's serial console: `Device /dev/sdb did not exist and was not created with a udevadm settle`. `/home/<user>` silently ends up on the ephemeral root disk instead, so it's wiped on every stop despite the template (and README) claiming it persists. - `region` defaults to `eu-marseille-1`, which most tenancies aren't subscribed to (including a fresh trial account, per the review thread, and my own tenancy's home region). Since the AD/image data sources aren't gated by `start_count`, this makes `coder templates push` itself fail with an auth-looking 401 before anyone even gets to create a workspace. - `ssh_public_key` is documented ("SSH public key for debugging access") but never wired into cloud-init, so there's no way to get into a workspace to debug it. This PR builds on top of this branch's `main.tf`/README/cloud-init with: - A `runcmd` script that waits for the volume device to show up, formats it only if it isn't already formatted (matched by filesystem label, so repeated stop/start cycles don't wipe it), and mounts it via `/etc/fstab`. Also fixes a subtler bug this introduced: ext4 labels are capped at 16 bytes, but the existing `home_volume_label` local is (correctly, for OCI's own 32-char display name limit) longer than that, so `mkfs` was silently truncating it and later lookups by the untruncated label never matched. Added a dedicated 16-byte `home_fs_label`. - `region` now defaults to `us-ashburn-1` instead of `eu-marseille-1`. Still not universal (no single default can be, since OCI only auto-subscribes your home region), but it's one of OCI's two original commercial regions and a much more common default than a specialty region — documented the caveat clearly either way. - `ssh_public_key` now actually lands in `~/.ssh/authorized_keys`. Had to provision it from inside the mount script rather than through cloud-init's normal `ssh_authorized_keys`, since that runs before the home volume mount and would otherwise get shadowed by it. Tested all of the above against a real OCI tenancy (instance + volume + attachment, connected agent, `df`/`lsblk` showing `/home` on the block volume, `ssh <user>@<ip>` working with the debug key). Opened this against your `oci-vm` branch directly rather than `main` so it's easy to pull in — feel free to merge, cherry-pick, or push back on any of it. ## Type of Change - [ ] New module - [ ] New template - [x] Bug fix - [ ] Feature/enhancement - [ ] Documentation - [ ] Other ## Template Information **Path:** `registry/anis/templates/oci-vm` ## Testing & Validation - [x] Tests pass (`bun test`) - [x] Code formatted (`bun fmt`) - [x] Changes tested locally ## Related Issues Related to #525
|
Merging for now! |
Description
Add oci-vm template to coder
Type of Change
Template Information
Path:
registry/anis/templates/oci-vmTesting & Validation
bun test)bun fmt)oracle_oci_coder_template_demo.mp4
Related Issues
Closes #201
/claim #201