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

azurerm_virtual_machine: make the vm_size property case insensitive #1131

Merged
merged 1 commit into from
Apr 17, 2018

Conversation

katbyte
Copy link
Collaborator

@katbyte katbyte commented Apr 17, 2018

Closes #1073.

Also switches some resource_group fields to use the shared schema function.

@katbyte katbyte added this to the Future milestone Apr 17, 2018
@katbyte katbyte force-pushed the b-vm_size_ignore_case branch 2 times, most recently from 1115b2d to 2e14ba6 Compare April 17, 2018 01:38
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.

LGTM 👍

@tombuildsstuff tombuildsstuff modified the milestones: Future, Soon Apr 17, 2018
@tombuildsstuff tombuildsstuff merged commit c54de98 into master Apr 17, 2018
@tombuildsstuff tombuildsstuff deleted the b-vm_size_ignore_case branch April 17, 2018 16:31
tombuildsstuff added a commit that referenced this pull request Apr 17, 2018
@amcguign
Copy link

amcguign commented May 8, 2018

@katbyte @tombuildsstuff @rohrerb I tried this just now in one of our deployments and the terraform plan output still appears to want to "fix" an existing VM :(

  ~ module.ltoo.azurerm_virtual_machine.vm
      vm_size:                           "Standard_D4_v2" => "Standard_D4_V2"

Versions:

$ terraform version
Terraform v0.11.7
+ provider.azurerm v1.4.0

@katbyte katbyte modified the milestones: Soon, 1.4.0 May 9, 2018
@katbyte
Copy link
Collaborator Author

katbyte commented May 9, 2018

Hi @amcguign,

I just verified this PR on my side by taking the example from the documentation and changing the vm_size's case a few different ways, none of witch resulted in a new plan.

If possible, could you please supply some terraform that triggers this bug and the steps to reproduce it in a new issue?

Thank you 🙂

@amcguign
Copy link

Hi @katbyte

#1257

Many thanks
Andy

@tombuildsstuff
Copy link
Contributor

👋

Since this PR has been merged some time ago - in order to prevent comments on older merged PR's - I'm going to lock this PR for the moment; if you're seeing issues with this functionality please feel free to open a new issue

Thanks!

@hashicorp hashicorp locked as resolved and limited conversation to collaborators May 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants