Repository navigation
Commit 4c61f38
K8s: Replace external scaler jobScalingStrategy with includeOngoingSessions (#3218)
* K8s: Replace external scaler jobScalingStrategy with includeOngoingSessions
Aligns the Selenium Grid KEDA external scaler with the trigger param
merged into the built-in scaler upstream (kedacore/keda#8107), so the two
implementations stay a mechanical swap of trigger `type`.
The external scaler previously took `jobScalingStrategy`
(default|custom|accurate|eager) and derived the count convention from it.
Upstream settled on a plain boolean instead, matching how other scalers
expose the same choice (SQS `scaleOnInFlight`, RabbitMQ
`excludeUnacknowledged`, Beanstalkd `includeDelayed`, Temporal
`includeRunningWorkflowCount`).
- metadata: `JobScalingStrategy string` -> `IncludeOngoingSessions bool`
(default true); enum validation replaced by strconv.ParseBool.
- server: scrapeAndCount adds onGoingSessions only when the flag is set,
matching upstream GetMetricsAndActivity.
- chart: `_helpers.tpl` derives `includeOngoingSessions` from
`autoscaling.scaledJobOptions.scalingStrategy.strategy` - false for
`accurate`/`eager`, true otherwise. Override key is now
`<node>.hpa.includeOngoingSessions`. The chart default strategy is
`accurate`, so the default render now emits "false", which is the fix
for the runaway Node creation in #3167.
`jobScalingStrategy` is removed rather than deprecated; it only ever
existed on the external scaler and never reached the built-in one.
Note the patched built-in scaler copy in `.keda/scalers/` is deliberately
left tracking upstream main; it picks the param up when #8107 merges.
Relates to #3167
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwsdd5D5hWWXiPJ1hdPBzh
* K8s: Point scale test's previous-version Nodes at GHCR
Previous-version Nodes in the TEST_MULTIPLE_VERSIONS scale test pulled
from Docker Hub, which hits the anonymous pull rate limit when a scale
test brings many of them up at once (observed in
https://github.com/SeleniumHQ/docker-selenium/actions/runs/33747779946/attempts/2).
Rather than editing charts/selenium-grid/multiple-nodes-platform-version.yaml
- shipped chart documentation/example content that should stay on the
default (Docker Hub) registry for real users - control this from tests/:
- Add tests/charts/ci/multiple-nodes-platform-version-values.yaml, a
test-only overlay that pins previous-version Nodes to
ghcr.io/seleniumhq (anonymous, unmetered for public images).
- Prune its version range to what tests/SeleniumTests/__init__.py
actually requests (LIST_CHROMIUM_VERSIONS / LIST_FIREFOX_VERSIONS =
144.0-147.0), instead of mirroring the shipped file's full historical
list. Versions outside that range never receive a session request
during the test, so they never scale up and never pull an image
anyway; keeping them only added scaler triggers with no coverage
benefit.
- Point chart_test.sh's TEST_MULTIPLE_VERSIONS branch at this overlay
instead of the chart's file.
All ten pinned tags (chrome/edge 144-146, firefox 144-147, each
-20260808) were confirmed present on ghcr.io/seleniumhq before this
change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwsdd5D5hWWXiPJ1hdPBzh
* K8s: Bump test matrix K8s/Helm versions by one minor release
Rolling maintenance bump: every k8s-version and helm-version entry
across the K8s test matrices advances by one minor release, each to
its latest published patch:
k8s: 1.29->1.30, 1.30->1.31, 1.31->1.32, 1.32->1.33, 1.33->1.34, 1.34->1.35
helm: 3.14->3.15, 3.15->3.16, 3.16->3.17, 3.17->3.18, 3.18->3.19, 3.19->3.20, 4.0->4.1
Applies to helm-chart-test.yml, k8s-scaling-test.yml, and
k8s-dynamic-grid-test.yml (k8s-version only, no helm-version there).
docker-version/python-version are untouched.
Note: this does not fully clear the isCIDR/Calico CRD failure seen on
https://github.com/SeleniumHQ/docker-selenium/actions/runs/33747779946/attempts/2
- job_https now lands on k8s v1.30.14, the same version that failed as
job_hostname before this bump (v1.31.14 is the confirmed-working floor
for Calico's current CRD). Tracked as a separate follow-up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwsdd5D5hWWXiPJ1hdPBzh
* K8s: Test both default and accurate ScaledJob strategies, hard-fail on pod-count mismatch
Scale testing only ever exercised the "default" ScaledJob strategy, even
though the shipped chart default is "accurate" - the exact strategy
#3167 and the includeOngoingSessions fix (PR #3218) target. Add matching
coverage, and turn the existing (unused) pod-count check into a real
assertion instead of a warn-only log.
Makefile:
- Add test_k8s_autoscaling_job_count_strategy_accurate(_in_chaos|
_with_node_max_sessions), mirroring the "default" targets by reusing
test_k8s_autoscaling_job_count_strategy_default with
SCALING_STRATEGY=accurate (already forwarded to
autoscaling.scaledJobOptions.scalingStrategy.strategy by chart_test.sh
- no new plumbing needed).
.github/workflows/k8s-scaling-test.yml:
- Add 3 matrix entries for the new "accurate" targets, each pinned to
the same k8s/helm/docker/python versions as its "default" counterpart
so a difference in results reflects the strategy, not the environment.
tests/AutoscalingTests/common.py:
- wait_for_count_matches() was dead code (never called) and, even if
called, only warned on mismatch. It also compared pod count to raw
session count, which is wrong once sessions span multiple browser
types Chrome/Firefox/Edge scale independently, so the correct expected
count is the sum of per-browser ceil(sessions/node_max_sessions), not
one ceiling over the total.
- Rewritten to compute that per-browser expected count and raise
AssertionError immediately if the poll times out without matching -
a hard failure, not a log line. This makes it a direct regression
guard for #3167: a strategy that double-counts on-going sessions
never converges pod count to the expected value, so the poll times
out and the test fails.
- create_session/create_sessions_in_parallel now track (driver,
browser_name) tuples so per-browser counts are available;
close_all_sessions/randomly_quit_sessions updated accordingly.
tests/AutoscalingTests/test_scale_up.py, test_scale_chaos.py:
- Call wait_for_count_matches(SESSIONS, TEST_NODE_MAX_SESSIONS) right
after each iteration's scale-up, before the next create/quit cycle.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwsdd5D5hWWXiPJ1hdPBzh
* K8s: Fix job_https Setup Kubernetes cluster failure (Calico isCIDR)
https://github.com/SeleniumHQ/docker-selenium/actions/runs/33781949421/job/100765325753
Confirms the residual risk flagged in 55011ea: after the +1 minor bump,
job_https landed on k8s v1.30.14 - the same version that failed as
job_hostname before that bump. Calico's ClusterNetworkPolicy CRD uses
the CEL builtin isCIDR(self), which v1.30's CEL library doesn't
support ("undeclared reference to 'isCIDR'"), so `minikube start`
fails applying the CNI manifest every retry.
Move job_https to v1.31.14 (matching helm v3.16.4) - the confirmed
working floor, since job_hostname already runs there successfully.
Every k8s-version across all three workflows is now >= v1.31.14.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hwsdd5D5hWWXiPJ1hdPBzh
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 1d1cf83 commit 4c61f38
22 files changed
Lines changed: 317 additions & 129 deletions
File tree
- .github/workflows
- .keda-external-scaler
- cmd/scaler
- deploy
- internal/gridscaler
- charts/selenium-grid
- templates
- tests
- AutoscalingTests
- charts
- ci
- make
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
52 | | - | |
| 52 | + | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
62 | | - | |
| 62 | + | |
63 | 63 | | |
64 | | - | |
| 64 | + | |
65 | 65 | | |
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
| 82 | + | |
83 | 83 | | |
84 | | - | |
| 84 | + | |
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
92 | | - | |
| 92 | + | |
93 | 93 | | |
94 | | - | |
| 94 | + | |
95 | 95 | | |
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | 101 | | |
102 | | - | |
| 102 | + | |
103 | 103 | | |
104 | | - | |
| 104 | + | |
105 | 105 | | |
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
109 | 109 | | |
110 | 110 | | |
111 | 111 | | |
112 | | - | |
| 112 | + | |
113 | 113 | | |
114 | | - | |
| 114 | + | |
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
121 | 121 | | |
122 | | - | |
| 122 | + | |
123 | 123 | | |
124 | | - | |
| 124 | + | |
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
75 | 75 | | |
76 | | - | |
| 76 | + | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
81 | | - | |
| 81 | + | |
82 | 82 | | |
83 | | - | |
| 83 | + | |
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
88 | | - | |
| 88 | + | |
89 | 89 | | |
90 | | - | |
| 90 | + | |
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
94 | 94 | | |
95 | | - | |
| 95 | + | |
96 | 96 | | |
97 | | - | |
98 | | - | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
99 | 113 | | |
100 | 114 | | |
101 | | - | |
| 115 | + | |
102 | 116 | | |
103 | 117 | | |
104 | 118 | | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
105 | 126 | | |
106 | 127 | | |
107 | 128 | | |
108 | 129 | | |
109 | | - | |
| 130 | + | |
110 | 131 | | |
111 | | - | |
| 132 | + | |
112 | 133 | | |
113 | 134 | | |
114 | 135 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
46 | | - | |
| 46 | + | |
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
54 | 78 | | |
55 | 79 | | |
56 | 80 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
81 | | - | |
82 | | - | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
83 | 84 | | |
84 | 85 | | |
85 | 86 | | |
| |||
151 | 152 | | |
152 | 153 | | |
153 | 154 | | |
154 | | - | |
| 155 | + | |
155 | 156 | | |
156 | 157 | | |
157 | 158 | | |
| |||
166 | 167 | | |
167 | 168 | | |
168 | 169 | | |
169 | | - | |
| 170 | + | |
170 | 171 | | |
171 | 172 | | |
172 | 173 | | |
| |||
236 | 237 | | |
237 | 238 | | |
238 | 239 | | |
239 | | - | |
| 240 | + | |
240 | 241 | | |
241 | 242 | | |
242 | 243 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
| 32 | + | |
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
50 | | - | |
| 50 | + | |
51 | 51 | | |
52 | 52 | | |
53 | 53 | | |
54 | | - | |
| 54 | + | |
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
82 | | - | |
83 | | - | |
| 82 | + | |
| 83 | + | |
84 | 84 | | |
85 | 85 | | |
86 | 86 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
9 | | - | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
45 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
46 | 49 | | |
47 | 50 | | |
48 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
4 | | - | |
5 | | - | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
| |||
0 commit comments