fix(redis): quiet skip on Regrade when namespace is missing (#36)

mastermanas805 · claude · web-flow · commit 6286073f9d11 · 2026-05-30T22:29:40.000+05:30
The reconciler iterates every active `resources` row of type=redis and calls provisioner Regrade per row. If a tenant namespace was force-deleted or a cluster wipe raced ahead of platform-DB cleanup, the namespace is gone — but the resource row is still status='active' so the reconciler keeps trying every ~5min tick. Pre-fix path: Secrets.Get → IsNotFound (b/c namespace missing) → exec fallback → Pods.List → empty → WARN `no Redis pod found — soft skip`. Twice per orphan per tick, indistinguishable in logs from a real legacy pod missing its Secret. Verified live in prod (commit eeb33ab) on 2026-05-30: two orphaned namespaces emitting ~576 WARN/day combined, drowning real signals (`maxmemory_policy CONFIG REWRITE failed`, `non-integer maxmemory`). Fix: at Regrade entry, do a single Namespaces.Get. If IsNotFound, return SkipReason="namespace not found (resource orphaned)" with an INFO log (distinct from exec-fallback WARNs so operators can differentiate). Saves the Secrets.Get + Pods.List round-trip per orphan per tick. Tests: * TestRegrade_NamespaceMissing_QuietSkip — new orphan path: zero exec calls, exact SkipReason match. * TestRegrade_NamespaceLookupTransientError_PropagatesError — non- IsNotFound errors from namespace lookup MUST propagate so the reconciler retries (paid customers' regrades not silently dropped). * TestRegrade_NoPodsAtAll_SoftSkip rewritten: now asserts namespace EXISTS but is pod-less (the genuine soft-skip case) and that the SkipReason is NOT the orphan sentinel. * Updated all 7 existing Regrade tests in coverage_test.go + k8s_test.go that previously relied on the empty-cluster path to add the corev1.Namespace fixture (via a new nsObject helper). Co-authored-by: Manas Srivastava <[email protected]> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
diff --git a/internal/backend/redis/coverage_test.go b/internal/backend/redis/coverage_test.go
@@ -1037,6 +1037,11 @@ func TestK8sBackend_Regrade_NoServiceSoftSkip(t *testing.T) {
 	b := newFakeK8sBackend(t)
 	ctx := context.Background()
 	const ns = "no-svc-regrade-ns"
+	// Namespace present (pre-orphan-check fix requires this) + secret present
+	// + service absent → soft-skip with the legacy-resource SkipReason.
+	_, _ = b.cs.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: ns},
+	}, metav1.CreateOptions{})
 	_, _ = b.cs.CoreV1().Secrets(ns).Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "redis-auth", Namespace: ns},
 		Data:       map[string][]byte{"REDIS_PASSWORD": []byte("x")},
@@ -1054,8 +1059,9 @@ func TestK8sBackend_Regrade_NoServiceSoftSkip(t *testing.T) {
 }
 
 // TestK8sBackend_Regrade_DefaultProviderResourceID — when providerResourceID is
-// empty, the namespace is derived from the token. Secret missing → exec path
-// (no pods → soft-skip), but the namespace lookup still happens.
+// empty, the namespace is derived from the token (redisK8sNsPrefix+token). With
+// an empty cluster the derived namespace is missing → orphan short-circuit fires
+// → quiet skip with SkipReason="namespace not found (resource orphaned)".
 func TestK8sBackend_Regrade_DefaultProviderResourceID(t *testing.T) {
 	b := newFakeK8sBackend(t)
 	result, err := b.Regrade(context.Background(), "tok-derive", "", 50)
@@ -1065,6 +1071,9 @@ func TestK8sBackend_Regrade_DefaultProviderResourceID(t *testing.T) {
 	if result.Applied {
 		t.Error("Applied should be false for empty cluster")
 	}
+	if !strings.Contains(result.SkipReason, "namespace not found") {
+		t.Errorf("SkipReason = %q; want 'namespace not found ...' (derived ns missing → orphan path)", result.SkipReason)
+	}
 }
 
 // TestK8sBackend_StorageBytes_DefaultNamespace exercises the empty-PRID branch
@@ -1263,6 +1272,9 @@ func TestK8sBackend_Regrade_DirectPath_HappyAndIdempotent(t *testing.T) {
 
 	b := newFakeK8sBackend(t)
 	const ns = "regrade-real-ns"
+	_, _ = b.cs.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: ns},
+	}, metav1.CreateOptions{})
 	_, _ = b.cs.CoreV1().Secrets(ns).Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "redis-auth", Namespace: ns},
 		Data:       map[string][]byte{"REDIS_PASSWORD": []byte("")},
@@ -1307,6 +1319,9 @@ func TestK8sBackend_Regrade_DirectPath_Unlimited(t *testing.T) {
 
 	b := newFakeK8sBackend(t)
 	const ns = "regrade-unlim-ns"
+	_, _ = b.cs.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: ns},
+	}, metav1.CreateOptions{})
 	_, _ = b.cs.CoreV1().Secrets(ns).Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "redis-auth", Namespace: ns},
 		Data:       map[string][]byte{"REDIS_PASSWORD": []byte("")},
@@ -1516,14 +1531,16 @@ func TestK8sBackend_StorageBytes_GetSecretError(t *testing.T) {
 }
 
 // TestK8sBackend_Regrade_GetSecretError covers the non-NotFound secret error
-// branch on Regrade.
+// branch on Regrade. The namespace fixture is required so the new orphan
+// short-circuit does not fire before the secret reactor runs.
 func TestK8sBackend_Regrade_GetSecretError(t *testing.T) {
-	cs := fake.NewClientset()
+	const ns = "ns"
+	cs := fake.NewClientset(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})
 	cs.PrependReactor("get", "secrets", func(_ k8stesting.Action) (bool, runtime.Object, error) {
 		return true, nil, fmt.Errorf("apiserver unavailable")
 	})
 	b := &K8sBackend{cs: cs}
-	_, err := b.Regrade(context.Background(), "tok", "ns", 50)
+	_, err := b.Regrade(context.Background(), "tok", ns, 50)
 	if err == nil {
 		t.Error("Regrade should propagate non-NotFound secret errors")
 	}
@@ -1532,8 +1549,8 @@ func TestK8sBackend_Regrade_GetSecretError(t *testing.T) {
 // TestK8sBackend_Regrade_GetServiceError covers the non-NotFound service error
 // branch on Regrade.
 func TestK8sBackend_Regrade_GetServiceError(t *testing.T) {
-	cs := fake.NewClientset()
 	const ns = "svcerr-ns"
+	cs := fake.NewClientset(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})
 	_, _ = cs.CoreV1().Secrets(ns).Create(context.Background(), &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "redis-auth", Namespace: ns},
 		Data:       map[string][]byte{"REDIS_PASSWORD": []byte("x")},
@@ -1858,6 +1875,9 @@ func TestK8sBackend_Regrade_ConfigGetError(t *testing.T) {
 	b := newFakeK8sBackend(t)
 	ctx := context.Background()
 	const ns = "cgerr-ns"
+	_, _ = b.cs.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: ns},
+	}, metav1.CreateOptions{})
 	_, _ = b.cs.CoreV1().Secrets(ns).Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{Name: "redis-auth", Namespace: ns},
 		Data:       map[string][]byte{"REDIS_PASSWORD": []byte("x")},
diff --git a/internal/backend/redis/k8s.go b/internal/backend/redis/k8s.go
@@ -623,6 +623,30 @@ func (b *K8sBackend) Regrade(ctx context.Context, token, providerResourceID stri
 		ns = redisK8sNsPrefix + token
 	}
 
+	// ── Orphaned-resource short-circuit ───────────────────────────────────────
+	//
+	// If the namespace itself is gone, the resource row is orphaned: deprovision
+	// raced ahead of platform-DB cleanup, the namespace was force-deleted, or a
+	// test cluster was wiped. Without this guard the reconciler hits the
+	// Secrets.Get → IsNotFound → exec-fallback → Pods.List → empty path on every
+	// sweep forever, logging WARN twice per orphan per ~5min tick (verified
+	// 2026-05-30 in prod: 2 orphaned namespaces emitting 576 WARN/day combined).
+	//
+	// Quiet skip: log at INFO (one line per tick is acceptable; debug-only would
+	// hide a real cluster-wide namespace outage) and return a distinct SkipReason
+	// so operators can differentiate orphaned rows ("namespace not found") from
+	// genuine legacy-pod drift ("exec fallback: no pod found"). Also saves two
+	// kube-apiserver round-trips (Secrets.Get + Pods.List) per orphaned row.
+	if _, err := b.cs.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{}); err != nil {
+		if k8serrors.IsNotFound(err) {
+			slog.Info("k8s.redis.Regrade: namespace not found — orphaned resource, skip",
+				"namespace", ns, "token", token)
+			return RegradeResult{Applied: false, SkipReason: "namespace not found (resource orphaned)"}, nil
+		}
+		// Other errors (permission, transport): surface so the caller can retry.
+		return RegradeResult{Applied: false}, fmt.Errorf("k8s redis.Regrade: get namespace: %w", err)
+	}
+
 	// ── Secret-based path (modern resources) ──────────────────────────────────
 	//
 	// Modern resources (provisioned after the redis-auth Secret convention) store
diff --git a/internal/backend/redis/k8s_test.go b/internal/backend/redis/k8s_test.go
@@ -45,6 +45,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes/fake"
+	ktesting "k8s.io/client-go/testing"
 )
 
 // ─── Tier sizing regression guards ──────────────────────────────────────────
@@ -489,6 +490,14 @@ func TestExecCommandConstruction_AlreadyCorrect(t *testing.T) {
 
 // ─── Integration-style tests: Regrade routing ────────────────────────────────
 
+// nsObject returns a minimal corev1.Namespace fixture for use with fake.NewClientset.
+// Required after the orphaned-resource short-circuit (k8s.go) — Regrade now returns
+// SkipReason="namespace not found (resource orphaned)" if the namespace is absent,
+// so every Regrade test that exercises the existing code paths must include it.
+func nsObject(name string) *corev1.Namespace {
+	return &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}}
+}
+
 // TestRegrade_SecretPresent_DoesNotUseExec asserts that when the redis-auth
 // Secret IS present, Regrade uses the direct-connection path and never calls
 // the exec fallback.
@@ -512,7 +521,7 @@ func TestRegrade_SecretPresent_DoesNotUseExec(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{Name: "redis", Namespace: ns},
 		Spec:       corev1.ServiceSpec{ClusterIP: "127.0.0.1"},
 	}
-	cs := fake.NewClientset(secret, svc)
+	cs := fake.NewClientset(nsObject(ns), secret, svc)
 
 	fe := &fakeExecor{}
 	b := &K8sBackend{cs: cs, execor: fe}
@@ -545,7 +554,7 @@ func TestRegrade_SecretAbsent_UsesExecPath(t *testing.T) {
 	// No secret — secret lookup returns NotFound.
 	// Pod is present and Running so exec can proceed.
 	pod := runningPod(ns, "redis-aaa", "redis")
-	cs := fake.NewClientset(pod)
+	cs := fake.NewClientset(nsObject(ns), pod)
 
 	fe := &fakeExecor{
 		responses: []fakeExecResponse{
@@ -596,7 +605,7 @@ func TestRegrade_ExecFails_SoftSkip(t *testing.T) {
 	const token = "tok"
 
 	pod := runningPod(ns, "redis-aaa", "redis")
-	cs := fake.NewClientset(pod)
+	cs := fake.NewClientset(nsObject(ns), pod)
 
 	fe := &fakeExecor{
 		responses: []fakeExecResponse{
@@ -633,7 +642,7 @@ func TestRegrade_NoPodRunning_SoftSkip(t *testing.T) {
 		},
 		Status: corev1.PodStatus{Phase: corev1.PodPending},
 	}
-	cs := fake.NewClientset(pod)
+	cs := fake.NewClientset(nsObject(ns), pod)
 
 	fe := &fakeExecor{}
 	b := &K8sBackend{cs: cs, execor: fe}
@@ -650,13 +659,15 @@ func TestRegrade_NoPodRunning_SoftSkip(t *testing.T) {
 	}
 }
 
-// TestRegrade_NoPodsAtAll_SoftSkip asserts that when the namespace has no pods
-// at all, the exec fallback returns a soft-skip.
+// TestRegrade_NoPodsAtAll_SoftSkip asserts that when the namespace EXISTS but has
+// no pods at all (e.g. a legacy pod was deleted but the namespace lingers), the
+// exec fallback returns a soft-skip.
 func TestRegrade_NoPodsAtAll_SoftSkip(t *testing.T) {
 	const ns = "instant-customer-tok"
 	const token = "tok"
 
-	cs := fake.NewClientset() // empty cluster
+	// Namespace exists, but no secret + no pods → exec fallback → no pod found.
+	cs := fake.NewClientset(nsObject(ns))
 
 	fe := &fakeExecor{}
 	b := &K8sBackend{cs: cs, execor: fe}
@@ -668,6 +679,78 @@ func TestRegrade_NoPodsAtAll_SoftSkip(t *testing.T) {
 	if result.Applied {
 		t.Errorf("Applied should be false when namespace has no pods")
 	}
+	// Distinct skip reason: not the orphan-namespace path.
+	if result.SkipReason == "namespace not found (resource orphaned)" {
+		t.Errorf("SkipReason should be exec-fallback reason, not orphan-ns reason; got %q", result.SkipReason)
+	}
+}
+
+// TestRegrade_NamespaceMissing_QuietSkip asserts the orphaned-resource short-circuit:
+// when the namespace is gone (deprovisioned tenant, force-deleted ns, wiped cluster),
+// Regrade returns a quiet skip with the orphan SkipReason and makes NO Secrets.Get
+// or Pods.List call — verified by checking the fakeExecor.calls count stays zero AND
+// the SkipReason exactly matches the orphan sentinel string.
+//
+// Regression guard: prevents the WARN-spam-per-tick behaviour observed in prod on
+// 2026-05-30 (2 orphaned namespaces emitting ~576 WARN/day combined).
+func TestRegrade_NamespaceMissing_QuietSkip(t *testing.T) {
+	const ns = "instant-customer-gone"
+	const token = "gone"
+
+	// Empty cluster — no namespace exists. The pre-fix code would have
+	// hit Secrets.Get → IsNotFound → exec-fallback → Pods.List → WARN.
+	cs := fake.NewClientset()
+
+	fe := &fakeExecor{}
+	b := &K8sBackend{cs: cs, execor: fe}
+
+	result, err := b.Regrade(context.Background(), token, ns, 50)
+	if err != nil {
+		t.Fatalf("Regrade must not propagate errors for missing namespace; got: %v", err)
+	}
+	if result.Applied {
+		t.Errorf("Applied should be false when namespace is missing")
+	}
+	const wantSkip = "namespace not found (resource orphaned)"
+	if result.SkipReason != wantSkip {
+		t.Errorf("SkipReason = %q; want %q (distinct from exec-fallback reasons so operators can differentiate orphans from real legacy drift)",
+			result.SkipReason, wantSkip)
+	}
+	// Critical: the exec path MUST NOT be entered for orphans. That was the
+	// pre-fix bug — fanout to Pods.List per orphan per tick.
+	if len(fe.calls) != 0 {
+		t.Errorf("fakeExecor must not be called when namespace is missing; got %d calls", len(fe.calls))
+	}
+}
+
+// TestRegrade_NamespaceLookupTransientError_PropagatesError asserts that a
+// non-IsNotFound error from the namespace lookup (e.g. permission denied,
+// apiserver transport failure) is surfaced to the caller — NOT swallowed as a
+// silent skip. The reconciler needs the error to schedule a retry; treating it
+// as "orphaned" would cause skipped grades the customer pays for.
+func TestRegrade_NamespaceLookupTransientError_PropagatesError(t *testing.T) {
+	const ns = "instant-customer-tok"
+	const token = "tok"
+
+	cs := fake.NewClientset(nsObject(ns))
+	// Inject a transient error on the namespace GET reactor.
+	cs.PrependReactor("get", "namespaces", func(_ ktesting.Action) (bool, runtime.Object, error) {
+		return true, nil, fmt.Errorf("etcd: leader changed")
+	})
+
+	fe := &fakeExecor{}
+	b := &K8sBackend{cs: cs, execor: fe}
+
+	result, err := b.Regrade(context.Background(), token, ns, 50)
+	if err == nil {
+		t.Fatalf("Regrade must propagate non-IsNotFound errors so the reconciler retries; got nil error, result=%+v", result)
+	}
+	if result.Applied {
+		t.Errorf("Applied should be false on namespace lookup error")
+	}
+	if len(fe.calls) != 0 {
+		t.Errorf("fakeExecor must not be called on namespace lookup error; got %d calls", len(fe.calls))
+	}
 }
 
 // TestRegrade_TeamTier_Unlimited verifies that the team tier (unlimited, targetMB=-1)
