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 cloud_init argument to image creation #188

Merged
merged 7 commits into from
Feb 5, 2024

Conversation

stvnjacobs
Copy link
Contributor

📝 Description

This adds the cloud_init flag to the image create step.

✔️ How to Test

There are no unit or integration tests included with this patch. I am putting it up for consideration before those are completed.

To test manually, and to get a working cloud-init image, an image can be created using an existing image which has the cloud-init capability.

$ linode-cli images list --text --is_public true --format id,capabilities | awk '/cloud-init/ { print $1 }'
linode/debian11
linode/ubuntu20.04
linode/ubuntu22.04
linode/ubuntu23.10

📷 Preview

locals { timestamp = regex_replace(timestamp(), "[- TZ:]", "") }

source "linode" "demo" {
  image          = "linode/debian11"
  image_label    = "demo-debian11"
  instance_label = "pkr-demo-debian11-${local.timestamp}"
  instance_type  = "g6-nanode-1"
  region         = "us-iad"
  ssh_username   = "root"
  cloud_init     = true
}

🤔 Additional Thoughts

Config structure

I was uncertain on the best way to format the config. I am sure I am over thinking it, but what good does that do if I don't share where my head is at.

POST /v4/images: When an image is created, cloud-init is a boolean.

GET /v4/images/{imageId}: When an image is viewed, cloud-init is one of an array of capabilities.

I was unsure whether to structure the config so it matches the create arguments:

source "linode" "demo" {
  image      = "linode/debian11"
  # ...
  cloud_init = true
}

Or to model it like the image resource it creates.

source "linode" "demo" {
  image        = "linode/debian11"
  # ...
  capabilities = ["cloud-init"]
}

In these situations I typically refer to official hashicorp cloud provider plugins, but I couldn't really find anything comparable.

Unfamiliar with codebase

I admit to being a bit rusty around these parts. the hcl2spec.go was new to me, and in config.go is wasn't sure if required:"false" was really needed. I am just committing this as a proof of what worked for me, not what is fully correct.

This adds the `cloud_init` flag to the image create step.
@stvnjacobs stvnjacobs requested a review from a team as a code owner January 30, 2024 16:35
@stvnjacobs stvnjacobs requested review from zliang-akamai and ykim-akamai and removed request for a team January 30, 2024 16:35
@lgarber-akamai lgarber-akamai self-requested a review January 30, 2024 16:36
@lgarber-akamai
Copy link
Contributor

lgarber-akamai commented Jan 31, 2024

@stvnjacobs I think the first implementation you mentioned (cloud_init = true) is ideal since Packer isn't stateful 👍

Copy link
Contributor

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

LGTM! This PR doesn't add proper E2E testing for cloud_init given #185 hasn't been merged quite yet but the change is simple enough that unit tests should be sufficient

lgarber-akamai and others added 3 commits February 5, 2024 13:00
Co-authored-by: Zhiwei Liang <121905282+zliang-akamai@users.noreply.github.com>
@lgarber-akamai
Copy link
Contributor

Failing tests are unrelated to this PR 🙂

@zliang-akamai zliang-akamai merged commit 211bdb1 into linode:dev Feb 5, 2024
9 of 10 checks passed
@stvnjacobs stvnjacobs deleted the feat/cloud-init-capability branch March 19, 2024 20:42
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