azurerm_linux_virtual_machine,azurerm_windows_virtual_machine - Support automatic_upgrade_enabled,disk_controller_type,os_image_notification,treat_failure_as_deployment_failure_enabled and vm_agent_platform_updates_enabled

[ms-zhenhua]

Swagger Link:

https://github.com/Azure/azure-rest-api-specs/tree/main/specification/compute/resource-manager/Microsoft.Compute/ComputeRP/stable/2023-03-01

Test Result

=== RUN TestAccWindowsVirtualMachine_diskOSControllerType
=== PAUSE TestAccWindowsVirtualMachine_diskOSControllerType
=== CONT TestAccWindowsVirtualMachine_diskOSControllerType
--- PASS: TestAccWindowsVirtualMachine_diskOSControllerType (683.48s)

=== RUN TestAccLinuxVirtualMachine_diskOSControllerType
=== PAUSE TestAccLinuxVirtualMachine_diskOSControllerType
=== CONT TestAccLinuxVirtualMachine_diskOSControllerType
--- PASS: TestAccLinuxVirtualMachine_diskOSControllerType (677.39s)

=== RUN TestAccLinuxVirtualMachine_otherVmAgentPlatformUpdates
=== PAUSE TestAccLinuxVirtualMachine_otherVmAgentPlatformUpdates
=== CONT TestAccLinuxVirtualMachine_otherVmAgentPlatformUpdates
--- PASS: TestAccLinuxVirtualMachine_otherVmAgentPlatformUpdates (405.47s)

=== RUN TestAccLinuxVirtualMachine_otherOsImageNotification
=== PAUSE TestAccLinuxVirtualMachine_otherOsImageNotification
=== CONT TestAccLinuxVirtualMachine_otherOsImageNotification
--- PASS: TestAccLinuxVirtualMachine_otherOsImageNotification (464.68s)

=== RUN TestAccWindowsVirtualMachine_otherVmAgentPlatformUpdates
=== PAUSE TestAccWindowsVirtualMachine_otherVmAgentPlatformUpdates
=== CONT TestAccWindowsVirtualMachine_otherVmAgentPlatformUpdates
--- PASS: TestAccWindowsVirtualMachine_otherVmAgentPlatformUpdates (413.93s)

=== RUN TestAccWindowsVirtualMachine_otherOsImageNotification
=== PAUSE TestAccWindowsVirtualMachine_otherOsImageNotification
=== CONT TestAccWindowsVirtualMachine_otherOsImageNotification
--- PASS: TestAccWindowsVirtualMachine_otherOsImageNotification (481.37s)

[catriona-m]

is there a reason why this is limited to 4.0?

[ms-zhenhua]

Hey @catriona-m, thank you for your review.
disk_controller_type is optional+computed, which may cause a diff after upgrading the provider for users in current provider version. I have added a comment to explain it.

[katbyte]

it doesn't appear to be optional and computed at the moment?

[ms-zhenhua]

The MS-DOC clarifies that This property will be set to the default disk controller type if not specified.
For example, it will return SCSI as the default value if storageProfile.disk_controller_type is not set with the following configuration:

PUT /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/acctestRG-240131131145275307/providers/Microsoft.Compute/virtualMachines/acctestVM-240131131145275307?api-version=2023-03-01 HTTP/1.1
{
  "identity": {
    "type": "None"
  },
  "location": "westeurope",
  "properties": {
    "additionalCapabilities": {},
    "applicationProfile": {
      "galleryApplications": []
    },
    "diagnosticsProfile": {
      "bootDiagnostics": {
        "enabled": false,
        "storageUri": ""
      }
    },
    "extensionsTimeBudget": "PT1H30M",
    "hardwareProfile": {
      "vmSize": "Standard_E2bds_v5"
    },
    "networkProfile": {
      "networkInterfaces": [
        {
          "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/acctestRG-240131131145275307/providers/Microsoft.Network/networkInterfaces/acctestnic-240131131145275307",
          "properties": {
            "primary": true
          }
        }
      ]
    },
    "osProfile": {
      "computerName": "acctestVM-240131131145275307",
      "adminUsername": "adminuser",
      "linuxConfiguration": {
        "disablePasswordAuthentication": true,
        "ssh": {
          "publicKeys": [
            {
              "path": "/home/adminuser/.ssh/authorized_keys",
              "keyData": "ssh-rsa <key_data>"
            }
          ]
        },
        "provisionVMAgent": true,
        "patchSettings": {
          "patchMode": "ImageDefault",
          "assessmentMode": "ImageDefault"
        },
        "enableVMAgentPlatformUpdates": false
      },
      "secrets": [],
      "allowExtensionOperations": true
    },
    "priority": "Regular",
    "storageProfile": {
      "imageReference": {
        "offer": "0001-com-ubuntu-server-jammy",
        "publisher": "Canonical",
        "sku": "22_04-lts-gen2",
        "version": "latest"
      },
      "osDisk": {
        "osType": "Linux",
        "caching": "ReadWrite",
        "writeAcceleratorEnabled": false,
        "createOption": "FromImage",
        "managedDisk": {
          "storageAccountType": "Standard_LRS"
        }
      },
      "dataDisks": []
    }
  },
  "tags": {}
}

AzureRM Response for https:
{
  "name": "acctestVM-240131131145275307",
  "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/acctestRG-240131131145275307/providers/Microsoft.Compute/virtualMachines/acctestVM-240131131145275307",
  "type": "Microsoft.Compute/virtualMachines",
  "location": "westeurope",
  "tags": {},
  "properties": {
    "hardwareProfile": {
      "vmSize": "Standard_E2bds_v5"
    },
    "provisioningState": "Creating",
    "vmId": "218d5175-6dad-4df9-9d2f-7f689f133c12",
    "storageProfile": {
      "imageReference": {
        "publisher": "Canonical",
        "offer": "0001-com-ubuntu-server-jammy",
        "sku": "22_04-lts-gen2",
        "version": "latest",
        "exactVersion": "22.04.202401250"
      },
      "osDisk": {
        "osType": "Linux",
        "createOption": "FromImage",
        "caching": "ReadWrite",
        "writeAcceleratorEnabled": false,
        "managedDisk": {
          "storageAccountType": "Standard_LRS"
        },
        "deleteOption": "Detach",
        "diskSizeGB": 30
      },
      "dataDisks": [],
      "diskControllerType": "SCSI"
    },
    "osProfile": {
      "computerName": "acctestVM-240131131145275307",
      "adminUsername": "adminuser",
      "linuxConfiguration": {
        "disablePasswordAuthentication": true,
        "ssh": {
          "publicKeys": [
            {
              "path": "/home/adminuser/.ssh/authorized_keys",
              "keyData": "ssh-rsa <key_data>"
            }
          ]
        },
        "provisionVMAgent": true,
        "patchSettings": {
          "patchMode": "ImageDefault",
          "assessmentMode": "ImageDefault"
        },
        "enableVMAgentPlatformUpdates": false
      },
      "secrets": [],
      "allowExtensionOperations": true,
      "requireGuestProvisionSignal": true
    },
    "networkProfile": {
      "networkInterfaces": [
        {
          "id": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/acctestRG-240131131145275307/providers/Microsoft.Network/networkInterfaces/acctestnic-240131131145275307",
          "properties": {
            "primary": true
          }
        }
      ]
    },
    "diagnosticsProfile": {
      "bootDiagnostics": {
        "enabled": false
      }
    },
    "priority": "Regular",
    "extensionsTimeBudget": "PT1H30M",
    "timeCreated": "2024-01-31T05:40:48.630956+00:00"
  }
}

[katbyte]

But the value is not optional computed here & if we know the default why can we not simply just configure that be default?

i still don't understand why this needs to wait till 4.0.

[ms-zhenhua]

Hi @katbyte, @tombuildsstuff,
If disk_controller_type is not set, its value in the Get-Response depends on the value of size, for example:

• if size = Standard_E2bds_v5, disk_controller_type will be returned as SCSI
• if size = Standard_E112ibds_v5, disk_controller_type will be returned as NVMe

Since disk_controller_type is not fixed for different size values, I cannot set a default value for it.

[katbyte]

you could just make it computed thou? while we would rather have a proper default in this case as we can't computed should be fine? why would making it computed cause a diff for existing users? it would just adopt the value they have picked?

[ms-zhenhua]

you could just make it computed thou? while we would rather have a proper default in this case as we can't computed should be fine? why would making it computed cause a diff for existing users? it would just adopt the value they have picked?

I cannot just define disk_controller_type as a computed property because this property supports to be overwritten by users. May I define it as optional+computed w/o a default value, which can avoid the diff for existing users?
Though I know the best practice does not recommend using optional+computed anymore.

[katbyte]

while generally yes that is correct and we shouldn't, there still are times where it has to be done because theres no "correct" way to dynamically specify a default. so in cases like this were the only way to get "the default" is from the API we have to use computed still.

[ms-zhenhua]

code updated. Thanks.

[tombuildsstuff]

Just a heads up that given the complexity of this one / the interconnectedness of the Virtual Machine resources we're going to need to take a deeper look into this one - to set expectations that's likely to be early next year at this point given the Christmas/New Year break.

[katbyte]

instead of having enabled here could we just use the presence of the block to determine if it is enabled?

[ms-zhenhua]

Make sense. Removed enabled. Thanks.

[katbyte]

it doesn't appear to be optional and computed at the moment?

[ms-zhenhua]

Hi @katbyte , thank you for your review.
I have updated the comments and code. Could you kindly take another review? 😄

[ms-zhenhua]

Hi @katbyte, I have updated the code and doc. Kindly take another review.
BTW, I got a Check for new usages of deprecated functionality error. But I did not change this file this time.

Test Results

=== RUN TestAccLinuxVirtualMachine_diskOSControllerType
=== PAUSE TestAccLinuxVirtualMachine_diskOSControllerType
=== CONT TestAccLinuxVirtualMachine_diskOSControllerType
--- PASS: TestAccLinuxVirtualMachine_diskOSControllerType (754.35s)

=== RUN TestAccWindowsVirtualMachine_diskOSControllerType
=== PAUSE TestAccWindowsVirtualMachine_diskOSControllerType
=== CONT TestAccWindowsVirtualMachine_diskOSControllerType
--- PASS: TestAccWindowsVirtualMachine_diskOSControllerType (660.84s)

[katbyte]

LGTM 🍴

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.