[Observability] Detector Controller Concurrent Reconciliation Issues Trigger SLO violations

[jabellard]

Summary

The detector controller frequently fails to create ResourceBindings with "already exists" errors, indicating a race condition where multiple reconciliations attempt to create the same ResourceBinding concurrently.

Problem Description

When creating Deployments, the detector controller reconciles them and creates ResourceBindings using controllerutil.CreateOrUpdate(). However, we observe frequent "already exists" errors:

Error Pattern:

E0118 02:08:38.005102 detector.go:496] Failed to apply policy(deployment-policy) for object: apps/v1, kind=Deployment, default/nginx-deployment-1. error: resourcebindings.work.karmada.io "nginx-deployment-1-deployment" already exists

Frequency: 218 errors in 5 minutes for 3 Deployments (~73 errors per deployment)

Sample Logs

The logs show two reconciliations starting within microseconds and one succeeding while the other fails:

I0118 02:08:37.988786 detector.go:242] Reconciling object: apps/v1, kind=Deployment, default/nginx-deployment-1
I0118 02:08:37.988852 detector.go:242] Reconciling object: apps/v1, kind=Deployment, default/nginx-deployment-1 

I0118 02:08:38.001183 detector.go:502] Create ResourceBinding(default/nginx-deployment-1-deployment) successfully.
E0118 02:08:38.005102 detector.go:496] Failed to apply policy(deployment-policy) for object: apps/v1, kind=Deployment, default/nginx-deployment-1. error: resourcebindings.work.karmada.io "nginx-deployment-1-deployment" already exists

Why CreateOrUpdate Should Not Return AlreadyExists

The controllerutil.CreateOrUpdate() function internally performs:

1. GET the resource
2. If NotFound → CREATE
3. If exists → UPDATE

Normal (non-concurrent) behavior:

• GET → NotFound → CREATE → Success ✅
• GET → Found → UPDATE → Success ✅

The AlreadyExists error can ONLY occur if:

Reconciliation A: GET → NotFound
Reconciliation B: GET → NotFound  ← Happens concurrently
Reconciliation A: CREATE → Success ✅
Reconciliation B: CREATE → AlreadyExists ❌  ← Race condition!

This indicates that two reconciliations are running concurrently for the same Deployment.

Root Cause

The race window exists in the ApplyPolicy function:

File: pkg/detector/detector.go:481-517

err = retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
    operationResult, err = controllerutil.CreateOrUpdate(context.TODO(), d.Client, bindingCopy, func() error {
        // ... update binding fields ...
    })
    return err
})

The problem:

• Both Conflict and AlreadyExists are HTTP 409 errors, but with different StatusReason values:
  • Conflict: Code=409, Reason="Conflict" (stale resourceVersion on UPDATE)
  • AlreadyExists: Code=409, Reason="AlreadyExists" (duplicate CREATE)
• retry.RetryOnConflict only retries on errors with StatusReason="Conflict"
• It does NOT retry on errors with StatusReason="AlreadyExists"
• When concurrent reconciliations race to CREATE, one fails with AlreadyExists and the error propagates

Proposed Fix

Replace retry.RetryOnConflict with retry.OnError that handles both Conflict and AlreadyExists:

err = retry.OnError(retry.DefaultRetry, func(err error) bool {
    // Retry on both Conflict and AlreadyExists errors
    return apierrors.IsConflict(err) || apierrors.IsAlreadyExists(err)
}, func() (err error) {
   ...

        return nil
    })
    return err
})

Affected Code Locations

The same pattern needs to be fixed in 3 locations:

1. detector.go:481-517 - ApplyPolicy() for PropagationPolicy → ResourceBinding
2. detector.go:573-602 - ApplyClusterPolicy() for ClusterPropagationPolicy → ResourceBinding (namespaced)
3. detector.go:624-651 - ApplyClusterPolicy() for ClusterPropagationPolicy → ClusterResourceBinding (cluster-scoped)

SLO Impact

This issue directly affects the policy-apply-availability SLO through the metrics.ObserveApplyPolicyAttemptAndLatency() metric recorded at detector.go:452:

defer func() {
    metrics.ObserveApplyPolicyAttemptAndLatency(err, start)
    if err != nil {
        d.EventRecorder.Eventf(object, corev1.EventTypeWarning, events.EventReasonApplyPolicyFailed, ...)
    }
}()

Each AlreadyExists error is counted as a failed policy application attempt, causing:

SLO Violation:

• Total attempts: 345 (in 5 minutes)
• Failures: 218 AlreadyExists errors (63.2%)
• Success rate: 36.8%
• SLO target: 99.9%
• Status: ❌ VIOLATED (63% failure rate vs 0.1% threshold)

Prometheus query:

# Policy apply failure rate
sum (rate(policy_apply_attempts_total{esult="error"}[5m])
/
sum(rate(policy_apply_attempts_total[5m]))

# Shows: 0.632 (63.2%) - CRITICAL VIOLATION

Expected Impact

Before fix:

• AlreadyExists errors: 218 in 5 minutes
• Policy apply success rate: 36.8%
• SLO status: ❌ VIOLATED (63% failure rate)
• User impact: Warning events, failed reconciliations

After fix:

• AlreadyExists errors: 0 (handled gracefully via retry)
• Policy apply success rate: 99.9%+
• SLO status: ✅ COMPLIANT (meets 99.9% target)
• User impact: Transparent retry, clean success

[jabellard]

Hey @RainbowMango. This is another issue simiar to this one. Looks like there's no guarantee for preventing the detector controller workers/threads from reconciling the same object concurrently.

[jabellard]

@RainbowMango , for more context, I have 3 concurrent jobs running to simulate some action for Karmada:

kubectl -n experiments get deployments.apps                                                                                      
NAME                     READY   UP-TO-DATE   AVAILABLE   AGE
karmada-schedule-job-1   1/1     1            1           28h
karmada-schedule-job-2   1/1     1            1           28h
karmada-schedule-job-3   1/1     1            1           28h

Each one of those jobs runs a shell script that does the following:

• Create a propagation policy
• In an infinite loop:
  • Create a deployment (e.g., nginx-deployment-{job-index}) followed by a small delay
  • Scale the deployment followed by a small delay
  • Delete the deployment followed by a small delay

Here's what the shell script looks like:

#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

KUBECONFIG_PATH=""
TRACE_ENABLED=false

usage() {
  cat <<USAGE
Usage: $0 --kubeconfig <path>  [--trace]

Options:
  --kubeconfig    Path to kubeconfig file (required)
  --trace         Enable bash xtrace (set -x)
USAGE
}

while [[ "$#" -gt 0 ]]; do
  case "$1" in
    --kubeconfig) KUBECONFIG_PATH="$2"; shift ;;
    --trace)      TRACE_ENABLED=true ;;
    -h|--help)    usage; exit 0 ;;
    *) echo "Unknown parameter passed: $1"; usage; exit 1 ;;
  esac
  shift
done

if [[ -z "${KUBECONFIG_PATH}" ]]; then
  echo "Error: --kubeconfig is required."
  usage
  exit 1
fi

if [[ ! -f "$KUBECONFIG_PATH" ]]; then
  echo "Error: kubeconfig file not found at $KUBECONFIG_PATH"
  exit 1
fi

if [ "$TRACE_ENABLED" = true ]; then
  set -o xtrace
fi

echo "Creating PropagationPolicy in karmada..."
kubectl --kubeconfig="$KUBECONFIG_PATH" apply -f - <<'POLICYEOF'
apiVersion: policy.karmada.io/v1alpha1
kind: PropagationPolicy
metadata:
  name: deployment-policy
  namespace: default
spec:
  resourceSelectors:
    - apiVersion: apps/v1
      kind: Deployment
  placement:
    replicaScheduling:
      replicaDivisionPreference: Weighted
      replicaSchedulingType: Divided
      weightPreference:
        dynamicWeight: AvailableReplicas
POLICYEOF

echo "Starting deployment lifecycle loop. Press Ctrl+C to stop."
while true; do
  echo "=== Applying nginx Deployment with 1 replicas..."
  kubectl --kubeconfig="$KUBECONFIG_PATH" apply -f - <<'INNEREOF'
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment-1
  namespace: default
  labels:
    app: nginx-1
    team: a
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx-1
  template:
    metadata:
      labels:
        app: nginx-1
        team: a
    spec:
      containers:
        - name: nginx
          image: nginx:latest
          ports:
            - containerPort: 80
INNEREOF

  echo "Waiting for initial rollout to complete (1 replicas)..."
  kubectl --kubeconfig="$KUBECONFIG_PATH" --namespace default rollout status deployment/nginx-deployment-1 --timeout=180s

  echo "Sleeping for 2 seconds..."
  sleep 2

  echo "Scaling to 2 replicas..."
  kubectl --kubeconfig="$KUBECONFIG_PATH" --namespace default scale deployment/nginx-deployment-1 --replicas=2

  echo "Waiting for all 2 replicas to be ready..."
  kubectl --kubeconfig="$KUBECONFIG_PATH" --namespace default rollout status deployment/nginx-deployment-1 --timeout=180s

  echo "Sleeping for 2 seconds..."
  sleep 2

  echo "Deleting deployment..."
  kubectl --kubeconfig="$KUBECONFIG_PATH" --namespace default delete deployment/nginx-deployment-1

  echo "Sleeping for 2 seconds..."
  sleep 2
done

The availability SLO based on the recorded metrics I referenced above that is getting violated:

The error budget for that SLO, for this scenario, is currently getting burned at 250x faster than allowed.

[RainbowMango]

Hey @RainbowMango. This is another issue simiar to this one. Looks like there's no guarantee for preventing the detector controller workers/threads from reconciling the same object concurrently.

No, I don't think so. Each object(resource template in the detector) should be reconciled by exactly one queue worker. That's the controller basis. Otherwise, the reconciled result would become unexpected, just like the bug we fixed in release v1.15(#6674).

Frequency: 218 errors in 5 minutes for 3 Deployments (~73 errors per deployment)
The logs show two reconciliations starting within microseconds and one succeeding while the other fails:

This is unusual, and I guess there is something wrong here. I think we need to dig into it and try find the root cause.