Skip to content

ADR-052: Resource Conflict Error Handling in Reconciliation Loop

Status: Accepted Date: 2025-11-24 Updated: 2026-01-24 Related: ADR-011 (Error Handling and Retry Strategy), ADR-037 (Build-Validation Sequencing) Supersedes: Previously numbered as ADR-042 (renumbered to resolve duplicate)

Status

Accepted

Context

During E2E testing on OpenShift 4.18, Tier 2 and Tier 3 tests (which use Tekton builds) were experiencing immediate failures with the error:

Operation cannot be fulfilled on notebookvalidationjobs.mlops.mlops.dev "tier2-test-01-sentiment-model":
the object has been modified; please apply your changes to the latest version and try again

Root Cause Analysis

Observation from Logs: 1. Job enters Building phase successfully 2. Tekton Pipeline and PipelineRun created successfully 3. Controller attempts to update BuildStatus in job status 4. Gets Kubernetes resource conflict error (HTTP 409) 5. Error handler incorrectly marks job as Failed instead of requeuing

Why Conflicts Occur: - Multiple reconciliation loops triggered simultaneously: - Initial job creation triggers reconciliation - Status updates trigger watch events → new reconciliation - Webhook mutations trigger watch events → new reconciliation - Each loop fetches the same resource version - When multiple loops try to Update() status, all but one get conflicts

Expected Behavior vs Actual: - ✅ Expected: Conflict errors should be treated as transient, causing a requeue without incrementing retry count - ❌ Actual: Conflict errors were classified as "Retriable", causing retry count increment and eventual job failure

Why This Affects Tier ⅔ But Not Tier 1

Tier 1 Tests (Simple, no builds): - Single reconciliation path: Initializing → ValidationRunning → Succeeded - Fewer status updates = lower chance of conflicts

Tier ⅔ Tests (Complex, with Tekton builds): - Complex reconciliation: Initializing → Building → (checking build status) → BuildComplete → ValidationRunning → Succeeded - Many more status updates for BuildStatus tracking - Pipeline/PipelineRun creation triggers multiple watch events - Much higher probability of concurrent reconciliation = higher conflict rate

Decision

Modify the classifyError() function (per ADR-011) to treat resource conflicts as transient errors.

Implementation

func classifyError(err error) string {
    if err == nil {
        return ""
    }

    // Transient errors - these should requeue without incrementing retry count
    // Conflicts are transient because they occur when multiple reconciliation loops
    // try to update the same resource - the next reconciliation will succeed
    if errors.IsConflict(err) || errors.IsServerTimeout(err) ||
       errors.IsTimeout(err) || errors.IsServiceUnavailable(err) {
        return "Transient"
    }

    // Terminal errors
    if errors.IsInvalid(err) || errors.IsBadRequest(err) || errors.IsForbidden(err) {
        return "Terminal"
    }

    // Default to retriable
    return "Retriable"
}

Error Handling Behavior

Transient Errors (includes conflicts): - Requeue after 1 minute - Do NOT increment retry count - Do NOT mark job as Failed - Allow controller-runtime to handle optimistic locking on retry

Retriable Errors (network issues, temporary failures): - Increment retry count - Exponential backoff: 1m, 2m, 5m - Mark as Failed after 3 retries

Terminal Errors (validation errors, permissions): - Immediate failure - No retry - Clear error message to user

Consequences

Positive

  1. Eliminates False Failures: Jobs no longer fail due to benign race conditions
  2. Aligns with Kubernetes Best Practices: Conflicts are expected in distributed systems
  3. Improves E2E Test Reliability: Tier ⅔ tests now pass consistently
  4. Preserves ADR-011 Strategy: Conflict handling fits naturally into existing error classification
  5. Zero User Impact: Users never see conflict errors as they're handled transparently

Negative

  1. Slightly Longer Reconciliation Time: Conflicts cause 1-minute requeue delay
  2. Mitigation: This is acceptable for build workflows that already take 5-20 minutes
  3. Masks Developer Errors: If controller code has actual bugs causing excessive conflicts
  4. Mitigation: Added logging and metrics to track conflict frequency

Testing

Unit Tests Added: - TestClassifyError_ConflictErrors: Verifies conflicts classified as "Transient" - TestClassifyError_TimeoutErrors: Verifies existing transient error handling - TestClassifyError_TerminalErrors: Verifies terminal error classification - TestClassifyError_RetriableErrors: Verifies default retriable classification

E2E Test Coverage: - Tier 1: Simple notebooks (no builds) - Already passing - Tier 2: Intermediate notebooks with Tekton builds - Now expected to pass - Tier 3: Complex integration tests with credentials - Now expected to pass

References

Implementation Checklist

  • Update classifyError() to check errors.IsConflict()
  • Add unit tests for conflict error classification
  • Verify existing tests still pass
  • Document decision in ADR-052
  • Run E2E tests to verify Tier ⅔ now pass
  • Monitor conflict rate in production (metrics)
  • Update troubleshooting guide with conflict information

Metrics to Monitor

After deployment, track: - notebook_validation_reconciliation_errors_total{type="conflict"} - Conflict error rate - notebook_validation_reconciliation_duration_seconds - Impact on reconciliation time - Tier ⅔ test pass rate in CI/CD

Expected: Conflict rate should be low (<5% of reconciliations) and have no user-visible impact.

  • GitHub Actions E2E Test Run: https://github.com/tosin2013/jupyter-notebook-validator-operator/actions/runs/19636832637
  • Issue: Tier ⅔ tests failing with "Operation cannot be fulfilled" error