Merge pull request #84801 from mikedanese/lebug

Fix panic on configmap and lease lock implementations

staging/src/k8s.io/client-go/tools/leaderelection/leaderelection_test.go

@@ -37,26 +37,27 @@ import (
 	"k8s.io/client-go/tools/record"
 )
 
-func createLockObject(t *testing.T, objectType, namespace, name string, record rl.LeaderElectionRecord) (obj runtime.Object) {
+func createLockObject(t *testing.T, objectType, namespace, name string, record *rl.LeaderElectionRecord) (obj runtime.Object) {
 	objectMeta := metav1.ObjectMeta{
 		Namespace: namespace,
 		Name:      name,
 	}
+	if record != nil {
+		recordBytes, _ := json.Marshal(record)
+		objectMeta.Annotations = map[string]string{
+			rl.LeaderElectionRecordAnnotationKey: string(recordBytes),
+		}
+	}
 	switch objectType {
 	case "endpoints":
-		recordBytes, _ := json.Marshal(record)
-		objectMeta.Annotations = map[string]string{
-			rl.LeaderElectionRecordAnnotationKey: string(recordBytes),
-		}
 		obj = &corev1.Endpoints{ObjectMeta: objectMeta}
 	case "configmaps":
-		recordBytes, _ := json.Marshal(record)
-		objectMeta.Annotations = map[string]string{
-			rl.LeaderElectionRecordAnnotationKey: string(recordBytes),
-		}
 		obj = &corev1.ConfigMap{ObjectMeta: objectMeta}
 	case "leases":
-		spec := rl.LeaderElectionRecordToLeaseSpec(&record)
+		var spec coordinationv1.LeaseSpec
+		if record != nil {
+			spec = rl.LeaderElectionRecordToLeaseSpec(record)
+		}
 		obj = &coordinationv1.Lease{ObjectMeta: objectMeta, Spec: spec}
 	default:
 		t.Fatal("unexpected objType:" + objectType)
@@ -108,13 +109,33 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
 			expectSuccess: true,
 			outHolder:     "baz",
 		},
+		{
+			name: "acquire from object without annotations",
+			reactors: []Reactor{
+				{
+					verb: "get",
+					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
+						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), nil), nil
+					},
+				},
+				{
+					verb: "update",
+					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
+						return true, action.(fakeclient.CreateAction).GetObject(), nil
+					},
+				},
+			},
+			expectSuccess:    true,
+			transitionLeader: true,
+			outHolder:        "baz",
+		},
 		{
 			name: "acquire from unled object",
 			reactors: []Reactor{
 				{
 					verb: "get",
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
+						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
 					},
 				},
 				{
@@ -135,7 +156,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
 				{
 					verb: "get",
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
@@ -158,7 +179,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
 				{
 					verb: "get",
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: ""}), nil
+						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: ""}), nil
 					},
 				},
 				{
@@ -180,7 +201,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
 				{
 					verb: "get",
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 			},
@@ -195,7 +216,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
 				{
 					verb: "get",
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
+						return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
 					},
 				},
 				{
@@ -421,7 +442,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
 					},
 				},
 				{
@@ -464,14 +485,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
 					},
 				},
 				{
@@ -485,7 +506,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
 					},
 				},
 				{
@@ -507,7 +528,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
@@ -528,7 +549,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
@@ -554,14 +575,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
@@ -575,7 +596,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
@@ -601,14 +622,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
 					},
 				},
 			},
@@ -626,14 +647,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
 					},
 				},
 				{
@@ -647,7 +668,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
 					},
 				},
 				{
@@ -673,7 +694,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
@@ -698,14 +719,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 			},
@@ -723,14 +744,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
 					},
 				},
 			},
@@ -748,14 +769,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: primaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
+						return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
 					},
 				},
 				{
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
 					},
 				},
 				{
@@ -769,7 +790,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
 					verb:       "get",
 					objectType: secondaryType,
 					reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
-						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
+						return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
 					},
 				},
 				{

staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/endpointslock.go

@@ -82,6 +82,9 @@ func (el *EndpointsLock) Update(ler LeaderElectionRecord) error {
 	if err != nil {
 		return err
 	}
+	if el.e.Annotations == nil {
+		el.e.Annotations = make(map[string]string)
+	}
 	el.e.Annotations[LeaderElectionRecordAnnotationKey] = string(recordBytes)
 	el.e, err = el.Client.Endpoints(el.EndpointsMeta.Namespace).Update(el.e)
 	return err

staging/src/k8s.io/client-go/tools/leaderelection/resourcelock/leaselock.go

@@ -96,25 +96,24 @@ func (ll *LeaseLock) Identity() string {
 }
 
 func LeaseSpecToLeaderElectionRecord(spec *coordinationv1.LeaseSpec) *LeaderElectionRecord {
-	holderIdentity := ""
+	var r LeaderElectionRecord
 	if spec.HolderIdentity != nil {
-		holderIdentity = *spec.HolderIdentity
+		r.HolderIdentity = *spec.HolderIdentity
 	}
-	leaseDurationSeconds := 0
 	if spec.LeaseDurationSeconds != nil {
-		leaseDurationSeconds = int(*spec.LeaseDurationSeconds)
+		r.LeaseDurationSeconds = int(*spec.LeaseDurationSeconds)
 	}
-	leaseTransitions := 0
 	if spec.LeaseTransitions != nil {
-		leaseTransitions = int(*spec.LeaseTransitions)
+		r.LeaderTransitions = int(*spec.LeaseTransitions)
 	}
-	return &LeaderElectionRecord{
+	if spec.AcquireTime != nil {
-		HolderIdentity:       holderIdentity,
+		r.AcquireTime = metav1.Time{spec.AcquireTime.Time}
-		LeaseDurationSeconds: leaseDurationSeconds,
-		AcquireTime:          metav1.Time{spec.AcquireTime.Time},
-		RenewTime:            metav1.Time{spec.RenewTime.Time},
-		LeaderTransitions:    leaseTransitions,
 	}
+	if spec.RenewTime != nil {
+		r.RenewTime = metav1.Time{spec.RenewTime.Time}
+	}
+	return &r
+
 }
 
 func LeaderElectionRecordToLeaseSpec(ler *LeaderElectionRecord) coordinationv1.LeaseSpec {