-
Notifications
You must be signed in to change notification settings - Fork 367
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
Support for availability_zone_hints for networks #196
Support for availability_zone_hints for networks #196
Conversation
This allows users to associate an availability zone with their resources so that the resources get high availability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stone Nice work again :)
One small change and one additional comment. Let me know if you have any questions.
@@ -56,6 +56,12 @@ func dataSourceNetworkingNetworkV2() *schema.Resource { | |||
Type: schema.TypeString, | |||
Computed: true, | |||
}, | |||
"availability_zone_hints": &schema.Schema{ | |||
Type: schema.TypeSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to make sure TypeSet
is the correct choice here.
If you specified ["zone2", "zone1"]
in Terraform and Neutron replied with ["zone1", "zone2"]
, then TypeSet
is the correct choice. However, if Neutron respects the ordering you gave, then we can use a TypeList
instead.
The benefit of using TypeList
is that the user can then use numerical indexing to access the elements.
Unfortunately I don't have access to an environment that has more than one AZ to test with. Are you able to confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have done some empirical testing, and it looks like the ordering is kept, tried to look into the source for the field and it looks like a Python list (and the List "type") which is ordered.
What I did to test was to set different ordering of zones, and then check with "openstack" cli output.
I will do some more testing, and if I don't find anything more I think TypeList is the way to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - thank you!
@@ -183,6 +192,7 @@ func resourceNetworkingNetworkV2Read(d *schema.ResourceData, meta interface{}) e | |||
d.Set("shared", strconv.FormatBool(n.Shared)) | |||
d.Set("tenant_id", n.TenantID) | |||
d.Set("region", GetRegion(d, config)) | |||
d.Set("availability_zone_hints", n.AvailabilityZoneHints) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change here: when setting a non-scalar type (Set or List), we need to check for an error. We've only recently begun to do this so there aren't many cases of it being done, so it's no fault this was missed.
if err := d.Set("availability_zone_hints", n.AvailabilityZoneHints); err != nil {
log.Printf("[DEBUG] unable to set availability_zone_hints: %s", err)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, np, added in latest push to this PR.
Based on some code digging and some empirical tests the availability_zone_hints is a List. The documentation here is not crystal clear. Ref: https://github.com/openstack/neutron/blob/4a128494ae4d7d9d0223dad136e858b7ead83122/neutron/objects/network.py#L190 https://git.openstack.org/cgit/openstack/oslo.versionedobjects/tree/oslo_versionedobjects/fields.py#n1117 https://git.openstack.org/cgit/openstack/oslo.versionedobjects/tree/oslo_versionedobjects/fields.py#n634
Build failed.
|
Hmm.. something didn't work as expected, looking into it.. |
Also fixed error check to use short version.
Build succeeded.
|
This looks good to me. Thank you for your work on this :) |
* vendor: Updating Gophercloud * Allow setting availability_zone_hints on router. This allows users to associate an availability zone with their resources. ``` resource "openstack_networking_router_v2" "testrouter" { name = "testrouter" availability_zone_hints = ["zone1"] } ``` * Cleanup of vendor/vendor.json A fat-finger error from my side. * Break out resourceNetworkingAvailabilityZoneHintsV2 And use it from both resource_openstack_networking_network and resource_openstack_networking_router_v2. * Doc for networking_router_v2 az-hints. * Update on doc for #196 on az-hints.
This allows users to associate an availability zone with their network