Skip to content

Commit 1c1fc73

Browse files
aroradamanaroradaman
committed
pkg/proxy/healthcheck: enhance testing
Signed-off-by: Daman Arora <[email protected]>
1 parent 64aac66 commit 1c1fc73

File tree

1 file changed

+102
-30
lines changed

1 file changed

+102
-30
lines changed

‎pkg/proxy/healthcheck/healthcheck_test.go‎

Lines changed: 102 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"k8s.io/apimachinery/pkg/types"
3535
"k8s.io/apimachinery/pkg/util/dump"
3636
"k8s.io/apimachinery/pkg/util/sets"
37+
"k8s.io/utils/ptr"
3738

3839
basemetrics "k8s.io/component-base/metrics"
3940
"k8s.io/kubernetes/pkg/proxy/metrics"
@@ -133,9 +134,9 @@ type hcPayload struct {
133134
}
134135

135136
type healthzPayload struct {
136-
LastUpdated string
137-
CurrentTime string
138-
NodeHealthy bool
137+
LastUpdated string
138+
CurrentTime string
139+
NodeEligible *bool
139140
}
140141

141142
type fakeProxyHealthChecker struct {
@@ -479,23 +480,30 @@ func TestHealthzServer(t *testing.T) {
479480
tracking503: 0,
480481
}
481482

482-
testProxyHealthUpdater(hs, hsTest, fakeClock, t)
483+
var expectedPayload healthzPayload
484+
// testProxyHealthUpdater only asserts on proxy health, without considering node eligibility
485+
// defaulting node health to true for testing.
486+
testProxyHealthUpdater(hs, hsTest, fakeClock, ptr.To(true), t)
483487

484488
// Should return 200 "OK" if we've synced a node, tainted in any other way
485489
hs.SyncNode(makeNode(tweakTainted("other")))
486-
testHTTPHandler(hsTest, http.StatusOK, t)
490+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: ptr.To(true)}
491+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
487492

488493
// Should return 503 "ServiceUnavailable" if we've synced a ToBeDeletedTaint node
489494
hs.SyncNode(makeNode(tweakTainted(ToBeDeletedTaint)))
490-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
495+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: ptr.To(false)}
496+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
491497

492498
// Should return 200 "OK" if we've synced a node, tainted in any other way
493499
hs.SyncNode(makeNode(tweakTainted("other")))
494-
testHTTPHandler(hsTest, http.StatusOK, t)
500+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: ptr.To(true)}
501+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
495502

496503
// Should return 503 "ServiceUnavailable" if we've synced a deleted node
497504
hs.SyncNode(makeNode(tweakDeleted()))
498-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
505+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: ptr.To(false)}
506+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
499507
}
500508

501509
func TestLivezServer(t *testing.T) {
@@ -514,23 +522,30 @@ func TestLivezServer(t *testing.T) {
514522
tracking503: 0,
515523
}
516524

517-
testProxyHealthUpdater(hs, hsTest, fakeClock, t)
525+
var expectedPayload healthzPayload
526+
// /livez doesn't have a concept of "nodeEligible", keeping the value
527+
// for node eligibility nil.
528+
testProxyHealthUpdater(hs, hsTest, fakeClock, nil, t)
518529

519530
// Should return 200 "OK" irrespective of node syncs
520531
hs.SyncNode(makeNode(tweakTainted("other")))
521-
testHTTPHandler(hsTest, http.StatusOK, t)
532+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String()}
533+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
522534

523535
// Should return 200 "OK" irrespective of node syncs
524536
hs.SyncNode(makeNode(tweakTainted(ToBeDeletedTaint)))
525-
testHTTPHandler(hsTest, http.StatusOK, t)
537+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String()}
538+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
526539

527540
// Should return 200 "OK" irrespective of node syncs
528541
hs.SyncNode(makeNode(tweakTainted("other")))
529-
testHTTPHandler(hsTest, http.StatusOK, t)
542+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String()}
543+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
530544

531545
// Should return 200 "OK" irrespective of node syncs
532546
hs.SyncNode(makeNode(tweakDeleted()))
533-
testHTTPHandler(hsTest, http.StatusOK, t)
547+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String()}
548+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
534549
}
535550

536551
type url string
@@ -540,78 +555,121 @@ var (
540555
livezURL url = "/livez"
541556
)
542557

543-
func testProxyHealthUpdater(hs *ProxyHealthServer, hsTest *serverTest, fakeClock *testingclock.FakeClock, t *testing.T) {
558+
func testProxyHealthUpdater(hs *ProxyHealthServer, hsTest *serverTest, fakeClock *testingclock.FakeClock, nodeEligible *bool, t *testing.T) {
559+
var expectedPayload healthzPayload
560+
// lastUpdated is used to track the time whenever any of the proxier update is simulated,
561+
// is used in assertion of the http response.
562+
var lastUpdated time.Time
563+
544564
// Should return 200 "OK" by default.
545-
testHTTPHandler(hsTest, http.StatusOK, t)
565+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
566+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
546567

547568
// Should return 200 "OK" after first update for both IPv4 and IPv6 proxiers.
548569
hs.Updated(v1.IPv4Protocol)
549570
hs.Updated(v1.IPv6Protocol)
550-
testHTTPHandler(hsTest, http.StatusOK, t)
571+
lastUpdated = fakeClock.Now()
572+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
573+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
574+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
575+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
551576

552577
// Should continue to return 200 "OK" as long as no further updates are queued for any proxier.
553578
fakeClock.Step(25 * time.Second)
554-
testHTTPHandler(hsTest, http.StatusOK, t)
579+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
580+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
581+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
582+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
555583

556584
// Should return 503 "ServiceUnavailable" if IPv4 proxier exceed max update-processing time.
557585
hs.QueuedUpdate(v1.IPv4Protocol)
558586
fakeClock.Step(25 * time.Second)
559-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
587+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
588+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
560589

561590
// Should return 200 "OK" after processing update for both IPv4 and IPv6 proxiers.
562591
hs.Updated(v1.IPv4Protocol)
563592
hs.Updated(v1.IPv6Protocol)
593+
lastUpdated = fakeClock.Now()
564594
fakeClock.Step(5 * time.Second)
565-
testHTTPHandler(hsTest, http.StatusOK, t)
595+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
596+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
597+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
598+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
566599

567600
// Should return 503 "ServiceUnavailable" if IPv6 proxier exceed max update-processing time.
568601
hs.QueuedUpdate(v1.IPv6Protocol)
569602
fakeClock.Step(25 * time.Second)
570-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
603+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
604+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
571605

572606
// Should return 200 "OK" after processing update for both IPv4 and IPv6 proxiers.
573607
hs.Updated(v1.IPv4Protocol)
574608
hs.Updated(v1.IPv6Protocol)
609+
lastUpdated = fakeClock.Now()
575610
fakeClock.Step(5 * time.Second)
576-
testHTTPHandler(hsTest, http.StatusOK, t)
611+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
612+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
613+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
614+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
577615

578616
// Should return 503 "ServiceUnavailable" if both IPv4 and IPv6 proxiers exceed max update-processing time.
579617
hs.QueuedUpdate(v1.IPv4Protocol)
580618
hs.QueuedUpdate(v1.IPv6Protocol)
581619
fakeClock.Step(25 * time.Second)
582-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
620+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
621+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
583622

584623
// Should return 200 "OK" after processing update for both IPv4 and IPv6 proxiers.
585624
hs.Updated(v1.IPv4Protocol)
586625
hs.Updated(v1.IPv6Protocol)
626+
lastUpdated = fakeClock.Now()
587627
fakeClock.Step(5 * time.Second)
588-
testHTTPHandler(hsTest, http.StatusOK, t)
628+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
629+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
630+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
631+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
589632

590633
// If IPv6 proxier is late for an update but IPv4 proxier is not then updating IPv4 proxier should have no effect.
591634
hs.QueuedUpdate(v1.IPv6Protocol)
592635
fakeClock.Step(25 * time.Second)
593-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
636+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
637+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
594638

595639
hs.Updated(v1.IPv4Protocol)
596-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
640+
lastUpdated = fakeClock.Now()
641+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
642+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
597643

598644
hs.Updated(v1.IPv6Protocol)
599-
testHTTPHandler(hsTest, http.StatusOK, t)
645+
lastUpdated = fakeClock.Now()
646+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
647+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
648+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
649+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
600650

601651
// If both IPv4 and IPv6 proxiers are late for an update, we shouldn't report 200 "OK" until after both of them update.
602652
hs.QueuedUpdate(v1.IPv4Protocol)
603653
hs.QueuedUpdate(v1.IPv6Protocol)
604654
fakeClock.Step(25 * time.Second)
605-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
655+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
656+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
606657

607658
hs.Updated(v1.IPv4Protocol)
608-
testHTTPHandler(hsTest, http.StatusServiceUnavailable, t)
659+
lastUpdated = fakeClock.Now()
660+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: lastUpdated.String(), NodeEligible: nodeEligible}
661+
testHTTPHandler(hsTest, http.StatusServiceUnavailable, expectedPayload, t)
609662

610663
hs.Updated(v1.IPv6Protocol)
611-
testHTTPHandler(hsTest, http.StatusOK, t)
664+
lastUpdated = fakeClock.Now()
665+
// for backward-compatibility returned last_updated is current_time when proxy is healthy,
666+
// using fakeClock.Now().String() instead of lastUpdated.String() here.
667+
expectedPayload = healthzPayload{CurrentTime: fakeClock.Now().String(), LastUpdated: fakeClock.Now().String(), NodeEligible: nodeEligible}
668+
testHTTPHandler(hsTest, http.StatusOK, expectedPayload, t)
612669
}
613670

614-
func testHTTPHandler(hsTest *serverTest, status int, t *testing.T) {
671+
func testHTTPHandler(hsTest *serverTest, status int, expectedPayload healthzPayload, t *testing.T) {
672+
t.Helper()
615673
handler := hsTest.server.(*fakeHTTPServer).handler
616674
req, err := http.NewRequest("GET", string(hsTest.url), nil)
617675
if err != nil {
@@ -629,17 +687,31 @@ func testHTTPHandler(hsTest *serverTest, status int, t *testing.T) {
629687
t.Fatal(err)
630688
}
631689

690+
// assert on payload
691+
if payload.LastUpdated != expectedPayload.LastUpdated {
692+
t.Errorf("expected last updated: %s; got: %s", expectedPayload.LastUpdated, payload.LastUpdated)
693+
}
694+
if payload.CurrentTime != expectedPayload.CurrentTime {
695+
t.Errorf("expected current time: %s; got: %s", expectedPayload.CurrentTime, payload.CurrentTime)
696+
}
697+
632698
if status == http.StatusOK {
633699
hsTest.tracking200++
634700
}
635701
if status == http.StatusServiceUnavailable {
636702
hsTest.tracking503++
637703
}
638704
if hsTest.url == healthzURL {
705+
if *payload.NodeEligible != *expectedPayload.NodeEligible {
706+
t.Errorf("expected node eligible: %v; got: %v", *payload.NodeEligible, *expectedPayload.NodeEligible)
707+
}
639708
testMetricEquals(metrics.ProxyHealthzTotal.WithLabelValues("200"), float64(hsTest.tracking200), t)
640709
testMetricEquals(metrics.ProxyHealthzTotal.WithLabelValues("503"), float64(hsTest.tracking503), t)
641710
}
642711
if hsTest.url == livezURL {
712+
if payload.NodeEligible != nil {
713+
t.Errorf("expected node eligible nil for %s response", livezURL)
714+
}
643715
testMetricEquals(metrics.ProxyLivezTotal.WithLabelValues("200"), float64(hsTest.tracking200), t)
644716
testMetricEquals(metrics.ProxyLivezTotal.WithLabelValues("503"), float64(hsTest.tracking503), t)
645717
}

0 commit comments

Comments
 (0)