Skip to content

Commit 05fb765

Browse files
dobrerazvanclaude
dobrerazvan
and
claude
authored
fix(kafka): resolve disk-removal deadlock during rolling upgrade (#254)
* Fix disk removal deadlock during rolling upgrade When a broker pod is deleted during rolling upgrade and a disk removal is pending (GracefulDiskRemovalScheduled), the operator enters a deadlock: reconcileKafkaPvc blocks the entire reconcile with "Disk removal pending", preventing reconcileKafkaPod from recreating the missing pod. Meanwhile, Cruise Control cannot complete the disk removal because the broker isn't running. Fix: move runningBrokers map building before reconcileKafkaPvc and pass it in. Before returning the blocking error, check if any broker with pending disk removal has a missing pod. If so, allow the reconcile to proceed so the pod can be recreated. The disk removal check is re-evaluated on the next cycle once the broker is back up. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address review findings: cover IsDiskRebalance, add mountPath to log, add tests - Fix #1 (HIGH): Override now checks IsDiskRebalance() in addition to IsDiskRemoval(), closing the same deadlock vector for rebalance states - Fix #2 (LOW): Include mountPath in the bypass log message for consistency with other disk-removal log messages - Fix #3 (LOW): Add tests for rebalance-state deadlock bypass and for newly-marked-for-removal with missing pod Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add OpenSpec artifacts for disk removal deadlock fix Proposal, design, and task tracking for the fix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(kafka): don't block reconcile on stalled disk removal/rebalance A terminally-failed or paused Cruise Control disk operation (GracefulDisk{Removal,Rebalance}CompletedWithError / Paused) was treated as "in progress" via IsDiskRemovalRunning(), so reconcileKafkaPvc returned CruiseControlTaskRunning indefinitely. With all broker pods present this froze the whole reconcile — including config rollout / rolling upgrade to healthy brokers — since reconcileKafkaPod runs downstream of the PVC block. Observed on a live 3-broker cluster stuck in ClusterRollingUpgrading with every removed disk in GracefulDiskRemovalCompletedWithError. Relaxing the block is data-safe: log.dirs retention (shouldKeepRemovedLogDirInConfig) and PVC mount retention both keep the removed disk in place until removal is confirmed *succeeded*, independent of this block. Cruise Control also does not hang on a dead broker — it marks the intra-broker task DEAD and completes with error — so a stalled state is genuinely terminal, not in-flight work a restart could disrupt. - add CruiseControlVolumeState.IsDiskOperationStalled() (CompletedWithError /Paused), mirroring IsDownscaleStalled - in handleDiskRemoval, set waitForDiskRemovalToFinish only for non-stalled IsDiskRemoval()/IsDiskRebalance() states; branch selection (PVC deletion, state marking) is unchanged - tests: removal CompletedWithError / Paused and rebalance CompletedWithError with pod present now proceed; Running + pod present still blocks Complements the existing missing-pod bypass: narrowing handles a stalled task with pods up; the bypass handles a genuinely-running task whose broker pod is missing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(e2e): verify config change + disk removal reconcile together Add an e2e spec that applies a single manifest carrying both a read-only broker config change (log.retention.hours, forces a rolling restart) and a disk removal, then asserts the cluster reconciles correctly: the config change propagates to broker ConfigMaps, the removed disk drops out of log.dirs, Cruise Control goes quiescent, and the cluster returns to ClusterRunning (a deadlock would time out this wait). - config/samples/simplekafkacluster_1disk_configchange.yaml: 2disk sample reduced to one disk (removes /kafka-logs3) plus a changed readOnlyConfig - tests/e2e/test_config_change_with_disk_removal.go: testConfigChangeWithDiskRemoval and brokerConfigMapsContainProperty helper - wire the spec into the suite after testMultiDiskRemoval (chains 2->1 disk) Covers the combined-operation happy path end to end; the stalled-removal and missing-pod deadlocks the fix targets remain covered by the unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(kafka): silence funlen for TestReconcileKafkaPvcDiskRemoval The added stalled-state cases pushed the table-driven test past the funlen limit (355 > 323). Annotate with //nolint:funlen, matching the existing convention for long table tests in this file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fb6d1c1 commit 05fb765

9 files changed

Lines changed: 930 additions & 14 deletions

File tree

‎api/v1beta1/common_types.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,18 @@ func (s CruiseControlVolumeState) IsDiskRemoval() bool {
102102
return s.IsDiskRemovalRunning() || s == GracefulDiskRemovalRequired
103103
}
104104

105+
// IsDiskOperationStalled returns true when a disk removal or rebalance task is in a
106+
// non-progressing state: CompletedWithError (the Cruise Control task finished with an
107+
// error) or Paused (halted, awaiting manual resume). In these states Cruise Control is
108+
// doing no work, so waiting on them to "finish" would block the reconcile indefinitely.
109+
// Mirrors IsDownscaleStalled at the broker-operation level.
110+
func (s CruiseControlVolumeState) IsDiskOperationStalled() bool {
111+
return s == GracefulDiskRemovalCompletedWithError ||
112+
s == GracefulDiskRebalanceCompletedWithError ||
113+
s == GracefulDiskRemovalPaused ||
114+
s == GracefulDiskRebalancePaused
115+
}
116+
105117
// IsUpscale returns true if CruiseControlState in GracefulUpscale* state.
106118
func (r CruiseControlState) IsUpscale() bool {
107119
return r == GracefulUpscaleRequired ||

‎config/samples/simplekafkacluster_1disk_configchange.yaml‎

Lines changed: 300 additions & 0 deletions
Large diffs are not rendered by default.

‎openspec/changes/fix-disk-removal-deadlock/design.md‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# Design: Fix Disk Removal Deadlock
2+
3+
## Architecture Context
4+
5+
The koperator reconcile loop in `pkg/resources/kafka/kafka.go` (main `Reconcile` function) runs these steps sequentially:
6+
7+
```
8+
reconcileKafkaPodDelete() // Line 265 - delete pods removed from spec
9+
↓
10+
reconcileKafkaPvc() // Line 326 - PVC lifecycle (create, resize, disk removal)
11+
↓ ← BLOCKS HERE when disk removal pending
12+
build runningBrokers map // Line 332 - query pod list
13+
↓
14+
reconcileKafkaPod() // Line 448 - per-broker pod create/update/rolling upgrade
15+
```
16+
17+
When `reconcileKafkaPvc` returns an error, all subsequent steps are skipped.
18+
19+
Inside `reconcileKafkaPvc`:
20+
- Iterates ALL brokers' PVCs
21+
- Calls `handleDiskRemoval()` when existing PVCs > desired PVCs
22+
- `handleDiskRemoval` sets `waitForDiskRemovalToFinish = true` for any non-succeeded removal state
23+
- At the end, if `waitForDiskRemovalToFinish` → returns `CruiseControlTaskRunning` error
24+
25+
Inside `reconcileKafkaPod`:
26+
- When `len(podList.Items) == 0` → creates pod (line 831-836)
27+
- When `len(podList.Items) == 1` → handles rolling upgrade via `handleRollingUpgrade()`
28+
29+
## Design Decision
30+
31+
### Where to put the check
32+
33+
**Option A**: Inside `handleDiskRemoval` — skip `waitForDiskRemovalToFinish = true` per-broker if pod missing.
34+
**Option B**: At the end of `reconcileKafkaPvc` — override the error if any broker with removal has missing pod.
35+
**Option C**: In the main reconcile — catch the error and decide whether to proceed.
36+
37+
**Chosen: Option B.** Reasons:
38+
- Minimal change surface (only the blocking decision at line 1278)
39+
- `handleDiskRemoval` still correctly tracks state and logs — no behavior change inside it
40+
- The main reconcile doesn't need to understand PVC internals
41+
- Easy to test: one function, one new parameter
42+
43+
### What data is needed
44+
45+
`reconcileKafkaPvc` needs to know which broker pods exist. The `runningBrokers` map (currently built at line 332) provides this. Move it earlier and pass it in.
46+
47+
### Behavioral change
48+
49+
| Scenario | Current | After Fix |
50+
|---|---|---|
51+
| Disk removal pending, all pods running | Block (error) | Block (error) — unchanged |
52+
| Disk removal pending, broker pod missing | Block (error) — DEADLOCK | Allow (nil) — pod gets created |
53+
| No disk removal pending | Allow (nil) | Allow (nil) — unchanged |
54+
55+
## Key Code Paths
56+
57+
### `handleDiskRemoval` (line 1285-1339)
58+
59+
```
60+
for each existing PVC not in desired:
61+
if volumeState not found → continue (removal done)
62+
if IsDiskRemovalSucceeded → delete PVC, delete status
63+
if IsDiskRemoval → waitForDiskRemovalToFinish = true ← these are the blocking states
64+
if IsDiskRebalance → waitForDiskRemovalToFinish = true ← (rebalance before removal)
65+
default → mark GracefulDiskRemovalRequired, wait = true ← initial marking
66+
return waitForDiskRemovalToFinish
67+
```
68+
69+
### `reconcileKafkaPvc` blocking (line 1278-1280)
70+
71+
```go
72+
if waitForDiskRemovalToFinish {
73+
return errorfactory.New(CruiseControlTaskRunning{}, "Disk removal pending", ...)
74+
}
75+
```
76+
77+
The fix adds a check before this return:
78+
```go
79+
if waitForDiskRemovalToFinish {
80+
// Check if any broker with pending removal has a missing pod
81+
for brokerId := range brokersDesiredPvcs {
82+
if _, podExists := runningBrokers[brokerId]; !podExists {
83+
if state has IsDiskRemoval volume → return nil
84+
}
85+
}
86+
return error // all relevant pods exist, block normally
87+
}
88+
```
89+
90+
## Edge Cases
91+
92+
1. **Multiple brokers with missing pods**: Still returns nil. All missing pods will be created on this reconcile cycle.
93+
94+
2. **Broker pod missing but NO disk removal for that broker**: `runningBrokers` missing + no IsDiskRemoval volume state → doesn't trigger the override. The error is still returned. This is intentional — we only bypass when the deadlock condition is present.
95+
96+
3. **Pod deleted between runningBrokers check and reconcileKafkaPod**: Possible but harmless — `reconcileKafkaPod` re-queries the pod list per broker (line 826).
97+
98+
4. **Disk removal completes while pod is being created**: The next reconcile cycle will see `IsDiskRemovalSucceeded` and clean up the PVC. No conflict.
99+
100+
## Follow-up: narrow the blocking wait to genuinely-progressing states
101+
102+
The missing-pod bypass above fixes the deadlock only while a pod is *missing*. A live incident (`pipeline-kafka`, 3 brokers, all `GracefulDiskRemovalCompletedWithError` on the removed disk) showed a second failure mode: with all pods **present** but the CC removal task terminally failed, `reconcileKafkaPvc` still blocks forever and the rolling upgrade (brokers `ConfigOutOfSync`) can never start a restart, because it lives downstream of the PVC block.
103+
104+
### Root cause
105+
106+
`GracefulDiskRemovalCompletedWithError` is bucketed under `IsDiskRemovalRunning()` (`common_types.go:83`), so a **failed** CC task is treated as "in progress" and blocks the reconcile indefinitely. There is no self-healing path unless the volume is on `ErrorPolicyIgnore` (`cruisecontroltask_types.go:147-148`).
107+
108+
### Why relaxing the block is data-safe
109+
110+
The blanket block is **not** what protects `log.dirs` integrity — two independent mechanisms already do, and both key off *success*, not the desired spec:
111+
112+
- **`log.dirs` retention**: `shouldKeepRemovedLogDirInConfig` (`configmap.go:312-335`) keeps the removed disk's mount path in `log.dirs` while the state is `IsDiskRemoval()`/`IsDiskRebalance()` (which includes `CompletedWithError`), dropping it only on `…Succeeded`. The KRaft path writes the shrunk set at `configmap.go:202`, but the protective merge at `configmap.go:90-97` runs afterward and overwrites it.
113+
- **PVC mount retention**: the pod mounts the *actually existing* PVCs (`generateDataVolumeAndVolumeMount`, `pod.go:438`; `getCreatedPvcForBroker`, `kafka.go:143` returns all existing PVCs). The removed disk's PVC is deleted only in `handleDiskRemoval`'s `IsDiskRemovalSucceeded()` branch.
114+
115+
So a `ConfigOutOfSync` broker can restart mid-removal with the disk still in `log.dirs` and still mounted — no stranded data.
116+
117+
### Cruise Control does not hang on a dead broker (verified against CC source)
118+
119+
`remove_disks` executes as intra-broker replica movement. If the broker (hence its destination disk) is down, CC's progress check marks the task DEAD ("Killing execution for task … because the destination disk is down", `Executor.java:2145-2151`), the wait loop exits (`Executor.java:2009`), and the user task reaches a terminal state — mapped by koperator to `…CompletedWithError`. It does **not** stay pinned in `IN_EXECUTION`. This is why the external-termination variant also lands in `CompletedWithError` and, pre-fix, deadlocks permanently.
120+
121+
### The change
122+
123+
Add `CruiseControlVolumeState.IsDiskOperationStalled()` (`CompletedWithError`/`Paused`, mirroring `IsDownscaleStalled`) and, in `handleDiskRemoval`, set `waitForDiskRemovalToFinish = true` only for non-stalled `IsDiskRemoval()`/`IsDiskRebalance()` states. Branch selection (PVC deletion, state marking) is unchanged. The `default`/`Required` branch still blocks (conservative; covered by the missing-pod bypass when the pod is also gone).
124+
125+
The two fixes are complementary:
126+
127+
| Scenario | Fixed by |
128+
|---|---|
129+
| Removal failed/paused, pods up | state-narrowing (`IsDiskOperationStalled`) |
130+
| Removal genuinely Running/Scheduled, this broker's pod missing | missing-pod bypass |
131+
132+
### Not fixed here
133+
134+
The failed removal itself (`CompletedWithError`) is not retried/cleared by this change — it stops that failure from freezing the cluster. Driving the removal to success (or surfacing it for manual action) is a separate concern in the CC task reconciler.

‎openspec/changes/fix-disk-removal-deadlock/proposal.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Fix: Disk Removal Deadlock During Rolling Upgrade
2+
3+
**Status**: proposed
4+
**Created**: 2026-05-18
5+
6+
## Problem
7+
8+
When a broker's pod is deleted during a rolling upgrade AND a disk removal is pending (`GracefulDiskRemovalScheduled`), the operator enters a deadlock:
9+
10+
1. `reconcileKafkaPvc` blocks the entire reconcile with "Disk removal pending" error
11+
2. `reconcileKafkaPod` is never reached, so the pod is never recreated
12+
3. Cruise Control cannot complete the disk removal because the broker isn't running
13+
4. The cluster is stuck in `ClusterRollingUpgrading` indefinitely
14+
15+
This was observed in production: broker 103 of a 9-broker cluster (`pipeline-kafka`) had its pod deleted at 14:27:33 and was never recreated. The operator looped every ~20s for 10+ minutes with "Disk removal pending".
16+
17+
## Root Cause
18+
19+
In `pkg/resources/kafka/kafka.go`, the main reconcile function processes steps sequentially:
20+
21+
```
22+
Line 326: reconcileKafkaPvc() ← blocks here with "Disk removal pending"
23+
Line 332: build runningBrokers ← never reached
24+
Line 448: reconcileKafkaPod() ← never reached (this creates missing pods)
25+
```
26+
27+
`reconcileKafkaPvc` checks disk removal for ALL brokers. If ANY broker has pending removal, it returns a `CruiseControlTaskRunning` error that aborts the ENTIRE reconcile — including pod creation for brokers whose pods are missing.
28+
29+
The deadlock emerges across reconcile cycles:
30+
- **Cycle N**: PVC check passes (states not set yet) → rolling upgrade deletes pod → returns
31+
- **Cycle N+1**: PVC check sets `GracefulDiskRemovalRequired` → blocks → pod never recreated
32+
- **Cycle N+2...∞**: Same. Deadlock.
33+
34+
## Proposed Fix
35+
36+
**Don't block on disk removal when a broker's pod doesn't exist.**
37+
38+
CC disk removal REQUIRES the broker to be running (it moves partition replicas off the disk). Blocking pod creation while waiting for CC is counterproductive. The disk removal check is re-evaluated every reconcile cycle, so once the pod is back up, the check will correctly block again if still needed.
39+
40+
### Changes
41+
42+
**`pkg/resources/kafka/kafka.go`**:
43+
44+
1. Move the `runningBrokers` map building (lines 332-343) to BEFORE `reconcileKafkaPvc` (line 326)
45+
2. Pass `runningBrokers` to `reconcileKafkaPvc`
46+
3. In `reconcileKafkaPvc` (line 1278), before returning "Disk removal pending" error: check if any broker with pending disk removal has a missing pod. If yes, return `nil` instead.
47+
48+
**`pkg/resources/kafka/kafka_test.go`**:
49+
50+
- Update existing `reconcileKafkaPvc` tests for new signature
51+
- Add test: disk removal pending + broker pod missing → returns `nil`
52+
- Add test: disk removal pending + all pods running → returns error (unchanged behavior)
53+
54+
## Scope
55+
56+
- This fix is purely in the reconcile ordering/blocking logic
57+
- No changes to Cruise Control integration, disk removal flow, or rolling upgrade semantics
58+
- Existing behavior is preserved when all broker pods are running
59+
- Only changes behavior when a broker pod is missing AND disk removal is pending
60+
61+
## Risk
62+
63+
**Low.** The fix only relaxes a blocking condition in a specific scenario (missing pod + pending disk removal) where the current behavior is provably wrong (deadlock). The disk removal check continues to work normally once the pod is recreated.

‎openspec/changes/fix-disk-removal-deadlock/tasks.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Tasks: Fix Disk Removal Deadlock
2+
3+
## Task 1: Move `runningBrokers` before `reconcileKafkaPvc` [x]
4+
- **File**: `pkg/resources/kafka/kafka.go`
5+
- **What**: Move the broker pod list query (lines 332-343) to before `reconcileKafkaPvc` call (line 326). Remove the duplicate query at its original location.
6+
- **Details**: The `var brokerPods` / `runningBrokers` block currently runs AFTER `reconcileKafkaPvc`. Move it before. Pass `runningBrokers` to `reconcileKafkaPvc`.
7+
8+
## Task 2: Update `reconcileKafkaPvc` to accept and use `runningBrokers` [x]
9+
- **File**: `pkg/resources/kafka/kafka.go`
10+
- **What**:
11+
1. Add `runningBrokers map[string]struct{}` parameter to `reconcileKafkaPvc`
12+
2. At line 1278, before returning "Disk removal pending" error: check if any broker in `brokersDesiredPvcs` has a missing pod AND has a `IsDiskRemoval()` volume state. If so, return nil.
13+
- **Details**: This is the core fix. The check iterates `brokersDesiredPvcs` keys, looks up `runningBrokers`, and if a pod is missing checks the broker's volume states for active disk removal.
14+
15+
## Task 3: Update tests [x]
16+
- **File**: `pkg/resources/kafka/kafka_test.go`
17+
- **What**:
18+
1. Update all existing callers of `reconcileKafkaPvc` to pass the new `runningBrokers` parameter
19+
2. Add test case: disk removal pending + broker pod missing → returns nil
20+
3. Add test case: disk removal pending + all pods present → returns CruiseControlTaskRunning error
21+
- **Details**: The new test cases should set up a KafkaCluster with a broker whose volume state is `GracefulDiskRemovalScheduled`, then call `reconcileKafkaPvc` with/without the broker in `runningBrokers`.
22+
23+
## Task 4: Verify [x]
24+
- Run `go test ./pkg/resources/kafka/...`
25+
- Run `make test` for full suite
26+
- Run `go vet ./...` and `go build ./...`

‎pkg/resources/kafka/kafka.go‎

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -327,13 +327,6 @@ func (r *Reconciler) Reconcile(log logr.Logger) error {
327327
brokersVolumes[strconv.Itoa(int(broker.Id))] = brokerVolumes
328328
}
329329
}
330-
if len(brokersVolumes) > 0 {
331-
err := r.reconcileKafkaPvc(ctx, log, brokersVolumes)
332-
if err != nil {
333-
return errors.WrapIfWithDetails(err, "failed to reconcile resource", "resources", "PersistentVolumeClaim")
334-
}
335-
}
336-
337330
var brokerPods corev1.PodList
338331
matchingLabels := client.MatchingLabels(apiutil.LabelsForKafka(r.KafkaCluster.Name))
339332
err = r.List(ctx, &brokerPods, client.ListOption(client.InNamespace(r.KafkaCluster.Namespace)), client.ListOption(matchingLabels))
@@ -347,6 +340,13 @@ func (r *Reconciler) Reconcile(log logr.Logger) error {
347340
runningBrokers[brokerID] = struct{}{}
348341
}
349342

343+
if len(brokersVolumes) > 0 {
344+
err := r.reconcileKafkaPvc(ctx, log, brokersVolumes, runningBrokers)
345+
if err != nil {
346+
return errors.WrapIfWithDetails(err, "failed to reconcile resource", "resources", "PersistentVolumeClaim")
347+
}
348+
}
349+
350350
var pvcList corev1.PersistentVolumeClaimList
351351
err = r.List(ctx, &pvcList, client.ListOption(client.InNamespace(r.KafkaCluster.Namespace)), client.ListOption(matchingLabels))
352352
if err != nil {
@@ -1189,7 +1189,7 @@ func (r *Reconciler) isPodTainted(log logr.Logger, pod *corev1.Pod) bool {
11891189
}
11901190

11911191
//nolint:funlen
1192-
func (r *Reconciler) reconcileKafkaPvc(ctx context.Context, log logr.Logger, brokersDesiredPvcs map[string][]*corev1.PersistentVolumeClaim) error {
1192+
func (r *Reconciler) reconcileKafkaPvc(ctx context.Context, log logr.Logger, brokersDesiredPvcs map[string][]*corev1.PersistentVolumeClaim, runningBrokers map[string]struct{}) error {
11931193
brokersVolumesState := make(map[string]map[string]banzaiv1beta1.VolumeState)
11941194
var brokerIds []string
11951195
waitForDiskRemovalToFinish := false
@@ -1328,6 +1328,23 @@ func (r *Reconciler) reconcileKafkaPvc(ctx context.Context, log logr.Logger, bro
13281328
}
13291329

13301330
if waitForDiskRemovalToFinish {
1331+
// Don't block if any broker with pending disk removal/rebalance has a missing pod.
1332+
// Blocking prevents pod recreation, creating a deadlock where CC can't
1333+
// complete the disk removal or rebalance because the broker isn't running.
1334+
for brokerId := range brokersDesiredPvcs {
1335+
if _, podExists := runningBrokers[brokerId]; !podExists {
1336+
if brokerState, ok := r.KafkaCluster.Status.BrokersState[brokerId]; ok {
1337+
for mountPath, volumeState := range brokerState.GracefulActionState.VolumeStates {
1338+
if volumeState.CruiseControlVolumeState.IsDiskRemoval() || volumeState.CruiseControlVolumeState.IsDiskRebalance() {
1339+
log.Info("Disk removal pending but broker pod is missing, "+
1340+
"allowing reconcile to proceed for pod recreation",
1341+
"brokerId", brokerId, "mountPath", mountPath)
1342+
return nil
1343+
}
1344+
}
1345+
}
1346+
}
1347+
}
13311348
return errorfactory.New(errorfactory.CruiseControlTaskRunning{}, errors.New("Disk removal pending"), "Disk removal pending")
13321349
}
13331350

@@ -1376,11 +1393,26 @@ func handleDiskRemoval(ctx context.Context, pvcList *corev1.PersistentVolumeClai
13761393
return false, errors.WrapIfWithDetails(err, "could not delete volume status for broker volume", "brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove)
13771394
}
13781395
case ccVolumeState.IsDiskRemoval():
1379-
log.Info("Graceful disk removal is in progress", "brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove)
1380-
waitForDiskRemovalToFinish = true
1396+
// Only block the reconcile while the CC task is actively progressing. When it has
1397+
// stalled (CompletedWithError/Paused) there is no in-flight work a broker restart
1398+
// could disrupt, and the removed disk stays in log.dirs and mounted until removal
1399+
// is confirmed succeeded (shouldKeepRemovedLogDirInConfig), so proceeding is
1400+
// data-safe. Blocking on a stalled task would freeze rolling upgrades indefinitely.
1401+
if ccVolumeState.IsDiskOperationStalled() {
1402+
log.Info("Graceful disk removal is not progressing (error/paused); not blocking reconcile",
1403+
"brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove, "volumeState", ccVolumeState)
1404+
} else {
1405+
log.Info("Graceful disk removal is in progress", "brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove)
1406+
waitForDiskRemovalToFinish = true
1407+
}
13811408
case ccVolumeState.IsDiskRebalance():
1382-
log.Info("Graceful disk rebalance is in progress, waiting for it to finish before marking disk for removal", "brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove)
1383-
waitForDiskRemovalToFinish = true
1409+
if ccVolumeState.IsDiskOperationStalled() {
1410+
log.Info("Graceful disk rebalance is not progressing (error/paused); not blocking reconcile",
1411+
"brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove, "volumeState", ccVolumeState)
1412+
} else {
1413+
log.Info("Graceful disk rebalance is in progress, waiting for it to finish before marking disk for removal", "brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove)
1414+
waitForDiskRemovalToFinish = true
1415+
}
13841416
default:
13851417
brokerVolumesState[mountPathToRemove] = banzaiv1beta1.VolumeState{CruiseControlVolumeState: banzaiv1beta1.GracefulDiskRemovalRequired}
13861418
log.Info("Marked the volume for removal", "brokerId", brokerId, mountPathAnnotationKey, mountPathToRemove)

0 commit comments

Comments
 (0)