Merge pull request #44286 from shashidharatd/leader-election

Automatic merge from submit-queue (batch tested with PRs 44286, 44209)

Refactor leader election package test code to remove redundant code

**What this PR does / why we need it**:
leader election object supports locking on 2 objects currently endpoints and configmap. The test code for both is almost identical. This pr attempts to remove redundancy in test code for easier maintenance and extension.

**Special notes for your reviewer**:
I am trying to add leader election feature to federation controller manager and this pr is one of sub-task of the overall work documented in #44283

**Release note**:
```
NONE
```

pkg/client/leaderelection/leaderelection_test.go

@@ -32,8 +32,24 @@ import (
 	rl "k8s.io/kubernetes/pkg/client/leaderelection/resourcelock"
 )
 
+func createLockObject(objectType string, objectMeta metav1.ObjectMeta) (obj runtime.Object) {
+	switch objectType {
+	case "endpoints":
+		obj = &v1.Endpoints{ObjectMeta: objectMeta}
+	case "configmaps":
+		obj = &v1.ConfigMap{ObjectMeta: objectMeta}
+	default:
+		panic("unexpected objType:" + objectType)
+	}
+	return
+}
+
 // Will test leader election using endpoints as the resource
 func TestTryAcquireOrRenewEndpoints(t *testing.T) {
+	testTryAcquireOrRenew(t, "endpoints")
+}
+
+func testTryAcquireOrRenew(t *testing.T, objectType string) {
 	future := time.Now().Add(1000 * time.Hour)
 	past := time.Now().Add(-1000 * time.Hour)
 
@@ -49,7 +65,7 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 		transitionLeader bool
 		outHolder        string
 	}{
-		// acquire from no endpoints
+		// acquire from no object
 		{
 			reactors: []struct {
 				verb     string
@@ -64,14 +80,14 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 				{
 					verb: "create",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.Endpoints), nil
+						return true, action.(core.CreateAction).GetObject(), nil
 					},
 				},
 			},
 			expectSuccess: true,
 			outHolder:     "baz",
 		},
-		// acquire from unled endpoints
+		// acquire from unled object
 		{
 			reactors: []struct {
 				verb     string
@@ -80,18 +96,17 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 				{
 					verb: "get",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.Endpoints{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-							},
-						}, nil
+						objectMeta := metav1.ObjectMeta{
+							Namespace: action.GetNamespace(),
+							Name:      action.(core.GetAction).GetName(),
+						}
+						return true, createLockObject(objectType, objectMeta), nil
 					},
 				},
 				{
 					verb: "update",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.Endpoints), nil
+						return true, action.(core.CreateAction).GetObject(), nil
 					},
 				},
 			},
@@ -100,7 +115,7 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 			transitionLeader: true,
 			outHolder:        "baz",
 		},
-		// acquire from led, unacked endpoints
+		// acquire from led, unacked object
 		{
 			reactors: []struct {
 				verb     string
@@ -109,21 +124,20 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 				{
 					verb: "get",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.Endpoints{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-								Annotations: map[string]string{
-									rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"bing"}`,
-								},
+						objectMeta := metav1.ObjectMeta{
+							Namespace: action.GetNamespace(),
+							Name:      action.(core.GetAction).GetName(),
+							Annotations: map[string]string{
+								rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"bing"}`,
 							},
-						}, nil
+						}
+						return true, createLockObject(objectType, objectMeta), nil
 					},
 				},
 				{
 					verb: "update",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.Endpoints), nil
+						return true, action.(core.CreateAction).GetObject(), nil
 					},
 				},
 			},
@@ -134,7 +148,7 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 			transitionLeader: true,
 			outHolder:        "baz",
 		},
-		// don't acquire from led, acked endpoints
+		// don't acquire from led, acked object
 		{
 			reactors: []struct {
 				verb     string
@@ -143,15 +157,14 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 				{
 					verb: "get",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.Endpoints{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-								Annotations: map[string]string{
-									rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"bing"}`,
-								},
+						objectMeta := metav1.ObjectMeta{
+							Namespace: action.GetNamespace(),
+							Name:      action.(core.GetAction).GetName(),
+							Annotations: map[string]string{
+								rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"bing"}`,
 							},
-						}, nil
+						}
+						return true, createLockObject(objectType, objectMeta), nil
 					},
 				},
 			},
@@ -160,7 +173,7 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 			expectSuccess: false,
 			outHolder:     "bing",
 		},
-		// renew already acquired endpoints
+		// renew already acquired object
 		{
 			reactors: []struct {
 				verb     string
@@ -169,21 +182,20 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 				{
 					verb: "get",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.Endpoints{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-								Annotations: map[string]string{
-									rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"baz"}`,
-								},
+						objectMeta := metav1.ObjectMeta{
+							Namespace: action.GetNamespace(),
+							Name:      action.(core.GetAction).GetName(),
+							Annotations: map[string]string{
+								rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"baz"}`,
 							},
-						}, nil
+						}
+						return true, createLockObject(objectType, objectMeta), nil
 					},
 				},
 				{
 					verb: "update",
 					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.Endpoints), nil
+						return true, action.(core.CreateAction).GetObject(), nil
 					},
 				},
 			},
@@ -200,17 +212,39 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 		var wg sync.WaitGroup
 		wg.Add(1)
 		var reportedLeader string
+		var lock rl.Interface
 
-		lock := rl.EndpointsLock{
-			EndpointsMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
-			LockConfig: rl.ResourceLockConfig{
-				Identity:      "baz",
-				EventRecorder: &record.FakeRecorder{},
-			},
+		objectMeta := metav1.ObjectMeta{Namespace: "foo", Name: "bar"}
+		resourceLockConfig := rl.ResourceLockConfig{
+			Identity:      "baz",
+			EventRecorder: &record.FakeRecorder{},
+		}
+		c := &fakeclientset.Clientset{Fake: core.Fake{}}
+		for _, reactor := range test.reactors {
+			c.AddReactor(reactor.verb, objectType, reactor.reaction)
+		}
+		c.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
+			t.Errorf("[%v] unreachable action. testclient called too many times: %+v", i, action)
+			return true, nil, fmt.Errorf("unreachable action")
+		})
+
+		switch objectType {
+		case "endpoints":
+			lock = &rl.EndpointsLock{
+				EndpointsMeta: objectMeta,
+				LockConfig:    resourceLockConfig,
+				Client:        c,
+			}
+		case "configmaps":
+			lock = &rl.ConfigMapLock{
+				ConfigMapMeta: objectMeta,
+				LockConfig:    resourceLockConfig,
+				Client:        c,
+			}
 		}
 
 		lec := LeaderElectionConfig{
-			Lock:          &lock,
+			Lock:          lock,
 			LeaseDuration: 10 * time.Second,
 			Callbacks: LeaderCallbacks{
 				OnNewLeader: func(l string) {
@@ -219,21 +253,11 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 				},
 			},
 		}
-		c := &fakeclientset.Clientset{Fake: core.Fake{}}
-		for _, reactor := range test.reactors {
-			c.AddReactor(reactor.verb, "endpoints", reactor.reaction)
-		}
-		c.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
-			t.Errorf("[%v] unreachable action. testclient called too many times: %+v", i, action)
-			return true, nil, fmt.Errorf("uncreachable action")
-		})
-
 		le := &LeaderElector{
 			config:         lec,
 			observedRecord: test.observedRecord,
 			observedTime:   test.observedTime,
 		}
-		lock.Client = c
 
 		if test.expectSuccess != le.tryAcquireOrRenew() {
 			t.Errorf("[%v]unexpected result of tryAcquireOrRenew: [succeded=%v]", i, !test.expectSuccess)
@@ -263,231 +287,6 @@ func TestTryAcquireOrRenewEndpoints(t *testing.T) {
 }
 
 // Will test leader election using configmap as the resource
-func TestTryAcquireOrRenewConfigMap(t *testing.T) {
-	future := time.Now().Add(1000 * time.Hour)
-	past := time.Now().Add(-1000 * time.Hour)
-
-	tests := []struct {
-		observedRecord rl.LeaderElectionRecord
-		observedTime   time.Time
-		reactors       []struct {
-			verb     string
-			reaction core.ReactionFunc
-		}
-
-		expectSuccess    bool
-		transitionLeader bool
-		outHolder        string
-	}{
-		// acquire from no onfigmap
-		{
-			reactors: []struct {
-				verb     string
-				reaction core.ReactionFunc
-			}{
-				{
-					verb: "get",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, nil, errors.NewNotFound(action.(core.GetAction).GetResource().GroupResource(), action.(core.GetAction).GetName())
-					},
-				},
-				{
-					verb: "create",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.ConfigMap), nil
-					},
-				},
-			},
-			expectSuccess: true,
-			outHolder:     "baz",
-		},
-		// acquire from unled configmap
-		{
-			reactors: []struct {
-				verb     string
-				reaction core.ReactionFunc
-			}{
-				{
-					verb: "get",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.ConfigMap{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-							},
-						}, nil
-					},
-				},
-				{
-					verb: "update",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.ConfigMap), nil
-					},
-				},
-			},
-
-			expectSuccess:    true,
-			transitionLeader: true,
-			outHolder:        "baz",
-		},
-		// acquire from led, unacked configmap
-		{
-			reactors: []struct {
-				verb     string
-				reaction core.ReactionFunc
-			}{
-				{
-					verb: "get",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.ConfigMap{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-								Annotations: map[string]string{
-									rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"bing"}`,
-								},
-							},
-						}, nil
-					},
-				},
-				{
-					verb: "update",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.ConfigMap), nil
-					},
-				},
-			},
-			observedRecord: rl.LeaderElectionRecord{HolderIdentity: "bing"},
-			observedTime:   past,
-
-			expectSuccess:    true,
-			transitionLeader: true,
-			outHolder:        "baz",
-		},
-		// don't acquire from led, acked configmap
-		{
-			reactors: []struct {
-				verb     string
-				reaction core.ReactionFunc
-			}{
-				{
-					verb: "get",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.ConfigMap{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-								Annotations: map[string]string{
-									rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"bing"}`,
-								},
-							},
-						}, nil
-					},
-				},
-			},
-			observedTime: future,
-
-			expectSuccess: false,
-			outHolder:     "bing",
-		},
-		// renew already acquired configmap
-		{
-			reactors: []struct {
-				verb     string
-				reaction core.ReactionFunc
-			}{
-				{
-					verb: "get",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, &v1.ConfigMap{
-							ObjectMeta: metav1.ObjectMeta{
-								Namespace: action.GetNamespace(),
-								Name:      action.(core.GetAction).GetName(),
-								Annotations: map[string]string{
-									rl.LeaderElectionRecordAnnotationKey: `{"holderIdentity":"baz"}`,
-								},
-							},
-						}, nil
-					},
-				},
-				{
-					verb: "update",
-					reaction: func(action core.Action) (handled bool, ret runtime.Object, err error) {
-						return true, action.(core.CreateAction).GetObject().(*v1.ConfigMap), nil
-					},
-				},
-			},
-			observedTime:   future,
-			observedRecord: rl.LeaderElectionRecord{HolderIdentity: "baz"},
-
-			expectSuccess: true,
-			outHolder:     "baz",
-		},
-	}
-
-	for i, test := range tests {
-		// OnNewLeader is called async so we have to wait for it.
-		var wg sync.WaitGroup
-		wg.Add(1)
-		var reportedLeader string
-
-		lock := rl.ConfigMapLock{
-			ConfigMapMeta: metav1.ObjectMeta{Namespace: "foo", Name: "bar"},
-			LockConfig: rl.ResourceLockConfig{
-				Identity:      "baz",
-				EventRecorder: &record.FakeRecorder{},
-			},
-		}
-
-		lec := LeaderElectionConfig{
-			Lock:          &lock,
-			LeaseDuration: 10 * time.Second,
-			Callbacks: LeaderCallbacks{
-				OnNewLeader: func(l string) {
-					defer wg.Done()
-					reportedLeader = l
-				},
-			},
-		}
-		c := &fakeclientset.Clientset{Fake: core.Fake{}}
-		for _, reactor := range test.reactors {
-			c.AddReactor(reactor.verb, "configmaps", reactor.reaction)
-		}
-		c.AddReactor("*", "*", func(action core.Action) (bool, runtime.Object, error) {
-			t.Errorf("[%v] unreachable action. testclient called too many times: %+v", i, action)
-			return true, nil, fmt.Errorf("uncreachable action")
-		})
-
-		le := &LeaderElector{
-			config:         lec,
-			observedRecord: test.observedRecord,
-			observedTime:   test.observedTime,
-		}
-		lock.Client = c
-
-		if test.expectSuccess != le.tryAcquireOrRenew() {
-			t.Errorf("[%v]unexpected result of tryAcquireOrRenew: [succeded=%v]", i, !test.expectSuccess)
-		}
-
-		le.observedRecord.AcquireTime = metav1.Time{}
-		le.observedRecord.RenewTime = metav1.Time{}
-		if le.observedRecord.HolderIdentity != test.outHolder {
-			t.Errorf("[%v]expected holder:\n\t%+v\ngot:\n\t%+v", i, test.outHolder, le.observedRecord.HolderIdentity)
-		}
-		if len(test.reactors) != len(c.Actions()) {
-			t.Errorf("[%v]wrong number of api interactions", i)
-		}
-		if test.transitionLeader && le.observedRecord.LeaderTransitions != 1 {
-			t.Errorf("[%v]leader should have transitioned but did not", i)
-		}
-		if !test.transitionLeader && le.observedRecord.LeaderTransitions != 0 {
-			t.Errorf("[%v]leader should not have transitioned but did", i)
-		}
-
-		le.maybeReportTransition()
-		wg.Wait()
-		if reportedLeader != test.outHolder {
-			t.Errorf("[%v]reported leader was not the new leader. expected %q, got %q", i, test.outHolder, reportedLeader)
-		}
-	}
+func TestTryAcquireOrRenewConfigMaps(t *testing.T) {
+	testTryAcquireOrRenew(t, "configmaps")
 }