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

provider/azurerm: suppport for primary network interface #12277

Closed
wants to merge 1 commit into from

Conversation

kvisscher
Copy link
Contributor

This brings support for setting the primary IP of a network interface.

@pmcatominey
Copy link
Contributor

Hi @kvisscher

Thanks for the PR!

Unfortunately we can't merge this right now as the feature is still in preview:

--- FAIL: TestAccAzureRMNetworkInterface_primaryNetworkInterface (148.04s)
    testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:
        
        * azurerm_network_interface.test1: 1 error(s) occurred:
        
        * azurerm_network_interface.test1: network.InterfacesClient#CreateOrUpdate: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="SubscriptionNotRegisteredForFeature" Message="Subscription xxx is not registered for feature Microsoft.Network/AllowMultipleIpConfigurationsPerNic required to carry out the requested operation." Details=[]

Once this feature is in general availability we will circle back to this PR, #9874 is also tracking this feature request.

@dominik-lekse
Copy link
Contributor

Hi @pmcatominey, @kvisscher,

as from 22nd March, the "Multiple IP addresses per network interface" feature has been announced to be generally available in all public regions (refer to https://azure.microsoft.com/de-de/updates/ga-multiple-ips-per-nic/).

Can we consider this PR to be merged into the upcoming release?

// Dominik

@tombuildsstuff
Copy link
Contributor

@dominik-lekse thanks for the update - it's great to see that this has hit GA :)

Hi @kvisscher

I've taken a look into your PR - and it looks good :) There's a couple of minor changes needed in order to be able to merge this:

  1. The Linter is complaining about the test file - can we run make fmt which will fix this?
  2. I've run the tests and the TestAccAzureRMNetworkInterface_primaryNetworkInterface test is failing - is it possible to fix this?
  3. Update the Documentation to include the primary field (and it's default value)

Out of interest - when specifying only a single IP Configuration, does the Azure API set a default value for primary? The Azure Docs don't state either way - but it'd be good to mention if it does :)

Thanks!

@dominik-lekse
Copy link
Contributor

@tombuildsstuff Regarding the value primary in case of a single IP configuration, you are right that the official docs are not quite clear on this. When checking this in the relevant implementation of Azure CLI 2.0 the value is falseby default and when using only a single IP configuration. Therefore, I would consider the default set in this pull request as correct. I will reserve time this week to test it.

Another point: We could add a validation that only one primary = true exists in all IP configurations of a NIC.

@apparentlymart apparentlymart changed the title feat: suppport for primary network interface provider/azurerm: suppport for primary network interface Apr 10, 2017
@dominik-lekse
Copy link
Contributor

Are there any plans yet to migrate this to the new terraform provider repository?

@kvisscher I can offer to help you here.

@422158
Copy link

422158 commented Aug 8, 2017

why is this not merged yet?
EDIT: or better yet, when is it going to part of release?

@tombuildsstuff
Copy link
Contributor

Hey @kvisscher

I hope you don't mind, but I've ported this PR over to the new repository in hashicorp/terraform-provider-azurerm#245 as this has now hit GA. As such I'm going to close this PR for the moment - but I'd like to thank you for your contribution here!

Thanks!

@ghost
Copy link

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

6 participants