Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for all informers to sync in /readyz. #92644

Merged
merged 1 commit into from Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 14 additions & 7 deletions staging/src/k8s.io/apiserver/pkg/server/config.go
Expand Up @@ -605,13 +605,20 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G
}

genericApiServerHookName := "generic-apiserver-start-informers"
if c.SharedInformerFactory != nil && !s.isPostStartHookRegistered(genericApiServerHookName) {
err := s.AddPostStartHook(genericApiServerHookName, func(context PostStartHookContext) error {
c.SharedInformerFactory.Start(context.StopCh)
return nil
})
if err != nil {
return nil, err
if c.SharedInformerFactory != nil {
if !s.isPostStartHookRegistered(genericApiServerHookName) {
err := s.AddPostStartHook(genericApiServerHookName, func(context PostStartHookContext) error {
c.SharedInformerFactory.Start(context.StopCh)
return nil
})
if err != nil {
return nil, err
}
// TODO: Once we get rid of /healthz consider changing this to post-start-hook.
err = s.addReadyzChecks(healthz.NewInformerSyncHealthz(c.SharedInformerFactory))
if err != nil {
return nil, err
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions staging/src/k8s.io/apiserver/pkg/server/config_test.go
Expand Up @@ -167,6 +167,7 @@ func TestNewWithDelegate(t *testing.T) {
"/metrics",
"/readyz",
"/readyz/delegate-health",
"/readyz/informer-sync",
"/readyz/log",
"/readyz/ping",
"/readyz/poststarthook/delegate-post-start-hook",
Expand Down Expand Up @@ -242,10 +243,10 @@ func checkExpectedPathsAtRoot(url string, expectedPaths []string, t *testing.T)
pathset.Insert(p.(string))
}
expectedset := sets.NewString(expectedPaths...)
for _, p := range pathset.Difference(expectedset) {
for p := range pathset.Difference(expectedset) {
t.Errorf("Got %v path, which we did not expect", p)
}
for _, p := range expectedset.Difference(pathset) {
for p := range expectedset.Difference(pathset) {
t.Errorf(" Expected %v path which we did not get", p)
}
})
Expand Down
1 change: 1 addition & 0 deletions staging/src/k8s.io/apiserver/pkg/server/healthz/BUILD
Expand Up @@ -31,6 +31,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/endpoints/metrics: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/v2:go_default_library",
],
)
Expand Down
34 changes: 34 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/server/healthz/healthz.go
Expand Up @@ -29,6 +29,7 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apiserver/pkg/endpoints/metrics"
"k8s.io/apiserver/pkg/server/httplog"
"k8s.io/client-go/informers"
"k8s.io/klog/v2"
)

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

type informerSync struct {
sharedInformerFactory informers.SharedInformerFactory
}

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 {
return &informerSync{
sharedInformerFactory: sharedInformerFactory,
}
}

func (i *informerSync) Name() string {
return "informer-sync"
}

func (i *informerSync) Check(_ *http.Request) error {
stopCh := make(chan struct{})
// 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[started] = append(informersByStarted[started], informerType.String())
}

if notStarted := informersByStarted[false]; len(notStarted) > 0 {
return fmt.Errorf("%d informers not started yet: %v", len(notStarted), notStarted)
}
return nil
}

// NamedCheck returns a healthz checker for the given name and function.
func NamedCheck(name string, check func(r *http.Request) error) HealthChecker {
return &healthzCheck{name, check}
Expand Down
18 changes: 11 additions & 7 deletions test/integration/master/kube_apiserver_test.go
Expand Up @@ -98,11 +98,15 @@ func TestRun(t *testing.T) {
}
}

func endpointReturnsStatusOK(client *kubernetes.Clientset, path string) bool {
res := client.CoreV1().RESTClient().Get().AbsPath(path).Do(context.TODO())
func endpointReturnsStatusOK(client *kubernetes.Clientset, path string) (bool, error) {
res := client.CoreV1().RESTClient().Get().RequestURI(path).Do(context.TODO())
var status int
res.StatusCode(&status)
return status == http.StatusOK
_, err := res.Raw()
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to do this, then we should probably just fully convert this function so that it doesn't return the bool.

if err != nil || status != http.StatusOK {
   t.Fatalf("got %v but wanted 200, error: %v", status, err)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then the calls on 121 and 124 don't have to be in conditional form.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed the function to return error actually

return false, err
}
return status == http.StatusOK, nil
}

func TestLivezAndReadyz(t *testing.T) {
Expand All @@ -113,11 +117,11 @@ func TestLivezAndReadyz(t *testing.T) {
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !endpointReturnsStatusOK(client, "/livez") {
t.Fatalf("livez should be healthy")
if statusOK, err := endpointReturnsStatusOK(client, "/livez"); err != nil || !statusOK {
t.Fatalf("livez should be healthy, got %v and error %v", statusOK, err)
}
if !endpointReturnsStatusOK(client, "/readyz") {
t.Fatalf("readyz should be healthy")
if statusOK, err := endpointReturnsStatusOK(client, "/readyz"); err != nil || !statusOK {
t.Fatalf("readyz should be healthy, got %v and error %v", statusOK, err)
}
}

Expand Down