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

New Resources: azurerm_linux_virtual_machine & azurerm_windows_virtual_machine #5550

Merged
merged 19 commits into from
Feb 3, 2020

Conversation

tombuildsstuff
Copy link
Contributor

This Pull Request introduces two new resources previously outlined in #2807:

  • azurerm_linux_virtual_machine
  • azurerm_windows_virtual_machine

As detailed in #2807 - the azurerm_virtual_machine resource was first added to Terraform in 2006 when Azure Resource Manager was in Beta. Over time both the behaviour and capabilities of the Virtual Machine API have diverged sufficiently that whilst the existing resource captures most of the use-cases - it's fallen out of sync.

Whilst it's potentially possible to fix some of these issues with the azurerm_virtual_machine resource - in practice some of these are larger design issues where the Azure API is now sufficiently generic it's hard to provide useful validation.

As such this PR introduces two resources (one for Linux/Windows) for managing Virtual Machines allowing us to provide more granular validation and the new features.

At this time there's a couple of known issues/failing tests which we'll fix prior to GA:

  1. The public_ip_address and public_ip_addresses fields aren't correctly populated when using a Dynamic Public IP
  2. The "image" acceptance tests need updating - on Linux the Public IP needs fixing; on Windows we need to switch to using WinRM

There's Acceptance Tests covering both of these issues, but these are acceptable limitations for the Beta.

Examples for both the new Virtual Machine & VM Scale Set resources will be added in a subsequent PR since this won't go live until the Beta / is out of scope for this PR.


Acceptance Tests for the VM resources:

Screenshot 2020-01-29 at 12 51 15

Acceptance Tests for the VM Scale set resources:

Screenshot 2020-01-29 at 12 51 34

(these are also expected/will be fixed by other open PR's)


Depends on #5542

Fixes #148
Fixes #184
Fixes #486
Fixes #603
Fixes #956
Fixes #1013
Fixes #1170
Fixes #1279
Fixes #1433
Fixes #1670
Fixes #2298
Fixes #3592
Fixes #4750
Fixes #4749
Fixes #5068
Fixes #5447
Fixes #5482

@tombuildsstuff tombuildsstuff added this to the v2.0.0 milestone Jan 29, 2020
@tombuildsstuff tombuildsstuff requested a review from a team January 29, 2020 11:52
@tombuildsstuff tombuildsstuff self-assigned this Jan 29, 2020
@ghost ghost added the size/XXL label Jan 29, 2020
@ghost ghost added the documentation label Jan 30, 2020
@tombuildsstuff tombuildsstuff marked this pull request as ready for review January 30, 2020 11:07
Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

A few minor comments that should possibly be addressed (nothing blocking / critical IMO). Otherwise LGTM 👍

@@ -53,7 +53,7 @@ func validateName(maxLength int) func(i interface{}, k string) (warnings []strin

// The value must be between 1 and 64 (Linux) or 16 (Windows) characters long.
Copy link
Member

Choose a reason for hiding this comment

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

Not part of the PR, but worth noting that Linux max hostname length is 63, not 64.

azurerm/internal/services/compute/virtual_machine.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Aside from a few minor comments LGTM 👍

@tombuildsstuff
Copy link
Contributor Author

Ignoring some expected (known / minor) test failures which are documented as known-issues for the Beta - the tests pass:

Screenshot 2020-02-03 at 18 11 14

@tombuildsstuff tombuildsstuff merged commit f201109 into master Feb 3, 2020
@tombuildsstuff tombuildsstuff deleted the f/virtual-machine branch February 3, 2020 17:37
@ghost
Copy link

ghost commented Feb 24, 2020

This has been released in version 2.0.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.0.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Mar 5, 2020

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 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.