Skip to content

spec.family — fail-fast on instanceArg conflict (design) #698

Description

@QuentinBisson

Design issue tracking the option to enforce `family.instanceArg` uniqueness at admission / reconciler time, rather than continue with the graceful-fallback behaviour in #670.

Split out of #697 because muster's existing reconcilers only handle Kubernetes optimistic-locking conflicts — there is no precedent in this codebase for cross-CR domain-conflict detection or `Ready=False, reason=…` patterns. Introducing one is a new pattern, not a follow-up bug fix.

Current behaviour (#670)

When two `MCPServer` CRs declare the same `spec.family.name` with different `spec.family.instanceArg`, the aggregator detects divergence at the family level (via `familyFallbackStatusLocked` in `internal/aggregator/registry.go`) and falls back the entire family to per-server prefixing. A single warning is logged at the moment the divergent server registers.

Operator UX:

  • Both CRs remain `Ready=True`.
  • No `x__*` tools surface.
  • The warning is only visible in aggregator logs.

Proposed alternative

Refuse to register the second CR. Surface `Status.Conditions[type=Ready, status=False, reason=FamilyInstanceArgConflict]` with a message identifying the offending sibling. Tiebreaker: older `creationTimestamp` wins; later CR enters `Ready=False` until the conflict is resolved.

What this depends on

  1. Design decision — is the new pattern worth introducing in muster? Trade-offs:
    • Operator UX: `Ready=False` + reason is clearly better than a log message.
    • Implementation cost: new pattern, more code, more state to manage.
    • Multi-tenant safety: with fail-fast a misconfigured CR cannot silently disable a family for everyone.
  2. `api.MCPServerInfo` extension — `internal/mcpserver/api_adapter.go::convertCRDToInfo` does not currently surface `Status.Conditions[]`. Adding the field is mechanical (~20 lines) but only useful if there is a condition to observe.
  3. Reconciler enforcement — add cross-CR `List` + tiebreaker logic in `internal/reconciler/mcpserver_reconciler.go`. Defence-in-depth at `aggregator.Register` returning `ErrFamilyInstanceArgConflict`.
  4. BDD scenarios — two scenarios from the original spec.family — deferred bug surfaces and behavioural reproducers #697 list become writeable once `api.MCPServerInfo.Conditions` exists:
    • `mcpserver-family-instance-arg-conflict-reconciler` — assert the loser carries the condition; patching it to match clears the condition.
    • `mcpserver-family-instance-arg-conflict-resolution-on-delete` — assert the loser becomes Ready after the winner is deleted.

Recommendation

Hold until either (a) a separate need lands for cross-CR conflict detection (e.g. another CR type also needs it), or (b) a real operator incident shows the silent-fallback behaviour caused unexpected outages.

If we do this, do it as a focused PR that also lays the groundwork (condition-surfacing infrastructure + reconciler tiebreaker pattern) so subsequent CR types can reuse it.

Related:

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions