Repository navigation
Commit 42cf2ec
committed
### What changes were proposed in this pull request?
This PR aims to support K8s events for `SparkApplication` and `SparkCluster` held by `spec.suspend`.
| Type | Reason | When |
|---|---|---|
| `Normal` | `SuspendHeld` | The resource is held by `spec.suspend`, so the driver (or master and worker) is not requested |
Like the `KueueAdmissionPending` event of SPARK-59638, it is republished while the hold lasts. The event sink keys the `Event` on the reason, so a repeat bumps the `count` of the one `Event` and refreshes it instead of creating another. An application held in `ScheduledToRestart` gets the same event, because the status of its previous attempt says that a restart is due, not that the next attempt is withheld. Resuming publishes no event of its own, since the following state transition already reports it.
Unlike the Kueue hold, which ends when quota arrives, this one ends only when a user clears `spec.suspend`. Republishing on every reconcile would therefore cost a read and a write on the API server for as long as a resource is parked, so the hold is requeued every 30 minutes instead, which still refreshes the event well within the one hour that the API server retains events by default. Nothing waits for that interval, since clearing `spec.suspend` arrives as a watch event.
The reason is `SuspendHeld` rather than `Suspended` because state transition events use the state name as both the reason and the dedup key, so the latter would collide with the dedicated `Suspended` state planned in a follow-up of SPARK-59542.
In addition,
- `SuspendUtils.holdForSuspend(BaseContext<?>, String)` holds the policy for both init steps, mirroring how SPARK-59638 extracted `KueueWorkloadUtils.holdForAdmission`. Each step keeps only its own guard.
- Suspending a queued resource releases its Kueue `Workload`, and the message now says so: the `KueueAdmissionPending` event it was queued with has no owner reference and outlives that `Workload`, so it would otherwise contradict the new event until the API server drops it.
- A new `suspend-events` E2E group asserts the event for both resource kinds, and a new `events` CI mode installs the operator with `spark.kubernetes.operator.events.enabled=true` through `tests/e2e/helm/events-config-values.yaml`. The existing `suspend` group keeps running with the shipped defaults.
- The description of `spark.kubernetes.operator.events.enabled` covers the new event.
### Why are the changes needed?
A suspended resource has no state transition, so users cannot see why it is stuck via `kubectl describe`. The initial `Submitted` status of a resource created with `suspend: true` is not persisted either, so `kubectl get` shows an empty `Current State` as well.
Publishing the event once would not be enough: Kubernetes retains events only for a limited time (one hour by default), so a resource suspended for longer would end up with neither a status nor an event.
### Does this PR introduce _any_ user-facing change?
Yes. When `spark.kubernetes.operator.events.enabled` is `true`, the new event is published for resources held by `spec.suspend`.
To opt out, add `SuspendHeld` to `spark.kubernetes.operator.events.excludedReasons`.
### How was this patch tested?
Pass the CIs with the newly added unit tests in `AppInitStepTest` and `ClusterInitStepTest`, and the new `suspend-events` E2E group which asserts the event for both `SparkApplication` and `SparkCluster`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 5
Closes #854 from dongjoon-hyun/SPARK-59677.
Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent 4147887 commit 42cf2ec
17 files changed
Lines changed: 686 additions & 56 deletions
File tree
- .github/workflows
- docs
- spark-operator/src
- main/java/org/apache/spark/k8s/operator
- config
- context
- kueue
- reconciler/reconcilesteps
- utils
- test/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps
- tests/e2e
- helm
- suspend-events
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
148 | 151 | | |
149 | 152 | | |
150 | 153 | | |
| |||
244 | 247 | | |
245 | 248 | | |
246 | 249 | | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
247 | 265 | | |
248 | 266 | | |
249 | 267 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| 55 | + | |
55 | 56 | | |
56 | 57 | | |
57 | 58 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
76 | 76 | | |
77 | 77 | | |
78 | 78 | | |
79 | | - | |
80 | | - | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
81 | 82 | | |
82 | 83 | | |
83 | 84 | | |
| 85 | + | |
84 | 86 | | |
85 | 87 | | |
86 | 88 | | |
87 | 89 | | |
88 | 90 | | |
89 | | - | |
| 91 | + | |
| 92 | + | |
90 | 93 | | |
91 | 94 | | |
92 | 95 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
536 | 536 | | |
537 | 537 | | |
538 | 538 | | |
539 | | - | |
540 | | - | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
541 | 546 | | |
542 | 547 | | |
543 | 548 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
262 | 262 | | |
263 | 263 | | |
264 | 264 | | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
265 | 294 | | |
266 | 295 | | |
267 | 296 | | |
| |||
292 | 321 | | |
293 | 322 | | |
294 | 323 | | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
299 | 328 | | |
300 | 329 | | |
301 | 330 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
100 | | - | |
| 100 | + | |
| 101 | + | |
101 | 102 | | |
102 | 103 | | |
103 | 104 | | |
104 | 105 | | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
105 | 126 | | |
106 | 127 | | |
107 | 128 | | |
| |||
120 | 141 | | |
121 | 142 | | |
122 | 143 | | |
123 | | - | |
124 | | - | |
125 | | - | |
126 | | - | |
127 | | - | |
128 | | - | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
140 | 152 | | |
141 | 153 | | |
| 154 | + | |
142 | 155 | | |
143 | 156 | | |
144 | 157 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
357 | 357 | | |
358 | 358 | | |
359 | 359 | | |
360 | | - | |
361 | | - | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
362 | 363 | | |
363 | 364 | | |
364 | 365 | | |
| 366 | + | |
365 | 367 | | |
366 | | - | |
| 368 | + | |
367 | 369 | | |
368 | 370 | | |
369 | | - | |
| 371 | + | |
370 | 372 | | |
371 | 373 | | |
372 | 374 | | |
373 | | - | |
| 375 | + | |
| 376 | + | |
374 | 377 | | |
375 | 378 | | |
| 379 | + | |
376 | 380 | | |
377 | 381 | | |
378 | 382 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
34 | 34 | | |
35 | 35 | | |
36 | 36 | | |
| 37 | + | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
| |||
69 | 70 | | |
70 | 71 | | |
71 | 72 | | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
77 | 86 | | |
78 | | - | |
79 | 87 | | |
80 | 88 | | |
81 | 89 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
66 | 66 | | |
67 | 67 | | |
68 | 68 | | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
| 69 | + | |
75 | 70 | | |
76 | 71 | | |
77 | 72 | | |
| |||
0 commit comments