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

Difference in behavior of managed vs unmanaged data disk parsing on azurerm_virtual_machine #136

Closed
cehoffman opened this issue Jun 27, 2017 · 5 comments

Comments

@cehoffman
Copy link

cehoffman commented Jun 27, 2017

On terraform 0.9.8

This is probably a little out there, but I wanted to create a module for creating a set of VMs that would autojoin a kubernetes cluster with a parameterized number of data disks. I eventually arrived at a solution that worked quite well for managed disks. I then wanted to copy this pattern for unmanaged disks and ran into a problem.

The basic formula I'm using is a storage_data_disk block in the azurerm_virtual_machine definition that looks like this.

  storage_data_disk = [
    "${slice(list(

        map(
          "name", "${var.cluster_name}-worker-${var.role}-${count.index}-l0",
          "lun", 0,
          "caching", "${var.disk_caching}",
          "create_option", "Empty",
          "disk_size_gb", "${var.disk_size}",
          "managed_disk_type", "${var.disk_storage_account}",
        ),

....

        map(
          "name", "${var.cluster_name}-worker-${var.role}-${count.index}-l63",
          "lun", 63,
          "caching", "${var.disk_caching}",
          "create_option", "Empty",
          "disk_size_gb", "${var.disk_size}",
          "managed_disk_type", "${var.disk_storage_account}",
        ),

      ), 0, var.disk_count)}"
  ]

This list me create basically a VM of any size with any number of attached disks using only 2 parameters. When creating the unmanaged disk version of this I ran into this problem.

3 error(s) occurred:

* module.storage-workers.azurerm_virtual_machine.worker-unmanaged: "storage_data_disk.0.create_option": required field is not set
* module.storage-workers.azurerm_virtual_machine.worker-unmanaged: "storage_data_disk.0.lun": required field is not set
* module.storage-workers.azurerm_virtual_machine.worker-unmanaged: "storage_data_disk.0.name": required field is not set
  storage_data_disk = [
    "${slice(list(

        map(
          "name", "${var.cluster_name}-worker-${var.role}-${count.index}-l0",
          "lun", 0,
          "caching", "${var.disk_caching}",
          "create_option", "Empty",
          "disk_size_gb", "${var.disk_size}",
          "vhd_uri", "${azurerm_storage_account.worker.primary_blob_endpoint}${azurerm_storage_container.worker.name}/${var.cluster_name}-worker-${var.role}-${count.index}-l0.vhd",
        ),
...
        map(
          "name", "${var.cluster_name}-worker-${var.role}-${count.index}-l63",
          "lun", 63,
          "caching", "${var.disk_caching}",
          "create_option", "Empty",
          "disk_size_gb", "${var.disk_size}",
          "vhd_uri", "${azurerm_storage_account.worker.primary_blob_endpoint}${azurerm_storage_container.worker.name}/${var.cluster_name}-worker-${var.role}-${count.index}-l1.vhd",
        ),
      ), 0, var.disk_count)}"
  ]

It is specifically switching managed_disk_type for vhd_uri that causes the problem.

@DevOpsFu
Copy link
Contributor

I came across this because I'm trying to solve exactly the same issue that you have done with Managed Disks. Unfortunately your solution for Managed Disks gives me the same errors as you get for Unmanaged.

I suspect that this implementation is unpredictable for the same reasons as in hashicorp/terraform#16582

@nbering
Copy link

nbering commented Nov 28, 2017

I seem to recall an issue on Terraform core that is similar in nature, and the resolution had something to do the the fact that passing a map as a config block like this is actually a hack that works because of an oddity in how the Go-lang type system works. The validation errors happen as a result of this oddity, and the accepting of the map in the first place is the actual bug, not the validation errors when you change a property.

The issue I'm referring to is much newer than this issue though, as I recall. If I can dredge it up, I'll link to it here, but there weren't any details remarkable enough for me to come up with clear search terms to find it.

@nbering
Copy link

nbering commented Nov 28, 2017

Hey look at that. @DevOpsFu did have the link, that's exactly the one I'm talking about. You can ignore me.

@achandmsft achandmsft modified the milestones: M1, 1.4.0 Mar 10, 2018
@achandmsft achandmsft added bug M1 and removed M1 labels Mar 10, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.4.0, Temp/To Be Sorted Apr 17, 2018
@achandmsft achandmsft modified the milestones: Temp/To Be Sorted, 1.4.0 Apr 19, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.4.0, 1.5.0 Apr 25, 2018
@tombuildsstuff tombuildsstuff modified the milestones: 1.5.0, 1.6.0 May 8, 2018
@tombuildsstuff
Copy link
Contributor

👋🏻 hey @cehoffman @DevOpsFu @nbering

This'll be fixed by the new azurerm_virtual_machine_data_disk_attachment resource which has been requested in #795 and has been added in #1207 - rather than having multiple issues tracking the same thing, I'm going to close this issue in favour of that one.

Thanks!

@ghost
Copy link

ghost commented Mar 31, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants