Azure / terraform-azurerm-virtual-machine

Terraform Azure RM Virtual Machine Module
MIT License
36 stars 36 forks source link

Support for VM Extension to be executed after a data disks attachment successfully executed #33

Closed tze-dev closed 1 year ago

tze-dev commented 1 year ago

Is there an existing issue for this?

Description

Currently the VM Extension. azurerm_virtual_machine_extension.extensions resource runs as soon as the azurerm_windows_virtual_machine or azurerm_linux_virtual_machine gets executed. However, there are cases where this is undesirable - e.g., if you want to attach multiple data disks to the VM and then use the VM Extension to configure span volumes across data disks -- In this case, the VM extension would run before the data disks are fully attached and therefore unable to properly complete the logic.

What would be better is to allow VM extension to run after all data disks are fully attached. One way to do this is to make the resource, azurerm_virtual_machine_extension.extensions depends on azurerm_virtual_machine_data_disk_attachment.attachment. And optionally, it would be great to also output the azurerm_virtual_machine_data_disk_attachment.attachment from the module - this is so that if people want to apply VM Extension outside of the module, they can manage the dependency using the output of data disk attachment - before running VM extension from the caller module.

New or Affected Resource(s)/Data Source(s)

azurerm_virtual_machine_extension.extensions

Potential Terraform Configuration

resource "azurerm_virtual_machine_extension" "extensions" {
  # The `sensitive` inside `nonsensitive` is a workaround for https://github.com/terraform-linters/tflint-ruleset-azurerm/issues/229
  for_each = nonsensitive({ for e in var.extensions : e.name => e })

  name                        = each.key
  publisher                   = each.value.publisher
  type                        = each.value.type
  type_handler_version        = each.value.type_handler_version
  virtual_machine_id          = local.virtual_machine.id
  auto_upgrade_minor_version  = each.value.auto_upgrade_minor_version
  automatic_upgrade_enabled   = each.value.automatic_upgrade_enabled
  failure_suppression_enabled = each.value.failure_suppression_enabled
  protected_settings          = each.value.protected_settings
  settings                    = each.value.settings
  tags                        = var.tags

  dynamic "protected_settings_from_key_vault" {
    for_each = each.value.protected_settings_from_key_vault == null ? [] : [
      "protected_settings_from_key_vault"
    ]

    content {
      secret_url      = each.value.protected_settings_from_key_vault.secret_url
      source_vault_id = each.value.protected_settings_from_key_vault.source_vault_id
    }
  }

  depends_on = [azurerm_virtual_machine_data_disk_attachment.attachment] # New addition
}

# The following section is for additional information and workflow flexibility to caller module

locals {
  data_disk_attachment = try(azurerm_virtual_machine_data_disk_attachment.attachment, {})
}

output "data_disk_attachment" {
  value = local.data_disk_attachment
}

References

No response

lonegunmanb commented 1 year ago

Thanks @tl-aiyor for opening this issue, awesome suggestion, I like this idea! Would you like to submit a pr to implement this feature?

Btw, I think outputing attachments' id list is good enough, basically, we don't output resources directly. And this local can be inlined into output's value I think.

tze-dev commented 1 year ago

Hi @lonegunmanb, sounds good. I can definitely submit a pr. And agree on the attachments' id list as output.