Stop rewriting machine conditions on every reconcile

[lukasfrank]

Proposed Changes

• Only update machine condition LastTransitionTime when the status changes

Fixes #1496

Summary by CodeRabbit

• Bug Fixes
  • Improved machine status condition updates.
  • Preserved transition timestamps when conditions remain unchanged.
  • Updated timestamps correctly when machine readiness changes.
  • Ensured volume and network-interface readiness conditions are created and retained consistently.

[coderabbitai]

Warning

Review limit reached

@lukasfrank, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 63db2d9d-fac9-4a72-a51f-4229ffb599fa

📥 Commits

Reviewing files that changed from the base of the PR and between 786b6b5 and cd9125d.

📒 Files selected for processing (2)

• poollet/machinepoollet/controllers/machine_controller.go
• poollet/machinepoollet/controllers/machine_controller_test.go

📝 Walkthrough

Walkthrough

The machine controller now exposes ComputeMachineConditions. It updates existing conditions, preserves timestamps for unchanged statuses, updates timestamps when statuses change, and includes volume and network-interface readiness conditions when status data exists.

Changes

Machine condition computation

Layer / File(s)	Summary
Condition computation and reconciliation wiring poollet/machinepoollet/controllers/machine_controller.go	updateMachineStatus delegates to ComputeMachineConditions and propagates errors. Standalone helpers compute machine, volume, and network-interface readiness conditions without receiving timestamps.
Condition timestamp validation poollet/machinepoollet/controllers/machine_controller_test.go	Tests verify condition creation, timestamp preservation for unchanged statuses, and timestamp updates when machine state changes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• ironcore-dev/ironcore#1476: Both changes modify machine condition helpers and transition timestamp handling.

Suggested labels: area/iaas

Suggested reviewers: adracus

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: preventing machine condition rewrites during every reconcile.
Description check	✅ Passed	The description states the condition timestamp change and links issue #1496, which satisfies the required template content.
Linked Issues check	✅ Passed	The changes preserve transition timestamps and prevent repeated condition updates, addressing the standstill issue in #1496.
Out of Scope Changes check	✅ Passed	The condition refactor and related tests directly support the linked issue and stated objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

poollet/machinepoollet/controllers/machine_controller_test.go (1)

1206-1208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the unchanged network-interface timestamp.

The second computation checks only VolumesReady. A change that resets NetworkInterfacesReady.LastTransitionTime can pass this test.

Capture and compare the NetworkInterfacesReady timestamp after the unchanged recomputation.

🤖 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 `@poollet/machinepoollet/controllers/machine_controller_test.go` around lines
1206 - 1208, Extend the unchanged recomputation test around
ComputeMachineConditions to capture NetworkInterfacesReady.LastTransitionTime
before the second call and assert it remains temporally equal afterward,
alongside the existing VolumesReady assertion.

🤖 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.

Inline comments:
In `@poollet/machinepoollet/controllers/machine_controller.go`:
- Around line 523-528: Update the condition-building logic around
computeVolumesReadyCondition and computeNetworkInterfacesReadyCondition so an
empty volumeStatuses or nicStatuses removes the corresponding existing
VolumesReady or NetworkInterfacesReady condition instead of leaving it stale;
retain the current append behavior for non-empty inputs, and add a regression
test covering each non-empty-to-empty transition.

---

Nitpick comments:
In `@poollet/machinepoollet/controllers/machine_controller_test.go`:
- Around line 1206-1208: Extend the unchanged recomputation test around
ComputeMachineConditions to capture NetworkInterfacesReady.LastTransitionTime
before the second call and assert it remains temporally equal afterward,
alongside the existing VolumesReady assertion.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fa3a21f5-95dc-4303-ad5b-53f35e51cfe9

📥 Commits

Reviewing files that changed from the base of the PR and between ceab5aa and 786b6b5.

📒 Files selected for processing (2)

• poollet/machinepoollet/controllers/machine_controller.go
• poollet/machinepoollet/controllers/machine_controller_test.go