From 710705cfe628f0ce8d987c74d7cb67553ac26049 Mon Sep 17 00:00:00 2001 From: Zachary Seguin Date: Sat, 3 Aug 2019 17:40:03 -0400 Subject: [PATCH 1/2] Generate complete leader election record to resolve leader election issues with LeaseLocks Kubernetes-commit: c902b8a20578a1d299e3563613a66ed32f70efad --- tools/leaderelection/leaderelection.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/leaderelection/leaderelection.go b/tools/leaderelection/leaderelection.go index e44fb473..3f6b898e 100644 --- a/tools/leaderelection/leaderelection.go +++ b/tools/leaderelection/leaderelection.go @@ -290,8 +290,12 @@ func (le *LeaderElector) release() bool { if !le.IsLeader() { return true } + now := metav1.Now() leaderElectionRecord := rl.LeaderElectionRecord{ - LeaderTransitions: le.observedRecord.LeaderTransitions, + LeaderTransitions: le.observedRecord.LeaderTransitions, + LeaseDurationSeconds: 1, + RenewTime: now, + AcquireTime: now, } if err := le.config.Lock.Update(context.TODO(), leaderElectionRecord); err != nil { klog.Errorf("Failed to release lock: %v", err) From 7efa263687790199f48780f6c6978ad76901135b Mon Sep 17 00:00:00 2001 From: Zachary Seguin Date: Wed, 22 Apr 2020 18:52:44 -0400 Subject: [PATCH 2/2] Add lease release tests in leader election Kubernetes-commit: a4979cd2a69192138b5998daca07d9b7ca6c8801 --- tools/leaderelection/leaderelection_test.go | 155 ++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/tools/leaderelection/leaderelection_test.go b/tools/leaderelection/leaderelection_test.go index 10acfa7d..c4703a90 100644 --- a/tools/leaderelection/leaderelection_test.go +++ b/tools/leaderelection/leaderelection_test.go @@ -917,3 +917,158 @@ func TestTryAcquireOrRenewEndpointsLeases(t *testing.T) { func TestTryAcquireOrRenewConfigMapsLeases(t *testing.T) { testTryAcquireOrRenewMultiLock(t, "configmapsleases") } + +func testReleaseLease(t *testing.T, objectType string) { + tests := []struct { + name string + observedRecord rl.LeaderElectionRecord + observedTime time.Time + reactors []Reactor + + expectSuccess bool + transitionLeader bool + outHolder string + }{ + { + name: "release acquired lock from no object", + reactors: []Reactor{ + { + 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()) + }, + }, + { + verb: "create", + objectType: objectType, + reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) { + return true, action.(fakeclient.CreateAction).GetObject(), nil + }, + }, + { + verb: "update", + objectType: objectType, + reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) { + return true, action.(fakeclient.UpdateAction).GetObject(), nil + }, + }, + }, + expectSuccess: true, + outHolder: "", + }, + } + + for i := range tests { + test := &tests[i] + t.Run(test.name, func(t *testing.T) { + // OnNewLeader is called async so we have to wait for it. + var wg sync.WaitGroup + wg.Add(1) + var reportedLeader string + var lock rl.Interface + + objectMeta := metav1.ObjectMeta{Namespace: "foo", Name: "bar"} + resourceLockConfig := rl.ResourceLockConfig{ + Identity: "baz", + EventRecorder: &record.FakeRecorder{}, + } + c := &fake.Clientset{} + for _, reactor := range test.reactors { + c.AddReactor(reactor.verb, objectType, reactor.reaction) + } + c.AddReactor("*", "*", func(action fakeclient.Action) (bool, runtime.Object, error) { + t.Errorf("unreachable action. testclient called too many times: %+v", action) + return true, nil, fmt.Errorf("unreachable action") + }) + + switch objectType { + case "endpoints": + lock = &rl.EndpointsLock{ + EndpointsMeta: objectMeta, + LockConfig: resourceLockConfig, + Client: c.CoreV1(), + } + case "configmaps": + lock = &rl.ConfigMapLock{ + ConfigMapMeta: objectMeta, + LockConfig: resourceLockConfig, + Client: c.CoreV1(), + } + case "leases": + lock = &rl.LeaseLock{ + LeaseMeta: objectMeta, + LockConfig: resourceLockConfig, + Client: c.CoordinationV1(), + } + } + + lec := LeaderElectionConfig{ + Lock: lock, + LeaseDuration: 10 * time.Second, + Callbacks: LeaderCallbacks{ + OnNewLeader: func(l string) { + defer wg.Done() + reportedLeader = l + }, + }, + } + observedRawRecord := GetRawRecordOrDie(t, objectType, test.observedRecord) + le := &LeaderElector{ + config: lec, + observedRecord: test.observedRecord, + observedRawRecord: observedRawRecord, + observedTime: test.observedTime, + clock: clock.RealClock{}, + } + if !le.tryAcquireOrRenew(context.Background()) { + t.Errorf("unexpected result of tryAcquireOrRenew: [succeeded=%v]", true) + } + + le.maybeReportTransition() + + // Wait for a response to the leader transition, and add 1 so that we can track the final transition. + wg.Wait() + wg.Add(1) + + if test.expectSuccess != le.release() { + t.Errorf("unexpected result of release: [succeeded=%v]", !test.expectSuccess) + } + + le.observedRecord.AcquireTime = metav1.Time{} + le.observedRecord.RenewTime = metav1.Time{} + if le.observedRecord.HolderIdentity != test.outHolder { + t.Errorf("expected holder:\n\t%+v\ngot:\n\t%+v", test.outHolder, le.observedRecord.HolderIdentity) + } + if len(test.reactors) != len(c.Actions()) { + t.Errorf("wrong number of api interactions") + } + if test.transitionLeader && le.observedRecord.LeaderTransitions != 1 { + t.Errorf("leader should have transitioned but did not") + } + if !test.transitionLeader && le.observedRecord.LeaderTransitions != 0 { + t.Errorf("leader should not have transitioned but did") + } + le.maybeReportTransition() + wg.Wait() + if reportedLeader != test.outHolder { + t.Errorf("reported leader was not the new leader. expected %q, got %q", test.outHolder, reportedLeader) + } + }) + } +} + +// Will test leader election using endpoints as the resource +func TestReleaseLeaseEndpoints(t *testing.T) { + testReleaseLease(t, "endpoints") +} + +// Will test leader election using endpoints as the resource +func TestReleaseLeaseConfigMaps(t *testing.T) { + testReleaseLease(t, "configmaps") +} + +// Will test leader election using endpoints as the resource +func TestReleaseLeaseLeases(t *testing.T) { + testReleaseLease(t, "leases") +}