Remove default from vmss "priority" property

[rrudduck]

We have a large number of virtual machine scale sets that were created before priority was an option on them. In those cases the priority property is not returned from ARM and it has no value. By setting a default on this property that "" gets changed to "Regular" and all existing scalesets are marked to be recreated. It appears in my testing that simply removing this default allows existing scalesets to function without this property and adding a value for it in the manifests generate the appropriate diffs.

[katbyte]

Hi @rrudduck,

Thank you for your PR to address this issue. While this might fix your use case, it would now cause anyone relying on the new default to have a diff to "". I think the correct way to solve this would be to add a CustomizeDiff that sets a new value overriding the default when diff'd against to "".

[rrudduck]

IMO, compatibility was broken when the change to add the default was made in May. Fixing the diff caused by this is simple as the Azure API returns a value if was previously set - thus all you need to do is add the property to your tf files. The other way - having a default for a field the Azure API makes optional - is more problematic.

I'm fine any way it's resolved. Right now we are having to fork this provider to keep it from incorrectly re-creating vmss instances.

[katbyte]

I agree that adding the default with no provision to handle the older scale sets broke backwards compatibility. However currently when creating a new VMSS in the portal priority Regular is assigned by default. Thus I think having the optional parameter default to Regular is the correct behaviour.

[rrudduck]

OK. I'm not familiar with the CustomizeDiff func. Looking at the docs it looks like it allows me to control the diff result but does it allow me to change the value that's ultimately passed to the API? Passing the value at all to the API when it's not originally set results in this error from Azure:

{
  "error": {
    "code": "BadRequest",
    "message": "The specified priority value 'Regular' cannot be applied to the VirtualMachineScaleSet '<vmss-name>' since no priority was specified while originally creating the VirtualMachineScaleSet."
  }
}

So even if I suppress the diff, any other change to the scale set would result in the default "Regular" being sent to the API correct? Or am I missing something here?

[tombuildsstuff]

hey @rrudduck

Apologies for the delayed response here - I've chatted with @katbyte about this and we believe this is probably the right approach - we just need to test the upgrade path here from both an older version of the Provider -> this fix and from the current master -> this fix - both of which I've got this on my list to do today.

Thanks!

[tombuildsstuff]

hey @rrudduck

Thanks for this PR - apologies for the delay reviewing this. I've done some edge-case testing for this and this behaves as expected. I'll kick off the Acceptance Tests not but this otherwise LGTM 👍

Thanks!

[metacpp]

LGTM.

[tombuildsstuff]

Tests pass:

[ghost]

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 👉 hashibot-feedback@hashicorp.com. Thanks!