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

Support for windows_virtual_machine license_type change without force replacement #7192

Closed
AndreasMWalter opened this issue Jun 3, 2020 · 11 comments · Fixed by #8542
Closed

Comments

@AndreasMWalter
Copy link

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

Remove the need to redeploy the VM when changing OS licensing on Windows Virtual Machine.

Changing the license type of a Windows virtual machine can be done online using the portal (or Powershell and CLI for that matter) without replacement of the resource, Terraform however will redeploy the machine rather than update in place.

New or Affected Resource(s)

  • azurerm_windows_virtual_machine
    (possible that other services offer the same hybrid activation, but force replacement?)

Potential Terraform Configuration

Unchanged

References

https://docs.microsoft.com/de-de/azure/virtual-machines/windows/hybrid-use-benefit-licensing
https://www.terraform.io/docs/providers/azurerm/r/windows_virtual_machine.html#license_type

@ArcturusZhang
Copy link
Contributor

Hi @AndreasMWalter

Thanks for opening this issue!

By playing with azure CLI, I found this upgrade of lisenceType is one-way, you can update from None to WindowsServer or WindowsClient, but you cannot downgrade from WindowsServer or WindowsClient back to None (the service does not return error but silently ignores your request of upgrade)

As a valid workaround, terraform previously using ForceNew to make the downgrade possible, but still, it would cause issues (like this one). This might be resolved by using CustomizeDiff, but I would make no promising here.

@AndreasMWalter
Copy link
Author

Not sure about the Azure CLI but in general I think there shouldn’t be an issue with changing back and forward as often as one would like. I am currently on vacation but might give it a short test next week. Otherwise might open a case with Microsoft as well since it is either an issue in their documentation or an issue with the Azure CLI/ API

@AndreasMWalter
Copy link
Author

Hi @ArcturusZhang so I checked using the Portal, started out with a machine without license, so the machine started out with "licenseType": null or not set in the resource explorer.

I then added a license and "licenseType" was then set to "Windows_Server" and the API confirmed the change with 200 OK

Then again I changed back to none, now obviously the licensetype got changed to "None" rather than not set. But the general function is working and the API also confirmed this with 200 OK.

I haven't yet checked this with the CLI since I didn't have the time yet.

@ArcturusZhang
Copy link
Contributor

Hi @ArcturusZhang so I checked using the Portal, started out with a machine without license, so the machine started out with "licenseType": null or not set in the resource explorer.

I then added a license and "licenseType" was then set to "Windows_Server" and the API confirmed the change with 200 OK

Then again I changed back to none, now obviously the licensetype got changed to "None" rather than not set. But the general function is working and the API also confirmed this with 200 OK.

I haven't yet checked this with the CLI since I didn't have the time yet.

Actually in my test, CLI would work the same, it just returned no error but the licenseType is not actually changed. Could you please confirm whether the licenseType could change from Windows_Server to None?

@AndreasMWalter
Copy link
Author

Well yes indeed I can confirm this I checked it in the resource explorer after each change, when I did it:
This is the one I changed from Null -> Windows_Server -> None
image

This is an untouched one:
image

@ArcturusZhang
Copy link
Contributor

ArcturusZhang commented Jun 17, 2020

Thanks for the update! @AndreasMWalter . I will be working on this very soon

@eldridgeb
Copy link

@ArcturusZhang any updates on this issue?

@NilsBusche
Copy link
Contributor

NilsBusche commented Sep 18, 2020

Hi together,

because we need this functionality immediately I have worked on implementing an update functionality. Basically this would be following changes:

index 9814dbf..445a3a1 100644
--- a/azurerm/internal/services/compute/windows_virtual_machine_resource.go
+++ b/azurerm/internal/services/compute/windows_virtual_machine_resource.go
@@ -175,7 +175,6 @@ func resourceWindowsVirtualMachine() *schema.Resource {
                        "license_type": {
                                Type:     schema.TypeString,
                                Optional: true,
-                               ForceNew: true,
                                ValidateFunc: validation.StringInSlice([]string{
                                        "None",
                                        "Windows_Client",
@@ -735,6 +734,13 @@ func resourceWindowsVirtualMachineUpdate(d *schema.ResourceData, meta interface{
                VirtualMachineProperties: &compute.VirtualMachineProperties{},
        }

+       if d.HasChange("license_type") {
+               shouldUpdate = true
+
+               license := d.Get("license_type").(string)
+               update.VirtualMachineProperties.LicenseType = &license
+       }
+
        if d.HasChange("boot_diagnostics") {
                shouldUpdate = true

Updating from null (license_type not set) to any other value works well. Updating from Windows_Server/Windows_Client back to None also works. But if you remove license_type completeley updating from Windows_Server/Windows_Client to null ends in a successful apply, but changes nothing in the live infrastructure and will show you this change at every plan/apply again.

I think there are two options to handle this issue:

  1. Accept this behavior, so the executor has to make sure, that he sets None explicitly if he wants to turn back from Windows_Server/Windows_Client
  2. Add something like the following in my changed section (pseudocode, not tested):
if license == "" {
   license = "None"
}

@ArcturusZhang: Do you have already looked into this problem? Then a short answer here would be great.
Otherwise I will provide a pull request with my preferred solution because we really need this functionality.

@NilsBusche
Copy link
Contributor

NilsBusche commented Sep 18, 2020

Add something like the following in my changed section (pseudocode, not tested):

Tested, this would work:

        if d.HasChange("license_type") {
                shouldUpdate = true

                license := d.Get("license_type").(string)
                if license == "" {
                        license = "None"
                }
                update.VirtualMachineProperties.LicenseType = &license
        }

@ghost
Copy link

ghost commented Sep 24, 2020

This has been released in version 2.29.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.29.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Oct 21, 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 as resolved and limited conversation to collaborators Oct 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants