Skip to content

Commit

Permalink
vultr: fix OS ID for snapshots
Browse files Browse the repository at this point in the history
The Vultr API models the the OS of an instance as being
dependent on _two_ things: the OS selected at creation AND the snapshot
selected at creation. This turns out to be the case as well for
applications, i.e. if a chosen snapshot is of an application server,
then once the instance boots, its application ID will be that of the
original snapshotted app.

To solve this in this provider:
the `os_id` field of the `vultr_instance` resource becomes optional
and is computed by the provider. When creating an instance based on a
snapshot, the user must not supply an `os_id`. Once the instance is up,
the `os_id` field is computed and set to the OS ID of the machine that
the snapshot was based on. This is a slightly larger public API change
but requires the user to configure fewer options.

This option simplifies the interface of the provider
and allows the user to do less work. From now on, when booting a
snapshot, only the `snapshot_id` must be configured. The `os_id` will be
computed.
  • Loading branch information
squat committed May 31, 2018
1 parent 560e0e7 commit 441156b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 17 deletions.
16 changes: 3 additions & 13 deletions examples/snapshot/example.tf
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@ provider "vultr" {
api_key = "<your-vultr-api-key>"
}

// Find the OS ID for snapshots.
data "vultr_os" "snapshot" {
filter {
name = "family"
values = ["snapshot"]
}
}

// Find the snapshot ID for a Kubernetes master.
data "vultr_snapshot" "master" {
description_regex = "master"
Expand All @@ -21,7 +13,7 @@ data "vultr_snapshot" "master" {
data "vultr_region" "silicon_valley" {
filter {
name = "name"
values = ["Silicon Valley"]
values = ["Frankfurt"]
}
}

Expand All @@ -39,11 +31,9 @@ data "vultr_plan" "starter" {
}

// Create a Vultr virtual machine.
resource "vultr_instance" "master" {
name = "master"
hostname = "master"
resource "vultr_instance" "snapshot" {
name = "snapshot"
region_id = "${data.vultr_region.silicon_valley.id}"
plan_id = "${data.vultr_plan.starter.id}"
os_id = "${data.vultr_os.snapshot.id}"
snapshot_id = "${data.vultr_snapshot.master.id}"
}
21 changes: 19 additions & 2 deletions vultr/resource_bare_metal.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func resourceBareMetal() *schema.Resource {
Schema: map[string]*schema.Schema{
"application_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
},

Expand Down Expand Up @@ -72,7 +73,8 @@ func resourceBareMetal() *schema.Resource {

"os_id": {
Type: schema.TypeInt,
Required: true,
Computed: true,
Optional: true,
},

"plan_id": {
Expand Down Expand Up @@ -125,6 +127,16 @@ func resourceBareMetal() *schema.Resource {
}

func resourceBareMetalCreate(d *schema.ResourceData, meta interface{}) error {
_, appOK := d.GetOk("application_id")
_, osOK := d.GetOk("os_id")
_, snapshotOK := d.GetOk("snapshot_id")
if appOK == snapshotOK {
return fmt.Errorf("One of %q and %q must be provided but not both", "application_id", "snapshot_id")
}
if osOK == snapshotOK {
return fmt.Errorf("One of %q and %q must be provided but not both", "os_id", "snapshot_id")
}

client := meta.(*Client)
options := &lib.BareMetalServerOptions{
AppID: d.Get("application_id").(string),
Expand All @@ -136,7 +148,12 @@ func resourceBareMetalCreate(d *schema.ResourceData, meta interface{}) error {
}

name := d.Get("name").(string)
osID := d.Get("os_id").(int)
var osID int
if snapshotOK {
osID = osIDSnapshot
} else {
osID = d.Get("os_id").(int)
}
planID := d.Get("plan_id").(int)
regionID := d.Get("region_id").(int)

Expand Down
25 changes: 23 additions & 2 deletions vultr/resource_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import (
"github.com/hashicorp/terraform/helper/schema"
)

const (
osIDSnapshot = 164
)

func resourceInstance() *schema.Resource {
return &schema.Resource{
Create: resourceInstanceCreate,
Expand All @@ -23,6 +27,7 @@ func resourceInstance() *schema.Resource {
Schema: map[string]*schema.Schema{
"application_id": {
Type: schema.TypeString,
Computed: true,
Optional: true,
},

Expand Down Expand Up @@ -90,7 +95,8 @@ func resourceInstance() *schema.Resource {

"os_id": {
Type: schema.TypeInt,
Required: true,
Computed: true,
Optional: true,
},

"plan_id": {
Expand Down Expand Up @@ -170,6 +176,16 @@ func resourceInstance() *schema.Resource {
}

func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
_, appOK := d.GetOk("application_id")
_, osOK := d.GetOk("os_id")
_, snapshotOK := d.GetOk("snapshot_id")
if appOK == snapshotOK {
return fmt.Errorf("One of %q and %q must be provided but not both", "application_id", "snapshot_id")
}
if osOK == snapshotOK {
return fmt.Errorf("One of %q and %q must be provided but not both", "os_id", "snapshot_id")
}

client := meta.(*Client)
options := &lib.ServerOptions{
AppID: d.Get("application_id").(string),
Expand All @@ -184,7 +200,12 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
}

name := d.Get("name").(string)
osID := d.Get("os_id").(int)
var osID int
if snapshotOK {
osID = osIDSnapshot
} else {
osID = d.Get("os_id").(int)
}
planID := d.Get("plan_id").(int)
regionID := d.Get("region_id").(int)

Expand Down

0 comments on commit 441156b

Please sign in to comment.