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

providers/aws: rework instance block devices #1045

Merged
merged 2 commits into from
Mar 19, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
332 changes: 234 additions & 98 deletions builtin/providers/aws/resource_aws_instance.go

Large diffs are not rendered by default.

107 changes: 107 additions & 0 deletions builtin/providers/aws/resource_aws_instance_migrate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package aws

import (
"fmt"
"log"
"strconv"
"strings"

"github.com/hashicorp/terraform/helper/hashcode"
"github.com/hashicorp/terraform/terraform"
)

func resourceAwsInstanceMigrateState(
v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
switch v {
case 0:
log.Println("[INFO] Found AWS Instance State v0; migrating to v1")
return migrateStateV0toV1(is)
default:
return is, fmt.Errorf("Unexpected schema version: %d", v)
}

return is, nil
}

func migrateStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes)
// Delete old count
delete(is.Attributes, "block_device.#")

oldBds, err := readV0BlockDevices(is)
if err != nil {
return is, err
}
// seed count fields for new types
is.Attributes["ebs_block_device.#"] = "0"
is.Attributes["ephemeral_block_device.#"] = "0"
// depending on if state was v0.3.7 or an earlier version, it might have
// root_block_device defined already
if _, ok := is.Attributes["root_block_device.#"]; !ok {
is.Attributes["root_block_device.#"] = "0"
}
for _, oldBd := range oldBds {
if err := writeV1BlockDevice(is, oldBd); err != nil {
return is, err
}
}
log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes)
return is, nil
}

func readV0BlockDevices(is *terraform.InstanceState) (map[string]map[string]string, error) {
oldBds := make(map[string]map[string]string)
for k, v := range is.Attributes {
if !strings.HasPrefix(k, "block_device.") {
continue
}
path := strings.Split(k, ".")
if len(path) != 3 {
return oldBds, fmt.Errorf("Found unexpected block_device field: %#v", k)
}
hashcode, attribute := path[1], path[2]
oldBd, ok := oldBds[hashcode]
if !ok {
oldBd = make(map[string]string)
oldBds[hashcode] = oldBd
}
oldBd[attribute] = v
delete(is.Attributes, k)
}
return oldBds, nil
}

func writeV1BlockDevice(
is *terraform.InstanceState, oldBd map[string]string) error {
code := hashcode.String(oldBd["device_name"])
bdType := "ebs_block_device"
if vn, ok := oldBd["virtual_name"]; ok && strings.HasPrefix(vn, "ephemeral") {
bdType = "ephemeral_block_device"
} else if dn, ok := oldBd["device_name"]; ok && dn == "/dev/sda1" {
bdType = "root_block_device"
}

switch bdType {
case "ebs_block_device":
delete(oldBd, "virtual_name")
case "root_block_device":
delete(oldBd, "virtual_name")
delete(oldBd, "encrypted")
delete(oldBd, "snapshot_id")
case "ephemeral_block_device":
delete(oldBd, "delete_on_termination")
delete(oldBd, "encrypted")
delete(oldBd, "iops")
delete(oldBd, "volume_size")
delete(oldBd, "volume_type")
}
for attr, val := range oldBd {
attrKey := fmt.Sprintf("%s.%d.%s", bdType, code, attr)
is.Attributes[attrKey] = val
}

countAttr := fmt.Sprintf("%s.#", bdType)
count, _ := strconv.Atoi(is.Attributes[countAttr])
is.Attributes[countAttr] = strconv.Itoa(count + 1)
return nil
}
135 changes: 135 additions & 0 deletions builtin/providers/aws/resource_aws_instance_migrate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
package aws

import (
"testing"

"github.com/hashicorp/terraform/terraform"
)

func TestAWSInstanceMigrateState(t *testing.T) {
cases := map[string]struct {
StateVersion int
Attributes map[string]string
Expected map[string]string
Meta interface{}
}{
"v0.3.6 and earlier": {
StateVersion: 0,
Attributes: map[string]string{
// EBS
"block_device.#": "2",
"block_device.3851383343.delete_on_termination": "true",
"block_device.3851383343.device_name": "/dev/sdx",
"block_device.3851383343.encrypted": "false",
"block_device.3851383343.snapshot_id": "",
"block_device.3851383343.virtual_name": "",
"block_device.3851383343.volume_size": "5",
"block_device.3851383343.volume_type": "standard",
// Ephemeral
"block_device.3101711606.delete_on_termination": "false",
"block_device.3101711606.device_name": "/dev/sdy",
"block_device.3101711606.encrypted": "false",
"block_device.3101711606.snapshot_id": "",
"block_device.3101711606.virtual_name": "ephemeral0",
"block_device.3101711606.volume_size": "",
"block_device.3101711606.volume_type": "",
// Root
"block_device.56575650.delete_on_termination": "true",
"block_device.56575650.device_name": "/dev/sda1",
"block_device.56575650.encrypted": "false",
"block_device.56575650.snapshot_id": "",
"block_device.56575650.volume_size": "10",
"block_device.56575650.volume_type": "standard",
},
Expected: map[string]string{
"ebs_block_device.#": "1",
"ebs_block_device.3851383343.delete_on_termination": "true",
"ebs_block_device.3851383343.device_name": "/dev/sdx",
"ebs_block_device.3851383343.encrypted": "false",
"ebs_block_device.3851383343.snapshot_id": "",
"ebs_block_device.3851383343.volume_size": "5",
"ebs_block_device.3851383343.volume_type": "standard",
"ephemeral_block_device.#": "1",
"ephemeral_block_device.2458403513.device_name": "/dev/sdy",
"ephemeral_block_device.2458403513.virtual_name": "ephemeral0",
"root_block_device.#": "1",
"root_block_device.3018388612.delete_on_termination": "true",
"root_block_device.3018388612.device_name": "/dev/sda1",
"root_block_device.3018388612.snapshot_id": "",
"root_block_device.3018388612.volume_size": "10",
"root_block_device.3018388612.volume_type": "standard",
},
},
"v0.3.7": {
StateVersion: 0,
Attributes: map[string]string{
// EBS
"block_device.#": "2",
"block_device.3851383343.delete_on_termination": "true",
"block_device.3851383343.device_name": "/dev/sdx",
"block_device.3851383343.encrypted": "false",
"block_device.3851383343.snapshot_id": "",
"block_device.3851383343.virtual_name": "",
"block_device.3851383343.volume_size": "5",
"block_device.3851383343.volume_type": "standard",
"block_device.3851383343.iops": "",
// Ephemeral
"block_device.3101711606.delete_on_termination": "false",
"block_device.3101711606.device_name": "/dev/sdy",
"block_device.3101711606.encrypted": "false",
"block_device.3101711606.snapshot_id": "",
"block_device.3101711606.virtual_name": "ephemeral0",
"block_device.3101711606.volume_size": "",
"block_device.3101711606.volume_type": "",
"block_device.3101711606.iops": "",
// Root
"root_block_device.#": "1",
"root_block_device.3018388612.delete_on_termination": "true",
"root_block_device.3018388612.device_name": "/dev/sda1",
"root_block_device.3018388612.snapshot_id": "",
"root_block_device.3018388612.volume_size": "10",
"root_block_device.3018388612.volume_type": "io1",
"root_block_device.3018388612.iops": "1000",
},
Expected: map[string]string{
"ebs_block_device.#": "1",
"ebs_block_device.3851383343.delete_on_termination": "true",
"ebs_block_device.3851383343.device_name": "/dev/sdx",
"ebs_block_device.3851383343.encrypted": "false",
"ebs_block_device.3851383343.snapshot_id": "",
"ebs_block_device.3851383343.volume_size": "5",
"ebs_block_device.3851383343.volume_type": "standard",
"ephemeral_block_device.#": "1",
"ephemeral_block_device.2458403513.device_name": "/dev/sdy",
"ephemeral_block_device.2458403513.virtual_name": "ephemeral0",
"root_block_device.#": "1",
"root_block_device.3018388612.delete_on_termination": "true",
"root_block_device.3018388612.device_name": "/dev/sda1",
"root_block_device.3018388612.snapshot_id": "",
"root_block_device.3018388612.volume_size": "10",
"root_block_device.3018388612.volume_type": "io1",
"root_block_device.3018388612.iops": "1000",
},
},
}

for tn, tc := range cases {
is := &terraform.InstanceState{
Attributes: tc.Attributes,
}
is, err := resourceAwsInstanceMigrateState(
tc.StateVersion, is, tc.Meta)

if err != nil {
t.Fatalf("bad: %s, err: %#v", tn, err)
}

for k, v := range tc.Expected {
if is.Attributes[k] != v {
t.Fatalf(
"bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v",
tn, k, v, k, is.Attributes[k], is.Attributes)
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole file is great.

41 changes: 24 additions & 17 deletions builtin/providers/aws/resource_aws_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,31 +111,33 @@ func TestAccAWSInstance_blockDevices(t *testing.T) {
resource.TestCheckResourceAttr(
"aws_instance.foo", "root_block_device.#", "1"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "root_block_device.0.device_name", "/dev/sda1"),
"aws_instance.foo", "root_block_device.3018388612.device_name", "/dev/sda1"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "root_block_device.0.volume_size", "11"),
// this one is important because it's the only root_block_device
// attribute that comes back from the API. so checking it verifies
// that we set state properly
"aws_instance.foo", "root_block_device.3018388612.volume_size", "11"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "root_block_device.0.volume_type", "gp2"),
"aws_instance.foo", "root_block_device.3018388612.volume_type", "gp2"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.#", "2"),
"aws_instance.foo", "ebs_block_device.#", "2"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.172787947.device_name", "/dev/sdb"),
"aws_instance.foo", "ebs_block_device.418220885.device_name", "/dev/sdb"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.172787947.volume_size", "9"),
"aws_instance.foo", "ebs_block_device.418220885.volume_size", "9"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.172787947.iops", "0"),
// Check provisioned SSD device
"aws_instance.foo", "ebs_block_device.418220885.volume_type", "standard"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.3336996981.volume_type", "io1"),
"aws_instance.foo", "ebs_block_device.1877654467.device_name", "/dev/sdc"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.3336996981.device_name", "/dev/sdc"),
"aws_instance.foo", "ebs_block_device.1877654467.volume_size", "10"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.3336996981.volume_size", "10"),
"aws_instance.foo", "ebs_block_device.1877654467.volume_type", "io1"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "block_device.3336996981.iops", "100"),
"aws_instance.foo", "ebs_block_device.1877654467.iops", "100"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "ephemeral_block_device.#", "1"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "ephemeral_block_device.2087552357.device_name", "/dev/sde"),
resource.TestCheckResourceAttr(
"aws_instance.foo", "ephemeral_block_device.2087552357.virtual_name", "ephemeral0"),
testCheck(),
),
},
Expand Down Expand Up @@ -420,21 +422,26 @@ resource "aws_instance" "foo" {
# us-west-2
ami = "ami-55a7ea65"
instance_type = "m1.small"

root_block_device {
device_name = "/dev/sda1"
volume_type = "gp2"
volume_size = 11
}
block_device {
ebs_block_device {
device_name = "/dev/sdb"
volume_size = 9
}
block_device {
ebs_block_device {
device_name = "/dev/sdc"
volume_size = 10
volume_type = "io1"
iops = 100
}
ephemeral_block_device {
device_name = "/dev/sde"
virtual_name = "ephemeral0"
}
}
`

Expand Down
22 changes: 13 additions & 9 deletions helper/schema/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (r *Resource) Apply(
err = r.Update(data, meta)
}

return data.State(), err
return r.recordCurrentSchemaVersion(data.State()), err
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the bug that wasted me a nice little bit of time. 😭 See the first commit's msg for details.

}

// Diff returns a diff of this resource and is API compatible with the
Expand Down Expand Up @@ -207,14 +207,7 @@ func (r *Resource) Refresh(
state = nil
}

if state != nil && r.SchemaVersion > 0 {
if state.Meta == nil {
state.Meta = make(map[string]string)
}
state.Meta["schema_version"] = strconv.Itoa(r.SchemaVersion)
}

return state, err
return r.recordCurrentSchemaVersion(state), err
}

// InternalValidate should be called to validate the structure
Expand All @@ -241,3 +234,14 @@ func (r *Resource) checkSchemaVersion(is *terraform.InstanceState) (bool, int) {
stateSchemaVersion, _ := strconv.Atoi(is.Meta["schema_version"])
return stateSchemaVersion < r.SchemaVersion, stateSchemaVersion
}

func (r *Resource) recordCurrentSchemaVersion(
state *terraform.InstanceState) *terraform.InstanceState {
if state != nil && r.SchemaVersion > 0 {
if state.Meta == nil {
state.Meta = make(map[string]string)
}
state.Meta["schema_version"] = strconv.Itoa(r.SchemaVersion)
}
return state
}
8 changes: 8 additions & 0 deletions helper/schema/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

func TestResourceApply_create(t *testing.T) {
r := &Resource{
SchemaVersion: 2,
Schema: map[string]*Schema{
"foo": &Schema{
Type: TypeInt,
Expand Down Expand Up @@ -51,6 +52,9 @@ func TestResourceApply_create(t *testing.T) {
"id": "foo",
"foo": "42",
},
Meta: map[string]string{
"schema_version": "2",
},
}

if !reflect.DeepEqual(actual, expected) {
Expand Down Expand Up @@ -339,6 +343,7 @@ func TestResourceInternalValidate(t *testing.T) {

func TestResourceRefresh(t *testing.T) {
r := &Resource{
SchemaVersion: 2,
Schema: map[string]*Schema{
"foo": &Schema{
Type: TypeInt,
Expand Down Expand Up @@ -368,6 +373,9 @@ func TestResourceRefresh(t *testing.T) {
"id": "bar",
"foo": "13",
},
Meta: map[string]string{
"schema_version": "2",
},
}

actual, err := r.Refresh(s, 42)
Expand Down
Loading