📖proposal for multi-az apiserver loadbalancer #2660
Conversation
k8s-ci-robot
added
the
do-not-merge/hold
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @sebltm. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| - [api/v1beta1/openstackcluster_types.go](api/v1beta1/openstackcluster_types.go) | ||
| - [api/v1beta1/types.go](api/v1beta1/types.go) |
| - availabilityZoneSubnets []AZSubnetMapping (required to enable multi-AZ) | ||
| - Explicit mapping; each entry includes: | ||
| - availabilityZone string | ||
| - subnet SubnetParam | ||
| - The LB network MUST be specified when using this mapping via spec.apiServerLoadBalancer.network. Each mapped subnet MUST belong to that network. | ||
| - allowCrossAZLoadBalancerMembers *bool | ||
| - Default false. | ||
| - When true, register control plane nodes to all per-AZ LBs; otherwise same-AZ only. | ||
| - additionalPorts []int | ||
| - Optional extra listener ports besides the Kubernetes API port. | ||
| - allowedCIDRs []string | ||
| - Optional VIP ACL list when supported by the Octavia provider. |
There was a problem hiding this comment.
Could you put the code/yaml fields inside ticks so it is easier to read?
For example availabilityZoneSubnets []AZSubnetMapping.
| - [config/crd/bases/](config/crd/bases/) | ||
| - [config/crd/patches/](config/crd/patches/) |
| - [controllers/openstackcluster_controller.go](controllers/openstackcluster_controller.go) | ||
| - [pkg/cloud/services/loadbalancer/](pkg/cloud/services/loadbalancer/) | ||
| - [pkg/cloud/services/networking/](pkg/cloud/services/networking/) |
| end | ||
| ``` | ||
|
|
||
| ## Integration with External Global Server Load Balancing (GSLB) |
There was a problem hiding this comment.
This section about GSLB seems quite redundant to me. It doesn't really have anything to do with CAPO or the implementation of this feature. It just explains how it can fit in the larger ecosystem. I would prefer to reduce this to just a paragraph under the Motivation section.
| Restrict access using allowed CIDRs | ||
| ```yaml | ||
| spec: | ||
| apiServerLoadBalancer: | ||
| enabled: true | ||
| network: | ||
| id: 6c90b532-7ba0-418a-a276-5ae55060b5b0 | ||
| availabilityZoneSubnets: | ||
| - availabilityZone: az1 | ||
| subnet: | ||
| id: cad5a91a-36de-4388-823b-b0cc82cadfdc | ||
| - availabilityZone: az2 | ||
| subnet: | ||
| id: e2407c18-c4e7-4d3d-befa-8eec5d8756f2 | ||
| allowedCIDRs: | ||
| - 192.0.2.0/24 | ||
| - 203.0.113.10 | ||
| ``` |
There was a problem hiding this comment.
This is not new in this proposal so I would leave it out.
| - status.apiServerLoadBalancers is populated alongside legacy status until further cleanup. | ||
| - Disabling multi-AZ: | ||
| - Remove the mapping; controller maintains single-LB behavior. | ||
| - Per-AZ LBs are not automatically deleted; operators may clean up unused resources. |
There was a problem hiding this comment.
Why would we not clean them up automatically?
| - [pkg/cloud/services/loadbalancer/](pkg/cloud/services/loadbalancer/) | ||
| - [controllers/](controllers/) | ||
| - [test/e2e/](test/e2e/) |
| ## Non-Goals | ||
| - Managing or provisioning DNS records. | ||
| - Provider-specific topologies such as ACTIVE_STANDBY across fault domains. | ||
| - Service type LoadBalancer for worker Services. |
There was a problem hiding this comment.
It seems to me that automatic creation of the network and subnets is also a non-goal, right?
| ## Design Overview | ||
|
|
||
| ### High-level behavior | ||
| - When enabled and configured with an explicit mapping, CAPO reconciles one LoadBalancer per Availability Zone (AZ). |
There was a problem hiding this comment.
Perhaps add here that the operator creates the network and subnets?
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
TODOs:
/hold