From 6046458062ac58656f0b350d8b094e3c6f35fe93 Mon Sep 17 00:00:00 2001 From: haojiwu Date: Thu, 12 Mar 2026 13:10:38 -0700 Subject: [PATCH] Fix RepairIPAddress controller startup failure when namespace informer is not yet synced Cherry-pick of https://github.com/kubernetes/kubernetes/pull/137147 --- .../ipallocator/controller/repairip.go | 13 ++- .../ipallocator/controller/repairip_test.go | 81 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/pkg/registry/core/service/ipallocator/controller/repairip.go b/pkg/registry/core/service/ipallocator/controller/repairip.go index feb7fce3076..79c5060e2e5 100644 --- a/pkg/registry/core/service/ipallocator/controller/repairip.go +++ b/pkg/registry/core/service/ipallocator/controller/repairip.go @@ -247,7 +247,18 @@ func (r *RepairIPAddress) RunUntil(onFirstSuccess func(), stopCh chan struct{}) // runOnce verifies the state of the ClusterIP allocations and returns an error if an unrecoverable problem occurs. func (r *RepairIPAddress) runOnce() error { - return retry.RetryOnConflict(retry.DefaultBackoff, r.doRunOnce) + return retry.OnError(retry.DefaultBackoff, func(err error) bool { + // When trying to repair the ClusterIP allocations, we may get a conflict or forbidden error. + // IsForbidden depends on the admission chain to be ready that may depend on the + // Namespace informer to be ready. + // Ref: https://issues.k8s.io/136288 + if apierrors.IsConflict(err) || apierrors.IsForbidden(err) { + klog.ErrorS(err, "Running ipallocator repair failed ... retrying") + return true + } + klog.ErrorS(err, "Running ipallocator repair failed with not retryable error") + return false + }, r.doRunOnce) } // doRunOnce verifies the state of the ClusterIP allocations and returns an error if an unrecoverable problem occurs. diff --git a/pkg/registry/core/service/ipallocator/controller/repairip_test.go b/pkg/registry/core/service/ipallocator/controller/repairip_test.go index 1e44687f025..8a4af0b8d4a 100644 --- a/pkg/registry/core/service/ipallocator/controller/repairip_test.go +++ b/pkg/registry/core/service/ipallocator/controller/repairip_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -707,3 +708,83 @@ func expectEvents(t *testing.T, actual <-chan string, expected []string) { } } } + +func TestRepairIPAddress_runOnce(t *testing.T) { + tests := []struct { + name string + conflictCount int + errorType func(string) error + expectedRuns int + expectedErr bool + }{ + { + name: "Retry on Conflict", + conflictCount: 2, + errorType: func(name string) error { + return apierrors.NewConflict(networkingv1.Resource("ipaddresses"), name, fmt.Errorf("conflict")) + }, + expectedRuns: 3, + expectedErr: false, + }, + { + name: "Retry on Forbidden", + conflictCount: 2, + errorType: func(name string) error { + return apierrors.NewForbidden(networkingv1.Resource("ipaddresses"), name, fmt.Errorf("forbidden")) + }, + expectedRuns: 3, + expectedErr: false, + }, + { + name: "No Retry on InternalError", + conflictCount: 1, // It will fail once and stop + errorType: func(name string) error { + return apierrors.NewInternalError(fmt.Errorf("internal error")) + }, + expectedRuns: 1, + expectedErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, r := newFakeRepair(testTimeNow) + // Add a service that needs repair (missing IPAddress) + svc := newService("test-svc", []string{"10.0.1.1"}) + err := r.serviceStore.Add(svc) + if err != nil { + t.Fatalf("Unexpected error adding service: %v", err) + } + r.servicesSynced = func() bool { return true } + r.ipAddressSynced = func() bool { return true } + r.serviceCIDRSynced = func() bool { return true } + + // Add a default ServiceCIDR + cidr := newServiceCIDR("kubernetes", serviceCIDRv4, serviceCIDRv6) + err = r.serviceCIDRStore.Add(cidr) + if err != nil { + t.Fatalf("Unexpected error adding ServiceCIDR: %v", err) + } + + // Track how many times create is called + createCalls := 0 + client.PrependReactor("create", "ipaddresses", func(action k8stesting.Action) (bool, runtime.Object, error) { + createCalls++ + if createCalls <= tt.conflictCount { + return true, nil, tt.errorType("10.0.1.1") + } + // Pass through to the default reactor (which creates the object) + return false, nil, nil + }) + + err = r.runOnce() + if (err != nil) != tt.expectedErr { + t.Errorf("runOnce() error = %v, expectedErr %v", err, tt.expectedErr) + } + + if createCalls != tt.expectedRuns { + t.Errorf("Expected %d create calls, got %d", tt.expectedRuns, createCalls) + } + }) + } +}