-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
provider/azurerm: Crash in Virtual Machines on an empty Windows Config block #13754
Conversation
@@ -354,10 +354,12 @@ func resourceArmVirtualMachine() *schema.Resource { | |||
"provision_vm_agent": { | |||
Type: schema.TypeBool, | |||
Optional: true, | |||
Default: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are changing these to have a default, we need to include a schema migration IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking the Azure docs & #9995 - the default value for the provisionVMAgent
field should be true if it's unset:
When this property is not specified in the request body, default behavior is to set it to true. This will ensure that VM Agent is installed on the VM so that extensions can be added to the VM later.
I'll update this and look into a schema migration
85b649e
to
e9b3d24
Compare
e9b3d24
to
95a8f48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Testing some edge-cases here, when using 0.9.3 and the following config:
Which generates the following plan when switching to this branch:
Which needs fixing |
Just a FYI - In cases where you are attaching an existing OS drive, the entire resp.VirtualMachineProperties.OsProfile can be nil. I ran into that issue and patched it on my branch, but haven't had time to commit the tests, etc. It's just a simple check for resp.VirtualMachineProperties.OsProfile being nil - if it is exlcude the whole block. |
(update: I've started fixing this & porting it over to the new repo) |
Hey @tombuildsstuff does this have a new home over in https://github.com/terraform-providers/terraform-provider-azurerm? |
I've migrated the fix for this crash identified in this issue over the new repository here: hashicorp/terraform-provider-azurerm#222 Rather than combine the fix for hashicorp/terraform-provider-azurerm#28 too - I've split this out into a separate PR so we can handle bumping the provider version at the same time Closing this PR in the interim.. |
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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Currently Terraform crashes with the following stack trace when specifying an empty
os_profile_windows_config
block in anazurerm_virtual_machine
resource (full details in #10744):Fixes #10744