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 - error when trying to create a VM with multiple NICs - Network interface must have Primary property set #6514

Closed
dougt1 opened this issue May 6, 2016 · 30 comments

Comments

@dougt1
Copy link

dougt1 commented May 6, 2016

Terraform Version

v0.6.15

Affected Resource

azurerm_virtual_machine

Terraform Configuration Files

resource "azurerm_network_interface" "public" {
    name = "public"
    location = "${var.location}"
    resource_group_name = "${azurerm_resource_group.azure_base.name}"
    network_security_group_id = "${azurerm_network_security_group.test-private-ssh-access-rule.id}"
    ip_configuration {
        name = "publicinterface"
        subnet_id = "${azurerm_subnet.public.id}"
        private_ip_address_allocation = "dynamic"
        public_ip_address_id = "${azurerm_public_ip.azure_base.id}"
    }
}
resource "azurerm_network_interface" "security" {
    name = "security"
    location = "${var.location}"
    resource_group_name = "${azurerm_resource_group.azure_base.name}"
    ip_configuration {
        name = "securityinterface"
        subnet_id = "${azurerm_subnet.security.id}"
        private_ip_address_allocation = "dynamic"
    }
}

resource "azurerm_virtual_machine" "bastion_host" {
    name = "acctvm"
    location = "${var.location}"
    resource_group_name = "${azurerm_resource_group.azure_base.name}"
    network_interface_ids = [
        "${azurerm_network_interface.security.id}",
        "${azurerm_network_interface.public.id}"
    ]
    vm_size = "Standard_A3"
    storage_image_reference {
    publisher = "Canonical"
    offer = "UbuntuServer"
    sku = "14.04.2-LTS"
    version = "latest"
    }

    storage_os_disk {
        name = "myosdisk1"
        vhd_uri = "${azurerm_storage_account.azure_base_storage.primary_blob_endpoint}${azurerm_storage_container.azure_base_container.name}/myosdisk1.vhd"
        caching = "ReadWrite"
        create_option = "FromImage"
    }

    os_profile {
    computer_name = "hostname"
    admin_username = "testadmin"
    admin_password = "Password1234!"
    }

    os_profile_linux_config {
    disable_password_authentication = false
    }
}

Error
* azurerm_virtual_machine.bastion_host: autorest:DoErrorUnlessStatusCode 400 PUT https://management.azure.com/subscriptions/e214ec76-5e8d-4923-8888-52758aede286/resourceGroups/azurebase/providers/Microsoft.Compute/virtualMachines/acctvm?api-version=2015-06-15 failed with 400 Bad Request

From Charles proxy

{
    "error": {
        "details": [],
        "code": "NetworkInterfaceMustHavePrimaryPropertySet",
        "message": "Network interface security must have Primary property set."
    }
}

Expected Behavior
Create a VM with two NICs. However it would appear that the Primary property can't be set in Terraform or i can't find it in order to set it. Without it i cant see how i can create a VM with multiple NIC's.

@stack72
Copy link
Contributor

stack72 commented May 6, 2016

Hi @dougt1

Apologies for the error here. I have actually already reported this to the Azure team (Azure/azure-sdk-for-go#259) - they suggest that multiple NIC configurations are not supported so far. I will chase this up again and let you know

Paul

@Felivel
Copy link
Contributor

Felivel commented May 9, 2016

Hey @stack72, I think we can fix this. They are two primary fields, one in the Network Interface itself and one in the IP configuration inside the Interface. This message refers to the one on the Network Interface.

I am going to take a look a this later to see if I can get it to work. (Probably not for 0.6.16)

Thanks,
Felivel

@stack72
Copy link
Contributor

stack72 commented May 9, 2016

No worries - this makes sense :)

thanks for the work here on this

P.

@dougt1
Copy link
Author

dougt1 commented Jul 8, 2016

Hi - just wondering if there has been any progress made on this?

Thanks,
Doug

@s-rusev
Copy link

s-rusev commented Oct 12, 2016

Hi,

Any update on this? Thanks!

@alexmags
Copy link

Hi Azure SDK for go team closed their case in August Azure/azure-rest-api-specs#305 Can this be progressed now so we can spin up dual homed VPN devices and firewalls on Azure.

@alexmags
Copy link

+1

@codyja
Copy link

codyja commented Dec 1, 2016

Hi All, interested in this too. Certain instance levels of VMs support multiple NICs, but their docs state they must be added during time of VM creation. Doesn't seem to allow you to add a second NIC after the VM is already built. Thanks!

@wwwlicious
Copy link

I'm getting the same problem still on v.0.8. Has anyone found a workaround until this is added?

@pearcec
Copy link
Contributor

pearcec commented Jan 20, 2017

@codyja Correct. We really need the code for this. I am on v0.8.4.

@pearcec
Copy link
Contributor

pearcec commented Jan 20, 2017

Looks like I am reading this wrong. We need to set the primary during the CRP. I think what I might do is have it define the first one in the list of network_interface_ids as primary. Alternatively I could ask which was is primary. Not sure there is an existing configuration that we can copy an example of best implementation. I am open to suggestions.

@pearcec
Copy link
Contributor

pearcec commented Jan 20, 2017

Stuff I learned. You can't rely on the order of the IDs passed to network_interface_ids. If you have a public IP set on a secondary interface it will fail. We should probably check for this. I don't have a good idea other than to define the primary_network_interface_id. We should also check count of the number of IDs and make sure the primary is set.

2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe:
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe: {
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe:   "error": {
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe:     "details": [],
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe:     "code": "SecondaryNetworkInterfaceMustNotUsePublicIP",
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe:     "message": "Network interface /subscriptions/XXX/resourceGroups/XXX/providers/Microsoft.Network/networkInterfaces/hub-mgmt-eth0 is not primary but it references public IP address /subscriptions/XXX/resourceGroups/XXX/providers/Microsoft.Network/publicIPAddresses/palo-public-ip. Only primary network interface can reference a public IP address."
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe:   }
2017/01/20 14:51:40 [DEBUG] plugin: terraform.exe: }

@pearcec
Copy link
Contributor

pearcec commented Jan 20, 2017

If we go the network_profile {} TypeSet route what is the best approach towards handling the existing network_interface_ids? Keep it in for backwards compatibility ? Break older configs and upgrade code? network_profile is similar to the scale sets. Any thoughts?

@pearcec
Copy link
Contributor

pearcec commented Jan 20, 2017

Here is a patch for consideration. https://github.com/pearcec/terraform/tree/azurerm-multi-network-interface-issue-6514 It is not ready for release.

  • Can we drop network_interface_ids if so how? If not I need guidance on how to support both. (How do I make one or the other required?)
  • Tests need to be rewritten
  • Need to decide if we make primary Optional and default to False
  • Should Terraform handle potential checks such as adding Primary on an interface which doesn't have a public IP, yet one of the other interfaces does have it. (We know this will fail) Or do we leave this as an exercise for the user?
  • Likewise do we check to see IF a primary is set to tree if we have a count of >1 on the number of interfaces?

Would appreciate a hashicorp person to offer up some advice. Thanks.

@pearcec
Copy link
Contributor

pearcec commented Jan 20, 2017

@stack72 Hi Paul I thought I would draw your attention since you are with HashiCorp.

@smeeus
Copy link

smeeus commented Jan 31, 2017

@pearcec Hi Christian

I am not entirely in favour of dropping network_interface_ids. Your suggestion of making one of both required sounds more logical, and would mean less problems with backward compatibility.

I am however more in favour of something similar to your first idea, simply have the first item in the list get the primary property set, with the necessary checks on the public IP address part off course.

Another fix/idea/thought I had in mind was to be able to pass on a dict instead of an id in the list, something more like:

network_interface_ids = [ <id>, ... ]
network_interface_ids = [ { "id" => <id>, "primary" => <true|false> }, ... ]

In the first case, the primary property is simply undefined and the terraform behaviour stays as it is now (0.8.4).
In the second case, the primary property is set to the value passed along via the dictionary value, or set to 'false' if not defined.

Either way, a fix for this is desirable.

@pearcec
Copy link
Contributor

pearcec commented Jan 31, 2017

@smeeus Hi Sven

Thanks for the idea.

I tried the idea of setting primary based first one, but I couldn't guarantee the order when I tried.

I thought about your idea of adding additional structure. I am not sure how to do that. But that seems like the most straightfoward path to keep backwards compatible. I also need to figure out do I have to test for the semantics of two primaries set as true?

@pearcec
Copy link
Contributor

pearcec commented Jan 31, 2017

So I think for now I will try to go with

network_interface_ids = [ <id>, ... ]
network_interface_ids = [ { "id" => <id>, "primary" => <true|false> }, ... ]

We can solicit further patches which check for validity of attaching a public interface to the primary and not sending in two primaries. The the AzureRM API will error out and give a sufficient error response.

@pearcec
Copy link
Contributor

pearcec commented Jan 31, 2017

I keep wondering is this possible to code? How do I tell the Schema to look for a TypeList or a TypeSet? Any working examples in the code I can read? @stack72 Can you offer guidance here?

@j00p34
Copy link

j00p34 commented Feb 15, 2017

@pearcec Doesn't this pull request: #11290 fix the problem? If so, maybe we should work towards getting that commited

@pearcec
Copy link
Contributor

pearcec commented Feb 16, 2017

@j00p34 A quick review of the code looks like a good fix. I like the solution of simply specifying the id (Why didn't I think of that?). Looks like we need some tests and some documentation. I am going to fetch the patch and run it myself to help approve.

@pearcec
Copy link
Contributor

pearcec commented Feb 17, 2017

@j00p34 It works. I submitted a test and some documentation. Hopefully we can get this into v0.9.0 or v0.9.1.

@kblackstone
Copy link

Can you create some type of device_id argument to set nic order? having undetermined order makes things a bit challenging during a deployment when you're expecting nics to arrive in one order and they arrive in another.

@pearcec
Copy link
Contributor

pearcec commented Jun 6, 2017

@kblackstone What has your experience been? Is terraform not ordering it based on order in networkProfile? Do you know if Azure guarantees order when you make a rest call? Can you open this as a new issue?

@kblackstone
Copy link

@pearcec other than the interface marked with primary_network_interface_id, the rest come in inconsistent order. I'm launching our firewall product (palo alto networks firewall) with 3 interfaces (sometimes we also use 4) to build out a customer demo environment and sometimes interface 2 and 3 are attaching to the vm inverted.

in both the azurerm_network_interface order and the azurerm_virtual_machine > network_interface_ids section i have them in a specific order that is not being honored. if i do the same type of action using either an azure template or powershell scripting, the order matches the order in the script itself.

i'll run the script again until the issue happens and then open a new issue with the evidence.

side note: i'm educating our customers on the importance of automation when implementing the immutable infrastructure concepts and would like to pitch terraform as a viable option, but until this is resolved i can't do so without mentioning this caveat. to be fair i will also re-run this to be sure the error is not on my side.

@pearcec
Copy link
Contributor

pearcec commented Jun 7, 2017

@kblackstone You can open an issue now. I have experienced it as well. I ignored the issue since we are using DHCP. Are you doing post automated post configuration work? At a minimum we plan to do rebuilds and use panorama to grab the existing configuration. having a different ordered interface would pose a problem.

@kblackstone
Copy link

@pearcec I opened it about 15 minutes ago, #15133. I'm not doing automated post configure work in my demonstration builds for customers, but I expect/hope they will plan to. This issue would break that process exactly as you mention. Using DHCP is fine but then you try to push panorama policy it wouldn't match and would fail if the interfaces are out of expected order.

@pearcec
Copy link
Contributor

pearcec commented Jun 7, 2017

@kblackstone please take another look at the issue you referenced. it doesn't look correct.

@kblackstone
Copy link

kblackstone commented Jun 7, 2017

@pearcec copy/paste error on my part. #15153

@ghost
Copy link

ghost commented Apr 11, 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 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.

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

No branches or pull requests