hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.61k stars 4.65k forks source link

ability to tag OS disk in azurerm_linux_virtual_machine/azurerm_windows_virtual_machine #8162

Open kasenna opened 4 years ago

kasenna commented 4 years ago

Community Note

Description

Hi all, Issue: It is not possible to provide tags for OS disk in azurerm_linux_virtual_machine/azurerm_windows_virtual_machine resources, os_disk block does not have such support. How i see this new feature:

resource "azurerm_linux_virtual_machine" "XXX" {
  os_disk {
    tags = {
       ...
      }
    }
  }

and

resource "azurerm_windows_virtual_machine" "XXX" {
  os_disk {
    tags = {
       ...
      }
    }
  }

OS disk tags we need for pricing reports.

Community Note

Description

New or Affected Resource(s)

Potential Terraform Configuration

# Copy-paste your Terraform configurations here - for large Terraform configs,
# please use a service like Dropbox and share a link to the ZIP file. For
# security, you can also encrypt the files using our GPG public key.

References

ArcturusZhang commented 4 years ago

Hi @kasenna thanks for this issue!

The VM API does create a managed disk resource on Azure when you are assigning this os_disk block, but the VM API does not support creating a OS Disk with tags on. You will have to assign those tags to the disk resource after the VM is created. Usually we do not manage the disk created by the VM ourselves, and we do not expose the disk id created by the VM. Could you explain your usage scenario of this feature requirement?

kasenna commented 4 years ago

Thank you for the quick response,

We're using tags to identify what project/service line the azure resource belongs to and using this information to calculate how much money we spend on each project.

And yes we definitely can assign tags to OS disk after VM being built, assign manually or with some script, but we would like to assign it with terraform so we don't have to perform extra steps, if it makes sense.

On Tue, Aug 18, 2020, 23:04 Arcturus notifications@github.com wrote:

Hi @kasenna https://github.com/kasenna thanks for this issue!

The VM API does create a managed disk resource on Azure when you are assigning this os_disk block, but the VM API does not support creating a OS Disk with tags on. You will have to assign those tags to the disk resource after the VM is created. Usually we do not manage the disk created by the VM ourselves, and we do not expose the disk id created by the VM. Could you explain your usage scenario of this feature requirement?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/terraform-providers/terraform-provider-azurerm/issues/8162#issuecomment-675838882, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKSFDC5YM336O64IMKTCFLSBNFLTANCNFSM4QD35N5A .

jpbuecken commented 4 years ago

Since the duplicate has been closed, kindly see my analysis and possible workaround https://github.com/terraform-providers/terraform-provider-azurerm/issues/2568#issuecomment-559754110 but it needs https://github.com/terraform-providers/terraform-provider-azurerm/issues/8195

tombuildsstuff commented 4 years ago

@jpbuecken thanks for the suggestions in that issue, but that's a different ask to this issue - which is for the VM resource to manage the tags for the OS Disk directly (#8195 is tracking support for an existing OS Disk)

kasenna commented 4 years ago

@jpbuecken thanks for the suggestions in that issue, but that's a different ask to this issue - which is for the VM resource to manage the tags for the OS Disk directly (#8195 is tracking support for an existing OS Disk)

thank you for the answer, since VM API can not set OS disk tags directly you can set it indirectly, via Managed Disk API, especially if you don't want to expose OS disk ID, and OS disk ID is known to terraform right, coz Azure will respond with this ID to VM create API call, or I may be wrong saying that Azure treats OS disk as a managed disk and in this case you just can't set these tags for sure

tombuildsstuff commented 4 years ago

@kasenna indeed, we've got several use-cases within the new VM resources where we call out to the Managed Disks API - so this is the approach we'd take here.

At this point the larger question is how folks would expect this field to behave, presumably this wants to be Optional & Computed - meaning that if you don't specify the field, we'll use whatever values exist (e.g. don't update them) - but if you do we'll apply those tags; which allows folks who are interested in managing tags within this resource to manage them here or using the separate resource - but not both. However it's worth having a few use-cases here to be sure this is the righr behaviour here, since these are popular resources :)

kasenna commented 4 years ago

thank you,

this means you are going to implement that feature?

anyway I appreciate your help

On Mon, Aug 31, 2020, 08:07 Tom Harvey notifications@github.com wrote:

@kasenna https://github.com/kasenna indeed, we've got several use-cases within the new VM resources where we call out to the Managed Disks API - so this is the approach we'd take here.

At this point the larger question is how folks would expect this field to behave, presumably this wants to be Optional & Computed - meaning that if you don't specify the field, we'll use whatever values exist (e.g. don't update them) - but if you do we'll apply those tags; which allows folks who are interested in managing tags within this resource to manage them here or using the separate resource - but not both. However it's worth having a few use-cases here to be sure this is the righr behaviour here, since these are popular resources :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/terraform-providers/terraform-provider-azurerm/issues/8162#issuecomment-683765640, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABKSFDGTH2JLOLJBA5YBQPTSDON77ANCNFSM4QD35N5A .

jpbuecken commented 4 years ago

Another approach would be a root_volume_tags resource, but you need to get the id anyway. So I like the solution as @kasenna describes, use disk api calls in instance resource

NilsBusche commented 3 years ago

@tombuildsstuff Are there already plans to implement this feature?

We are struggling with this issue, because sometimes we need to change cost-related tags on VM and all related resources. After applying the changes via Terraform you have do manually change the tags at all of the root volumes because of this limitation, which causes additional efforts.

Dilergore commented 2 years ago

Hi @tombuildsstuff,

Are there any plans to implement the "workaround" mentioned above by using Managed Disk API calls? This is really a required feature to be able to tag the disks properly.

Klaas- commented 1 year ago

I fell over this when I was trying to modify tags of the OS Disk, I was expecting them to update with the VM Tags (because that is where the OS Disk tags originate at creation). But they are not updated.

As a workaround: I wrote a simple graph query to check where disk tags differ from VM tags: Resources | where type =~ "Microsoft.Compute/disks" | where isnotempty(managedBy) | project managedBy=tolower(managedBy) , dstags=dynamic_to_json(tags) , dsid=id | join  kind=leftouter( Resources | where type =~ "Microsoft.Compute/virtualmachines" | project id=tolower(id) , vmtags=dynamic_to_json(tags) ) on $left.managedBy == $right.id | where tostring(dstags) != tostring(vmtags) Then I can just iterate over the VM Ids and get tags and set them on the corresponding disks. But I'd much rather see terraform do this :)

ChrisTav424 commented 1 year ago

I've just been caught out by this too. Updated some tags on the azurerm_windows_virtual_machine and terraform made changes to the tags on the VM but not the OS Disk, leaving out of date tags on the OS Disk.

I will write some PowerShell or use the above to work around it but is there a plan to fix this?

ChrisTav424 commented 1 year ago

Wrote the following to update all OSDisks with tags, posting it here in case it helps anyone else

Connect-AzAccount

#Get all subscriptions
$Subscriptions = Get-AzSubscription

# Loop through each subscription
ForEach ($Subcription in $Subscriptions)
{
Select-AzSubscription -Subscription $Sub.id

$VMs = Get-AZVM
ForEach ($Vm in $VMs)
    {
    Write-Host $Vm.Name

    #Get the existing tags from the VM
    $Tags = (Get-AzTag -ResourceId $Vm.Id).Properties.TagsProperty
    #Get the OSDisk associated to the VM
    $OSDiskId = (Get-AzDisk -DiskName $Vm.StorageProfile.OsDisk.Name).Id

    #Apply the tags
    Update-AzTag -ResourceId $OSDiskId -Tag $Tags -Operation Replace #Add -WhatIf here for testing

    }

}

Any idea when this will be fixed?

nrjohnstone commented 1 year ago

If the tags on the os disk are going to be created by using the tags on the VM, then it is very unexpected behavior that updating the tags on the VM does NOT update the tags on the OS disk... Just got caught by this when updating tags for budgets and owners of existing VMs :-(

Klaas- commented 1 year ago

Can you leverage the new import feature of Terraform 1.5 for a real solution here https://developer.hashicorp.com/terraform/language/import // https://github.com/hashicorp/terraform/pull/33153 ?

Klaas- commented 1 year ago

Can you leverage the new import feature of Terraform 1.5 for a real solution here https://developer.hashicorp.com/terraform/language/import // hashicorp/terraform#33153 ?

This sadly does not work, the new import feature only works for resources that already exist prior to running your terraform, not for resources that get created during your terraform apply.

Klaas- commented 1 year ago

I opened a case with Microsoft and they came up with a workaround by directly using the api via azapi provider

data "azurerm_managed_disk" "test" {
  name = azurerm_linux_virtual_machine.virtual_machine.os_disk[0].name
  resource_group_name = local.resource_group_name
}

resource "azapi_resource_action" "test" {
  type        = "Microsoft.Compute/disks@2022-03-02"
  resource_id = data.azurerm_managed_disk.test.id
  method      = "PATCH"

  body = jsonencode({
    tags = local.tags
  })
}

And if you also want to catch a configuration drift:

data "azurerm_managed_disk" "validate" {
  name = azurerm_linux_virtual_machine.virtual_machine.os_disk[0].name
  resource_group_name = local.resource_group_name
  depends_on = [ azapi_resource_action.test ]
}

output "hasTagDrift" {
  value = data.azurerm_managed_disk.validate.tags != tomap(local.tags)
}
sshipway commented 1 year ago

This is an issue for us, as well. I had naively assumed that the storage_os_disk and storage_data_disk(s) would inherit the same tags the azurerm_virtual_machine was defined with, but no such luck. We need to have these tags set for cost tracking reports, and the azapi workaround above is clever but feels like a kluge that will break in the future. Please add support for tags on the disk definitions, and default them to be those supplied for the parent VM

Klaas- commented 1 year ago

@sshipway if you are a hashicorp/azure customer you can try to open up a support case to show them there is a customer demand for this, apart from this I do not see Azure changing their API to accommodate terraform -- so best scenario I see at the moment is the possibility to allow providers to import resources that get implicitly created.

ajdann commented 1 year ago

I opened a case with Microsoft and they came up with a workaround by directly using the api via azapi provider

data "azurerm_managed_disk" "test" {
  name = azurerm_linux_virtual_machine.virtual_machine.os_disk[0].name
  resource_group_name = local.resource_group_name
}

resource "azapi_resource_action" "test" {
  type        = "Microsoft.Compute/disks@2022-03-02"
  resource_id = data.azurerm_managed_disk.test.id
  method      = "PATCH"

  body = jsonencode({
    tags = local.tags
  })
}

And if you also want to catch a configuration drift:

data "azurerm_managed_disk" "validate" {
  name = azurerm_linux_virtual_machine.virtual_machine.os_disk[0].name
  resource_group_name = local.resource_group_name
  depends_on = [ azapi_resource_action.test ]
}

output "hasTagDrift" {
  value = data.azurerm_managed_disk.validate.tags != tomap(local.tags)
}

Thank you, this has worked perfecntly fine for me !

KrazeyKami commented 1 year ago

Just confirmed; When creating the VM from scratch and setting tags, they get nicely propagated to the OS disk. Thanks!

LairdWilliams commented 7 months ago

I opened a case with Microsoft and they came up with a workaround by directly using the api via azapi provider


data "azurerm_managed_disk" "test" {
  name = azurerm_linux_virtual_machine.virtual_machine.os_disk[0].name
  resource_group_name = local.resource_group_name
}

resource "azapi_resource_action" "test" {
...

Thanks! This is nice to have - but as a work-around it only suffices in cases where you are not managing the VM resource in the same module in which this code appears, or in cases where you are making incremental updates and don't care about being able to recreate your environment end-to-end "from scratch". This is because the data block involved will usually fail at plan time if the VM does not already exist.

As such, it would be far better if the provider were to either propagate the VM tags to the os_disk on update in addition to on create (which it does already), or to give us a mechanism for managing the os_disk tags ourselves, or both - where the VM tags propagate consistently unless overridden by explicit os_disk tags.

fcobo95 commented 4 months ago

I am facing the same exact issue. Is there any ETA or TF target version for this feature request/fix? I am working on maintaining an environment and be able to propagate tags across all resources and I am not able to update the os_disk as part of these updates. I have to manually go an update the os disks each time.