-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Normalizes availability_set_id to lowercase to avoid spurious diffs #6768
Normalizes availability_set_id to lowercase to avoid spurious diffs #6768
Conversation
Hi @Bowbaq Thanks for the PR here - it's awesome to see someone else try to fix this. I have been thinking about this a lot and I am not quite sure it is as easy as this. The Azure API is actually case-sensitive i.e. parsing a resource ID for VirtualMachines will return no result but virtualMachines will (notice the lowercase v here) Therefore, we need to run some tests on this to make sure that the virtual machine is actually being added to the correct availability_set when using lowercase Does this make sense? Have you tried the tests? Paul |
Seems to work fine in this case |
This is good for me :) We can merge it and then let it bake in the nightly tests :) Thanks so much for the work here |
Hi @stack72 & @Bowbaq - i think this change may have introduced a bug. I am running v 0.7.0 in Azure (Resource Manager) with 2 VMs in an availability set - and now on every run my location is seen as different (as appears case sensitive) - this results in the forced creation of new resources within the availability group - resulting in the deletion and re-creation of all vm's in the availability set. It doesn't seem to matter what case I specify the location as - on subsequent runs it always returns as a change and wants to recreate the resource group and the VM's within the group. Results from a terraform plan below:- -/+ module.availability_set_test.azurerm_availability_set.availability_set Also - when i look at the Travis CI build results, unless I am reading it wrong it looks like there are no tests for Azure RM:- ok github.com/hashicorp/terraform/builtin/bins/provider-azure 0.006s so maybe we need to look at adding in some automated tests as well? Thanks. |
Morning @dougt1 So this PR only touched Also, FYI, the make test is a unit tests thing - we have a full suite of automated tests for AzureRM :) https://travis-ci.org/hashicorp/terraform/builds/152307805 We run these each night I will try and dig into the location thing - can you open a new issue to mark the work on that? P. |
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. |
Azure RM seems to return uppercased availability set ids from the API, which causes spurious diffs if the case used in the definition is anything else than uppercase. This normalizes the id to lowercase.