Skip to content

Commit

Permalink
azurerm_netapp_volume - support NFSv4.1, vol sizing issue (#5485)
Browse files Browse the repository at this point in the history
Hi Terraform team,

I'm part of Azure NetApp team at Microsoft and I'm providing this PR that adds support for NFSv4.1 since we have a partner that needs this support to work internally and with their own customers.

This is a summary of changes I made:

- Bumped up the ANF's Go SDK version to the latest (2019-10-01)
- Implemented an attribute name change for the ExportPolicyRule that got renamed to Nfsv41 since api version 2019-07-01 and applied the same renaming concept to Go's own object properties to avoid confusion.
- Implemented the property called protocol_types as additional volume property since this is the one that controls what protocol the volume will support (it is defined as a list following our API definition but only one is supported) and included validation.
- Implemented some changes on Tests so we can check if a NFSv4.1 volume gets created, plus testing for NFSv3 value on basic volume test
- Stepped down the ServiceLevel from Premium to Standard on Tests so developer incur in less costs while working on ANF resource.
- Fixed a bug related to volume size that was limiting a volume to 4TB while the correct maximum size is 100TB.
  • Loading branch information
katbyte authored Mar 5, 2020
1 parent 89b04b0 commit 740af45
Show file tree
Hide file tree
Showing 24 changed files with 958 additions and 154 deletions.
2 changes: 1 addition & 1 deletion azurerm/internal/services/netapp/client/client.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package client

import (
"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-06-01/netapp"
"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-10-01/netapp"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/common"
)

Expand Down
12 changes: 12 additions & 0 deletions azurerm/internal/services/netapp/data_source_netapp_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ func dataSourceArmNetAppVolume() *schema.Resource {
Type: schema.TypeInt,
Computed: true,
},

"protocols": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
},
}
}
Expand Down Expand Up @@ -101,6 +107,12 @@ func dataSourceArmNetAppVolumeRead(d *schema.ResourceData, meta interface{}) err
d.Set("service_level", props.ServiceLevel)
d.Set("subnet_id", props.SubnetID)

protocolTypes := make([]string, 0)
if prtclTypes := props.ProtocolTypes; prtclTypes != nil {
protocolTypes = append(protocolTypes, *prtclTypes...)
}
d.Set("protocols", protocolTypes)

if props.UsageThreshold != nil {
d.Set("storage_quota_in_gb", *props.UsageThreshold/1073741824)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-06-01/netapp"
"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-10-01/netapp"
"github.com/hashicorp/go-azure-helpers/response"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"strconv"
"time"

"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-06-01/netapp"
"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-10-01/netapp"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"log"
"time"

"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-06-01/netapp"
"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-10-01/netapp"
"github.com/hashicorp/go-azure-helpers/response"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
Expand Down
142 changes: 114 additions & 28 deletions azurerm/internal/services/netapp/resource_arm_netapp_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-06-01/netapp"
"github.com/Azure/azure-sdk-for-go/services/netapp/mgmt/2019-10-01/netapp"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
Expand Down Expand Up @@ -36,7 +36,7 @@ func resourceArmNetAppVolume() *schema.Resource {
Create: schema.DefaultTimeout(30 * time.Minute),
Read: schema.DefaultTimeout(5 * time.Minute),
Update: schema.DefaultTimeout(30 * time.Minute),
Delete: schema.DefaultTimeout(30 * time.Minute),
Delete: schema.DefaultTimeout(60 * time.Minute),
},

Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -75,6 +75,7 @@ func resourceArmNetAppVolume() *schema.Resource {
"service_level": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
string(netapp.Premium),
string(netapp.Standard),
Expand All @@ -89,14 +90,28 @@ func resourceArmNetAppVolume() *schema.Resource {
ValidateFunc: azure.ValidateResourceID,
},

"protocols": {
Type: schema.TypeSet,
ForceNew: true,
Optional: true,
Computed: true,
MaxItems: 2,
Elem: &schema.Schema{Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
"NFSv3",
"NFSv4.1",
"CIFS",
}, false)},
},

"storage_quota_in_gb": {
Type: schema.TypeInt,
Required: true,
ValidateFunc: validation.IntBetween(100, 4096),
},

"export_policy_rule": {
Type: schema.TypeSet,
Type: schema.TypeList,
Optional: true,
MaxItems: 5,
Elem: &schema.Resource{
Expand All @@ -106,6 +121,7 @@ func resourceArmNetAppVolume() *schema.Resource {
Required: true,
ValidateFunc: validation.IntBetween(1, 5),
},

"allowed_clients": {
Type: schema.TypeSet,
Required: true,
Expand All @@ -114,22 +130,47 @@ func resourceArmNetAppVolume() *schema.Resource {
ValidateFunc: validate.CIDR,
},
},

"protocols_enabled": {
Type: schema.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
MinItems: 1,
Elem: &schema.Schema{Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
"NFSv3",
"NFSv4.1",
"CIFS",
}, false)},
},

"cifs_enabled": {
Type: schema.TypeBool,
Required: true,
Type: schema.TypeBool,
Optional: true,
Computed: true,
Deprecated: "Deprecated in favor of `protocols_enabled`",
},

"nfsv3_enabled": {
Type: schema.TypeBool,
Required: true,
Type: schema.TypeBool,
Optional: true,
Computed: true,
Deprecated: "Deprecated in favor of `protocols_enabled`",
},

"nfsv4_enabled": {
Type: schema.TypeBool,
Required: true,
Type: schema.TypeBool,
Optional: true,
Computed: true,
Deprecated: "Deprecated in favor of `protocols_enabled`",
},

"unix_read_only": {
Type: schema.TypeBool,
Optional: true,
},

"unix_read_write": {
Type: schema.TypeBool,
Optional: true,
Expand Down Expand Up @@ -167,15 +208,21 @@ func resourceArmNetAppVolumeCreateUpdate(d *schema.ResourceData, meta interface{
volumePath := d.Get("volume_path").(string)
serviceLevel := d.Get("service_level").(string)
subnetId := d.Get("subnet_id").(string)
protocols := d.Get("protocols").(*schema.Set).List()
if len(protocols) == 0 {
protocols = append(protocols, "NFSv3")
}

storageQuotaInGB := int64(d.Get("storage_quota_in_gb").(int) * 1073741824)
exportPolicyRule := d.Get("export_policy_rule").(*schema.Set).List()
exportPolicyRule := d.Get("export_policy_rule").([]interface{})

parameters := netapp.Volume{
Location: utils.String(location),
VolumeProperties: &netapp.VolumeProperties{
CreationToken: utils.String(volumePath),
ServiceLevel: netapp.ServiceLevel(serviceLevel),
SubnetID: utils.String(subnetId),
ProtocolTypes: utils.ExpandStringSlice(protocols),
UsageThreshold: utils.Int64(storageQuotaInGB),
ExportPolicy: expandArmNetAppVolumeExportPolicyRule(exportPolicyRule),
},
Expand Down Expand Up @@ -236,7 +283,7 @@ func resourceArmNetAppVolumeRead(d *schema.ResourceData, meta interface{}) error
d.Set("volume_path", props.CreationToken)
d.Set("service_level", props.ServiceLevel)
d.Set("subnet_id", props.SubnetID)

d.Set("protocols", props.ProtocolTypes)
if props.UsageThreshold != nil {
d.Set("storage_quota_in_gb", *props.UsageThreshold/1073741824)
}
Expand Down Expand Up @@ -310,17 +357,42 @@ func expandArmNetAppVolumeExportPolicyRule(input []interface{}) *netapp.VolumePr
v := item.(map[string]interface{})
ruleIndex := int32(v["rule_index"].(int))
allowedClients := strings.Join(*utils.ExpandStringSlice(v["allowed_clients"].(*schema.Set).List()), ",")
cifsEnabled := v["cifs_enabled"].(bool)
nfsv3Enabled := v["nfsv3_enabled"].(bool)
nfsv4Enabled := v["nfsv4_enabled"].(bool)

cifsEnabled := false
nfsv3Enabled := false
nfsv41Enabled := false

if vpe := v["protocols_enabled"]; vpe != nil {
protocolsEnabled := vpe.([]interface{})
if len(protocolsEnabled) != 0 {
for _, protocol := range protocolsEnabled {
if protocol != nil {
switch strings.ToLower(protocol.(string)) {
case "cifs":
cifsEnabled = true
case "nfsv3":
nfsv3Enabled = true
case "nfsv4.1":
nfsv41Enabled = true
}
}
}
} else {
// TODO: Remove in next major version
cifsEnabled = v["cifs_enabled"].(bool)
nfsv3Enabled = v["nfsv3_enabled"].(bool)
nfsv41Enabled = v["nfsv4_enabled"].(bool)
}
}

unixReadOnly := v["unix_read_only"].(bool)
unixReadWrite := v["unix_read_write"].(bool)

result := netapp.ExportPolicyRule{
AllowedClients: utils.String(allowedClients),
Cifs: utils.Bool(cifsEnabled),
Nfsv3: utils.Bool(nfsv3Enabled),
Nfsv4: utils.Bool(nfsv4Enabled),
Nfsv41: utils.Bool(nfsv41Enabled),
RuleIndex: utils.Int32(ruleIndex),
UnixReadOnly: utils.Bool(unixReadOnly),
UnixReadWrite: utils.Bool(unixReadWrite),
Expand Down Expand Up @@ -350,17 +422,29 @@ func flattenArmNetAppVolumeExportPolicyRule(input *netapp.VolumePropertiesExport
if v := item.AllowedClients; v != nil {
allowedClients = strings.Split(*v, ",")
}
//TODO: Start - Remove in next major version
cifsEnabled := false
nfsv3Enabled := false
nfsv4Enabled := false
// End - Remove in next major version
protocolsEnabled := []string{}
if v := item.Cifs; v != nil {
cifsEnabled = *v
if *v {
protocolsEnabled = append(protocolsEnabled, "CIFS")
}
cifsEnabled = *v //TODO: Remove in next major version
}
nfsv3Enabled := false
if v := item.Nfsv3; v != nil {
nfsv3Enabled = *v
if *v {
protocolsEnabled = append(protocolsEnabled, "NFSv3")
}
nfsv3Enabled = *v //TODO: Remove in next major version
}
nfsv4Enabled := false
if v := item.Nfsv4; v != nil {
nfsv4Enabled = *v
if v := item.Nfsv41; v != nil {
if *v {
protocolsEnabled = append(protocolsEnabled, "NFSv4.1")
}
nfsv4Enabled = *v //TODO: Remove in next major version
}
unixReadOnly := false
if v := item.UnixReadOnly; v != nil {
Expand All @@ -372,13 +456,15 @@ func flattenArmNetAppVolumeExportPolicyRule(input *netapp.VolumePropertiesExport
}

results = append(results, map[string]interface{}{
"rule_index": ruleIndex,
"allowed_clients": utils.FlattenStringSlice(&allowedClients),
"cifs_enabled": cifsEnabled,
"nfsv3_enabled": nfsv3Enabled,
"nfsv4_enabled": nfsv4Enabled,
"unix_read_only": unixReadOnly,
"unix_read_write": unixReadWrite,
"rule_index": ruleIndex,
"allowed_clients": utils.FlattenStringSlice(&allowedClients),
"unix_read_only": unixReadOnly,
"unix_read_write": unixReadWrite,
"protocols_enabled": utils.FlattenStringSlice(&protocolsEnabled),
//TODO: Remove in next major version
"cifs_enabled": cifsEnabled,
"nfsv3_enabled": nfsv3Enabled,
"nfsv4_enabled": nfsv4Enabled,
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func TestAccDataSourceAzureRMNetAppVolume_basic(t *testing.T) {
resource.TestCheckResourceAttrSet(data.ResourceName, "service_level"),
resource.TestCheckResourceAttrSet(data.ResourceName, "subnet_id"),
resource.TestCheckResourceAttrSet(data.ResourceName, "storage_quota_in_gb"),
resource.TestCheckResourceAttrSet(data.ResourceName, "protocols.0"),
),
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestAccAzureRMNetAppPool_update(t *testing.T) {
Config: testAccAzureRMNetAppPool_basic(data),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMNetAppPoolExists(data.ResourceName),
resource.TestCheckResourceAttr(data.ResourceName, "service_level", "Premium"),
resource.TestCheckResourceAttr(data.ResourceName, "service_level", "Standard"),
resource.TestCheckResourceAttr(data.ResourceName, "size_in_tb", "4"),
),
},
Expand Down Expand Up @@ -181,7 +181,7 @@ resource "azurerm_netapp_pool" "test" {
account_name = azurerm_netapp_account.test.name
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
service_level = "Premium"
service_level = "Standard"
size_in_tb = 4
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,10 @@ resource "azurerm_subnet" "update" {
resource_group_name = azurerm_resource_group.test.name
virtual_network_name = azurerm_virtual_network.update.name
address_prefix = "10.0.2.0/24"
delegation {
name = "netapp"
service_delegation {
name = "Microsoft.Netapp/volumes"
actions = ["Microsoft.Network/networkinterfaces/*", "Microsoft.Network/virtualNetworks/subnets/join/action"]
Expand Down
Loading

0 comments on commit 740af45

Please sign in to comment.