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

Jump to bottom

Fix for Bugs #8227, #11226 - subnet info lost on vNet update #2

Closed
wants to merge 5 commits into from
Closed

Fix for Bugs #8227, #11226 - subnet info lost on vNet update #2

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
85ed0f6
Merge branch 'master' of https://github.com/hashicorp/terraform
echuvyrov Apr 9, 2017
be80f8a
Merge branch 'master' of https://github.com/harijayms/terraform
echuvyrov Apr 12, 2017
772fac7
Changes to vNet and subnet files to allow subnet updates without losi…
echuvyrov Apr 5, 2017
0810627
Minor changes to vNet properties retrieval
echuvyrov Apr 21, 2017
0787abd
Merge branch 'master' into bug8227
echuvyrov Apr 21, 2017
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
249 changes: 249 additions & 0 deletions builtin/providers/azurerm/resource_arm_subnet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package azurerm

import (
"fmt"
"log"
"net/http"
"strings"
"testing"

"github.com/hashicorp/terraform/helper/acctest"
Expand Down Expand Up @@ -30,6 +32,55 @@ func TestAccAzureRMSubnet_basic(t *testing.T) {
})
}

func TestAccAzureRMSubnet_routeTable(t *testing.T) {

ri := acctest.RandInt()
initConfig := fmt.Sprintf(testAccAzureRMSubnet_routeTable, ri, ri, ri)
updatedConfig := fmt.Sprintf(testAccAzureRMSubnet_updatedRouteTable, ri, ri, ri)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMSubnetDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: initConfig,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMSubnetExists("azurerm_subnet.rtTable2"),
),
},

resource.TestStep{
Config: updatedConfig,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMSubnetRouteTableExists("azurerm_subnet.rtTable2", "rtTable2-RT"),
),
},
},
})
}

func TestAccAzureRMSubnet_routeTable_subnetInline(t *testing.T) {

ri := acctest.RandInt()
//initConfig := fmt.Sprintf(testAccAzureRMSubnet_routeTable_subnetInline, ri, ri, ri)
updatedConfig := fmt.Sprintf(testAccAzureRMSubnet_updatedRouteTable, ri, ri, ri)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMSubnetDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: updatedConfig,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMSubnetRouteTableExists("azurerm_subnet.rtTable2", "rtTable2-RT"),
),
},
},
})
}

func TestAccAzureRMSubnet_disappears(t *testing.T) {

ri := acctest.RandInt()
Expand Down Expand Up @@ -60,6 +111,8 @@ func testCheckAzureRMSubnetExists(name string) resource.TestCheckFunc {
return fmt.Errorf("Not found: %s", name)
}

log.Printf("[INFO] Checking Subnet addition.")

name := rs.Primary.Attributes["name"]
vnetName := rs.Primary.Attributes["virtual_network_name"]
resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
Expand All @@ -78,6 +131,60 @@ func testCheckAzureRMSubnetExists(name string) resource.TestCheckFunc {
return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not exist", name, resourceGroup)
}

if resp.RouteTable == nil {
return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not contain route tables after add", name, resourceGroup)
}

return nil
}
}

func testCheckAzureRMSubnetRouteTableExists(subnetName string, routeTableId string) resource.TestCheckFunc {
return func(s *terraform.State) error {
// Ensure we have enough information in state to look up in API
rs, ok := s.RootModule().Resources[subnetName]
if !ok {
return fmt.Errorf("Not found: %s", subnetName)
}

log.Printf("[INFO] Checking Subnet update.")

name := rs.Primary.Attributes["name"]
vnetName := rs.Primary.Attributes["virtual_network_name"]
resourceGroup, hasResourceGroup := rs.Primary.Attributes["resource_group_name"]
if !hasResourceGroup {
return fmt.Errorf("Bad: no resource group found in state for subnet: %s", name)
}

vnetConn := testAccProvider.Meta().(*ArmClient).vnetClient
vnetResp, vnetErr := vnetConn.Get(resourceGroup, vnetName, "")
if vnetErr != nil {
return fmt.Errorf("Bad: Get on vnetClient: %s", vnetErr)
}

if vnetResp.Subnets == nil {
return fmt.Errorf("Bad: Vnet %q (resource group: %q) does not have subnets after update", vnetName, resourceGroup)
}

conn := testAccProvider.Meta().(*ArmClient).subnetClient

resp, err := conn.Get(resourceGroup, vnetName, name, "")
if err != nil {
return fmt.Errorf("Bad: Get on subnetClient: %s", err)
}

if resp.StatusCode == http.StatusNotFound {
return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not exist", subnetName, resourceGroup)
}

if resp.RouteTable == nil {
return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not contain route tables after update", subnetName, resourceGroup)
}

if !strings.Contains(*resp.RouteTable.ID, routeTableId) {
return fmt.Errorf("Bad: Subnet %q (resource group: %q) does not have route table %q", subnetName, resourceGroup, routeTableId)
}

return nil
}
}
Expand Down Expand Up @@ -154,3 +261,145 @@ resource "azurerm_subnet" "test" {
address_prefix = "10.0.2.0/24"
}
`

var testAccAzureRMSubnet_routeTable = `
resource "azurerm_resource_group" "rtTable2" {
name = "acctestRG-%d"
location = "West US"
}

resource "azurerm_virtual_network" "rtTable2" {
name = "acctestvirtnet%d"
address_space = ["10.0.0.0/16"]
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
}

resource "azurerm_subnet" "rtTable2" {
name = "acctestsubnet%d"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
virtual_network_name = "${azurerm_virtual_network.rtTable2.name}"
address_prefix = "10.0.2.0/24"
route_table_id = "${azurerm_route_table.rtTable2.id}"
}

resource "azurerm_route_table" "rtTable2" {
name = "rtTable2-RT"
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
}

resource "azurerm_route" "route_a" {
name = "TestRouteA"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
route_table_name = "${azurerm_route_table.rtTable2.name}"

address_prefix = "10.100.0.0/14"
next_hop_type = "VirtualAppliance"
next_hop_in_ip_address = "10.10.1.1"
}`

var testAccAzureRMSubnet_routeTable_subnetInline = `
resource "azurerm_resource_group" "rtTable2" {
name = "acctestRG-%d"
location = "West US"
}

resource "azurerm_virtual_network" "rtTable2" {
name = "acctestvirtnet%d"
address_space = ["10.0.0.0/16"]
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
subnet {
name = "rtTable2"
address_prefix = "10.0.3.0/24"
route_table = "${azurerm_route_table.rtTable2.id}"
}
tags {
environment = "Testing"
}
}

resource "azurerm_route_table" "rtTable2" {
name = "rtTable2-RT"
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
}

resource "azurerm_route" "route_a" {
name = "TestRouteA"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
route_table_name = "${azurerm_route_table.rtTable2.name}"

address_prefix = "10.100.0.0/14"
next_hop_type = "VirtualAppliance"
next_hop_in_ip_address = "10.10.1.1"
}`

var testAccAzureRMSubnet_updatedRouteTable = `
resource "azurerm_resource_group" "rtTable2" {
name = "acctestRG-%d"
location = "West US"
tags {
environment = "Testing"
}
}

resource "azurerm_network_security_group" "rtTable2_secgroup" {
name = "acceptanceTestSecurityGroup1"
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"

security_rule {
name = "test123"
priority = 100
direction = "Inbound"
access = "Allow"
protocol = "Tcp"
source_port_range = "*"
destination_port_range = "*"
source_address_prefix = "*"
destination_address_prefix = "*"
}

tags {
environment = "Testing"
}
}

resource "azurerm_virtual_network" "rtTable2" {
name = "acctestvirtnet%d"
address_space = ["10.0.0.0/16"]
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
tags {
environment = "Testing"
}
}

resource "azurerm_subnet" "rtTable2" {
name = "acctestsubnet%d"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
virtual_network_name = "${azurerm_virtual_network.rtTable2.name}"
address_prefix = "10.0.2.0/24"
route_table_id = "${azurerm_route_table.rtTable2.id}"
}

resource "azurerm_route_table" "rtTable2" {
name = "rtTable2-RT"
location = "West US"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
tags {
environment = "Testing"
}
}

resource "azurerm_route" "route_a" {
name = "TestRouteA"
resource_group_name = "${azurerm_resource_group.rtTable2.name}"
route_table_name = "${azurerm_route_table.rtTable2.name}"

address_prefix = "10.100.0.0/14"
next_hop_type = "VirtualAppliance"
next_hop_in_ip_address = "10.10.1.1"
}`
72 changes: 66 additions & 6 deletions builtin/providers/azurerm/resource_arm_virtual_network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ func resourceArmVirtualNetwork() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"route_table": {
Type: schema.TypeString,
Optional: true,
},
},
},
Set: resourceAzureSubnetHash,
Expand Down Expand Up @@ -93,7 +97,7 @@ func resourceArmVirtualNetworkCreate(d *schema.ResourceData, meta interface{}) e
vnet := network.VirtualNetwork{
Name: &name,
Location: &location,
VirtualNetworkPropertiesFormat: getVirtualNetworkProperties(d),
VirtualNetworkPropertiesFormat: getVirtualNetworkProperties(d, meta),
Tags: expandTags(tags),
}

Expand Down Expand Up @@ -187,7 +191,7 @@ func resourceArmVirtualNetworkDelete(d *schema.ResourceData, meta interface{}) e
return err
}

func getVirtualNetworkProperties(d *schema.ResourceData) *network.VirtualNetworkPropertiesFormat {
func getVirtualNetworkProperties(d *schema.ResourceData, meta interface{}) *network.VirtualNetworkPropertiesFormat {
// first; get address space prefixes:
prefixes := []string{}
for _, prefix := range d.Get("address_space").([]interface{}) {
Expand All @@ -207,20 +211,41 @@ func getVirtualNetworkProperties(d *schema.ResourceData) *network.VirtualNetwork
subnet := subnet.(map[string]interface{})

name := subnet["name"].(string)
log.Printf("[INFO] setting subnets inside vNet, processing %q", name)
//since subnets can also be created outside of vNet definition (as root objects)
// do a GET on subnet properties from the server before setting them
resGroup := d.Get("resource_group_name").(string)
vnetName := d.Get("name").(string)
subnetObj := getExistingSubnet(resGroup, vnetName, name, meta)
log.Printf("[INFO] Completer GET of Subnet props ")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Completed? :-)


prefix := subnet["address_prefix"].(string)
secGroup := subnet["security_group"].(string)
routeTable := subnet["route_table"].(string)
Copy link
Collaborator

Choose a reason for hiding this comment

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

route_table and security_group are optional, you need error checking.


var subnetObj network.Subnet
subnetObj.Name = &name
subnetObj.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{}
subnetObj.SubnetPropertiesFormat.AddressPrefix = &prefix
if name != "" {
log.Printf("[INFO] setting SUBNETNAME TO %q", name)
subnetObj.Name = &name
}
if subnetObj.SubnetPropertiesFormat == nil {
subnetObj.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{}
}
if subnetObj.SubnetPropertiesFormat.AddressPrefix == nil {
subnetObj.SubnetPropertiesFormat.AddressPrefix = &prefix
}

if secGroup != "" {
subnetObj.SubnetPropertiesFormat.NetworkSecurityGroup = &network.SecurityGroup{
ID: &secGroup,
}
}

if routeTable != "" {
subnetObj.SubnetPropertiesFormat.RouteTable = &network.RouteTable{
ID: &routeTable,
}
}

subnets = append(subnets, subnetObj)
}
}
Expand All @@ -245,3 +270,38 @@ func resourceAzureSubnetHash(v interface{}) int {
}
return hashcode.String(subnet)
}

func getExistingSubnet(resGroup string, vnetName string, subnetName string, meta interface{}) network.Subnet {
//attempt to retrieve existing subnet from the server
existingSubnet := network.Subnet{}
subnetClient := meta.(*ArmClient).subnetClient
resp, err := subnetClient.Get(resGroup, vnetName, subnetName, "")

if err != nil {
if resp.StatusCode == http.StatusNotFound {
return existingSubnet
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we be doing something with other errors?

}

existingSubnet.SubnetPropertiesFormat = &network.SubnetPropertiesFormat{}
existingSubnet.SubnetPropertiesFormat.AddressPrefix = resp.SubnetPropertiesFormat.AddressPrefix

if resp.SubnetPropertiesFormat.NetworkSecurityGroup != nil {
existingSubnet.SubnetPropertiesFormat.NetworkSecurityGroup = resp.SubnetPropertiesFormat.NetworkSecurityGroup
}

if resp.SubnetPropertiesFormat.RouteTable != nil {
existingSubnet.SubnetPropertiesFormat.RouteTable = resp.SubnetPropertiesFormat.RouteTable
}

if resp.SubnetPropertiesFormat.IPConfigurations != nil {
ips := make([]string, 0, len(*resp.SubnetPropertiesFormat.IPConfigurations))
for _, ip := range *resp.SubnetPropertiesFormat.IPConfigurations {
ips = append(ips, *ip.ID)
}

existingSubnet.SubnetPropertiesFormat.IPConfigurations = resp.SubnetPropertiesFormat.IPConfigurations
}

return existingSubnet
}