fix reconciliation cycle

[Despire]

Summary by CodeRabbit

• Bug Fixes
  • Prevented conflicting node pool target-size updates when a rollback is already in-flight, improving stability during autoscaling failure recovery.
• Chores
  • Pinned multiple component container images to a new shared tag.
  • Updated the testing framework container image tag.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: be56be40-2cef-427c-8054-d7f8af838f9f

📥 Commits

Reviewing files that changed from the base of the PR and between bc00718 and 082ff11.

📒 Files selected for processing (2)

• manifests/claudie/kustomization.yaml
• manifests/testing-framework/kustomization.yaml

✅ Files skipped from review due to trivial changes (2)

• manifests/claudie/kustomization.yaml
• manifests/testing-framework/kustomization.yaml

---

Walkthrough

This change adds a conditional guard to nodepool reconciliation logic, ensuring that historical scale-up failure counter checks and target-size rollbacks only occur when no in-flight task is active. The guard prevents counter-based state updates from interfering with ongoing in-flight rollback operations.

Changes

Reconciliation scale-up failure rollback guard

Layer / File(s)	Summary
Guard scale-up failure rollback when in-flight task exists services/manager/internal/service/reconciliation.go	The nodepool target-size rollback logic based on historical K8SNodePoolScaleUpFailed counters is now wrapped in if state.InFlight == nil, ensuring this counter-based update is skipped when an in-flight task exists.

Kustomize image tag updates

Layer / File(s)	Summary
Pin Claudie component images to new tag manifests/claudie/kustomization.yaml, manifests/testing-framework/kustomization.yaml	Update newTag values for multiple Claudie component images and the testing-framework image to 4f42164-4266.

Possibly related PRs

• berops/claudie#2129: touches the same reconciliation rollback logic and autoscaled nodepool target-size adjustments.
• berops/claudie#2038: modifies in-flight/rollback flow in reconciliate, adjacent to the current guard addition.
• berops/claudie#2069: implements historical K8SNodePoolScaleUpFailed counters and related autoscaling rollback behavior.

Suggested labels

test-set-autoscaling, test-set-ordinary

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix reconciliation cycle' is specific and directly related to the main change—guarding counter-based target-size logic with an InFlight check in the reconciliation flow.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/reconciliation-target-size

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

services/manager/internal/service/reconciliation.go (1)

185-238: 💤 Low value

Guard is correct and necessary to prevent conflicting rollbacks.

The guard correctly prevents counter-based target-size rollback from modifying state.Current when an in-flight rollback task exists. Without this guard, the counter-based logic would modify the outer current (which is state.Current from line 61) while the diff-based rollback uses the in-flight state as current (inner current reassigned at line 298), leading to conflicting rollback behaviors.

The implementation aligns with the reconciliation flow where in-flight failed tasks are rolled back via diff-based logic, making this guard essential for correctness.

Optional: Consider more defensive guard condition

For consistency with line 66's hasInFlightState check, the guard could also check Task:

-			if state.InFlight == nil {
+			if state.InFlight == nil || state.InFlight.Task == nil {

This would be more defensive, though the current implementation appears safe based on the codebase patterns.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/manager/internal/service/reconciliation.go` around lines 185 - 238,
The guard that skips counter-based target-size rollback when an in-flight state
exists should also ensure there is no in-flight Task; change the condition
around the block that currently reads "if state.InFlight == nil" to also check
the Task (e.g., run the rollback block only when state.InFlight == nil ||
state.InFlight.Task == nil) so you don't modify state.Current while an in-flight
task-based rollback is active; update the check near the event_switch that
surrounds the K8SNodePoolScaleUpFailed loop (references: state.InFlight,
state.InFlight.Task, clusterResult, nodepools.FindByName,
dyn.AutoscalerConfig.TargetSize).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@services/manager/internal/service/reconciliation.go`:
- Around line 185-238: The guard that skips counter-based target-size rollback
when an in-flight state exists should also ensure there is no in-flight Task;
change the condition around the block that currently reads "if state.InFlight ==
nil" to also check the Task (e.g., run the rollback block only when
state.InFlight == nil || state.InFlight.Task == nil) so you don't modify
state.Current while an in-flight task-based rollback is active; update the check
near the event_switch that surrounds the K8SNodePoolScaleUpFailed loop
(references: state.InFlight, state.InFlight.Task, clusterResult,
nodepools.FindByName, dyn.AutoscalerConfig.TargetSize).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8c56fdb-8dee-4ec6-b826-51104279d69d

📥 Commits

Reviewing files that changed from the base of the PR and between ed8fd69 and bc00718.

📒 Files selected for processing (1)

• services/manager/internal/service/reconciliation.go