Skip to content

feat(kubernetes): gate zone-aware routing on ingress-controller zone balance - #4377

Open
MustafaSaber wants to merge 8 commits into
masterfrom
opt-out-zone-aware
Open

feat(kubernetes): gate zone-aware routing on ingress-controller zone balance#4377
MustafaSaber wants to merge 8 commits into
masterfrom
opt-out-zone-aware

Conversation

@MustafaSaber

@MustafaSaber MustafaSaber commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Serve zone-filtered routes only when skipper-ingress itself is balanced
across zones; otherwise fall back to the global table. Adds configurable
namespace/name/max-skew, closes a data race and a silent-misconfiguration
gap, and adds unit/config/end-to-end test coverage.

088ef1d show no regression in benchmarks against master

@MustafaSaber MustafaSaber added the architectural all changes in the hot path, big changes in the control plane, control flow changes in filters label Oct 23, 2026
greeshma1196
greeshma1196 reviewed Oct 25, 2026
View reviewed changes
Comment thread dataclients/kubernetes/kube.go Resolved
greeshma1196
greeshma1196 reviewed Oct 25, 2026
View reviewed changes
Comment thread config/config.go Resolved
@zalando-robot

zalando-robot commented Oct 25, 2026

Copy link
Copy Markdown

Docker image "registry-write.opensource.zalan.do/teapot/skipper:4537e8c34c46b2c4fcab78a4d70e474400ab4515" is not based on an approved base image. Any production deployment relying on this image will be blocked.

To create a compliant Docker image of your application, you must reference an allowed Docker image as its base image in your Dockerfile. This base image must come from the Zalando Container Registry namespace library and use a recommended version as listed in the documentation.

@greeshma1196

greeshma1196 commented Oct 25, 2026

Copy link
Copy Markdown
Member

👍🏽

szuecs
szuecs requested changes Oct 25, 2026
View reviewed changes
Comment thread dataclients/kubernetes/clusterclient.go Outdated
ingressValidator *definitions.IngressV1Validator
balancedAcrossZones atomic.Bool
ingressControllerNamespace string
ingressControllerName string

@szuecs szuecs Oct 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not a label selector?

@MustafaSaber MustafaSaber Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because we need namespace and name for the ResourceID anyway that we use to get endpointSlices

@szuecs szuecs Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But then we have not canary possible to include, what I don't like.
Please think about the problem of multiple deployments like we have in our setup.

@MustafaSaber MustafaSaber Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yeah nice catch, totally forgot about that. Updated now

Comment thread dataclients/kubernetes/clusterclient.go Resolved
Comment thread routesrv/polling.go Resolved
Comment thread routesrv/eskipbytes.go Resolved
@MustafaSaber
MustafaSaber force-pushed the opt-out-zone-aware branch 2 times, most recently from 1fa4be9 to 6888da1 Compare October 26, 2026 20:09
@MustafaSaber
use a zone aware clouser to check if ingress controller (skipper) was…
fec7ddb 
… distributed across zone in this current poll

Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@MustafaSaber
make it configurable and work on the skew not only missing zones
2020a23 
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@MustafaSaber
fix(kubernetes): close race, silent-misconfig, and validation gaps in…
a493a79 
… zone-aware gate

Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@MustafaSaber
naming is hard :)
ec341e7 
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@MustafaSaber
cover as much cases as we can with and without mock
78123d4 
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@MustafaSaber
Rebase and add another test and benchmark
91c67c2 
Benchmark results compared against origin/master

```
goos: linux
goarch: amd64
pkg: github.com/zalando/skipper/routesrv
cpu: Intel(R) Core(TM) Ultra 7 265U
                              │    before    │               after                │
                              │    sec/op    │    sec/op     vs base              │
FormatAndSet-14                 268.7m ± ∞ ¹   277.8m ± ∞ ¹       ~ (p=0.548 n=5)
FormatAndSetNoChange-14         60.45m ± ∞ ¹   63.66m ± ∞ ¹       ~ (p=0.151 n=5)
FormatAndSetOneZoneChanged-14   232.6m ± ∞ ¹   227.9m ± ∞ ¹       ~ (p=0.222 n=5)
geomean                         155.8m         159.1m        +2.16%
¹ need >= 6 samples for confidence interval at level 0.95

                │   before    │             after              │
                │ retained_MB │ retained_MB  vs base           │
FormatAndSet-14   1.986 ± ∞ ¹   1.986 ± ∞ ¹  ~ (p=1.000 n=5) ²
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal

                              │    before     │                after                │
                              │     B/op      │     B/op       vs base              │
FormatAndSet-14                 227.8Mi ± ∞ ¹   227.8Mi ± ∞ ¹  +0.00% (p=0.008 n=5)
FormatAndSetNoChange-14         86.34Mi ± ∞ ¹   86.35Mi ± ∞ ¹       ~ (p=0.421 n=5)
FormatAndSetOneZoneChanged-14   229.7Mi ± ∞ ¹   217.8Mi ± ∞ ¹  -5.15% (p=0.008 n=5)
geomean                         165.3Mi         162.4Mi        -1.75%
¹ need >= 6 samples for confidence interval at level 0.95

                              │    before    │               after                │
                              │  allocs/op   │  allocs/op    vs base              │
FormatAndSet-14                 2.380M ± ∞ ¹   2.380M ± ∞ ¹  +0.00% (p=0.008 n=5)
FormatAndSetNoChange-14         700.0k ± ∞ ¹   700.0k ± ∞ ¹       ~ (p=1.000 n=5)
FormatAndSetOneZoneChanged-14   2.483M ± ∞ ¹   2.268M ± ∞ ¹  -8.65% (p=0.008 n=5)
geomean                         1.605M         1.558M        -2.97%
¹ need >= 6 samples for confidence interval at level 0.95
```

Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@MustafaSaber
update current test to check for global routing table hash
5d8f4c9 
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@zalando-robot

zalando-robot commented Oct 26, 2026

Copy link
Copy Markdown

Docker image "registry-write.opensource.zalan.do/teapot/skipper:5d8f4c937c408106579470772154973481211225" is not based on an approved base image. Any production deployment relying on this image will be blocked.

To create a compliant Docker image of your application, you must reference an allowed Docker image as its base image in your Dockerfile. This base image must come from the Zalando Container Registry namespace library and use a recommended version as listed in the documentation.

@MustafaSaber
Remove atomic bool, use labelSelectors instead of name/namespace comb…
3de4f88 
…ination to also catch canary

Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
@zalando-robot

zalando-robot commented Oct 28, 2026

Copy link
Copy Markdown

Docker image "registry-write.opensource.zalan.do/teapot/skipper:3de4f887b0d42adce00bb4307645af6fe3f1e772" is not based on an approved base image. Any production deployment relying on this image will be blocked.

To create a compliant Docker image of your application, you must reference an allowed Docker image as its base image in your Dockerfile. This base image must come from the Zalando Container Registry namespace library and use a recommended version as listed in the documentation.

@greeshma1196

greeshma1196 commented Oct 30, 2026

Copy link
Copy Markdown
Member

👍🏽

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szuecs szuecs szuecs requested changes
@greeshma1196 greeshma1196 greeshma1196 left review comments

Assignees

No one assigned

Labels

architectural all changes in the hot path, big changes in the control plane, control flow changes in filters

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MustafaSaber @zalando-robot @greeshma1196 @szuecs