operator(P3): headless coverage Service + status.coverageEndpoint

[ianp94]

Operator P3 — headless coverage Service + status.coverageEndpoint

The operator now closes the loop with DD-023 (multi-pod union coverage): when spec.coverageService is on, it creates a headless Service (clusterIP: None) selecting the target's pods on the coverage port. The Service's cluster DNS resolves to every pod IP, which is exactly what the DD-023 driver flag (-Dclosurejvm.coverage.jacoco=<host>:<port>) + InetAddress.getAllByName consume to union-merge coverage across replicas. status.coverageEndpoint publishes <svc>.<ns>.svc.cluster.local:<port> so the flag is copy-pasteable.

Behavior

• Create/update the headless Service (selector from the referenced Deployment, coverage port) when coverageService && coverage.enabled; delete it and clear the endpoint when toggled off or coverage disabled.
• Owner-referenced to the target → garbage-collected when the target is deleted (the Service is created by the operator, unlike the Deployment it patches, so an owner ref is the right lifecycle here — not the finalizer).
• core/services RBAC added (get;list;watch;create;update;patch;delete).

Verification

• envtest (13/13): creates the headless Service with the right selector/port + owner ref and publishes status.coverageEndpoint; toggling coverageService: false deletes the Service and clears the endpoint.
• In-cluster e2e (9/9): deploy/e2e/e2e.sh now also asserts the coverage Service is headless, has the app pod as an endpoint, and status.coverageEndpoint is published — end to end against a raw JPetStore.

Notes

• Endpoint uses the .svc.cluster.local FQDN (assumes the default cluster domain).
• Wiring an actual in-cluster driver at this endpoint is P5 (ClosureJVMCampaign); P3 stops at exposing it.

envtest 13/13; e2e 9/9. Design doc §8 P3 + status header and TODO updated.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JuGqspSB9zpA5AyvHJBiDK

[ianp94]

@claude please review — operator P3 (headless coverage Service + status.coverageEndpoint).

Worth checking:

1. Service lifecycle — owner-ref for GC (the Service is created by the operator, so owner-ref, unlike the patched Deployment which uses the finalizer). Create-if-missing / update-mutable-fields-only (clusterIP is immutable) / delete-on-toggle-off. Any lifecycle gap (e.g. leak if the Deployment's selector changes, or coverage disabled but coverageService still true)?
2. RBAC — added core/services get;list;watch;create;update;patch;delete. Right minimal set?
3. status.coverageEndpoint — the .svc.cluster.local FQDN + port; is that the right form for the DD-023 getAllByName flag?
4. The reconcile order (coverage Service before status update) and the requeue behavior during rollout.

Verified: envtest 13/13 (create/publish + toggle-off/delete), e2e 9/9 in-cluster (headless, has-endpoint, endpoint-published).

[claude]

Claude finished @ianp94's task in 2m 38s —— View job

---

Review — operator P3 (headless coverage Service + status.coverageEndpoint)

Todo list

• Gather context (PR diff, changed files)
• Read coverage_service.go and controller reconcile changes
• Review RBAC diff
• Review status.coverageEndpoint construction + DD-023 flag form
• Review reconcile ordering / requeue behavior
• Check envtest/e2e coverage for the lifecycle gaps mentioned
• Post final review

Diff matches the PR description exactly (218/-9 across the 7 listed files). Solid, well-tested change overall — going through your four questions:

1. Service lifecycle

The create/update/delete logic in reconcileCoverageService (closurejvmtarget_controller.go:219-247) is correct for the cases it handles: create-if-missing, update selector/ports only (leaving immutable clusterIP alone), delete when either coverageService or Agents.Coverage.Enabled flips off. Owner-ref vs. finalizer choice is right and well-justified in the comment.

Gap you didn't ask about but is real: when spec.deploymentRef.name stops resolving (Deployment deleted/renamed) after a coverage Service was already created, Reconcile hits the apierrors.IsNotFound branch at closurejvmtarget_controller.go:105-124 and returns early — before reconcileCoverageService is ever called. That leaves:

• The coverage Service orphaned (not deleted) — it'll sit there with a now-stale selector matching zero pods.
• status.CoverageEndpoint not cleared — it keeps publishing a dead endpoint to anyone reading status, which is worse than a missing endpoint since it looks valid.

It'll eventually get GC'd if the target itself is deleted (owner-ref), but while the target lingers in Pending/DeploymentNotFound, the Service+endpoint are stale. Worth either calling reconcileCoverageService's delete-path before the early return, or explicitly clearing CoverageEndpoint in that branch.

Selector edge case: desiredCoverageService (coverage_service.go:61-64) only copies deploy.Spec.Selector.MatchLabels, dropping MatchExpressions. corev1.ServiceSpec.Selector can't represent expressions anyway (K8s limitation, not fixable here), but if a Deployment ever used expression-only selectors, selector would end up nil/empty — which Kubernetes treats as a selectorless Service (no auto-managed Endpoints), silently breaking DD-023 with no error surfaced. Probably fine to leave given kubebuilder-generated Deployments here always use MatchLabels, but worth a one-line guard/log if MatchExpressions is ever non-empty.

The "Deployment's selector changes" scenario you flagged isn't actually reachable — spec.selector on a Deployment is immutable in apps/v1, so no leak there.

2. RBAC

get;list;watch;create;update;patch;delete on core/services (role.yaml:39-49) — patch is unused (matches the existing deployments RBAC block's style of including it defensively, so consistent, not a real issue). More notable: list/watch are also currently unused — the reconciler only ever does a targeted Get by name, and SetupWithManager (closurejvmtarget_controller.go:288-292) has no .Owns(&corev1.Service{}) or Watches for Services. That ties into #4 below — if you add that watch (recommended), list/watch become load-bearing; until then they're granted-but-dormant. Not a security issue (namespaced, no verbs beyond what a Service owner needs), just flagging it isn't "currently minimal," it's "minimal for where this is headed."

3. status.coverageEndpoint form

Confirmed correct against the consumer: JacocoCoverageProvider.parseEndpoints (runner/coverage/JacocoCoverageProvider.java:77-85) expects a bare host:port (comma-separated for multiple), and JacocoEndpointParseTest.java:48 explicitly tests a *.svc.cluster.local:6300-shaped string through getAllByName. coverageEndpoint() (coverage_service.go:51-54) produces exactly that shape. Good match.

4. Reconcile order / requeue

Coverage Service reconcile running before the single Status().Update (closurejvmtarget_controller.go:164-167 → status block at ~193-217) is correct — CoverageEndpoint and Phase/InstrumentedReplicas land in the same status write, no extra round-trip. During rollout, the existing RequeueAfter: 5s while PhaseInjecting also re-reconciles the coverage Service each pass, which is harmless (idempotent, cheap Get+maybe-Update) but means the Service selector/ports get needlessly re-diffed every 5s during a rollout — not a bug, just a minor inefficiency, not worth changing.

The more actionable point on requeue/self-healing: since the Service is owner-ref'd but there's no Owns(&corev1.Service{}) watch, an out-of-band edit or delete of the coverage Service (e.g. kubectl delete svc) won't be noticed until something else re-triggers reconcile (a Target spec change or a Deployment event). Adding .Owns(&corev1.Service{}) in SetupWithManager would close that gap and also justify the list/watch RBAC from #2.

Tests

envtest additions (closurejvmtarget_controller_test.go:353-397) look right: verifies headless clusterIP, selector, port, owner ref, status.coverageEndpoint format, and toggle-off deletes the Service + clears the endpoint. AfterEach correctly accounts for envtest not running GC. e2e additions assert headless + has-endpoint + published endpoint in-cluster. Good coverage of the paths that exist; the DeploymentNotFound-while-coverage-was-on gap above isn't covered by either suite (understandably, since it's a gap, not a tested behavior).

Summary

No blocking issues. One real gap worth a follow-up: stale coverage Service + endpoint when deploymentRef stops resolving after coverage was already provisioned (§1). Everything else is either working as intended or a minor efficiency/consistency note, not correctness. Nice, tight PR with genuinely good envtest + e2e coverage of the paths it implements.

[ianp94]

Fixed the real gap in 54cd6e2:

• Stale endpoint on DeploymentNotFound — extracted removeCoverageService (delete Service + clear endpoint) and call it in the DeploymentNotFound branch, so when the referenced Deployment disappears the coverage Service is torn down and status.coverageEndpoint is cleared rather than left publishing a dead one. New envtest asserts this.
• Self-heal — added Owns(&corev1.Service{}) to SetupWithManager, so an out-of-band kubectl delete svc re-triggers reconcile (and makes the list;watch services RBAC load-bearing, per your v0.9: exploration metrics in the live status UI #2).
• Selectorless guard — log a warning if the derived selector is empty (matchExpressions-only Deployment), so DD-023 silently resolving to nothing is surfaced.

Left patch in RBAC (consistency, as noted). envtest 14/14, e2e 9/9. Merging.