Transient leafref validation failures are treated as unrecoverable, causing multi-minute stalls on deletion

[steiler]

Summary

When a Config or ConfigSet deletion is processed, data-server validates the transaction against the syncTree (the cached running device state). If the device was recently changed outside SDCio (e.g. a direct gNMI Set), the syncTree may be stale for up to one sync interval (default 10 s in CI, configurable via the sync profile). During that window, a leafref that points into the changed path still appears valid in the syncTree, causing data-server to reject the deletion with a leafref validation error.

config-server currently treats all intent-level validation errors as unrecoverable, so it stamps the affected Config with FailedUnRecoverable and returns ctrl.Result{} with an error. This hands control to controller-runtime's exponential back-off rate limiter (5 ms base, 1000 s max). By the time the back-off reaches a long-enough interval for the syncTree to have been refreshed by a sync cycle, many minutes have already elapsed — observed as ~7 minutes in CI.

Why running must be included

data-server intentionally includes the running layer (syncTree) in leafref resolution to support brownfield config: configuration that exists on the device but has no SDCio intent. This is a valid design requirement and cannot simply be removed. However, the syncTree is only as fresh as the last completed sync cycle.

Root cause

The implicit assumption in config-server is that the syncTree is always in sync with the actual device state. In practice there is a bounded gap window:

direct gNMI delete on device
        │
        │  ← gap: up to 1 sync interval (10 s)
        ▼
sync process writes updated state to syncTree

Any SDCio delete transaction that touches leafrefs involving paths changed during this window will fail. The failure is transient — it resolves automatically once the next sync cycle completes — but config-server has no retry path designed for this case.

Code path

1. lowlevelTransactionSet in data-server builds the merged tree (syncTree running layer + intent layers), runs YANG validation including leafrefs, and returns (response_with_intent_errors, nil) on failure — the gRPC call succeeds, errors are in response.Intents[x].Errors.
2. analyzeIntentResponse in config-server (pkg/sdc/target/manager/transactor.go):

   result.Recoverable = false // any intent error is non-recoverable

   No distinction between permanent schema violations and transient stale-state failures.
3. handleTransactionErrors marks each affected Config as FailedUnRecoverable with the current ResourceVersion stored in the condition message.
4. Reconciler returns ctrl.Result{} with an error → controller-runtime exponential back-off.
5. The status patch triggers watch events that eventually re-queue the item, but by then the back-off is compounding (after ~14 failed attempts the per-item delay exceeds 1 minute).

Impact

• Observed in CI: kubectl delete configsets.config.sdcio.dev customer stalled for ~7 minutes waiting for finalizer removal.
• Reproducible in production: any operator-initiated direct device change (CLI, NETCONF, another gNMI client) followed by an SDCio delete within one sync interval triggers the same stall.
• The stall resolves on its own but degrades UX and inflates CI runtimes.

Proposed fix

Add a bounded fast retry in config-server before marking a transaction error as unrecoverable. The key is to use ctrl.Result{RequeueAfter: ...} with a nil error to bypass the exponential back-off rate limiter:

// In Transact or handleTransactionErrors — before stamping FailedUnRecoverable,
// check a per-object retry counter (e.g. annotation or status field).
// If under threshold, return a short requeue instead:
return true, ctrl.Result{RequeueAfter: 15 * time.Second}, nil
// RequeueAfter > sync interval (10 s) guarantees at least one full sync cycle.
// After N retries (e.g. 3–5), fall through to FailedUnRecoverable as today.

This covers the transient case (resolved within 1–2 retries) without masking permanent schema errors (they will exhaust the retry budget and be marked unrecoverable as before).

Related

• Sync interval is configured via the target's sync profile, not a global constant.
• The Sleep 15s added to tests/02-crud/11-sros-create-delete.robot in sdcio/integration-tests is a workaround that prevents the test from hitting this window; it does not fix the underlying behaviour.
• Observed in config-server CI run: https://github.com/sdcio/config-server/actions/runs/26513046368/job/78256037289

[henderiw]

see we had this discussion before.

• Let's define what is recoverable and not. Doing a retry all the time is not a good option either in my view
• also we need to know the reconcile reason as we need to find out what to apply again

[steiler]

Sure, I'm just raising an issue we, at least I haven't had on my radar. So we need to have a discussion how to handle this, because the issue is real.