diff --git a/tools/leaderelection/leaderelection.go b/tools/leaderelection/leaderelection.go index c3c1d9be..07180630 100644 --- a/tools/leaderelection/leaderelection.go +++ b/tools/leaderelection/leaderelection.go @@ -306,18 +306,30 @@ func (le *LeaderElector) renew(ctx context.Context) { // release attempts to release the leader lease if we have acquired it. func (le *LeaderElector) release() bool { + ctx := context.Background() + timeoutCtx, timeoutCancel := context.WithTimeout(ctx, le.config.RenewDeadline) + defer timeoutCancel() + // update the resourceVersion of lease + oldLeaderElectionRecord, _, err := le.config.Lock.Get(timeoutCtx) + if err != nil { + if !errors.IsNotFound(err) { + klog.Errorf("error retrieving resource lock %v: %v", le.config.Lock.Describe(), err) + return false + } + klog.Infof("lease lock not found: %v", le.config.Lock.Describe()) + return false + } + if !le.IsLeader() { return true } now := metav1.NewTime(le.clock.Now()) leaderElectionRecord := rl.LeaderElectionRecord{ - LeaderTransitions: le.observedRecord.LeaderTransitions, + LeaderTransitions: oldLeaderElectionRecord.LeaderTransitions, LeaseDurationSeconds: 1, RenewTime: now, AcquireTime: now, } - timeoutCtx, timeoutCancel := context.WithTimeout(context.Background(), le.config.RenewDeadline) - defer timeoutCancel() if err := le.config.Lock.Update(timeoutCtx, leaderElectionRecord); err != nil { klog.Errorf("Failed to release lock: %v", err) return false diff --git a/tools/leaderelection/leaderelection_test.go b/tools/leaderelection/leaderelection_test.go index 211f11df..eff46177 100644 --- a/tools/leaderelection/leaderelection_test.go +++ b/tools/leaderelection/leaderelection_test.go @@ -557,7 +557,7 @@ func testReleaseLease(t *testing.T, objectType string) { verb: "get", objectType: objectType, reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) { - return true, nil, errors.NewNotFound(action.(fakeclient.GetAction).GetResource().GroupResource(), action.(fakeclient.GetAction).GetName()) + return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil }, }, { @@ -574,6 +574,13 @@ func testReleaseLease(t *testing.T, objectType string) { return true, action.(fakeclient.UpdateAction).GetObject(), nil }, }, + { + verb: "get", + objectType: objectType, + reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, errors.NewNotFound(action.(fakeclient.GetAction).GetResource().GroupResource(), action.(fakeclient.GetAction).GetName()) + }, + }, }, expectSuccess: true, outHolder: "", @@ -677,6 +684,59 @@ func TestReleaseLeaseLeases(t *testing.T) { testReleaseLease(t, "leases") } +// TestReleaseMethodCallsGet test release method calls Get +func TestReleaseMethodCallsGet(t *testing.T) { + objectType := "leases" + getCalled := false + + lockMeta := metav1.ObjectMeta{Namespace: "foo", Name: "bar"} + recorder := record.NewFakeRecorder(100) + resourceLockConfig := rl.ResourceLockConfig{ + Identity: "baz", + EventRecorder: recorder, + } + c := &fake.Clientset{} + c.AddReactor("get", objectType, func(action fakeclient.Action) (bool, runtime.Object, error) { + // flag to check if Get is called + getCalled = true + return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{ + HolderIdentity: "baz", + LeaseDurationSeconds: 10, + }), nil + }) + c.AddReactor("update", objectType, func(action fakeclient.Action) (bool, runtime.Object, error) { + return true, action.(fakeclient.UpdateAction).GetObject(), nil + }) + + lock := &rl.LeaseLock{ + LeaseMeta: lockMeta, + LockConfig: resourceLockConfig, + Client: c.CoordinationV1(), + } + lec := LeaderElectionConfig{ + Lock: lock, + LeaseDuration: 10 * time.Second, + Callbacks: LeaderCallbacks{ + OnNewLeader: func(l string) {}, + }, + } + observedRawRecord := GetRawRecordOrDie(t, objectType, rl.LeaderElectionRecord{HolderIdentity: "baz"}) + le := &LeaderElector{ + config: lec, + observedRecord: rl.LeaderElectionRecord{HolderIdentity: "baz"}, + observedRawRecord: observedRawRecord, + observedTime: time.Now(), + clock: clock.RealClock{}, + metrics: globalMetricsFactory.newLeaderMetrics(), + } + + le.release() + + if !getCalled { + t.Errorf("release method does not call Get") + } +} + func TestReleaseOnCancellation_Leases(t *testing.T) { testReleaseOnCancellation(t, "leases") } @@ -791,9 +851,11 @@ func testReleaseOnCancellation(t *testing.T, objectType string) { if lockObj != nil { // Third and more get (first create, second renew) should return our canceled error // FakeClient doesn't do anything with the context so we're doing this ourselves - if gets >= 3 { - close(onRenewCalled) - <-onRenewResume + if gets >= 4 { + if gets == 4 { + close(onRenewCalled) + <-onRenewResume + } return true, nil, context.Canceled } return true, lockObj, nil