diff --git a/pkg/registry/core/service/ipallocator/controller/repairip.go b/pkg/registry/core/service/ipallocator/controller/repairip.go index bb7c2d877ec..feb7fce3076 100644 --- a/pkg/registry/core/service/ipallocator/controller/repairip.go +++ b/pkg/registry/core/service/ipallocator/controller/repairip.go @@ -530,6 +530,23 @@ func (r *RepairIPAddress) syncIPAddress(key string) error { return nil } } + + // Service storage implements transactions. It creates an IPAddress object first and then creates + // the Service object, and if the Service object already exists the complete transaction is + // reverted. There can be race conditions when the repair loop picks up the new IPAddress object + // for reconciliation before the transaction is reverted. This leads to spurious + // IPAddressWrongReference warnings, to suppress these warnings we delay the processing of the new + // IPAddress object by 5 seconds. The service allocation creates the IPAddress object before creating + // the Service object, we easily identify this scenario when the IPAddress object creation timestamp + // is after the Service creation timestamp. We do this only when the IPAddress object is created + // recently in order to avoid indefinitely requeue/delay in IPAddress cleanup if for some reason + // the service transaction revert fails. + if ipAddress.CreationTimestamp.After(svc.CreationTimestamp.Time) && + r.clock.Now().Sub(ipAddress.CreationTimestamp.Time) < 5*time.Second { + // requeue after the grace period + r.ipQueue.AddAfter(key, 5*time.Second) + return nil + } runtime.HandleError(fmt.Errorf("the IPAddress: %s for Service %s/%s has a wrong reference %#v; cleaning up", ipAddress.Name, svc.Name, svc.Namespace, ipAddress.Spec.ParentRef)) r.recorder.Eventf(ipAddress, nil, v1.EventTypeWarning, "IPAddressWrongReference", "IPAddressAllocation", "IPAddress: %s for Service %s/%s has a wrong reference; cleaning up", ipAddress.Name, svc.Namespace, svc.Name) err = r.client.NetworkingV1().IPAddresses().Delete(context.Background(), ipAddress.Name, metav1.DeleteOptions{}) diff --git a/pkg/registry/core/service/ipallocator/controller/repairip_test.go b/pkg/registry/core/service/ipallocator/controller/repairip_test.go index f716b6035c1..1e44687f025 100644 --- a/pkg/registry/core/service/ipallocator/controller/repairip_test.go +++ b/pkg/registry/core/service/ipallocator/controller/repairip_test.go @@ -372,6 +372,40 @@ func TestRepairServiceIP(t *testing.T) { actions: [][]string{{"delete", "ipaddresses"}}, events: []string{"Warning IPAddressWrongReference IPAddress: 10.0.1.2 for Service bar/test-svc has a wrong reference; cleaning up"}, }, + { + name: "Two IPAddresses referencing the same service when service create is called again and ip address is in grace period", + svcs: []*v1.Service{ + newServiceWithCreationTimestamp("test-svc", []string{"10.0.1.1"}, testTimeNow.Add(1*time.Second)), + }, + ipAddresses: []*networkingv1.IPAddress{ + newIPAddressWithCreationTimestamp("10.0.1.1", newService("test-svc", []string{"10.0.1.1"}), testTimeNow), + // ensure the second IPAddress object has a creation timestamp greater than service creation timestamp + newIPAddressWithCreationTimestamp("10.0.1.2", newService("test-svc", []string{"10.0.1.1"}), testTimeNow.Add(2*time.Second)), + }, + cidrs: []*networkingv1.ServiceCIDR{ + newServiceCIDR("kubernetes", serviceCIDRv4, serviceCIDRv6), + }, + // update the test time, ensuring 5 seconds have not passed after creation of the second IPAddress Object + testTime: testTimeNow.Add(3 * time.Second), + }, + { + name: "Two IPAddresses referencing the same service when service create is called again and ip address is not in grace period", + svcs: []*v1.Service{ + newServiceWithCreationTimestamp("test-svc", []string{"10.0.1.1"}, testTimeNow.Add(1*time.Second)), + }, + ipAddresses: []*networkingv1.IPAddress{ + newIPAddressWithCreationTimestamp("10.0.1.1", newService("test-svc", []string{"10.0.1.1"}), testTimeNow), + // ensure the second IPAddress object has a creation timestamp greater than service creation timestamp + newIPAddressWithCreationTimestamp("10.0.1.2", newService("test-svc", []string{"10.0.1.1"}), testTimeNow.Add(2*time.Second)), + }, + cidrs: []*networkingv1.ServiceCIDR{ + newServiceCIDR("kubernetes", serviceCIDRv4, serviceCIDRv6), + }, + // update the test time, ensuring 5 seconds have passed after creation of the second IPAddress Object + testTime: testTimeNow.Add(10 * time.Second), + actions: [][]string{{"delete", "ipaddresses"}}, + events: []string{"Warning IPAddressWrongReference IPAddress: 10.0.1.2 for Service bar/test-svc has a wrong reference; cleaning up"}, + }, { name: "Two Services with same ClusterIP", svcs: []*v1.Service{