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

azurerm_netapp_volume - support NFSv4.1, vol sizing issue #5485

Merged
merged 24 commits into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c13aa59
NFSv4.1 implementation+vol size bug fix+api bump
paulomarquesc Jan 22, 2020
31bd408
Module updates for ANF
paulomarquesc Jan 22, 2020
d014210
Default Nfsv3 test when missing protocol_types
paulomarquesc Jan 23, 2020
edc6ebf
Fixed format with terrafmt tool
paulomarquesc Jan 23, 2020
dfa8017
Attribute deprecation work starting
paulomarquesc Jan 23, 2020
ae59c78
Update azurerm/internal/services/netapp/data_source_netapp_volume.go
paulomarquesc Jan 24, 2020
d2899fa
Applying changes related to PR review
paulomarquesc Jan 26, 2020
6ddef14
Adding protocols and protocols_enabled to example
paulomarquesc Jan 26, 2020
4fa08cd
NFSv4.1 updates
paulomarquesc Jan 26, 2020
df8e8b8
Chaging wording
paulomarquesc Jan 26, 2020
b155983
Supporting only one item on protocols_enabled
paulomarquesc Jan 26, 2020
9ca983e
Reverting changes on go.mod and go.sum
paulomarquesc Jan 26, 2020
96e9a71
Fixing formatting
paulomarquesc Jan 26, 2020
2ddcac2
Removed unnecessary blank lines and comments
paulomarquesc Jan 26, 2020
92ed424
Adding extra line on go.mod
paulomarquesc Jan 26, 2020
db86619
Fixes to latest review round
paulomarquesc Feb 5, 2020
a5126b7
Removed not necessary log entry
paulomarquesc Feb 5, 2020
ab8c302
Fixfing tflint and lintrest CI issues
paulomarquesc Feb 5, 2020
0f40b36
More tflint fixes for CI
paulomarquesc Feb 5, 2020
3345b29
Update website/docs/r/netapp_volume.html.markdown
paulomarquesc Feb 6, 2020
940cd9b
Removing ForceNew and type fix on datasource
paulomarquesc Feb 6, 2020
da8ec80
ForceNew for volume and Type List on export policy
paulomarquesc Mar 4, 2020
af138cb
Changing comments related to v2
paulomarquesc Mar 4, 2020
389941e
git merge master
katbyte Mar 5, 2020
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
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
136 changes: 110 additions & 26 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 @@ -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,6 +90,19 @@ func resourceArmNetAppVolume() *schema.Resource {
ValidateFunc: azure.ValidateResourceID,
},

"protocols": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
MaxItems: 2,
paulomarquesc marked this conversation as resolved.
Show resolved Hide resolved
Elem: &schema.Schema{Type: schema.TypeString,
ValidateFunc: validation.StringInSlice([]string{
"NFSv3",
"NFSv4.1",
"CIFS",
}, false)},
},

"storage_quota_in_gb": {
Type: schema.TypeInt,
Required: true,
Expand All @@ -106,6 +120,7 @@ func resourceArmNetAppVolume() *schema.Resource {
Required: true,
ValidateFunc: validation.IntBetween(1, 5),
},

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

"protocols_enabled": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
MaxItems: 2,
Copy link
Member

Choose a reason for hiding this comment

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

The docs say only one value is supported at this time. Is that right? If so, we should match that here? From local testing, it seemed like the API accepted all three values being passed in at the same time.

Copy link
Contributor Author

@paulomarquesc paulomarquesc Feb 20, 2020

Choose a reason for hiding this comment

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

Hi @mbfrahry, to your first question, this will be supported in the future and @katbyte asked me to use TypeSet and I tried to reduce the items to 1 but TypeSet does not supports it, only TypeList.
Regarding the second part, as of today it is not supported to have more than one at this point, we have on the roadmap support for two, CIFS+NFS3 and CIFS+NFS4 but never the three of them or NFS3+NFS4 combination. Have you checked if the export policy got created? It will not throw an error, it just don't get the export policy created.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that makes sense. Thanks for clarifying. I just saw the values were put into the template but I didn't deploy the template.

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

"cifs_enabled": {
paulomarquesc marked this conversation as resolved.
Show resolved Hide resolved
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,6 +207,10 @@ 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()

Expand All @@ -176,6 +220,7 @@ func resourceArmNetAppVolumeCreateUpdate(d *schema.ResourceData, meta interface{
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 +281,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 @@ -280,7 +325,7 @@ func resourceArmNetAppVolumeDelete(d *schema.ResourceData, meta interface{}) err
if features.SupportsCustomTimeouts() {
stateConf.Timeout = d.Timeout(schema.TimeoutDelete)
} else {
stateConf.Timeout = 20 * time.Minute
stateConf.Timeout = 60 * time.Minute
}

if _, err := stateConf.WaitForState(); err != nil {
Expand Down Expand Up @@ -314,17 +359,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.(*schema.Set).List()
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 v2
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 @@ -354,17 +424,29 @@ func flattenArmNetAppVolumeExportPolicyRule(input *netapp.VolumePropertiesExport
if v := item.AllowedClients; v != nil {
allowedClients = strings.Split(*v, ",")
}
// Start - Remove in v2.0
cifsEnabled := false
nfsv3Enabled := false
nfsv4Enabled := false
// End - Remove in v2.0
protocolsEnabled := []string{}
if v := item.Cifs; v != nil {
cifsEnabled = *v
if *v {
protocolsEnabled = append(protocolsEnabled, "CIFS")
}
cifsEnabled = *v // Remove in v2.0
}
nfsv3Enabled := false
if v := item.Nfsv3; v != nil {
nfsv3Enabled = *v
if *v {
protocolsEnabled = append(protocolsEnabled, "NFSv3")
}
nfsv3Enabled = *v // Remove in v2.0
}
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 // Remove in v2.0
}
unixReadOnly := false
if v := item.UnixReadOnly; v != nil {
Expand All @@ -376,13 +458,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),
// Remove in v2.0
"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 @@ -177,7 +177,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