Repository navigation
Shared Filesystems: Share Export Locations - #381
Conversation
9260137 to
e63f72a
Compare
e63f72a to
56c4efc
Compare
pospispa
changed the title
56c4efc to
5561eaf
Compare
pospispa
changed the title
5561eaf to
9eb7beb
Compare
pospispa
changed the title
In order to mount a share one needs to know the share export location. That's why the Get Share Export Locations request is added.
9eb7beb to
7b621d3
Compare
|
@jtopjian can we just add the Get Export Locations functionality without microversion check and let the server return an error in case the client did not use a proper microversion? Should I create an issue that the microversion check shall be added later? |
| Path string `json:"path"` | ||
| // The UUID of the share instance that this export location belongs to. | ||
| ShareInstanceID string `json:"share_instance_id"` | ||
| // Defines purpose of an export location. If set to true, then it is expected to be used for service needs and by administrators only. If it is set to false, then this export location can be used by end users. |
There was a problem hiding this comment.
nit: Can you wrap this comment to around 80 characters?
| IsAdminOnly bool `json:"is_admin_only"` | ||
| // The share export location UUID. | ||
| ID string `json:"id"` | ||
| // Drivers may use this field to identify which export locations are most efficient and should be used preferentially by clients. By default it is set to false value. New in version 2.14 |
There was a problem hiding this comment.
nit: Can you wrap this comment to around 80 characters?
| // ExtractExportLocations will get the Export Locations from the commonResult | ||
| func (r commonResult) ExtractExportLocations() ([]ExportLocation, error) { | ||
| var s struct { | ||
| GetExportLocationsRes []ExportLocation `json:"export_locations"` |
There was a problem hiding this comment.
I think this should be:
ExportLocations []ExportLocation `json:"export_locations"`
|
This looks good to me. I was able to get a share created in a test environment and the API results matched with what's here and in the referenced Manila code. |
| } | ||
|
|
||
| // ExtractExportLocations will get the Export Locations from the commonResult | ||
| func (r commonResult) ExtractExportLocations() ([]ExportLocation, error) { |
There was a problem hiding this comment.
Why is this a commonResult instead of a GetExportLocationResult? Can every response type extract this data?
There was a problem hiding this comment.
I caught that as well last night and I saw something else in the sharedfilesystems packages that made me think it was ok, but I most likely got it mixed up with the same occurrence in #346.
I doubt this is function is applicable to all. I don't see how a share result can extract this information. I can follow-up with a PR to fix.
There was a problem hiding this comment.
Ah! This is what it was: https://github.com/gophercloud/gophercloud/blob/master/openstack/sharedfilesystems/v2/sharenetworks/results.go#L172-L182
But upon reading further, I see that those actions return a share network and not some other result. So this is my mistake for not reading further.
New versions of OpenStack are automatically adding a property of os_hidden with a boolean value. This is conflicting with Terraform's TypeMap which expects all values to be a string. This commit fixes the issue by filtering out non-string properties.
This commit updates the task to monitor the SSH port closure using local_action to avoid remote not reachable. This change aligns with StarlingX upstream code: https://opendev.org/starlingx/ansible-playbooks/src/branch/master/playbookconfig/src/playbooks/roles/common/host-unlock/tasks/main.yml#L46-L62
4 participants
For #380
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
https://github.com/openstack/manila/blob/master/manila/api/v2/share_export_locations.py#L25