Skip to content

Conversation

kayrus
Copy link
Contributor

@kayrus kayrus commented Jul 2, 2025

I could not find any docs about external_fixed_ip struct to support omitted subnet_id, but I created test case in terraform provider and it works as expected, see terraform-provider-openstack/terraform-provider-openstack#1915

As for the port's subnet_id, the omission behavior is documented here:

The IP addresses for the port. If you would like to assign multiple IP addresses for the port, specify multiple entries in this field. Each entry consists of IP address (ip_address) and the subnet ID from which the IP address is assigned (subnet_id).
If you specify both a subnet ID and an IP address, OpenStack Networking tries to allocate the IP address on that subnet to the port.
If you specify only a subnet ID, OpenStack Networking allocates an available IP from that subnet to the port.
If you specify only an IP address, OpenStack Networking tries to allocate the IP address if the address is a valid IP for any of the subnets on the specified network.

@github-actions github-actions bot added edit:networking This PR updates networking code semver:patch No API change backport-v2 This PR will be backported to v2 labels Jul 2, 2025
@coveralls
Copy link

Coverage Status

coverage: 63.702%. remained the same
when pulling 3a9dda3 on kayrus:router-gateway-omit-subnet
into f98c2f9 on gophercloud:main.

Copy link
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

I don't think this has any effect. You're adding an omitempty for fields in structs that are returned to the user, but according to the docs, the subnet ID is always populated:

The IP addresses for the port. If the port has multiple IP addresses, this field has multiple entries. Each entry consists of IP address (ip_address) and the subnet ID from which the IP address is assigned (subnet_id).

That said, the change is not harmful, and it doesn't really makes sense to me why IPAddress had an omitempty tag, so... I guess we can add one for SubnetID for consistency?

@kayrus
Copy link
Contributor Author

kayrus commented Jul 9, 2025

You're adding an omitempty for fields in structs that are returned to the user

not really, this struct is used in CreateOpts and UpdateOpts as well and these tags are taken into account in json marshaller.

@mandre
Copy link
Contributor

mandre commented Jul 9, 2025

You're adding an omitempty for fields in structs that are returned to the user

not really, this struct is used in CreateOpts and UpdateOpts as well and these tags are taken into account in json marshaller.

OK, I see the ExternalFixedIP being used via GatewayInfo in the router's request, but I can't find where the IP struct is used in port's requests.

Copy link
Contributor

@mandre mandre left a comment

Choose a reason for hiding this comment

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

The neutron docs for the router creation states that:

The external gateway information of the router. If the router has an external gateway, this would be a dict with network_id, enable_snat, external_fixed_ips and qos_policy_id. Otherwise, this would be null.

However, it doesn't say what format external_fixed_ips must have. I think it's fair to add an omitempty for the subnet ID. And you have test cases to show that SubnetID is indeed optional, so all good.

@kayrus
Copy link
Contributor Author

kayrus commented Jul 9, 2025

I can't find where the IP struct is used in port's requests.

you're right. I was referring to port CreateOpts, but then I realized that it has FixedIPs any, which gives a lot of freedom.

so all good.

👍

@kayrus kayrus merged commit 71abd83 into gophercloud:main Jul 9, 2025
18 checks passed
@kayrus kayrus deleted the router-gateway-omit-subnet branch July 9, 2025 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v2 This PR will be backported to v2 edit:networking This PR updates networking code semver:patch No API change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants