Add virtual machines data sources

[inkel]

This pull request adds two new Virtual Machine data sources:

• azurerm_virtual_machine to retrieve information about a single Virtual Machine.
• azurerm_virtual_machines to retrieve information about many Virtual Machines.

Usage

azurerm_virtual_machine

data "azurerm_virtual_machine" "test" {
  name                = "production"
  resource_group_name = "networking"
}

output "virtual_machine_id" {
  value = "${data.azurerm_virtual_machine.test.id}"
}

azurerm_virtual_machines

data "azurerm_virtual_machines" "test" {
  name_prefix         = "prod-"
  resource_group_name = "networking"
}

output "virtual_machine_ids" {
  value = "${join(", ", data.azurerm_virtual_machine.test.ids)}"
}

output "virtual_machine_names" {
  value = "${join(", ", data.azurerm_virtual_machine.test.names)}"
}

TODO

• add tests for azurerm_virtual_machines
• add additional parameters for azurerm_virtual_machine (e.g. location)
• add additional parameters for azurerm_virtual_machines (e.g. location)

[tombuildsstuff]

hey @inkel

Thanks for this PR :)

Out of interest what's the use-case you're looking to solve with this Data Source? We've previously discussed adding a Data Source for a Virtual Machine but ultimately found that most of the information you'd want to obtain from it can be found via another Data Source (e.g. the IP Addresses of the VM can be obtained using the azurerm_network_interface Data Source instead)

One thing of particular note here is that the azurerm_virtual_machine resource will be being replaced in 2.0 of the AzureRM Provider in favour of two split-out resources (one for a Linux VM / one for a Windows VM) which are going to have different schemas than the VM resource - as such I think it could make sense to to hold off and add support for this for the newer resources instead?

Thanks!

[inkel]

Hi @tombuildsstuff! Thanks for taking the time for this. I wasn't aware of the newest resources you will be adding, but for the current state of the provider, my use case is the following:

I'm working as a contractor for a company that already has infrastructure created outside of Terraform. My task was to add some monitoring metrics for that infrastructure, and we've chosen to use Terraform to simplify the process; migrating all of the infrastructure to Terraform is in the roadmap but further in the future, this task is the selling point for IaaC.

Given we are going to add the same metrics to each of our different virtual machines, we created a Terraform module, let's name it vm_alerts. This module has two configuration variables, resource_group_name and vm_id.

This is where these two data sources I'm proposing come at hand: without them, we have to manually pass the VM IDs to the module, and as you might recall, Azure IDs aren't just a simply ID but almost an URI, so we ended up with a main.tf like this (not the actual code, but enough to get the idea):

variable "vm_server_id" {
  default = "/subscriptions/${var.subscription_id}/resourceGroups/${var.resource_group_name}/providers/Microsoft.Compute/virtualMachines/${var.vm_server_name}"
}

In the case of only one VM, this is somewhat easy to maintain, though still error-prone to typos. But when we have a cluster of machines, all named using a fixed prefix and a variable numeric suffix, things got more complicated, as we had to declare one variable for each resource.

So far we managed to get this going by manually having to terraform import all the machines in the cluster and do so hacks within terraform with count and format. Also, as I've told you before, given that these VMs are created outside of Terraform, if they ever change we need to update our Terraform files with unnecessary changes only to maintain state.

Now, if we were to have this data sources in our tooling, we could make things much resilient and easy, as we could have something like:

# main.tf
data "azurerm_virtual_machine" "vm_server" {
  resource_group_name = "${var.resource_group_name}"
  name = "vm_server"
}

module "vm_server_alerts" {
  source = "modules/vm_server_alerts"
  resource_group_name = "${var.resource_group_name}"
  vm_id = "${data.azurerm_virtual_machine.vm_server.id}"
}

data "azurerm_virtual_machines" "vm_cluster" {
  resource_group_name = "${var.resource_group_name}"
  name_prefix = "vm_cluster_server_"
}

module "vm_cluster_alerts" {
  source = "modules/vm_cluster_alerts"
  resource_group_name = "${var.resource_group_name}"
  vm_ids = "${data.azurerm_virtual_machines.vm_cluster.ids}"
  vm_names = "${data.azurerm_virtual_machines.vm_cluster.names}"
}

I hope this is clear to understand. Feel free to ask me any further questions!

[tombuildsstuff]

hey @inkel

Taking a little time to think this through some more - I could definitely see this being useful, but only for the ID of the Virtual Machine (and not the other fields).

Based on the scenario you've described above - I believe it should be possible to achieve this either using one data source either for a single machine:

data "azurerm_virtual_machine" "example" {
  name                = "vm-name-1"
  resource_group_name = "example-resources"
}

output "virtual_machine_id" {
  value = "${data.azurerm_virtual_machine.example.id}"
}

module "monitoring" {
  source              = "./modules/monitoring"
  virtual_machine_ids = [ "${data.azurerm_virtual_machine.example.id}" ]
}

or for multiple virtual machines:

data "azurerm_virtual_machine" "example" {
  count               = 5
  name                = "vm-name-${count.index}"
  resource_group_name = "example-resources"
}

output "virtual_machine_id" {
  value = "${data.azurerm_virtual_machine.example.id}"
}

module "monitoring" {
  source              = "./modules/monitoring"
  virtual_machine_ids = [ "${data.azurerm_virtual_machine.example.*.id}" ]
}

This'd allow the Virtual Machine ID to be used with the azurerm_virtual_machine_data_disk_attachment resource - presumably we'd also want to update the azurerm_virtual_machine_extension resource to take the Virtual Machine ID in addition to/instead of the Virtual Machine Name (this has previously been requested in another issue - but I can't immediately find it at the moment)

As such I believe by using Count with the Data Source that the azurerm_virtual_machines Data Source wouldn't be necessary - what do you think?

Thanks!

[inkel]

As such I believe by using Count with the Data Source that the azurerm_virtual_machines Data Source wouldn't be necessary - what do you think?

I like this idea! Let me try it locally and see how does thing look, but I'm positive it will work. Also, it will remove the need of adding an additional data resource (and I'll get to avoid writing more tests 😛)

I'm more than ok with only returning the VM ID.

This'd allow the Virtual Machine ID to be used with the azurerm_virtual_machine_data_disk_attachment resource - presumably we'd also want to update the azurerm_virtual_machine_extension resource to take the Virtual Machine ID in addition to/instead of the Virtual Machine Name (this has previously been requested in another issue - but I can't immediately find it at the moment)

I'm not sure I'm following you on this comment, care to explain it again to me? Thanks!

[inkel]

Yup, the data plus count attribute will work more than ok. What would be the next steps, then?

• make azurerm_virtual_machine only return id
• remove azurerm_virtual_machines
• profit? 😬

[tombuildsstuff]

@inkel

This'd allow the Virtual Machine ID to be used with the azurerm_virtual_machine_data_disk_attachment resource - presumably we'd also want to update the azurerm_virtual_machine_extension resource to take the Virtual Machine ID in addition to/instead of the Virtual Machine Name (this has previously been requested in another issue - but I can't immediately find it at the moment)
I'm not sure I'm following you on this comment, care to explain it again to me? Thanks!

There's been a feature request recently to enable the azurerm_virtual_machine_extension resource to allow specifying a Virtual Machine ID rather than the Virtual Machine Name as an argument - this was more thinking out loud that this would also be enabled from this Data Source too :)

Yup, the data plus count attribute will work more than ok. What would be the next steps, then?

Yep basically as you've mentioned above - ensure the azurerm_virtual_machine data source does a Get to ensure the VM exists and then just set the ID (and a test) - but that should be all, let me take a quick look through 👍

Thanks!

[tombuildsstuff]

hey @inkel

Thanks for this PR :)

I've taken a look through and left some minor comments inline, but this is mostly looking good - if we can fix up the comments (and remove the Virtual Machines Data Source) we should be able to run the tests and get this merged 👍

Thanks!

[tombuildsstuff]

I think it'd be better to use a Linux VM here (since they tend to boot faster)

[inkel]

I just copied the same definition from azurerm/data_source_arm_virtual_network_test.go, but sure, I can do that 👌

[inkel]

I've should've known better, but the GitHub suggestion feature, awesome as it is, only applied to one line, so when I applied your suggestion to rename the function it broke the build 😞

Anyway, that's fixed now, alongside with all your other suggestions, so this should be ready for another review. Thank you @tombuildsstuff for your time and feedback!

[tombuildsstuff]

hey @inkel

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

Thanks!

[tombuildsstuff]

Tests pass:

[inkel]

Yoohoo! Thanks! It feels so good to contribute, you are an amazing maintainer! 👏

PS: any ideas on when to expect a new release with this data source? 😬

[ghost]

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 👉 hashibot-feedback@hashicorp.com. Thanks!