service-ip-alloc: delay ip processing on service recreate

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.

Signed-off-by: Daman Arora <aroradaman@gmail.com>
This commit is contained in:
Daman Arora
2025-07-13 23:59:30 +05:30
parent 8de14b526e
commit fcb595a2df
2 changed files with 51 additions and 0 deletions

View File

@@ -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{})

View File

@@ -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{