Skip to content

Commit 17d99df

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#84801 from mikedanese/lebug
Fix panic on configmap and lease lock implementations
2 parents 17874d6 + 7907b29 commit 17d99df

File tree

3 files changed

+70
-47
lines changed

3 files changed

+70
-47
lines changed

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

Lines changed: 56 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,27 @@ import (
3737
"k8s.io/client-go/tools/record"
3838
)
3939

40-
func createLockObject(t *testing.T, objectType, namespace, name string, record rl.LeaderElectionRecord) (obj runtime.Object) {
40+
func createLockObject(t *testing.T, objectType, namespace, name string, record *rl.LeaderElectionRecord) (obj runtime.Object) {
4141
objectMeta := metav1.ObjectMeta{
4242
Namespace: namespace,
4343
Name: name,
4444
}
45-
switch objectType {
46-
case "endpoints":
45+
if record != nil {
4746
recordBytes, _ := json.Marshal(record)
4847
objectMeta.Annotations = map[string]string{
4948
rl.LeaderElectionRecordAnnotationKey: string(recordBytes),
5049
}
50+
}
51+
switch objectType {
52+
case "endpoints":
5153
obj = &corev1.Endpoints{ObjectMeta: objectMeta}
5254
case "configmaps":
53-
recordBytes, _ := json.Marshal(record)
54-
objectMeta.Annotations = map[string]string{
55-
rl.LeaderElectionRecordAnnotationKey: string(recordBytes),
56-
}
5755
obj = &corev1.ConfigMap{ObjectMeta: objectMeta}
5856
case "leases":
59-
spec := rl.LeaderElectionRecordToLeaseSpec(&record)
57+
var spec coordinationv1.LeaseSpec
58+
if record != nil {
59+
spec = rl.LeaderElectionRecordToLeaseSpec(record)
60+
}
6061
obj = &coordinationv1.Lease{ObjectMeta: objectMeta, Spec: spec}
6162
default:
6263
t.Fatal("unexpected objType:" + objectType)
@@ -108,13 +109,33 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
108109
expectSuccess: true,
109110
outHolder: "baz",
110111
},
112+
{
113+
name: "acquire from object without annotations",
114+
reactors: []Reactor{
115+
{
116+
verb: "get",
117+
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
118+
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), nil), nil
119+
},
120+
},
121+
{
122+
verb: "update",
123+
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
124+
return true, action.(fakeclient.CreateAction).GetObject(), nil
125+
},
126+
},
127+
},
128+
expectSuccess: true,
129+
transitionLeader: true,
130+
outHolder: "baz",
131+
},
111132
{
112133
name: "acquire from unled object",
113134
reactors: []Reactor{
114135
{
115136
verb: "get",
116137
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
117-
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
138+
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
118139
},
119140
},
120141
{
@@ -135,7 +156,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
135156
{
136157
verb: "get",
137158
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
138-
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
159+
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
139160
},
140161
},
141162
{
@@ -158,7 +179,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
158179
{
159180
verb: "get",
160181
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
161-
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: ""}), nil
182+
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: ""}), nil
162183
},
163184
},
164185
{
@@ -180,7 +201,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
180201
{
181202
verb: "get",
182203
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
183-
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
204+
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
184205
},
185206
},
186207
},
@@ -195,7 +216,7 @@ func testTryAcquireOrRenew(t *testing.T, objectType string) {
195216
{
196217
verb: "get",
197218
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
198-
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
219+
return true, createLockObject(t, objectType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
199220
},
200221
},
201222
{
@@ -421,7 +442,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
421442
verb: "get",
422443
objectType: primaryType,
423444
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
424-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
445+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
425446
},
426447
},
427448
{
@@ -464,14 +485,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
464485
verb: "get",
465486
objectType: primaryType,
466487
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
467-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
488+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
468489
},
469490
},
470491
{
471492
verb: "get",
472493
objectType: secondaryType,
473494
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
474-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
495+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
475496
},
476497
},
477498
{
@@ -485,7 +506,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
485506
verb: "get",
486507
objectType: secondaryType,
487508
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
488-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{}), nil
509+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{}), nil
489510
},
490511
},
491512
{
@@ -507,7 +528,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
507528
verb: "get",
508529
objectType: primaryType,
509530
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
510-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
531+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
511532
},
512533
},
513534
{
@@ -528,7 +549,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
528549
verb: "get",
529550
objectType: secondaryType,
530551
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
531-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
552+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
532553
},
533554
},
534555
{
@@ -554,14 +575,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
554575
verb: "get",
555576
objectType: primaryType,
556577
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
557-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
578+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
558579
},
559580
},
560581
{
561582
verb: "get",
562583
objectType: secondaryType,
563584
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
564-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
585+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
565586
},
566587
},
567588
{
@@ -575,7 +596,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
575596
verb: "get",
576597
objectType: secondaryType,
577598
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
578-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
599+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
579600
},
580601
},
581602
{
@@ -601,14 +622,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
601622
verb: "get",
602623
objectType: primaryType,
603624
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
604-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
625+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
605626
},
606627
},
607628
{
608629
verb: "get",
609630
objectType: secondaryType,
610631
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
611-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
632+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
612633
},
613634
},
614635
},
@@ -626,14 +647,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
626647
verb: "get",
627648
objectType: primaryType,
628649
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
629-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
650+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
630651
},
631652
},
632653
{
633654
verb: "get",
634655
objectType: secondaryType,
635656
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
636-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
657+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
637658
},
638659
},
639660
{
@@ -647,7 +668,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
647668
verb: "get",
648669
objectType: secondaryType,
649670
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
650-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
671+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: rl.UnknownLeader}), nil
651672
},
652673
},
653674
{
@@ -673,7 +694,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
673694
verb: "get",
674695
objectType: primaryType,
675696
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
676-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
697+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
677698
},
678699
},
679700
{
@@ -698,14 +719,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
698719
verb: "get",
699720
objectType: primaryType,
700721
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
701-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
722+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
702723
},
703724
},
704725
{
705726
verb: "get",
706727
objectType: secondaryType,
707728
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
708-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
729+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
709730
},
710731
},
711732
},
@@ -723,14 +744,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
723744
verb: "get",
724745
objectType: primaryType,
725746
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
726-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
747+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
727748
},
728749
},
729750
{
730751
verb: "get",
731752
objectType: secondaryType,
732753
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
733-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
754+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "bing"}), nil
734755
},
735756
},
736757
},
@@ -748,14 +769,14 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
748769
verb: "get",
749770
objectType: primaryType,
750771
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
751-
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
772+
return true, createLockObject(t, primaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
752773
},
753774
},
754775
{
755776
verb: "get",
756777
objectType: secondaryType,
757778
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
758-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
779+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
759780
},
760781
},
761782
{
@@ -769,7 +790,7 @@ func testTryAcquireOrRenewMultiLock(t *testing.T, objectType string) {
769790
verb: "get",
770791
objectType: secondaryType,
771792
reaction: func(action fakeclient.Action) (handled bool, ret runtime.Object, err error) {
772-
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
793+
return true, createLockObject(t, secondaryType, action.GetNamespace(), action.(fakeclient.GetAction).GetName(), &rl.LeaderElectionRecord{HolderIdentity: "baz"}), nil
773794
},
774795
},
775796
{

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ func (el *EndpointsLock) Update(ler LeaderElectionRecord) error {
8282
if err != nil {
8383
return err
8484
}
85+
if el.e.Annotations == nil {
86+
el.e.Annotations = make(map[string]string)
87+
}
8588
el.e.Annotations[LeaderElectionRecordAnnotationKey] = string(recordBytes)
8689
el.e, err = el.Client.Endpoints(el.EndpointsMeta.Namespace).Update(el.e)
8790
return err

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,25 +96,24 @@ func (ll *LeaseLock) Identity() string {
9696
}
9797

9898
func LeaseSpecToLeaderElectionRecord(spec *coordinationv1.LeaseSpec) *LeaderElectionRecord {
99-
holderIdentity := ""
99+
var r LeaderElectionRecord
100100
if spec.HolderIdentity != nil {
101-
holderIdentity = *spec.HolderIdentity
101+
r.HolderIdentity = *spec.HolderIdentity
102102
}
103-
leaseDurationSeconds := 0
104103
if spec.LeaseDurationSeconds != nil {
105-
leaseDurationSeconds = int(*spec.LeaseDurationSeconds)
104+
r.LeaseDurationSeconds = int(*spec.LeaseDurationSeconds)
106105
}
107-
leaseTransitions := 0
108106
if spec.LeaseTransitions != nil {
109-
leaseTransitions = int(*spec.LeaseTransitions)
107+
r.LeaderTransitions = int(*spec.LeaseTransitions)
110108
}
111-
return &LeaderElectionRecord{
112-
HolderIdentity: holderIdentity,
113-
LeaseDurationSeconds: leaseDurationSeconds,
114-
AcquireTime: metav1.Time{spec.AcquireTime.Time},
115-
RenewTime: metav1.Time{spec.RenewTime.Time},
116-
LeaderTransitions: leaseTransitions,
109+
if spec.AcquireTime != nil {
110+
r.AcquireTime = metav1.Time{spec.AcquireTime.Time}
117111
}
112+
if spec.RenewTime != nil {
113+
r.RenewTime = metav1.Time{spec.RenewTime.Time}
114+
}
115+
return &r
116+
118117
}
119118

120119
func LeaderElectionRecordToLeaseSpec(ler *LeaderElectionRecord) coordinationv1.LeaseSpec {

0 commit comments

Comments
 (0)