Skip to content

Commit 3987d85

Browse files
stttssttts
committed
kube-apiserver/leaderelection/test: clean up controller test
Signed-off-by: Dr. Stefan Schimanski <[email protected]>
1 parent b13aab9 commit 3987d85

File tree

2 files changed

+137
-52
lines changed

2 files changed

+137
-52
lines changed

‎pkg/controlplane/controller/leaderelection/leaderelection_controller.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,6 @@ func (c *Controller) electionNeeded(candidates []*v1alpha1.LeaseCandidate, lease
245245
// PingTime + electionDuration < time.Now: Candidate has not responded within the appropriate PingTime. Continue the election.
246246
// RenewTime + 5 seconds > time.Now: All candidates acked in the last 5 seconds, continue the election.
247247
func (c *Controller) reconcileElectionStep(ctx context.Context, leaseNN types.NamespacedName) (requeue time.Duration, err error) {
248-
249248
candidates, err := c.listAdmissableCandidates(leaseNN)
250249
if err != nil {
251250
return defaultRequeueInterval, err

‎pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go‎

Lines changed: 137 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package leaderelection
1919
import (
2020
"context"
2121
"fmt"
22+
"math/rand"
2223
"testing"
2324
"time"
2425

@@ -27,7 +28,6 @@ import (
2728
"k8s.io/apimachinery/pkg/api/errors"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/types"
30-
"k8s.io/apimachinery/pkg/util/runtime"
3131
"k8s.io/apimachinery/pkg/util/wait"
3232
"k8s.io/client-go/informers"
3333
"k8s.io/client-go/kubernetes/fake"
@@ -442,16 +442,16 @@ func TestController(t *testing.T) {
442442
fakeClock := testingclock.NewFakeClock(time.Now())
443443

444444
cases := []struct {
445-
name string
446-
leaseNN types.NamespacedName
447-
createAfterControllerStart []*v1alpha1.LeaseCandidate
448-
deleteAfterControllerStart []types.NamespacedName
449-
expectedLeaderLeases []*v1.Lease
445+
name string
446+
leases []*v1.Lease
447+
candidates []*v1alpha1.LeaseCandidate
448+
createAfterControllerStart []*v1alpha1.LeaseCandidate
449+
deleteLeaseAfterControllerStart []types.NamespacedName
450+
expectedLeaderLeases []*v1.Lease
450451
}{
451452
{
452-
name: "single candidate leader election",
453-
leaseNN: types.NamespacedName{Namespace: "kube-system", Name: "component-A"},
454-
createAfterControllerStart: []*v1alpha1.LeaseCandidate{
453+
name: "single candidate leader election",
454+
candidates: []*v1alpha1.LeaseCandidate{
455455
{
456456
ObjectMeta: metav1.ObjectMeta{
457457
Namespace: "kube-system",
@@ -479,9 +479,8 @@ func TestController(t *testing.T) {
479479
},
480480
},
481481
{
482-
name: "multiple candidate leader election",
483-
leaseNN: types.NamespacedName{Namespace: "kube-system", Name: "component-A"},
484-
createAfterControllerStart: []*v1alpha1.LeaseCandidate{
482+
name: "multiple candidate leader election",
483+
candidates: []*v1alpha1.LeaseCandidate{
485484
{
486485
ObjectMeta: metav1.ObjectMeta{
487486
Namespace: "kube-system",
@@ -535,17 +534,21 @@ func TestController(t *testing.T) {
535534
},
536535
},
537536
{
538-
name: "deletion of lease triggers reelection",
539-
leaseNN: types.NamespacedName{Namespace: "kube-system", Name: "component-A"},
540-
createAfterControllerStart: []*v1alpha1.LeaseCandidate{
537+
name: "deletion of lease triggers reelection",
538+
leases: []*v1.Lease{
541539
{
542-
// Leader lease
543540
ObjectMeta: metav1.ObjectMeta{
544541
Namespace: "kube-system",
545542
Name: "component-A",
546543
},
547-
Spec: v1alpha1.LeaseCandidateSpec{},
544+
Spec: v1.LeaseSpec{
545+
HolderIdentity: ptr.To("some-other-component"),
546+
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(time.Second))),
547+
LeaseDurationSeconds: ptr.To(int32(10)),
548+
},
548549
},
550+
},
551+
candidates: []*v1alpha1.LeaseCandidate{
549552
{
550553
ObjectMeta: metav1.ObjectMeta{
551554
Namespace: "kube-system",
@@ -560,7 +563,7 @@ func TestController(t *testing.T) {
560563
},
561564
},
562565
},
563-
deleteAfterControllerStart: []types.NamespacedName{
566+
deleteLeaseAfterControllerStart: []types.NamespacedName{
564567
{Namespace: "kube-system", Name: "component-A"},
565568
},
566569
expectedLeaderLeases: []*v1.Lease{
@@ -576,17 +579,65 @@ func TestController(t *testing.T) {
576579
},
577580
},
578581
{
579-
name: "better candidate triggers reelection",
580-
leaseNN: types.NamespacedName{Namespace: "kube-system", Name: "component-A"},
581-
createAfterControllerStart: []*v1alpha1.LeaseCandidate{
582+
name: "expired lease is replaced",
583+
leases: []*v1.Lease{
584+
{
585+
ObjectMeta: metav1.ObjectMeta{
586+
Namespace: "kube-system",
587+
Name: "component-A",
588+
},
589+
Spec: v1.LeaseSpec{
590+
HolderIdentity: ptr.To("some-other-component"),
591+
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(-11 * time.Second))),
592+
LeaseDurationSeconds: ptr.To(int32(10)),
593+
Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"),
594+
},
595+
},
596+
},
597+
candidates: []*v1alpha1.LeaseCandidate{
598+
{
599+
ObjectMeta: metav1.ObjectMeta{
600+
Namespace: "kube-system",
601+
Name: "component-identity-1",
602+
},
603+
Spec: v1alpha1.LeaseCandidateSpec{
604+
LeaseName: "component-A",
605+
EmulationVersion: "1.19.0",
606+
BinaryVersion: "1.19.0",
607+
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
608+
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
609+
},
610+
},
611+
},
612+
expectedLeaderLeases: []*v1.Lease{
613+
{
614+
ObjectMeta: metav1.ObjectMeta{
615+
Namespace: "kube-system",
616+
Name: "component-A",
617+
},
618+
Spec: v1.LeaseSpec{
619+
HolderIdentity: ptr.To("component-identity-1"),
620+
},
621+
},
622+
},
623+
},
624+
{
625+
name: "better candidate triggers reelection",
626+
leases: []*v1.Lease{
582627
{
583-
// Leader lease
584628
ObjectMeta: metav1.ObjectMeta{
585629
Namespace: "kube-system",
586630
Name: "component-A",
587631
},
588-
Spec: v1alpha1.LeaseCandidateSpec{},
632+
Spec: v1.LeaseSpec{
633+
HolderIdentity: ptr.To("component-identity-1"),
634+
Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"),
635+
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(time.Second))),
636+
LeaseDurationSeconds: ptr.To(int32(10)),
637+
},
589638
},
639+
},
640+
candidates: []*v1alpha1.LeaseCandidate{
590641
{
591642
ObjectMeta: metav1.ObjectMeta{
592643
Namespace: "kube-system",
@@ -600,6 +651,8 @@ func TestController(t *testing.T) {
600651
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
601652
},
602653
},
654+
},
655+
createAfterControllerStart: []*v1alpha1.LeaseCandidate{
603656
{
604657
ObjectMeta: metav1.ObjectMeta{
605658
Namespace: "kube-system",
@@ -645,9 +698,30 @@ func TestController(t *testing.T) {
645698
t.Fatal(err)
646699
}
647700

701+
for _, obj := range tc.leases {
702+
t.Logf("Pre-creating lease %s/%s", obj.Namespace, obj.Name)
703+
_, err := client.CoordinationV1().Leases(obj.Namespace).Create(ctx, obj, metav1.CreateOptions{})
704+
if err != nil {
705+
t.Fatalf("Error pre-creating lease %s/%s: %v", obj.Namespace, obj.Name, err)
706+
}
707+
}
708+
for _, obj := range tc.candidates {
709+
t.Logf("Pre-creating lease candidate %s/%s", obj.Namespace, obj.Name)
710+
_, err := client.CoordinationV1alpha1().LeaseCandidates(obj.Namespace).Create(ctx, obj, metav1.CreateOptions{})
711+
if err != nil {
712+
t.Fatalf("Error pre-creating lease candidate %s/%s: %v", obj.Namespace, obj.Name, err)
713+
}
714+
}
715+
648716
informerFactory.Start(ctx.Done())
717+
informerFactory.WaitForCacheSync(ctx.Done())
649718
go controller.Run(ctx, 1)
650719

720+
if rand.Intn(2) == 0 {
721+
t.Logf("Giving controller a chance to run")
722+
time.Sleep(time.Second)
723+
}
724+
651725
go func() {
652726
ticker := time.NewTicker(10 * time.Millisecond)
653727
// Mock out the removal of preferredHolder leases.
@@ -658,14 +732,24 @@ func TestController(t *testing.T) {
658732
return
659733
case <-ticker.C:
660734
for _, expectedLease := range tc.expectedLeaderLeases {
661-
lease, err := client.CoordinationV1().Leases(expectedLease.Namespace).Get(ctx, expectedLease.Name, metav1.GetOptions{})
662-
if err == nil {
663-
if preferredHolder := lease.Spec.PreferredHolder; preferredHolder != nil {
664-
err = client.CoordinationV1().Leases(expectedLease.Namespace).Delete(ctx, expectedLease.Name, metav1.DeleteOptions{})
665-
if err != nil {
666-
runtime.HandleError(err)
667-
}
668-
}
735+
l, err := client.CoordinationV1().Leases(expectedLease.Namespace).Get(ctx, expectedLease.Name, metav1.GetOptions{})
736+
if err != nil {
737+
continue
738+
}
739+
ph := l.Spec.PreferredHolder
740+
if ph == nil || l.Spec.HolderIdentity == nil {
741+
continue
742+
}
743+
if *ph == *l.Spec.HolderIdentity {
744+
continue
745+
}
746+
if _, err := client.CoordinationV1alpha1().LeaseCandidates(expectedLease.Namespace).Get(ctx, *l.Spec.HolderIdentity, metav1.GetOptions{}); err != nil {
747+
continue // only candidate-aware controllers will follow preferredHolder
748+
}
749+
750+
t.Logf("Deleting lease %s/%s because of preferredHolder %q != %q", l.Namespace, l.Name, *ph, *l.Spec.HolderIdentity)
751+
if err = client.CoordinationV1().Leases(expectedLease.Namespace).Delete(ctx, expectedLease.Name, metav1.DeleteOptions{}); err != nil {
752+
t.Logf("Error deleting lease %s/%s: %v", l.Namespace, l.Name, err)
669753
}
670754
}
671755
}
@@ -682,16 +766,15 @@ func TestController(t *testing.T) {
682766
return
683767
case <-ticker.C:
684768
for _, lc := range tc.createAfterControllerStart {
685-
lease, err := client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Get(ctx, lc.Name, metav1.GetOptions{})
769+
c, err := client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Get(ctx, lc.Name, metav1.GetOptions{})
686770
if err == nil {
687-
if lease.Spec.PingTime != nil {
688-
c := lease.DeepCopy()
771+
if c.Spec.PingTime != nil {
772+
t.Logf("Answering ping for %s/%s", c.Namespace, c.Name)
689773
c.Spec.RenewTime = &metav1.MicroTime{Time: fakeClock.Now()}
690774
_, err = client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Update(ctx, c, metav1.UpdateOptions{})
691775
if err != nil {
692-
runtime.HandleError(err)
776+
t.Logf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err)
693777
}
694-
695778
}
696779
}
697780
}
@@ -700,45 +783,48 @@ func TestController(t *testing.T) {
700783
}()
701784

702785
for _, obj := range tc.createAfterControllerStart {
786+
t.Logf("Post-creating lease candidate %s/%s", obj.Namespace, obj.Name)
703787
_, err := client.CoordinationV1alpha1().LeaseCandidates(obj.Namespace).Create(ctx, obj, metav1.CreateOptions{})
704788
if err != nil {
705-
t.Fatal(err)
789+
t.Fatalf("Error post-creating lease candidate %s/%s: %v", obj.Namespace, obj.Name, err)
706790
}
707791
}
708792

709-
for _, obj := range tc.deleteAfterControllerStart {
710-
err := client.CoordinationV1alpha1().LeaseCandidates(obj.Namespace).Delete(ctx, obj.Name, metav1.DeleteOptions{})
793+
for _, obj := range tc.deleteLeaseAfterControllerStart {
794+
t.Logf("Post-deleting lease %s/%s", obj.Namespace, obj.Name)
795+
err := client.CoordinationV1().Leases(obj.Namespace).Delete(ctx, obj.Name, metav1.DeleteOptions{})
711796
if err != nil {
712-
t.Fatal(err)
797+
t.Fatalf("Error post-deleting lease %s/%s: %v", obj.Namespace, obj.Name, err)
713798
}
714799
}
715800

716801
for _, expectedLease := range tc.expectedLeaderLeases {
717802
var lease *v1.Lease
718-
err = wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, 600*time.Second, true, func(ctx context.Context) (done bool, err error) {
803+
t.Logf("Waiting for lease %s/%s with holder %q", expectedLease.Namespace, expectedLease.Name, strOrNil(expectedLease.Spec.HolderIdentity))
804+
err = wait.PollUntilContextTimeout(ctx, 100*time.Millisecond, wait.ForeverTestTimeout, true, func(ctx context.Context) (done bool, err error) {
719805
lease, err = client.CoordinationV1().Leases(expectedLease.Namespace).Get(ctx, expectedLease.Name, metav1.GetOptions{})
720806
if err != nil {
721807
if errors.IsNotFound(err) {
722808
return false, nil
723809
}
724810
return true, err
725811
}
726-
if expectedLease.Spec.HolderIdentity == nil || lease.Spec.HolderIdentity == nil {
727-
return expectedLease.Spec.HolderIdentity == nil && lease.Spec.HolderIdentity == nil, nil
812+
if expectedLease.Spec.HolderIdentity == nil {
813+
return lease.Spec.HolderIdentity == nil, nil
814+
}
815+
if lease.Spec.HolderIdentity == nil {
816+
return false, nil
728817
}
729-
if expectedLease.Spec.HolderIdentity != nil && lease.Spec.HolderIdentity != nil && *expectedLease.Spec.HolderIdentity != *lease.Spec.HolderIdentity {
818+
if *expectedLease.Spec.HolderIdentity != *lease.Spec.HolderIdentity {
730819
return false, nil
731820
}
732821
return true, nil
733822
})
734823
if err != nil {
735-
t.Fatal(err)
736-
}
737-
if lease.Spec.HolderIdentity == nil {
738-
t.Fatalf("Expected HolderIdentity of %s but got nil", expectedLease.Name)
739-
}
740-
if *lease.Spec.HolderIdentity != *expectedLease.Spec.HolderIdentity {
741-
t.Errorf("Expected HolderIdentity of %s but got %s", *expectedLease.Spec.HolderIdentity, *lease.Spec.HolderIdentity)
824+
if lease == nil {
825+
t.Fatalf("Error waiting for lease %s/%s to exist: %v", expectedLease.Namespace, expectedLease.Name, err)
826+
}
827+
t.Fatalf("Error waiting for lease %s/%s with holder %q, but got %q: %v", expectedLease.Namespace, expectedLease.Name, strOrNil(expectedLease.Spec.HolderIdentity), strOrNil(lease.Spec.HolderIdentity), err)
742828
}
743829
}
744830
})

0 commit comments

Comments
 (0)