From 56f277ca0f2f49ad03db3f98ac38fe734a40ca2c Mon Sep 17 00:00:00 2001 From: Maciej Borsz Date: Thu, 18 Jun 2020 15:21:12 +0200 Subject: [PATCH] Wait for all informers to sync in /readyz. --- .../src/k8s.io/apiserver/pkg/server/config.go | 21 ++++++++---- .../apiserver/pkg/server/config_test.go | 5 +-- .../k8s.io/apiserver/pkg/server/healthz/BUILD | 1 + .../apiserver/pkg/server/healthz/healthz.go | 34 +++++++++++++++++++ .../integration/master/kube_apiserver_test.go | 18 ++++++---- 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 998999e8ab46c..b8cfa1d834814 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -559,13 +559,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 + } } } diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index bf9daf844b69a..fac76e42b8e1d 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -158,6 +158,7 @@ func TestNewWithDelegate(t *testing.T) { "/metrics", "/readyz", "/readyz/delegate-health", + "/readyz/informer-sync", "/readyz/log", "/readyz/ping", "/readyz/poststarthook/delegate-post-start-hook", @@ -233,10 +234,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) } }) diff --git a/staging/src/k8s.io/apiserver/pkg/server/healthz/BUILD b/staging/src/k8s.io/apiserver/pkg/server/healthz/BUILD index 64ffaa25d1347..82a940b776205 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/healthz/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/server/healthz/BUILD @@ -27,6 +27,7 @@ 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", ], ) diff --git a/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz.go b/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz.go index a55e201e7043c..0cacdf6a0725b 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz.go +++ b/staging/src/k8s.io/apiserver/pkg/server/healthz/healthz.go @@ -28,6 +28,7 @@ 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" ) @@ -80,6 +81,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} diff --git a/test/integration/master/kube_apiserver_test.go b/test/integration/master/kube_apiserver_test.go index daa511599e940..4f58b8c3e48a3 100644 --- a/test/integration/master/kube_apiserver_test.go +++ b/test/integration/master/kube_apiserver_test.go @@ -96,11 +96,15 @@ func TestRun(t *testing.T) { } } -func endpointReturnsStatusOK(client *kubernetes.Clientset, path string) bool { - res := client.CoreV1().RESTClient().Get().AbsPath(path).Do() +func endpointReturnsStatusOK(client *kubernetes.Clientset, path string) (bool, error) { + res := client.CoreV1().RESTClient().Get().RequestURI(path).Do() var status int res.StatusCode(&status) - return status == http.StatusOK + _, err := res.Raw() + if err != nil { + return false, err + } + return status == http.StatusOK, nil } func TestLivezAndReadyz(t *testing.T) { @@ -111,11 +115,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) } }