WIP: Application load balancer controller#3
Conversation
|
@fischerman either mark the PR as "Draft" or use "WIP" as the title. The PR will then automatically marked as WIP/Draft |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Co-authored-by: Kamil Przybyl <Kamil.Przybyl@digits.schwarz>
There was a problem hiding this comment.
I've copied the file over from the CCM.
| } | ||
|
|
||
| ingressList := &networkingv1.IngressList{} | ||
| if err := c.List(ctx, ingressList, client.InNamespace(secret.Namespace)); err != nil { |
There was a problem hiding this comment.
That could probably be optimized with MatchingFields
dergeberl
left a comment
There was a problem hiding this comment.
This is my first batch of review comments. Only had a look into the /cmd/*, /pkg/controller/* (except the tests) and the *.md files, yet. I will continue with my review for the rest.
Maybe some comments are already resolved as you also pushed during my review (I did not crosscheck again, before submitting the comments now).
| | `alb.stackit.cloud/traget-pool-tls-skip-certificate-validation`| Boolean | IngressClass, Ingress, Service | Optional | Enables TLS bridging but skips certificate validation. | | ||
| | `alb.stackit.cloud/allowed-source-ranges`| String | IngressClass | Accepts a comma-separated list of IP ranges. E.g. 10.0.0.0/24,1.2.3.4/32. If unset, all IPs are allowed. | | ||
|
|
||
| ### Known Limitations |
There was a problem hiding this comment.
Maybe we add the NodePort requirement here? Also there are limits in the amount of resources (like listeners) which we should add here (or somewhere else).
| ### Limits | ||
|
|
||
| The following limitations are imposed directly by the STACKIT ALB API (not the controller itself): | ||
| - Maximum targets per pool: An individual target pool can contain a maximum of 250 targets. |
|
|
||
| #### When to watch out for the listener limit | ||
|
|
||
| Because each IngressClass provisions a dedicated ALB instance, hitting the 20-listener threshold is rarely an issue for a basic setup but becomes a real risk when you start stacking custom ports across multiple applications sharing that same ALB. If your Ingress resources use the `alb.stackit.cloud/http-port` or `alb.stackit.cloud/https-port` annotations to expose different apps on unique custom port numbers, each distinctive port allocates its own listener on the shared ALB instance. This risk compounds quickly when those applications also require TLS encryption; since the controller must keep an extra HTTP listener active alongside the HTTPS listener to smoothly process automated ACME certificate challenges, a single secure app immediately consumes two slots instead of one, accelerating how fast you approach the API limit if multiple unique custom ports are configured. |
There was a problem hiding this comment.
This needs a rephrase. The ALB per ingress does not make it rarely to hit the limit (It is the default of 2 ports).
|
|
||
| A "target" in a pool corresponds directly to a worker node in your cluster. If you run a large cluster with a high number of worker nodes, or expect your cluster to dynamically scale to a large size, keep this limit in mind since a single backend Service port mapping cannot route traffic to more than 250 worker nodes simultaneously. | ||
|
|
||
| #### When to watch out for the listener limit |
There was a problem hiding this comment.
Also something for the Known Limitations.
|
|
||
| Because each IngressClass provisions a dedicated ALB instance, hitting the 20-listener threshold is rarely an issue for a basic setup but becomes a real risk when you start stacking custom ports across multiple applications sharing that same ALB. If your Ingress resources use the `alb.stackit.cloud/http-port` or `alb.stackit.cloud/https-port` annotations to expose different apps on unique custom port numbers, each distinctive port allocates its own listener on the shared ALB instance. This risk compounds quickly when those applications also require TLS encryption; since the controller must keep an extra HTTP listener active alongside the HTTPS listener to smoothly process automated ACME certificate challenges, a single secure app immediately consumes two slots instead of one, accelerating how fast you approach the API limit if multiple unique custom ports are configured. | ||
|
|
||
| ### Configuration |
There was a problem hiding this comment.
Please check that this are all, WAF seems to be missing
| @@ -0,0 +1,5 @@ | |||
| # Application load balancer (ALB) controller | |||
|
|
|||
There was a problem hiding this comment.
Please add a short description what this controller does and a hint in the beginning that this is alpha and probably get bigger changes.
|
@fischerman: The following test failed, say
Full PR test history. Your PR dashboard. Command help for this repository. DetailsInstructions 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. I understand the commands that are listed here. |
No description provided.