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.

Sign up for GitHub

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

Add Support for Accelerated Networking (SR-IOV) #672

Merged
merged 10 commits into from
Jan 11, 2018
Merged

Add Support for Accelerated Networking (SR-IOV) #672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0524fce
Add support for the enable accelerated networking flag
jzampieron Jan 5, 2018
134c215
Add migration function to handle the schema change
jzampieron Jan 5, 2018
4787ce6
Add documentation for the enable_accelerated_networking field
jzampieron Jan 9, 2018
0e1e5f0
Updated comments now that AN is GA on both Windows and Linux
jzampieron Jan 9, 2018
6b6f1fe
Add a VM with managed disks with attached AN NIC
jzampieron Jan 9, 2018
3b8e5c0
Move AN enabled VM test from Network Interface to VM with Managed Disks
jzampieron Jan 10, 2018
e6c9531
Adjust doc language per feedback from Tom
jzampieron Jan 11, 2018
b4e892d
Change image to Ubuntu 16.04 LTS and change password for consistency …
jzampieron Jan 11, 2018
67e30cf
Remove unnecessary migration and update read function to allow defaul…
jzampieron Jan 11, 2018
80db544
Removing an invalid check
tombuildsstuff Jan 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion azurerm/resource_arm_network_interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,20 @@ func resourceArmNetworkInterface() *schema.Resource {
Computed: true,
},

/**
* As of 2018-01-06: AN (aka. SR-IOV) on Azure is GA on Windows and Linux.
*
* Refer to: https://azure.microsoft.com/en-us/blog/maximize-your-vm-s-performance-with-accelerated-networking-now-generally-available-for-both-windows-and-linux/
*
* Refer to: https://docs.microsoft.com/en-us/azure/virtual-network/create-vm-accelerated-networking-cli
* For details, VM configuration and caveats.
*/
"enable_accelerated_networking": {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add an entry to the documentation for this new field in website/docs/r/network_interface.html.markdown?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. I was looking for the docs, but didn't quite know the code base well enough yet to find them.

Type: schema.TypeBool,
Optional: true,
Default: false,
},

"enable_ip_forwarding": {
Type: schema.TypeBool,
Optional: true,
Expand Down Expand Up @@ -173,10 +187,12 @@ func resourceArmNetworkInterfaceCreateUpdate(d *schema.ResourceData, meta interf
location := d.Get("location").(string)
resGroup := d.Get("resource_group_name").(string)
enableIpForwarding := d.Get("enable_ip_forwarding").(bool)
enableAcceleratedNetworking := d.Get("enable_accelerated_networking").(bool)
tags := d.Get("tags").(map[string]interface{})

properties := network.InterfacePropertiesFormat{
EnableIPForwarding: &enableIpForwarding,
EnableIPForwarding: &enableIpForwarding,
EnableAcceleratedNetworking: &enableAcceleratedNetworking,
}

if v, ok := d.GetOk("network_security_group_id"); ok {
Expand Down Expand Up @@ -347,6 +363,7 @@ func resourceArmNetworkInterfaceRead(d *schema.ResourceData, meta interface{}) e
d.Set("applied_dns_servers", appliedDNSServers)
d.Set("dns_servers", dnsServers)
d.Set("enable_ip_forwarding", resp.EnableIPForwarding)
d.Set("enable_accelerated_networking", resp.EnableAcceleratedNetworking)

flattenAndSetTags(d, resp.Tags)

Expand Down
56 changes: 56 additions & 0 deletions azurerm/resource_arm_network_interface_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,25 @@ func TestAccAzureRMNetworkInterface_enableIPForwarding(t *testing.T) {
})
}

func TestAccAzureRMNetworkInterface_enableAcceleratedNetworking(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also add a test that provisions a VM using a Managed Disk in this file? That would allow us to have an end-to-end example of this working

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure can. I'll also rebase against the latest master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The one caveat here is that only certain instance types support AN. I will select the smallest I know of, which is a Standard_D8_v3, but it is not an inexpensive machine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the acceptance tests only spin the machines up for tests for a short while, I don't think that'll be that bad for us (HashiCorp) - however I think it's worth mentioning for contributors. In the AWS Provider we document this kind of thing at the top of the file, for instance:

// NOTE: Test `X` requires a machine of size `D8_v3` which is large/expensive - you may wish to ignore this test"

As such - can we update the top of the resource_arm_virtual_machine_managed_disk_test.go file with a comment to this effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tombuildsstuff I put that test in the wrong file.... it's in the network interface tests. I'll move it over now and update the top as requested.

resourceName := "azurerm_network_interface.test"
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMNetworkInterfaceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAzureRMNetworkInterface_acceleratedNetworking(rInt, testLocation()),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMNetworkInterfaceExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "enable_accelerated_networking", "true"),
),
},
},
})
}

func TestAccAzureRMNetworkInterface_multipleLoadBalancers(t *testing.T) {
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{
Expand Down Expand Up @@ -524,6 +543,43 @@ resource "azurerm_network_interface" "test" {
`, rInt, location, rInt, rInt)
}

func testAccAzureRMNetworkInterface_acceleratedNetworking(rInt int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctest-rg-%d"
location = "%s"
}

resource "azurerm_virtual_network" "test" {
name = "acctestvn-%d"
address_space = ["10.0.0.0/16"]
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
}

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

resource "azurerm_network_interface" "test" {
name = "acctestni-%d"
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
enable_ip_forwarding = false
enable_accelerated_networking = true

ip_configuration {
name = "testconfiguration1"
subnet_id = "${azurerm_subnet.test.id}"
private_ip_address_allocation = "dynamic"
}
}
`, rInt, location, rInt, rInt)
}

func testAccAzureRMNetworkInterface_withTags(rInt int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
Expand Down
95 changes: 95 additions & 0 deletions azurerm/resource_arm_virtual_machine_managed_disks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"github.com/hashicorp/terraform/terraform"
)

// NOTE: Test `TestAccAzureRMVirtualMachine_enableAnWithVM` requires a machine of size `D8_v3` which is large/expensive - you may wish to ignore this test"

func TestAccAzureRMVirtualMachine_basicLinuxMachine_managedDisk_explicit(t *testing.T) {
var vm compute.VirtualMachine
ri := acctest.RandInt()
Expand Down Expand Up @@ -341,6 +343,25 @@ func TestAccAzureRMVirtualMachine_managedServiceIdentity(t *testing.T) {
})
}

func TestAccAzureRMVirtualMachine_enableAnWithVM(t *testing.T) {
var vm compute.VirtualMachine
resourceName := "azurerm_virtual_machine.test"
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMVirtualMachineDestroy,
Steps: []resource.TestStep{
{
Config: testAccAzureRMVirtualMachine_anWithVM(rInt, testLocation()),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMVirtualMachineExists(resourceName, &vm),
),
},
},
})
}

func testAccAzureRMVirtualMachine_withManagedServiceIdentity(rInt int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
Expand Down Expand Up @@ -1576,3 +1597,77 @@ resource "azurerm_virtual_machine" "test" {
}
`, rInt, location, rInt, rInt, rInt, rInt, rString, rString)
}

func testAccAzureRMVirtualMachine_anWithVM(rInt int, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctest-rg-%d"
location = "%s"
}

resource "azurerm_virtual_network" "test" {
name = "acctestvn-%d"
address_space = ["10.0.0.0/16"]
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
}

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

resource "azurerm_network_interface" "test" {
name = "acctestni-%d"
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
enable_ip_forwarding = false
enable_accelerated_networking = true

ip_configuration {
name = "testconfiguration1"
subnet_id = "${azurerm_subnet.test.id}"
private_ip_address_allocation = "dynamic"
}
}

resource "azurerm_virtual_machine" "test" {
name = "acctestvm-%d"
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
primary_network_interface_id = "${azurerm_network_interface.test.id}"
network_interface_ids = [ "${azurerm_network_interface.test.id}" ]
// Only large VMs allow AN
vm_size = "Standard_D8_v3"
delete_os_disk_on_termination = true

storage_image_reference {
publisher = "Canonical"
offer = "UbuntuServer"
sku = "16.04-LTS"
version = "latest"
}

storage_os_disk {
name = "antest-%d-OSDisk"
caching = "ReadWrite"
create_option = "FromImage"
managed_disk_type = "Standard_LRS"
disk_size_gb = 32
}

os_profile {
computer_name = "antestMachine-%d"
admin_username = "antestuser"
admin_password = "Password1234!"
}

os_profile_linux_config {
disable_password_authentication = false
}

}
`, rInt, location, rInt, rInt, rInt, rInt, rInt)
}
2 changes: 2 additions & 0 deletions website/docs/r/network_interface.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ The following arguments are supported:

* `enable_ip_forwarding` - (Optional) Enables IP Forwarding on the NIC. Defaults to `false`.

* `enable_accelerated_networking` - (Optional) Enables Azure Accelerated Networking using SR-IOV. Only certain VM instance sizes are supported. Refer to [Create a Virtual Machine with Accelerated Networking](https://docs.microsoft.com/en-us/azure/virtual-network/create-vm-accelerated-networking-cli). Defaults to `false`.

* `dns_servers` - (Optional) List of DNS servers IP addresses to use for this NIC, overrides the VNet-level server list

* `ip_configuration` - (Required) One or more `ip_configuration` associated with this NIC as documented below.
Expand Down