Skip to content

Commit

Permalink
Merge pull request #93643 from ialidzhikov/automated-cherry-pick-of-#…
Browse files Browse the repository at this point in the history
…93600-upstream-release-1.17

Automated cherry pick of #93600: Fix panic on /readyz
  • Loading branch information
k8s-ci-robot committed Aug 4, 2020
2 parents 7d89b9a + 548db28 commit 1033539
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 8 deletions.
2 changes: 1 addition & 1 deletion staging/src/k8s.io/apiserver/pkg/server/healthz/BUILD
Expand Up @@ -11,6 +11,7 @@ go_test(
srcs = ["healthz_test.go"],
embed = [":go_default_library"],
deps = [
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
],
)
Expand All @@ -27,7 +28,6 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/httplog:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
)
Expand Down
18 changes: 11 additions & 7 deletions staging/src/k8s.io/apiserver/pkg/server/healthz/healthz.go
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"fmt"
"net/http"
"reflect"
"strings"
"sync"
"sync/atomic"
Expand All @@ -28,7 +29,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/server/httplog"
"k8s.io/client-go/informers"
"k8s.io/klog"
)

Expand Down Expand Up @@ -81,16 +81,20 @@ func (l *log) Check(_ *http.Request) error {
return fmt.Errorf("logging blocked")
}

type cacheSyncWaiter interface {
WaitForCacheSync(stopCh <-chan struct{}) map[reflect.Type]bool
}

type informerSync struct {
sharedInformerFactory informers.SharedInformerFactory
cacheSyncWaiter cacheSyncWaiter
}

var _ HealthChecker = &informerSync{}

// NewInformerSyncHealthz returns a new HealthChecker that will pass only if all informers in the given sharedInformerFactory sync.
func NewInformerSyncHealthz(sharedInformerFactory informers.SharedInformerFactory) HealthChecker {
// NewInformerSyncHealthz returns a new HealthChecker that will pass only if all informers in the given cacheSyncWaiter sync.
func NewInformerSyncHealthz(cacheSyncWaiter cacheSyncWaiter) HealthChecker {
return &informerSync{
sharedInformerFactory: sharedInformerFactory,
cacheSyncWaiter: cacheSyncWaiter,
}
}

Expand All @@ -103,8 +107,8 @@ func (i *informerSync) Check(_ *http.Request) error {
// Close stopCh to force checking if informers are synced now.
close(stopCh)

var informersByStarted map[bool][]string
for informerType, started := range i.sharedInformerFactory.WaitForCacheSync(stopCh) {
informersByStarted := make(map[bool][]string)
for informerType, started := range i.cacheSyncWaiter.WaitForCacheSync(stopCh) {
informersByStarted[started] = append(informersByStarted[started], informerType.String())
}

Expand Down
41 changes: 41 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go
Expand Up @@ -25,6 +25,7 @@ import (
"reflect"
"testing"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
)

Expand Down Expand Up @@ -240,3 +241,43 @@ func createGetRequestWithUrl(rawUrlString string) *http.Request {
URL: url,
}
}

func TestInformerSyncHealthChecker(t *testing.T) {
t.Run("test that check returns nil when all informers are started", func(t *testing.T) {
healthChecker := NewInformerSyncHealthz(cacheSyncWaiterStub{
startedByInformerType: map[reflect.Type]bool{
reflect.TypeOf(corev1.Pod{}): true,
},
})

err := healthChecker.Check(nil)
if err != nil {
t.Errorf("Got %v, expected no error", err)
}
})

t.Run("test that check returns err when there is not started informer", func(t *testing.T) {
healthChecker := NewInformerSyncHealthz(cacheSyncWaiterStub{
startedByInformerType: map[reflect.Type]bool{
reflect.TypeOf(corev1.Pod{}): true,
reflect.TypeOf(corev1.Service{}): false,
reflect.TypeOf(corev1.Node{}): true,
},
})

err := healthChecker.Check(nil)
if err == nil {
t.Errorf("expected error, got: %v", err)
}
})
}

type cacheSyncWaiterStub struct {
startedByInformerType map[reflect.Type]bool
}

// WaitForCacheSync is a stub implementation of the corresponding func
// that simply returns the value passed during stub initialization.
func (s cacheSyncWaiterStub) WaitForCacheSync(_ <-chan struct{}) map[reflect.Type]bool {
return s.startedByInformerType
}

0 comments on commit 1033539

Please sign in to comment.