github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #126446 from Jefftree/fix-leaderelection-flake-testcontroller

Browse Source 
Use fake clock for controller/leaderelection:TestController
This commit is contained in:
Kubernetes Prow Robot
2024-07-30 02:16:01 -07:00
committed by GitHub
parent 17d7d28620 634c9cd135
commit 0fc1671031
2 changed files with 37 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
pkg/controlplane/controller/leaderelection/leaderelection_controller.go
View File

@@ -278,7 +278,8 @@ func (c *Controller) reconcileElectionStep(ctx context.Context, leaseNN types.Na
if candidate.Spec.PingTime == nil || if candidate.Spec.PingTime == nil ||
// If PingTime is outdated, send another PingTime only if it already acked the first one. // If PingTime is outdated, send another PingTime only if it already acked the first one.
(candidate.Spec.PingTime.Add(electionDuration).Before(now) && candidate.Spec.PingTime.Before(candidate.Spec.RenewTime)) { // This checks for pingTime <= renewTime because equality is possible in unit tests using a fake clock.
(candidate.Spec.PingTime.Add(electionDuration).Before(now) && !candidate.Spec.RenewTime.Before(candidate.Spec.PingTime)) {
// TODO(jefftree): We should randomize the order of sending pings and do them in parallel // TODO(jefftree): We should randomize the order of sending pings and do them in parallel
// so that all candidates have equal opportunity to ack. // so that all candidates have equal opportunity to ack.
clone := candidate.DeepCopy() clone := candidate.DeepCopy()
@@ -300,7 +301,7 @@ func (c *Controller) reconcileElectionStep(ctx context.Context, leaseNN types.Na
continue // shouldn't be the case after the above continue // shouldn't be the case after the above
} }
if candidate.Spec.RenewTime != nil && candidate.Spec.PingTime.Before(candidate.Spec.RenewTime) { if candidate.Spec.RenewTime != nil && !candidate.Spec.RenewTime.Before(candidate.Spec.PingTime) {
continue // this has renewed already continue // this has renewed already
} }

62
pkg/controlplane/controller/leaderelection/leaderelection_controller_test.go
View File

@@ -20,6 +20,7 @@ import (
"context" "context"
"fmt" "fmt"
"math/rand" "math/rand"
"sync"
"testing" "testing"
"time" "time"
@@ -174,7 +175,7 @@ func TestReconcileElectionStep(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.20.0", EmulationVersion: "1.20.0",
BinaryVersion: "1.20.0", BinaryVersion: "1.20.0",
PingTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(-1 * time.Millisecond))), PingTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
@@ -439,8 +440,6 @@ func TestReconcileElectionStep(t *testing.T) {
} }
func TestController(t *testing.T) { func TestController(t *testing.T) {
fakeClock := testingclock.NewFakeClock(time.Now())
cases := []struct { cases := []struct {
name string name string
leases []*v1.Lease leases []*v1.Lease
@@ -461,7 +460,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.19.0", EmulationVersion: "1.19.0",
BinaryVersion: "1.19.0", BinaryVersion: "1.19.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -490,7 +489,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.19.0", EmulationVersion: "1.19.0",
BinaryVersion: "1.19.0", BinaryVersion: "1.19.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -503,7 +502,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.19.0", EmulationVersion: "1.19.0",
BinaryVersion: "1.20.0", BinaryVersion: "1.20.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -516,7 +515,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.20.0", EmulationVersion: "1.20.0",
BinaryVersion: "1.20.0", BinaryVersion: "1.20.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -543,7 +542,7 @@ func TestController(t *testing.T) {
}, },
Spec: v1.LeaseSpec{ Spec: v1.LeaseSpec{
HolderIdentity: ptr.To("some-other-component"), HolderIdentity: ptr.To("some-other-component"),
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(time.Second))), RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(time.Second))),
LeaseDurationSeconds: ptr.To(int32(10)), LeaseDurationSeconds: ptr.To(int32(10)),
}, },
}, },
@@ -558,7 +557,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.19.0", EmulationVersion: "1.19.0",
BinaryVersion: "1.19.0", BinaryVersion: "1.19.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -588,7 +587,7 @@ func TestController(t *testing.T) {
}, },
Spec: v1.LeaseSpec{ Spec: v1.LeaseSpec{
HolderIdentity: ptr.To("some-other-component"), HolderIdentity: ptr.To("some-other-component"),
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(-11 * time.Second))), RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(-11 * time.Second))),
LeaseDurationSeconds: ptr.To(int32(10)), LeaseDurationSeconds: ptr.To(int32(10)),
Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"), Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"),
}, },
@@ -604,7 +603,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.19.0", EmulationVersion: "1.19.0",
BinaryVersion: "1.19.0", BinaryVersion: "1.19.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -632,7 +631,7 @@ func TestController(t *testing.T) {
Spec: v1.LeaseSpec{ Spec: v1.LeaseSpec{
HolderIdentity: ptr.To("component-identity-1"), HolderIdentity: ptr.To("component-identity-1"),
Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"), Strategy: ptr.To[v1.CoordinatedLeaseStrategy]("OldestEmulationVersion"),
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now().Add(time.Second))), RenewTime: ptr.To(metav1.NewMicroTime(time.Now().Add(time.Second))),
LeaseDurationSeconds: ptr.To(int32(10)), LeaseDurationSeconds: ptr.To(int32(10)),
}, },
}, },
@@ -647,7 +646,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.20.0", EmulationVersion: "1.20.0",
BinaryVersion: "1.20.0", BinaryVersion: "1.20.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -662,7 +661,7 @@ func TestController(t *testing.T) {
LeaseName: "component-A", LeaseName: "component-A",
EmulationVersion: "1.19.0", EmulationVersion: "1.19.0",
BinaryVersion: "1.19.0", BinaryVersion: "1.19.0",
RenewTime: ptr.To(metav1.NewMicroTime(fakeClock.Now())), RenewTime: ptr.To(metav1.NewMicroTime(time.Now())),
PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion}, PreferredStrategies: []v1.CoordinatedLeaseStrategy{v1.OldestEmulationVersion},
}, },
}, },
@@ -683,6 +682,10 @@ func TestController(t *testing.T) {
for _, tc := range cases { for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
// collect go routines using t.logf
var wg sync.WaitGroup
defer wg.Wait()
ctx, cancel := context.WithTimeout(context.Background(), time.Minute) ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel() defer cancel()
@@ -722,7 +725,9 @@ func TestController(t *testing.T) {
time.Sleep(time.Second) time.Sleep(time.Second)
} }
wg.Add(1)
go func() { go func() {
defer wg.Done()
ticker := time.NewTicker(10 * time.Millisecond) ticker := time.NewTicker(10 * time.Millisecond)
// Mock out the removal of preferredHolder leases. // Mock out the removal of preferredHolder leases.
// When controllers are running, they are expected to do this voluntarily // When controllers are running, they are expected to do this voluntarily
@@ -756,7 +761,9 @@ func TestController(t *testing.T) {
} }
}() }()
wg.Add(1)
go func() { go func() {
defer wg.Done()
ticker := time.NewTicker(10 * time.Millisecond) ticker := time.NewTicker(10 * time.Millisecond)
// Mock out leasecandidate ack. // Mock out leasecandidate ack.
// When controllers are running, they are expected to watch and ack // When controllers are running, they are expected to watch and ack
@@ -765,16 +772,18 @@ func TestController(t *testing.T) {
case <-ctx.Done(): case <-ctx.Done():
return return
case <-ticker.C: case <-ticker.C:
for _, lc := range tc.createAfterControllerStart { cs, err := client.CoordinationV1alpha1().LeaseCandidates("").List(ctx, metav1.ListOptions{})
c, err := client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Get(ctx, lc.Name, metav1.GetOptions{}) if err != nil {
if err == nil { t.Logf("Error listing lease candidates: %v", err)
if c.Spec.PingTime != nil { continue
t.Logf("Answering ping for %s/%s", c.Namespace, c.Name) }
c.Spec.RenewTime = &metav1.MicroTime{Time: fakeClock.Now()} for _, c := range cs.Items {
_, err = client.CoordinationV1alpha1().LeaseCandidates(lc.Namespace).Update(ctx, c, metav1.UpdateOptions{}) if c.Spec.PingTime != nil && (c.Spec.RenewTime == nil || c.Spec.PingTime.Time.After(c.Spec.RenewTime.Time)) {
if err != nil { t.Logf("Answering ping for %s/%s", c.Namespace, c.Name)
t.Logf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err) c.Spec.RenewTime = &metav1.MicroTime{Time: time.Now()}
} _, err = client.CoordinationV1alpha1().LeaseCandidates(c.Namespace).Update(ctx, &c, metav1.UpdateOptions{})
if err != nil {
t.Logf("Error updating lease candidate %s/%s: %v", c.Namespace, c.Name, err)
} }
} }
} }
@@ -815,10 +824,7 @@ func TestController(t *testing.T) {
if lease.Spec.HolderIdentity == nil { if lease.Spec.HolderIdentity == nil {
return false, nil return false, nil
} }
if *expectedLease.Spec.HolderIdentity != *lease.Spec.HolderIdentity { return *expectedLease.Spec.HolderIdentity == *lease.Spec.HolderIdentity, nil
return false, nil
}
return true, nil
}) })
if err != nil { if err != nil {
if lease == nil { if lease == nil {