[ip-pools] Allow multiple default IP pools per silo #9561
Conversation
zeeshanlakhani
force-pushed
the
zl/multiple-default-pools
branch
from
7f3a5b6 to
661d029
Compare
Previously, each silo could only have one default IP pool. This change allows one default pool per (pool_type, ip_version) combination, enabling silos to have separate defaults for: - Unicast IPv4 - Unicast IPv6 - Multicast IPv4 - Multicast IPv6 This work previously branched off #9451, but is now off `main`, involving changes that have to do with the mcast lifecycle changes. Includes: - Each default can now be set or unset and demoted independently. Unsetting the unicast IPv4 default does not affect the multicast IPv4 default, for example. - Add `pool_type` and `ip_version` columns to `ip_pool_resource` (denormalized from parent `ip_pool` for unique index) - Replace unique index with partial index on (resource_id, pool_type, ip_version) WHERE is_default = true - Rename `IpPoolResourceLink` to `IncompleteIpPoolResource` to reflect that pool_type/ip_version are actually populated by the linking query - Add `ip_version` field to API params for default pool disambiguation - API versioning for backwards compatibility with older clients
zeeshanlakhani
force-pushed
the
zl/multiple-default-pools
branch
from
661d029 to
c11aae0
Compare
zeeshanlakhani
requested review from
bnaecker,
internet-diglett and
rcgoodfellow
|
FYI, even without the implicit lifecycle work, this still has mcast changes (as mcast is on |
bnaecker
left a comment
bnaecker
left a comment
There was a problem hiding this comment.
Thanks for splitting this out! Seems pretty straightforward to me, just a couple of quick questions.
| &instance_lookup, | ||
| ip_to_create.into_inner().pool, | ||
| pool, | ||
| ip_version, |
There was a problem hiding this comment.
It seems like somewhere we want to fail if you provide an IP version and a pool, and the version you gave doesn't match the version of the pool. Is that the case?
There was a problem hiding this comment.
I need to add this match check, good point.
| -- IPv6, preparing the schema for future multicast support. | ||
|
|
||
| -- Drop the old single-default constraint | ||
| DROP INDEX IF EXISTS omicron.public.one_default_ip_pool_per_resource; |
There was a problem hiding this comment.
I'm not certain that it matters, but we have generally had only one statement per SQL update file. I'd probably split these and the contents of up03.sql into two files.
|
@zeeshanlakhani Let me know if you need anything else here. I'd really like to get this merged so we can move forward on the IPv6 integration work. |
Previously, each silo could only have one default IP pool. This change allows one default pool per (pool_type, ip_version) combination, enabling silos to have separate defaults for:
This work previously branched off #9451, but is now off
main, NOT involving changes that have to do with the mcast lifecycle work. This supersedes #9452.Includes:
pool_typeandip_versioncolumns toip_pool_resource(denormalized from parentip_poolfor unique index)IpPoolResourceLinktoIncompleteIpPoolResourceto reflect that pool_type/ip_version are actually populated by the linking queryip_versionfield to API params for default pool disambiguation