diff --git a/staging/src/k8s.io/component-helpers/apimachinery/lease/controller.go b/staging/src/k8s.io/component-helpers/apimachinery/lease/controller.go index 843eb36133c..282eeea2426 100644 --- a/staging/src/k8s.io/component-helpers/apimachinery/lease/controller.go +++ b/staging/src/k8s.io/component-helpers/apimachinery/lease/controller.go @@ -65,9 +65,8 @@ type controller struct { latestLease *coordinationv1.Lease // newLeasePostProcessFunc allows customizing a lease object (e.g. setting OwnerReference) - // before every time the lease is created/refreshed(updated). Note that an error will block - // a lease CREATE, causing the controller to retry next time, but an error won't block a - // lease UPDATE. + // before every time the lease is created/refreshed(updated). + // Note that an error will block the lease operation. newLeasePostProcessFunc ProcessLeaseFunc } @@ -184,17 +183,21 @@ func (c *controller) ensureLease(ctx context.Context) (*coordinationv1.Lease, bo // call this once you're sure the lease has been created func (c *controller) retryUpdateLease(ctx context.Context, base *coordinationv1.Lease) error { for i := 0; i < maxUpdateRetries; i++ { - leaseToUpdate, _ := c.newLease(base) - lease, err := c.leaseClient.Update(ctx, leaseToUpdate, metav1.UpdateOptions{}) - if err == nil { - c.latestLease = lease - return nil - } - klog.FromContext(ctx).Error(err, "Failed to update lease") - // OptimisticLockError requires getting the newer version of lease to proceed. - if apierrors.IsConflict(err) { - base, _ = c.backoffEnsureLease(ctx) - continue + leaseToUpdate, err := c.newLease(base) + if err != nil { + klog.FromContext(ctx).Error(err, "Failed to prepare lease") + } else { + lease, err := c.leaseClient.Update(ctx, leaseToUpdate, metav1.UpdateOptions{}) + if err == nil { + c.latestLease = lease + return nil + } + klog.FromContext(ctx).Error(err, "Failed to update lease") + // OptimisticLockError requires getting the newer version of lease to proceed. + if apierrors.IsConflict(err) { + base, _ = c.backoffEnsureLease(ctx) + continue + } } if i > 0 && c.onRepeatedHeartbeatFailure != nil { c.onRepeatedHeartbeatFailure() diff --git a/staging/src/k8s.io/component-helpers/apimachinery/lease/controller_test.go b/staging/src/k8s.io/component-helpers/apimachinery/lease/controller_test.go index 42ab50efe5e..2729ba8edfa 100644 --- a/staging/src/k8s.io/component-helpers/apimachinery/lease/controller_test.go +++ b/staging/src/k8s.io/component-helpers/apimachinery/lease/controller_test.go @@ -227,6 +227,7 @@ func TestRetryUpdateNodeLease(t *testing.T) { getReactor func(action clienttesting.Action) (bool, runtime.Object, error) onRepeatedHeartbeatFailure func() expectErr bool + client *fake.Clientset }{ { desc: "no errors", @@ -236,6 +237,7 @@ func TestRetryUpdateNodeLease(t *testing.T) { getReactor: nil, onRepeatedHeartbeatFailure: nil, expectErr: false, + client: fake.NewSimpleClientset(node), }, { desc: "connection errors", @@ -245,6 +247,7 @@ func TestRetryUpdateNodeLease(t *testing.T) { getReactor: nil, onRepeatedHeartbeatFailure: nil, expectErr: true, + client: fake.NewSimpleClientset(node), }, { desc: "optimistic lock errors", @@ -267,12 +270,24 @@ func TestRetryUpdateNodeLease(t *testing.T) { }, onRepeatedHeartbeatFailure: func() { t.Fatalf("onRepeatedHeartbeatFailure called") }, expectErr: false, + client: fake.NewSimpleClientset(node), + }, + { + desc: "node not found errors", + updateReactor: func(action clienttesting.Action) (bool, runtime.Object, error) { + t.Fatalf("lease was updated when node does not exist!") + return true, nil, nil + }, + getReactor: nil, + onRepeatedHeartbeatFailure: nil, + expectErr: true, + client: fake.NewSimpleClientset(), }, } for _, tc := range cases { t.Run(tc.desc, func(t *testing.T) { _, ctx := ktesting.NewTestContext(t) - cl := fake.NewSimpleClientset(node) + cl := tc.client if tc.updateReactor != nil { cl.PrependReactor("update", "leases", tc.updateReactor) }