vultr / packer-plugin-vultr

Packer Builder plugin for Vultr snapshots
Mozilla Public License 2.0
48 stars 20 forks source link

Add support for uploading an ISO for building #208

Closed jasites closed 1 year ago

jasites commented 1 year ago

Description

Adds iso_url option to use the create-iso API endpoint to upload an ISO, updates the Config struct to use the newly created ISO's ID for the instance, and deletes the ISO upon completion/cancellation

Checklist:

optik-aper commented 1 year ago

This needs to handle invalid configs upfront. For instance, using this config:

packer {
  required_plugins {
  }
}

build {

  sources = [
    "source.vultr.iso"
  ]
}

variable "vultr_api_key" {
  type    = string
  default = "${env("VULTR_API_KEY")}"
}

source "vultr" "iso" {
  api_key              = "${var.vultr_api_key}"
  os_id                = 387
  iso_url              = "https://dl-cdn.alpinelinux.org/alpine/v3.17/releases/x86_64/alpine-virt-3.17.1-x86_64.iso"
  region_id            = "ewr"
  plan_id              = "vhf-2c-4gb"
  snapshot_description = "iso-test"
  state_timeout        = "20m"
  ssh_username         = "root"
}

os_id and iso_url shouldn't be allowed in the same config. The plugin proceeds but, eventually, the API will reject:

michael  ~/code-review/packer-plugin-vultr/packer-plugin-vultr-cust-128-iso (:|✔)
 Φ packer build iso-test.pkr.hcl                                                                                                 1 ↵ 15:45
vultr.ubuntu: output will be in this color.

==> vultr.ubuntu: Running Vultr builder...
==> vultr.ubuntu: Creating temporary SSH key...
==> vultr.ubuntu: Creating ISO in Vultr account...
==> vultr.ubuntu: Waiting 1200s for ISO alpine-virt-3.17.1-x86_64.iso (8f894776-7536-47bc-a824-3a9a364dcff2) to complete uploading...
==> vultr.ubuntu: Creating Vultr instance...
==> vultr.ubuntu: Error creating server: {"message":"please provide one app_id, image_id, iso_id, os_id, or snapshot_id","status":400}
==> vultr.ubuntu: Destroying ISO 8f894776-7536-47bc-a824-3a9a364dcff2
==> vultr.ubuntu: Deleting temporary SSH key...
Build 'vultr.ubuntu' errored after 15 seconds 606 milliseconds: Error creating server: {"message":"please provide one app_id, image_id, iso_id, os_id, or snapshot_id","status":400}

==> Wait completed after 15 seconds 606 milliseconds

==> Some builds didn't complete successfully and had errors:
--> vultr.ubuntu: Error creating server: {"message":"please provide one app_id, image_id, iso_id, os_id, or snapshot_id","status":400}

==> Builds finished but no artifacts were created.

Handle these exclusive cases before proceeding in the config prepare function.

optik-aper commented 1 year ago

What about doing this instead of the complicated && || chain?

    imageConfig := []bool{(c.ISOID != ""), (c.ISOURL != ""), (c.AppID != 0), (c.SnapshotID != ""), (c.OSID != 0)}
    imageDefined := false
    for _, isDefined := range imageConfig {
        if isDefined {
            if imageDefined {
                errs = packer.MultiErrorAppend(errs, errors.New("you can only set one of the following: `os_id`, `app_id`, `snapshot_id`, `iso_id`, `iso_url`"))
                break
            }
            imageDefined = true
        }
    }

This one will also handled os_id which should be exclusive.

jasites commented 1 year ago

Thanks for the feedback! Your suggestion definitely seems like it should do the trick in a simpler and more idiomatic manner... my inexperience with Go is showing :wink:

optik-aper commented 1 year ago

For sure! It took some figuring, and I'm still a little leery of people defining app_id or os_id as 0 in the configs since that will pass this check. We could update the mapstructure stuff to return the typed nil in that case but it seems like a minor enough case that I didn't want to rejigger the config values downstream.