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: Add support for setting the primary network interface #11290

Merged

Conversation

kvisscher
Copy link
Contributor

I'm trying to add support for setting the primary network interface for an Azure VM. This works (we deployed something with this change :-)), but I'm not sure how to write a test to proof it to you guys.

Can you guys point me in the right direction?

@radeksimko radeksimko changed the title [WIP] add support for setting the primary network interface [WIP] provider/azurerm: Add support for setting the primary network interface Jan 22, 2017
@pearcec
Copy link
Contributor

pearcec commented Feb 17, 2017

I just validated this patch works. Much better approach then anything I had planned on #6514 . Keeps the code from breaking and easy fix. Suggest we need documentation added as well. I am going to fork off your repo and work of your branch. I will try to add some tests and documentation and give you a PR.

@pearcec
Copy link
Contributor

pearcec commented Feb 17, 2017

I just pushed and submitted a PR for this PR kvisscher#1 Hopefully @kvisscher is still active on this thread. If not we can just reroll on the latest branch and submit a new PR.

@skinny
Copy link

skinny commented Feb 24, 2017

I'll poke @kvisscher (he's normally sitting a couple of meters away from my desk) when he's back from his holiday next week! Thanks for the work!

@kvisscher kvisscher force-pushed the feat/primary-network-interface branch from 602745e to cda7e55 Compare February 27, 2017 08:45
@kvisscher
Copy link
Contributor Author

@pearcec I've updated my PR with your changes. Can you verify that this is OK 👍 ?

@pearcec
Copy link
Contributor

pearcec commented Mar 3, 2017

Looks good. Did you pull of the latest master? Initially I tried to merge what you had off master and the PR was a bit stale.

@kvisscher
Copy link
Contributor Author

@pearcec I pulled from master 8 days ago :-). Want me to rebase again?

@pearcec
Copy link
Contributor

pearcec commented Mar 9, 2017

You are probably good then. Send out the PR and see what happens.

@kvisscher
Copy link
Contributor Author

@pearcec I made be mistaken, but this PR is already pending against the hashicorp/terrform master.

@pearcec
Copy link
Contributor

pearcec commented Mar 10, 2017

Right I think you are good. Hopefully it gets picked up soon.

@kvisscher
Copy link
Contributor Author

@radeksimko Don't want to bother you, but is there any chance you know someone in the community who has the super powers to merge this in?

Copy link
Contributor

@pmcatominey pmcatominey left a comment

Choose a reason for hiding this comment

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

Nice work!

Fixes #6514.

@pmcatominey
Copy link
Contributor

Tests pass!

TF_ACC=1 go test ./builtin/providers/azurerm -v -run TestAccAzureRMVirtualMachine_primary -timeout 120m
=== RUN   TestAccAzureRMVirtualMachine_primaryNetworkInterfaceId
--- PASS: TestAccAzureRMVirtualMachine_primaryNetworkInterfaceId (420.95s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	420.962s

@kvisscher if you're happy with the PR could you remove the WIP tag from the title?

@kvisscher kvisscher changed the title [WIP] provider/azurerm: Add support for setting the primary network interface provider/azurerm: Add support for setting the primary network interface Mar 23, 2017
@kvisscher
Copy link
Contributor Author

@pmcatominey Done 😄

@pmcatominey pmcatominey merged commit 8bc049f into hashicorp:master Mar 23, 2017
Copy link
Contributor

@StephenWeatherford StephenWeatherford left a comment

Choose a reason for hiding this comment

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

(wrong PR)

@kblackstone
Copy link

what is the syntax to set an interface as primary?

@kblackstone
Copy link

kblackstone commented Jun 2, 2017

@StephenWeatherford thank you!

@kvisscher kvisscher deleted the feat/primary-network-interface branch June 7, 2017 08:39
@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

Successfully merging this pull request may close these issues.

7 participants