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

network_profile.*.ip_configuration.*.primary now required for azure_virtual_machine_scale_set #2035

Merged
merged 2 commits into from
Oct 10, 2018
Merged

network_profile.*.ip_configuration.*.primary now required for azure_virtual_machine_scale_set #2035

merged 2 commits into from
Oct 10, 2018

Conversation

manicminer
Copy link
Contributor

The primary attribute for a VirtualMachineScaleSet IPConfiguration (https://docs.microsoft.com/en-us/rest/api/compute/virtualmachinescalesets/createorupdate#virtualmachinescalesetipconfiguration) is now apparently mandatory even with single IP configurations.

The VMs in our virtual machine scale sets only have a single interface, and each interface a single IP configuration, but we started getting this on Friday.

  • azurerm_virtual_machine_scale_set.main: compute.VirtualMachineScaleSetsClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="VMScaleSetNetworkInterfaceConfigurationMustHaveOneIpConfigAsPrimary" Message="Network Interface Configuration the-service-eth0 in VM Scale Set the-service-dev-vmss must have one designated primary IP Configuration." Details=[]

Discussions with an Azure support engineer confirmed this change. Simply adding the azurerm_virtual_machine_scale_set.main.network_profile.*.ip_configuration.*.primary attribute resolved the 400 error.

This is my first Terraform (actually Go project) contribution. Please let me know if I've messed up! Thanks!

@ghost ghost added the size/S label Oct 8, 2018
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @manicminer

Thanks for this PR - this mostly LGTM, if we can fix up a couple of minor issues then this should be good to go 👍

Thanks!

if properties.Primary != nil {
config["primary"] = *properties.Primary
}
config["primary"] = *properties.Primary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we revert this? the Azure API's return the state of the resource at the last time it was modified, meaning it's possible this won't be returned (and will crash here)

@tombuildsstuff tombuildsstuff added enhancement service/vmss Virtual Machine Scale Sets labels Oct 8, 2018
@tombuildsstuff tombuildsstuff added this to the 1.17.0 milestone Oct 8, 2018
@manicminer
Copy link
Contributor Author

manicminer commented Oct 9, 2018

@tombuildsstuff Thanks for the feedback! I've updated the docs and reverted the hunk from azurerm/resource_arm_virtual_machine_scale_set.go#1161

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @manicminer

Thanks for pushing the latest changes - this now LGTM 👍 - I'll kick off the tests shortly

Thanks!

@tombuildsstuff
Copy link
Contributor

hey @manicminer

Running the tests for this I notice there's a couple of configurations where the primary = true attribute is missing from the tests; so that we can get this merged - I've pushed a commit to fix this - I hope you don't mind :)

Thanks!

@tombuildsstuff
Copy link
Contributor

Tests pass:

screenshot 2018-10-10 at 13 22 23

@tombuildsstuff tombuildsstuff merged commit 71519fe into hashicorp:master Oct 10, 2018
tombuildsstuff added a commit that referenced this pull request Oct 10, 2018
@manicminer manicminer deleted the azure-vmss-primary-ip-configuration branch October 10, 2018 07:48
katbyte added a commit that referenced this pull request Oct 16, 2018
dghubble added a commit to poseidon/typhoon that referenced this pull request Oct 27, 2018
dghubble added a commit to poseidon/typhoon that referenced this pull request Oct 27, 2018
@ghost
Copy link

ghost commented Mar 6, 2019

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 and limited conversation to collaborators Mar 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement service/vmss Virtual Machine Scale Sets size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants