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

Azure virtual machine interfaces are not being created and attached in the order they are defined within the main.tf script #99

Closed
hashibot opened this issue Jun 13, 2017 · 13 comments · Fixed by #426

Comments

@hashibot
Copy link

This issue was originally opened by @kblackstone as hashicorp/terraform#15153. It was migrated here as part of the provider split. The original body of the issue is below.


Terraform Version

0.9.6

Affected Resource(s)

  • azurerm_network_interface
  • azurerm_virtual_machine

Terraform Configuration Files

https://www.dropbox.com/s/q4pnvy0bs2812vk/main.tf?dl=0

Expected Behavior

The interfaces should be consistently created and attached in the following order: management, untrust, trust as defined within the main.tf script.

Actual Behavior

Terraform plan is showing they will be attached in this order: management, trust, untrust. This is the actual order they get attached in on the virtual machine when launched via terraform apply, which is incorrect. The attachment order is inconsistent. On one run of terraform plan/apply it may create in the right order, on another run of terraform plan/apply it will be in the incorrect order. The first interface is ok due to primary_network_interface_id. I have screen shots of desired.

Steps to Reproduce

  1. terraform plan
  2. terraform apply
@hashibot hashibot added the bug label Jun 13, 2017
@kblackstone
Copy link

is there a rough eta on when this will be fixed? it's blowing up my customer demonstrations.

@tombuildsstuff
Copy link
Contributor

Hey @kblackstone

So that we can take a look into this would it be possible to post the Terraform config associated with this Issue? The download link appears to have stopped working?

Thanks!

@kblackstone
Copy link

kblackstone commented Jul 18, 2017

Sure Tom. https://www.dropbox.com/s/wxo2368wujb3bdn/main.tf?dl=0

This is the full main.tf and here are the specific parts.

Create the network interfaces

resource "azurerm_network_interface" "Untrust" {
count = "${var.azurerm_instances}"
name = "fw-${count.index}-untrust"
location = "${azurerm_resource_group.demo.location}"
resource_group_name = "${azurerm_resource_group.demo.name}"
enable_ip_forwarding = "true"

ip_configuration {
	name							= "fw${count.index}-ip-0"
	subnet_id						= "${azurerm_subnet.Untrust.id}"
	private_ip_address_allocation 	= "dynamic"
}

}

Create the network interfaces

resource "azurerm_network_interface" "Trust" {
count = "${var.azurerm_instances}"
name = "fw-${count.index}-trust"
location = "${azurerm_resource_group.demo.location}"
resource_group_name = "${azurerm_resource_group.demo.name}"
enable_ip_forwarding = "true"

ip_configuration {
	name							= "fw${count.index}-ip-0"
	subnet_id						= "${azurerm_subnet.Trust.id}"
	private_ip_address_allocation 	= "dynamic"
}

}

*** further down in the file ***

network_interface_ids =
[
"${element(azurerm_network_interface.Management..id, count.index)}",
"${element(azurerm_network_interface.Untrust.
.id, count.index)}",
"${element(azurerm_network_interface.Trust.*.id, count.index)}",
]

@kblackstone
Copy link

I just ran through it yesterday setting up a demo environment. 1 firewall had the interfaces in the right order, the other had them flipped. This is a non-starter for auto-deploying environments that rely on consistent interface placement. Until that gets fixed i have to tell customers to go in and manually check or potentially re-create the virtual machine via arm templates, which is exactly what they don't want to do, otherwise they would scrap terraform and template the whole thing.

@tombuildsstuff
Copy link
Contributor

Pasting the full config here just incase the original download link 404's again:

# Configure the Microsoft Azure Provider
provider "azurerm" {
	client_id 		= "${var.azurerm_client_id}"
	client_secret	= "${var.azurerm_client_secret}"
	subscription_id	= "${var.azurerm_subscription_id}"
	tenant_id		= "${var.azurerm_tenant_id}"
}

# Create a resource group
resource "azurerm_resource_group" "demo" {
	name		= "blackstone-rg1"
	location	= "east us"
}

# Create a virtual network in the resource group
resource "azurerm_virtual_network" "demo" {
	name				= "vnet-fw"
	address_space		= ["10.0.0.0/16"]
	location			= "${azurerm_resource_group.demo.location}"
	resource_group_name	= "${azurerm_resource_group.demo.name}"
}

resource "azurerm_subnet" "Mgmt" {
  name                 = "Mgmt"
  resource_group_name  = "${azurerm_resource_group.demo.name}"
  virtual_network_name = "${azurerm_virtual_network.demo.name}"
  address_prefix       = "10.0.0.0/24"
}

resource "azurerm_subnet" "Untrust" {
  name                 = "Untrust"
  resource_group_name  = "${azurerm_resource_group.demo.name}"
  virtual_network_name = "${azurerm_virtual_network.demo.name}"
  address_prefix       = "10.0.1.0/24"
}

resource "azurerm_subnet" "Trust" {
  name                 = "Trust"
  resource_group_name  = "${azurerm_resource_group.demo.name}"
  virtual_network_name = "${azurerm_virtual_network.demo.name}"
  address_prefix       = "10.0.2.0/24"
}

# Generate a random id for the storage account due to the need to be unique across azure.
# Here we are generating a random hex value of length 4 (2*2) that is prefixed with
# the static string "kbstorageaccount". For example: kbstorageaccount1n8a
resource "random_id" "storage_account" {
	prefix 		= "storageblackstone"
	byte_length = "2"
}

# Create the storage account
resource "azurerm_storage_account" "demo" {
	name				= "${lower(random_id.storage_account.hex)}"
	resource_group_name	= "${azurerm_resource_group.demo.name}"
	location			= "${azurerm_resource_group.demo.location}"
	account_type		= "Standard_LRS"
}

# Create the storage account container
resource "azurerm_storage_container" "demo" {
	name							= "vhds"
	resource_group_name				= "${azurerm_resource_group.demo.name}"
	storage_account_name			= "${azurerm_storage_account.demo.name}"
	container_access_type			= "private" 
}


# Create the public IP address
resource "azurerm_public_ip" demo {
	count							= "${var.azurerm_instances}"
	name							= "kbrg1-fw${count.index}-mgmt"
	location						= "${azurerm_resource_group.demo.location}"
	resource_group_name				= "${azurerm_resource_group.demo.name}"
	public_ip_address_allocation	= "static"
}

# Create the network interfaces
resource "azurerm_network_interface" "Management" {
	count								= "${var.azurerm_instances}"
	name								= "fw-${count.index}-mgmt"
	location							= "${azurerm_resource_group.demo.location}"
	resource_group_name 				= "${azurerm_resource_group.demo.name}"
	
	ip_configuration {
		name							= "fw${count.index}-ip-0"
		subnet_id						= "${azurerm_subnet.Mgmt.id}"
		private_ip_address_allocation 	= "dynamic"
		public_ip_address_id = "${element(azurerm_public_ip.demo.*.id, count.index)}"
	}
}

# Create the network interfaces
resource "azurerm_network_interface" "Untrust" {
	count								= "${var.azurerm_instances}"
	name								= "fw-${count.index}-untrust"
	location							= "${azurerm_resource_group.demo.location}"
	resource_group_name 				= "${azurerm_resource_group.demo.name}"
	enable_ip_forwarding				= "true"
	
	ip_configuration {
		name							= "fw${count.index}-ip-0"
		subnet_id						= "${azurerm_subnet.Untrust.id}"
		private_ip_address_allocation 	= "dynamic"
	}
}

# Create the network interfaces
resource "azurerm_network_interface" "Trust" {
	count								= "${var.azurerm_instances}"
	name								= "fw-${count.index}-trust"
	location							= "${azurerm_resource_group.demo.location}"
	resource_group_name 				= "${azurerm_resource_group.demo.name}"
	enable_ip_forwarding				= "true"
	
	ip_configuration {
		name							= "fw${count.index}-ip-0"
		subnet_id						= "${azurerm_subnet.Trust.id}"
		private_ip_address_allocation 	= "dynamic"
	}
}


# Create the availability set
resource "azurerm_availability_set" "demo" {
	name								= "as-fw"
	location							= "${azurerm_resource_group.demo.location}"
	resource_group_name					= "${azurerm_resource_group.demo.name}"
	platform_update_domain_count = "5"
  	platform_fault_domain_count  = "3"
}

# Create the virtual machine. Use the "count" variable to define how many
# to create.
resource "azurerm_virtual_machine" "demo" {
	count								= "${var.azurerm_instances}"
	name								= "fw-${count.index}"
	location							= "${azurerm_resource_group.demo.location}"
	resource_group_name					= "${azurerm_resource_group.demo.name}"
	network_interface_ids = 
	[
		"${element(azurerm_network_interface.Management.*.id, count.index)}",
		"${element(azurerm_network_interface.Untrust.*.id, count.index)}",
		"${element(azurerm_network_interface.Trust.*.id, count.index)}",
	]

	primary_network_interface_id		= "${element(azurerm_network_interface.Management.*.id, count.index)}"
	vm_size								= "Standard_D3_v2"
	availability_set_id					= "${azurerm_availability_set.demo.id}"
	
	storage_image_reference	{
		publisher 	= "paloaltonetworks"
		offer		= "vmseries1"
		sku			= "byol"
		version		= "latest"
	}

	plan {
    	name = "byol"
    	product = "vmseries1"
    	publisher = "paloaltonetworks"
  	}
	
	storage_os_disk {
		name			= "pa-vm-osdisk-${count.index}"
		vhd_uri			= "${azurerm_storage_account.demo.primary_blob_endpoint}${element(azurerm_storage_container.demo.*.name, count.index)}/fwDisk${count.index}.vhd"
		caching 		= "ReadWrite"
		create_option	= "FromImage"
	}
	
	delete_os_disk_on_termination    = true
	delete_data_disks_on_termination = true
	
	os_profile 	{
		computer_name	= "pa-vm"
		admin_username	= "${var.azurerm_vm_admin_username}"
		admin_password	= "${var.azurerm_vm_admin_password}"
	}
}

@kblackstone
Copy link

any update on this?

@kblackstone
Copy link

for what it's worth, the same behavior happens if you need to make network interfaces for multiple virtual machines (ubuntu for example). You'll see nic.0, nic.2, nic.1 in that order be created.

@rcarun
Copy link

rcarun commented Sep 27, 2017

Hi @kblackstone,
In order to prioritize this issue correctly, could you please let us know the scenario you are using this in and if this issue is currently a blocker? Any information related to the customer you can share will also be valuable. Please feel free to email me privately at achand @ microsoft dot com if need be.

Thanks,
Arun Chandrasekhar
Principal Program Manager
Azure OSS Integrations

@pzskc383
Copy link

Hi, I ran into this very issue last friday and after much time spent debugging why my network interfaces are sometimes out of order, I figured what happens:
network_interfaces_ids are defined in azurerm_virtual_machine schema as TypeSet.
Set doesn't preserve order of elements from constructor, it uses SchemaSetFunc on element to determine it's hash and then returns slice ordered by hashes. While you can supply a custom function for determining element's hash, it receives element only, not the whole list, so it won't help here.

A naiive fix would be just change network_interface_ids type to List:

diff --git a/azurerm/resource_arm_virtual_machine.go b/azurerm/resource_arm_virtual_machine.go
index 26bbd00..22a228c 100644
--- a/azurerm/resource_arm_virtual_machine.go
+++ b/azurerm/resource_arm_virtual_machine.go
@@ -479,12 +479,11 @@ func resourceArmVirtualMachine() *schema.Resource {
 			},
 
 			"network_interface_ids": {
-				Type:     schema.TypeSet,
+				Type:     schema.TypeList,
 				Required: true,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
 				},
-				Set: schema.HashString,
 			},
 
 			"primary_network_interface_id": {
@@ -1432,7 +1431,7 @@ func expandAzureRmVirtualMachineImageReference(d *schema.ResourceData) (*compute
 }
 
 func expandAzureRmVirtualMachineNetworkProfile(d *schema.ResourceData) compute.NetworkProfile {
-	nicIds := d.Get("network_interface_ids").(*schema.Set).List()
+	nicIds := d.Get("network_interface_ids").([]interface{})
 	primaryNicId := d.Get("primary_network_interface_id").(string)
 	network_interfaces := make([]compute.NetworkInterfaceReference, 0, len(nicIds))

Note that this will make supplying non-repeating values in network_interface_ids a user's responsibility. Also, your vm's current state won't be valid, and removing state item and removing/importing won't help with that.

A proper fix might be making OrderedSet schema type which does preserve order of elements passed into it, but I didn't wrote that.

Hope this helps.

@kblackstone
Copy link

My solution to this is to deploy another virtual machine (some bastion box) after the firewall. It seems to resolve the issue, or at least pass it down the line.

@tombuildsstuff
Copy link
Contributor

hey @pzskc383

Thanks for looking into this and confirming that - I'd been meaning to look into that this week anyway whilst reviewing the VM and VMSS resources - so I'll include those changes in the PR's I'll be making later this week :)

Thanks!

@rcarun rcarun added the M1 label Oct 11, 2017
@rcarun rcarun added this to the M1 milestone Oct 11, 2017
@tombuildsstuff
Copy link
Contributor

This is fixed in #426

@ghost
Copy link

ghost commented Apr 1, 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants