Adding disk type of Thick Lazy

[dkalleg]

Tests passing for me. Hoping we can get this on in 0.7.0.

[jen20]

We can probably get it into 0.7.1 :-)

[dkalleg]

FYI, test output for the two I touched:

$ make testacc TEST=./builtin/providers/vsphere/ TESTARGS="-run TestAccVSphereVirtualMachine_diskInitTypeLazy"
==> Checking that code complies with gofmt requirements...
/home/dkalleg/goworkspace/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/02 12:06:38 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/vsphere/ -v -run TestAccVSphereVirtualMachine_diskInitTypeLazy -timeout 120m
=== RUN   TestAccVSphereVirtualMachine_diskInitTypeLazy
2016/08/02 12:07:07 [DEBUG] template=
resource "vsphere_virtual_machine" "lazy" {
    name = "terraform-test"

%s
    vcpu = 2
    memory = 1024
    network_interface {
        label = "%s"
        ipv4_address = "%s"
        ipv4_prefix_length = %s
        ipv4_gateway = "%s"
    }
     disk {
%s
        template = "%s"
        iops = 500
    }

    disk {
        size = 1
        iops = 500
        controller_type = "scsi"
        name = "one"
    }
    disk {
        size = 1
        controller_type = "ide"
        type = "lazy"
        name = "two"
    }
}
2016/08/02 12:07:07 [DEBUG] template config=
resource "vsphere_virtual_machine" "lazy" {
    name = "terraform-test"

    datacenter = "FTC"
    resource_pool = "Cluster1"

    vcpu = 2
    memory = 1024
    network_interface {
        label = "VM Private"
        ipv4_address = ""
        ipv4_prefix_length = 24
        ipv4_gateway = ""
    }
     disk {
        datastore = "san1"

        template = "DansTfTest/danTestTemplate"
        iops = 500
    }

    disk {
        size = 1
        iops = 500
        controller_type = "scsi"
        name = "one"
    }
    disk {
        size = 1
        controller_type = "ide"
        type = "lazy"
        name = "two"
    }
}
2016/08/02 12:07:07 [WARN] Invalid log level: "all". Defaulting to level: TRACE. Valid levels are: [TRACE DEBUG INFO WARN ERROR]
--- PASS: TestAccVSphereVirtualMachine_diskInitTypeLazy (182.32s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/vsphere    182.364s

$ make testacc TEST=./builtin/providers/vsphere/ TESTARGS="-run TestAccVSphereVirtualMachine_diskInitTypeEager"
==> Checking that code complies with gofmt requirements...
/home/dkalleg/goworkspace/bin/stringer
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/08/02 12:10:41 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/vsphere/ -v -run TestAccVSphereVirtualMachine_diskInitTypeEager -timeout 120m
=== RUN   TestAccVSphereVirtualMachine_diskInitTypeEager
2016/08/02 12:11:12 [DEBUG] template=
resource "vsphere_virtual_machine" "thickEagerZero" {
    name = "terraform-test"

%s
    vcpu = 2
    memory = 1024
    network_interface {
        label = "%s"
        ipv4_address = "%s"
        ipv4_prefix_length = %s
        ipv4_gateway = "%s"
    }
     disk {
%s
        template = "%s"
        iops = 500
    }

    disk {
        size = 1
        iops = 500
        controller_type = "scsi"
        name = "one"
    }
    disk {
        size = 1
        controller_type = "ide"
        type = "eager_zeroed"
        name = "two"
    }
}
2016/08/02 12:11:12 [DEBUG] template config=
resource "vsphere_virtual_machine" "thickEagerZero" {
    name = "terraform-test"

    datacenter = "FTC"
    resource_pool = "Cluster1"

    vcpu = 2
    memory = 1024
    network_interface {
        label = "VM Private"
        ipv4_address = ""
        ipv4_prefix_length = 24
        ipv4_gateway = ""
    }
     disk {
        datastore = "san1"

        template = "DansTfTest/danTestTemplate"
        iops = 500
    }

    disk {
        size = 1
        iops = 500
        controller_type = "scsi"
        name = "one"
    }
    disk {
        size = 1
        controller_type = "ide"
        type = "eager_zeroed"
        name = "two"
    }
}
2016/08/02 12:11:12 [WARN] Invalid log level: "all". Defaulting to level: TRACE. Valid levels are: [TRACE DEBUG INFO WARN ERROR]
--- PASS: TestAccVSphereVirtualMachine_diskInitTypeEager (182.17s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/vsphere    182.209s

[stack72]

Hi @dkalleg

So I think this may be an issue, we are effectvely changing the default from eagerZeroedThick to eager_zeroed. This field is a ForceNew: true so anyone who is using the default value here will have their infrastructure change

I believe we are going to add this to the state migration

Thoughts?

Paul

[dkalleg]

@stack72 Yeah I realized it was a breaking change. If someone used TF to provision resources (prior to this change) then tried to modify anything (after applying this change) it would be a headache... I'm not sure how Hashicorp treats cases this that. But the name change is not for any technical reason, I only changed it to be inline with the virtual_machine resource. It seemed odd to me that the vm resource uses eager_zeroed while this disk resource used eagerZeroedThick when they're really the same thing. Looks like the underlying govmomi calls require those two respective names, which probably explains how the naming conventions came about.

Anywho, if you prefer to keep the naming schemes, just say the word and I'll change it back to eagerZeroedThick.

Also, I'm not sure what you meant by "I believe we are going to add this to the state migration". Is there some TF feature to help migrate attribute refactors like this?

Thanks,
Dan

[stack72]

@dkalleg so I think we should keep the Default value as it was - it will certainly prevent less of a headache ;)

There are migration files https://github.com/hashicorp/terraform/blob/master/builtin/providers/vsphere/resource_vsphere_virtual_machine_migrate.go

This allows us to manipulate the state for addition of new fields etc. This is typically how we would make changes like this

If we change it back to what it was, then we can proceed with the rest of the PR for now though :)

Paul

[dkalleg]

@stack72 Done-zo

[stack72]

Thanks @dkalleg

This LGTM now :)

[dkalleg]

Thanks @stack72 !

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.