Fix 727 network plugin fails silently after certificate rotation fails

[friegger]

Errors yielded by the clients config controller during cert rotation are now hooked up to the health checks.

Fixes #727

Summary by CodeRabbit

• New Features
  • Health status endpoints now include configured network interface checks.
  • Health checks report 200 OK only when libvirt and all configured checks pass; otherwise, they return 503 Service Unavailable and identify the failing check.
• Bug Fixes
  • Provider startup failures are now logged at the normal error level, making them visible without increased verbosity.
• Tests
  • Added coverage for healthy states, libvirt disconnections, and individual or partial health-check failures.

[coderabbitai]

Warning

Review limit reached

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 2090344b-6f90-4a2f-8358-038680c81c91

📥 Commits

Reviewing files that changed from the base of the PR and between fba8c9a and c8b404a.

📒 Files selected for processing (5)

• cmd/libvirt-provider/app/app.go
• internal/healthcheck/health_check.go
• internal/healthcheck/health_check_test.go
• internal/healthcheck/libvirt.go
• internal/healthcheck/libvirt_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e96092fa-adcc-47b6-8f4a-34cea78a318b

📥 Commits

Reviewing files that changed from the base of the PR and between b976243 and fba8c9a.

📒 Files selected for processing (3)

• cmd/libvirt-provider/app/app.go
• internal/healthcheck/health_check.go
• internal/healthcheck/health_check_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The provider now reports network controller health with libvirt connectivity. Health checks use a connector abstraction, evaluate named checkers, return HTTP 200 or 503, and log command failures at normal verbosity.

Changes

Provider health checks

Layer / File(s)	Summary
Health check contract and aggregation internal/libvirt/utils/libvirtutils.go, internal/healthcheck/health_check.go	HealthCheck accepts a Connector and evaluates libvirt connectivity plus named checkers.
Provider health wiring and error logging cmd/libvirt-provider/app/app.go, cmd/libvirt-provider/main.go	The application registers the network controller checker when available. Command errors use the default log level.
Health check test coverage internal/healthcheck/health_check_suite_test.go, internal/healthcheck/health_check_test.go	Tests cover healthy checks, checker failures, multiple checkers, and libvirt disconnection.

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

Merge Risk: ⚪ Minimal · up to fba8c

Network-controller rotation failures now make the provider health endpoint unavailable and identify the failing check, while healthy and libvirt-failure behavior remains covered. No concrete merge-blocking risk remains.

Suggested reviewers: lukasfrank

Sequence Diagram(s)

sequenceDiagram
  participant HTTPClient
  participant HealthCheckHandler
  participant Connector
  participant HealthChecker
  HTTPClient->>HealthCheckHandler: request health status
  HealthCheckHandler->>Connector: check IsConnected
  HealthCheckHandler->>HealthChecker: run named Check
  HealthCheckHandler-->>HTTPClient: return 200 or 503

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the network plugin failure after certificate rotation and matches the primary change.
Description check	✅ Passed	The description explains that client configuration controller errors are connected to health checks and references issue #727. It omits the template's Proposed Changes heading and bullet format, but i…
Linked Issues check	✅ Passed	The changes address issue #727 by registering the network interface controller with health checks, reporting checker failures as HTTP 503, and adding tests for controller and libvirt failures. This pr…
Out of Scope Changes check	✅ Passed	The changes remain within scope. The connector abstraction, health-check behavior, tests, and normal-level provider error logging support detection and visibility of network plugin failures.

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