Monitor MachinePool Health

[friegger]

1. A MachinePool lifecycle controller:

Implements machinepool-lifecycle-controller monitoring MachinePool health, with corresponding API additions for MachinePool/VolumePool/BucketPool conditions, codegen updates

2. A pool health heartbeat runnable in the machinepoollet:

The poollet now actively reports pool liveness so the lifecycle controller can set pools whose poollets have gone away to Unknown. It also includes kustomizations adding the Lease namespace and RBAC adjustments. The MachinePoolHeartbeat, a ticker-driven Runnable that probes the IRI runtime via Status, renews the pool's Lease in ironcore-machinepool-lease, and patches Ready only when its value or observedGeneration actually changes. Errors on either sub-step are logged and retried on the next tick; the lifecycle controller's grace period absorbs short blips. Lease takeover from a previous holder is logged at Info. Contains app arguments that make the heartbeat intervals configurable, defaulting to the IEP-15 values.

3. Lease namespaces and RBACs

Contributes to #1472

Summary by CodeRabbit

• New Features

  • Machine pool lifecycle monitoring with configurable grace period to detect unresponsive pools
  • Heartbeat-based health checks that probe runtimes and reflect readiness into pool status
  • New Ready condition type for machine pools and automatic status transitions when health signals stall
  • Lease-based coordination so heartbeats drive centralized lease renewal and controller watches

• Configuration

  • New namespace for machine pool lease coordination
  • New CLI flags to configure lifecycle grace period and heartbeat timings

[adracus]

What about the remaining pool health controllers / reconcilers? If we want to make this change only about machine pools, we should either remove everything else or add the rest.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Machine pool health monitoring: adds a MachinePoolReady condition and LastUpdateTime, poollet heartbeat that probes runtime and maintains Leases, a lifecycle reconciler that marks stale pools Unknown after a grace period, and namespace/RBAC manifests plus tests and generated updates.

Changes

Machine pool health monitoring

Layer / File(s)	Summary
Compute condition contract and helper behavior api/compute/v1alpha1/common.go, api/compute/v1alpha1/machinepool_types.go, api/compute/v1alpha1/conditions.go, api/compute/v1alpha1/conditions_test.go, api/compute/v1alpha1/v1alpha1_suite_test.go, api/compute/v1alpha1/zz_generated.deepcopy.go, internal/apis/compute/machinepool_types.go, internal/apis/compute/v1alpha1/zz_generated.conversion.go, internal/apis/compute/zz_generated.deepcopy.go, client-go/applyconfigurations/compute/v1alpha1/machinepoolcondition.go, client-go/openapi/zz_generated.openapi.go	Introduces NamespaceMachinePoolLease constant and MachinePoolReady condition type, adds LastUpdateTime to MachinePoolCondition, implements FindMachinePoolCondition and SetMachinePoolCondition with timestamp semantics, and updates generated/internal surfaces.
Poollet heartbeat runnable and ready condition publishing poollet/machinepoollet/controllers/machinepool_heartbeat.go, poollet/machinepoollet/controllers/machinepool_heartbeat_test.go, poollet/machinepoollet/controllers/machinepool_heartbeat_envtest_test.go, poollet/machinepoollet/controllers/machinepool_controller.go, poollet/machinepoollet/cmd/machinepoollet/app/app.go, poollet/machinepoollet/controllers/controllers_suite_test.go, poollet/machinepoollet/controllers/rbac.go	Adds MachinePoolHeartbeat runnable: probes IRI runtime, reconciles per-pool Lease, publishes probe results to MachinePoolReadyState, emits events on change, and wires heartbeat options and event-channel integration into the poollet reconciler. Includes unit and envtest coverage.
Lifecycle controller stale-health reconciliation internal/controllers/compute/machinepool_lifecycle_controller.go, internal/controllers/compute/machinepool_lifecycle_controller_test.go, internal/controllers/compute/machinepool_lifecycle_controller_unit_test.go, internal/controllers/compute/suite_test.go, cmd/ironcore-controller-manager/main.go	Adds MachinePoolLifecycleReconciler with per-pool health cache comparing Lease.renewTime and MachinePoolReady; when neither updates for configured grace period, patches MachinePoolReady to Unknown (unless already Unknown). Wires controller flag and watches both MachinePool and Lease. Tests cover timed transitions and cache cleanup.
Lease namespace manifests and RBAC/deployment wiring config/namespaces/machinepool-lease/namespace.yaml, config/namespaces/machinepool-lease/poollet_role.yaml, config/namespaces/machinepool-lease/poollet_rolebinding.yaml, config/namespaces/machinepool-lease/controller_role.yaml, config/namespaces/machinepool-lease/controller_rolebinding.yaml, config/namespaces/machinepool-lease/kustomization.yaml, config/machinepoollet-broker/poollet-rbac/role.yaml, config/apiserver/rbac/machinepool_role.yaml, config/default/kustomization.yaml, config/apiserver/standalone/kustomization.yaml, config/apiserver/standalone-etcdless/kustomization.yaml, config/controller/standalone/kustomization.yaml, config/etcdless/kustomization.yaml	Creates ironcore-machinepool-lease namespace, poollet/controller Roles and RoleBindings for leases, updates broker role, and includes the namespace in deployment kustomizations.
Dependency and codegen script adjustments go.mod, hack/update-codegen.sh	Moves github.com/google/uuid to direct dependency and removes -- from chmod invocation in codegen script.

Sequence Diagram(s)

sequenceDiagram
  participant Poollet as MachinePoollet
  participant IRI as IRI Runtime
  participant ReadyState as MachinePoolReadyState
  participant Events as Event Channel
  participant Lease as Lease Storage

  Poollet->>IRI: Status probe (with timeout)
  IRI-->>Poollet: probe result (success/error)
  Poollet->>ReadyState: Set(probeErr)
  alt probe error changed
    ReadyState-->>Poollet: changed=true
    Poollet->>Events: emit GenericEvent for MachinePool
  else probe error unchanged
    ReadyState-->>Poollet: changed=false
  end
  Poollet->>Lease: reconcileLease (create/update renewTime)
  Lease-->>Poollet: Lease created/updated

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• Implement Pool Health Proposal #1472: Implements the machinepool heartbeat, lease namespace, and lifecycle reconciler described by that proposal.

Suggested reviewers

• afritzler
• lukasfrank

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title is concise and matches the main change: monitoring MachinePool health.
Description check	✅ Passed	The description covers the main changes and issue reference, but it does not follow the required Proposed Changes/Fixes # template exactly.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch enh/1472-pool-health-on-standalone-ks

---

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: 2

♻️ Duplicate comments (1)

internal/controllers/compute/machinepool_lifecycle_controller.go (1)

44-46: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Memory leak: clean up healthData when MachinePool is deleted.

When a MachinePool is deleted, client.IgnoreNotFound(err) returns without removing the corresponding entry from the r.healthData map. Over time, this will cause the map to accumulate stale entries for deleted pools, leading to a memory leak.

As per the past review comment from adracus, you should free the health data when the machine pool is not found.

🛠️ Proposed fix

 func (r *MachinePoolLifecycleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 
 	machinePool := &computev1alpha1.MachinePool{}
 	if err := r.Get(ctx, req.NamespacedName, machinePool); err != nil {
+		if apierrors.IsNotFound(err) {
+			r.healthDataMu.Lock()
+			delete(r.healthData, req.Name)
+			r.healthDataMu.Unlock()
+		}
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}

🤖 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 `@internal/controllers/compute/machinepool_lifecycle_controller.go` around
lines 44 - 46, The current Reconcile branch returns client.IgnoreNotFound(err)
after r.Get and thus never cleans up r.healthData for deleted MachinePools;
modify the error handling so that when r.Get returns a NotFound error (use
apierrors.IsNotFound(err) or check client.IgnoreNotFound result), you remove the
corresponding entry from r.healthData using req.NamespacedName (or its string
form) as the key, then return ctrl.Result{} and nil (or the ignored-not-found
result). Keep the rest of the error path unchanged for non-NotFound errors.

🤖 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 `@client-go/openapi/api_violations.report`:
- Line 42: The Conditions slice in BucketPoolStatus lacks explicit list
semantics; add kubebuilder markers above the Conditions field in
api/storage/v1alpha1/bucketpool_types.go (e.g., annotate
BucketPoolStatus.Conditions with +listType=map and +listMapKey=type) so the list
is treated as a map keyed by condition Type, then run the CRD/OpenAPI generation
step to regenerate the openapi/report artifacts.

In `@poollet/machinepoollet/controllers/machinepool_heartbeat_test.go`:
- Around line 56-58: The test titled "ignores LastUpdateTime and
LastTransitionTime" currently compares an unchanged copy; update it so desired
mutates the condition timestamps before asserting: create desired := base (value
copy) then modify desired.Status.Conditions[*].LastUpdateTime and
LastTransitionTime (or at least one condition) to different non-zero values
(e.g., time.Now() or a different metav1.Time) and then call
controllers.ReadyConditionsDiffer(&base, desired) and Expect(...).To(BeFalse())
to ensure timestamp differences are ignored; reference ReadyConditionsDiffer,
base, desired, LastUpdateTime and LastTransitionTime to locate the change.

---

Duplicate comments:
In `@internal/controllers/compute/machinepool_lifecycle_controller.go`:
- Around line 44-46: The current Reconcile branch returns
client.IgnoreNotFound(err) after r.Get and thus never cleans up r.healthData for
deleted MachinePools; modify the error handling so that when r.Get returns a
NotFound error (use apierrors.IsNotFound(err) or check client.IgnoreNotFound
result), you remove the corresponding entry from r.healthData using
req.NamespacedName (or its string form) as the key, then return ctrl.Result{}
and nil (or the ignored-not-found result). Keep the rest of the error path
unchanged for non-NotFound errors.

🪄 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

Run ID: 5d7b923a-79b4-4d83-aee3-392f96b423a5

📥 Commits

Reviewing files that changed from the base of the PR and between 455f76c and 5e558bc.

📒 Files selected for processing (49)

• api/compute/v1alpha1/common.go
• api/compute/v1alpha1/conditions.go
• api/compute/v1alpha1/conditions_test.go
• api/compute/v1alpha1/machinepool_types.go
• api/compute/v1alpha1/v1alpha1_suite_test.go
• api/compute/v1alpha1/zz_generated.deepcopy.go
• api/storage/v1alpha1/bucketpool_types.go
• api/storage/v1alpha1/common.go
• api/storage/v1alpha1/volumepool_types.go
• api/storage/v1alpha1/zz_generated.deepcopy.go
• api/storage/v1alpha1/zz_generated.model_name.go
• client-go/applyconfigurations/compute/v1alpha1/machinepoolcondition.go
• client-go/applyconfigurations/storage/v1alpha1/bucketpoolcondition.go
• client-go/applyconfigurations/storage/v1alpha1/bucketpoolstatus.go
• client-go/applyconfigurations/storage/v1alpha1/volumepoolcondition.go
• client-go/applyconfigurations/utils.go
• client-go/openapi/api_violations.report
• client-go/openapi/zz_generated.openapi.go
• cmd/ironcore-controller-manager/main.go
• config/apiserver/standalone-etcdless/kustomization.yaml
• config/apiserver/standalone/kustomization.yaml
• config/controller/standalone/kustomization.yaml
• config/default/kustomization.yaml
• config/etcdless/kustomization.yaml
• config/machinepoollet-broker/poollet-rbac/role.yaml
• config/namespaces/machinepool-lease/controller_role.yaml
• config/namespaces/machinepool-lease/controller_rolebinding.yaml
• config/namespaces/machinepool-lease/kustomization.yaml
• config/namespaces/machinepool-lease/namespace.yaml
• config/namespaces/machinepool-lease/poollet_role.yaml
• config/namespaces/machinepool-lease/poollet_rolebinding.yaml
• go.mod
• hack/update-codegen.sh
• internal/apis/compute/machinepool_types.go
• internal/apis/compute/v1alpha1/zz_generated.conversion.go
• internal/apis/compute/zz_generated.deepcopy.go
• internal/apis/storage/bucketpool_types.go
• internal/apis/storage/v1alpha1/zz_generated.conversion.go
• internal/apis/storage/volumepool_types.go
• internal/apis/storage/zz_generated.deepcopy.go
• internal/controllers/compute/machinepool_lifecycle_controller.go
• internal/controllers/compute/machinepool_lifecycle_controller_test.go
• internal/controllers/compute/suite_test.go
• poollet/machinepoollet/cmd/machinepoollet/app/app.go
• poollet/machinepoollet/controllers/controllers_suite_test.go
• poollet/machinepoollet/controllers/machinepool_heartbeat.go
• poollet/machinepoollet/controllers/machinepool_heartbeat_envtest_test.go
• poollet/machinepoollet/controllers/machinepool_heartbeat_test.go
• poollet/machinepoollet/controllers/rbac.go

[friegger]

What about the remaining pool health controllers / reconcilers? If we want to make this change only about machine pools, we should either remove everything else or add the rest.

Volume and Bucket changes have been removed from this PR.

[lukasfrank]

Can you also please update the PR title and description.
For me it's not clear what based on standalone kustomizations means.

[lukasfrank]

Can you also please use komega for the tests.

[lukasfrank]

Many thanks for the great work! LGTM