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

azurerm_netapp_volume - support mount_ip_addresses #5526

Merged
merged 18 commits into from
May 20, 2020
Merged

azurerm_netapp_volume - support mount_ip_addresses #5526

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f810b05
Add more support for mountTargets
Jan 26, 2020
f607b5f
Update code
Jan 26, 2020
b79415e
Update code
Jan 28, 2020
eaaf155
Merge remote-tracking branch 'upstream/master' into fix_netapp_volume
Feb 4, 2020
c27bf6c
Update code
Feb 4, 2020
3e43f53
Update code
Feb 5, 2020
566602b
Update code
Mar 5, 2020
0a28add
Update code
Mar 5, 2020
372438f
Merge remote-tracking branch 'upstream/master' into fix_netapp_volume
Apr 13, 2020
ff0e09a
Update code
Apr 13, 2020
d076ce3
Update code
Apr 13, 2020
367f3ce
Update code
Apr 13, 2020
84e02a9
Update code
May 18, 2020
70f7012
go mod vendor
May 18, 2020
81f1171
Update code
May 18, 2020
1ca3640
Update code
May 18, 2020
10c4129
Update code
May 18, 2020
db409de
Update code
May 20, 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
16 changes: 16 additions & 0 deletions azurerm/internal/services/netapp/data_source_netapp_volume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ func dataSourceArmNetAppVolume() *schema.Resource {
ValidateFunc: ValidateNetAppPoolName,
},

"mount_targets": {
Type: schema.TypeSet,
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be a TypeList if it's a Computed field else it's not easily indexable - however - what are the mount targets here, a list of IP Addresses which should be used to mount the volume? If so it's likely this'd make more sense as a list of strings being mount_ip_addresses?

can we also update the test to confirm this field is correctly set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"ip_address": {
Type: schema.TypeString,
Computed: true,
},
},
},
},

"volume_path": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -104,6 +117,9 @@ func dataSourceArmNetAppVolumeRead(d *schema.ResourceData, meta interface{}) err
if props.UsageThreshold != nil {
d.Set("storage_quota_in_gb", *props.UsageThreshold/1073741824)
}
if err := d.Set("mount_targets", flattenArmNetAppVolumeMountTargets(props.MountTargets)); err != nil {
return fmt.Errorf("Error setting `mount_targets`: %+v", err)
}
}

return nil
Expand Down
20 changes: 20 additions & 0 deletions azurerm/internal/services/netapp/resource_arm_netapp_volume.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,3 +388,23 @@ func flattenArmNetAppVolumeExportPolicyRule(input *netapp.VolumePropertiesExport

return results
}

func flattenArmNetAppVolumeMountTargets(input interface{}) []interface{} {
results := make([]interface{}, 0)
if input == nil {
return results
}

for _, item := range input.([]interface{}) {
if item != nil {
v := item.(map[string]interface{})
ipAddress := v["ipAddress"]
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be:

Suggested change
ipAddress := v["ipAddress"]
ipAddress := v["ip_address"].(string)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It cannot be updated to "ip_address" since api returns ""mountTargets":[{"provisioningState":"Succeeded","ipAddress":"10.0.2.4"}]" for property mountTargets.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah so this nested object isn't modelled as an object in the Swagger? in which case can we send a PR to the Swagger repo to fix that

Copy link
Contributor

Choose a reason for hiding this comment

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

When it comes to mountTargets, it reminds me that I changed that some time ago... and it may be a mistake. I will take a look at this and see how to change it correctly (and when changing the swagger we should let the service team know and approve, which may take a long time).


results = append(results, map[string]interface{}{
"ip_address": ipAddress,
})
}
}

return results
}
8 changes: 8 additions & 0 deletions website/docs/d/netapp_volume.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ The following attributes are exported:

* `location` - The Azure Region where the NetApp Volume exists.

* `mount_targets` - The `mount_targets` block as defined below.

* `volume_path` - The unique file path of the volume.

* `service_level` - The service level of the file system.
Expand All @@ -51,6 +53,12 @@ The following attributes are exported:

* `storage_quota_in_gb` - The maximum Storage Quota in Gigabytes allowed for a file system.

---

The `mount_targets` block exports the following:

* `ip_address` - The IPv4 address of mount target.

### Timeouts

~> **Note:** Custom Timeouts are available [as an opt-in Beta in version 1.43 of the Azure Provider](/docs/providers/azurerm/guides/2.0-beta.html) and will be enabled by default in version 2.0 of the Azure Provider.
Expand Down