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

ability to tag OS disk in azurerm_linux_virtual_machine/azurerm_windows_virtual_machine #8162

Open
kasenna opened this issue Aug 18, 2020 · 24 comments

Comments

@kasenna
Copy link

kasenna commented Aug 18, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

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

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

New or Affected Resource(s)

  • azurerm_XXXXX

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

  • #0000
@ArcturusZhang
Copy link
Contributor

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
Copy link
Author

kasenna commented Aug 20, 2020 via email

@jpbuecken
Copy link

Since the duplicate has been closed, kindly see my analysis and possible workaround
#2568 (comment)
but it needs
#8195

@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented Aug 28, 2020

@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
Copy link
Author

kasenna commented Aug 29, 2020

@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
Copy link
Contributor

@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
Copy link
Author

kasenna commented Aug 31, 2020 via email

@jpbuecken
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

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-
Copy link
Contributor

Klaas- commented Dec 9, 2022

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
Copy link

ChrisTav424 commented Feb 6, 2023

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
Copy link

ChrisTav424 commented Feb 10, 2023

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
Copy link

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-
Copy link
Contributor

Klaas- commented Jun 26, 2023

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 ?

@Klaas-
Copy link
Contributor

Klaas- commented Jun 28, 2023

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-
Copy link
Contributor

Klaas- commented Jun 29, 2023

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
Copy link

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-
Copy link
Contributor

Klaas- commented Jul 11, 2023

@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
Copy link

ajdann commented Jul 13, 2023

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
Copy link

KrazeyKami commented Nov 8, 2023

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

@LairdWilliams
Copy link

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
Copy link

fcobo95 commented Jul 17, 2024

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.

@Klaas-
Copy link
Contributor

Klaas- commented Nov 28, 2024

azapi v2 had a breaking change -- https://registry.terraform.io/providers/Azure/azapi/latest/docs/guides/2.0-upgrade-guide#breaking-changes

now it looks like this:

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 = {
    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)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests