owncloud · fschade · Oct 17, 2024 · Oct 17, 2024
diff --git a/changelog/unreleased/fix-health-and-ready-endpoints.md b/changelog/unreleased/fix-health-and-ready-endpoints.md
@@ -0,0 +1,15 @@
+Bugfix: Fix health and ready endpoints
+
+We added new checks to the `/readyz` and `/healthz` endpoints to ensure that the services are ready and healthy.
+This change ensures that the endpoints return the correct status codes, which is needed to stabilize the k8s deployments.
+
+https://github.com/owncloud/ocis/pull/10163
+https://github.com/owncloud/ocis/pull/10301
+https://github.com/owncloud/ocis/pull/10302
+https://github.com/owncloud/ocis/pull/10303
+https://github.com/owncloud/ocis/pull/10308
+https://github.com/owncloud/ocis/pull/10323
+https://github.com/owncloud/ocis/pull/10163
+https://github.com/owncloud/ocis/pull/10333
+https://github.com/owncloud/ocis/issues/10316
+https://github.com/owncloud/ocis/issues/10281
diff --git a/ocis-pkg/handlers/checker.go b/ocis-pkg/handlers/checker.go
@@ -15,12 +15,9 @@ import (
 // check is a function that performs a check.
 type checker func(ctx context.Context) error
 
-// checks is a map of check names to check functions.
-type checkers map[string]func(ctx context.Context) error
-
 // CheckHandlerConfiguration defines the configuration for the CheckHandler.
 type CheckHandlerConfiguration struct {
-	checks        checkers
+	checks        map[string]checker
 	logger        log.Logger
 	limit         int
 	statusFailed  int
@@ -30,7 +27,7 @@ type CheckHandlerConfiguration struct {
 // NewCheckHandlerConfiguration initializes a new CheckHandlerConfiguration.
 func NewCheckHandlerConfiguration() CheckHandlerConfiguration {
 	return CheckHandlerConfiguration{
-		checks: make(checkers),
+		checks: make(map[string]checker),
 
 		limit:         -1,
 		statusFailed:  http.StatusInternalServerError,
@@ -56,15 +53,6 @@ func (c CheckHandlerConfiguration) WithCheck(name string, check checker) CheckHa
 	return c
 }
 
-// WithChecks adds multiple checks to the CheckHandlerConfiguration.
-func (c CheckHandlerConfiguration) WithChecks(checks checkers) CheckHandlerConfiguration {
-	for name, check := range checks {
-		c.checks = c.WithCheck(name, check).checks
-	}
-
-	return c
-}
-
 // WithLimit limits the number of active goroutines for the checks to at most n
 func (c CheckHandlerConfiguration) WithLimit(n int) CheckHandlerConfiguration {
 	c.limit = n
@@ -96,10 +84,6 @@ func NewCheckHandler(c CheckHandlerConfiguration) *CheckHandler {
 	}
 }
 
-func (h *CheckHandler) Checks() map[string]func(ctx context.Context) error {
-	return maps.Clone(h.conf.checks)
-}
-
 func (h *CheckHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	g, ctx := errgroup.WithContext(r.Context())
 	g.SetLimit(h.conf.limit)

diff --git a/ocis-pkg/handlers/checker_test.go b/ocis-pkg/handlers/checker_test.go
@@ -17,19 +17,18 @@ import (
 )
 
 func TestCheckHandlerConfiguration(t *testing.T) {
-	nopCheck := func(_ context.Context) error { return nil }
+	nopCheckCounter := 0
+	nopCheck := func(_ context.Context) error { nopCheckCounter++; return nil }
 	handlerConfiguration := handlers.NewCheckHandlerConfiguration().WithCheck("check-1", nopCheck)
 
 	t.Run("add check", func(t *testing.T) {
-		inheritedHandlerConfiguration := handlerConfiguration.WithCheck("check-2", nopCheck)
-		require.Equal(t, 1, len(handlers.NewCheckHandler(handlerConfiguration).Checks()))
-		require.Equal(t, 2, len(handlers.NewCheckHandler(inheritedHandlerConfiguration).Checks()))
-	})
+		localCounter := 0
+		handlers.NewCheckHandler(handlerConfiguration).ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/", nil))
+		require.Equal(t, 1, nopCheckCounter)
 
-	t.Run("add checks", func(t *testing.T) {
-		inheritedHandlerConfiguration := handlerConfiguration.WithChecks(map[string]func(ctx context.Context) error{"check-2": nopCheck, "check-3": nopCheck})
-		require.Equal(t, 1, len(handlers.NewCheckHandler(handlerConfiguration).Checks()))
-		require.Equal(t, 3, len(handlers.NewCheckHandler(inheritedHandlerConfiguration).Checks()))
+		handlers.NewCheckHandler(handlerConfiguration.WithCheck("check-2", func(_ context.Context) error { localCounter++; return nil })).ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/", nil))
+		require.Equal(t, 2, nopCheckCounter)
+		require.Equal(t, 1, localCounter)
 	})
 
 	t.Run("checks are unique", func(t *testing.T) {
@@ -40,6 +39,7 @@ func TestCheckHandlerConfiguration(t *testing.T) {
 		}()
 
 		handlerConfiguration.WithCheck("check-1", nopCheck)
+		require.Equal(t, 3, nopCheckCounter)
 	})
 }
 

diff --git a/ocis-pkg/service/debug/service.go b/ocis-pkg/service/debug/service.go
@@ -12,10 +12,26 @@ import (
 	"go.opentelemetry.io/contrib/zpages"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/cors"
+	"github.com/owncloud/ocis/v2/ocis-pkg/handlers"
+	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	"github.com/owncloud/ocis/v2/ocis-pkg/middleware"
 	graphMiddleware "github.com/owncloud/ocis/v2/services/graph/pkg/middleware"
 )
 
+var handleProbe = func(mux *http.ServeMux, pattern string, h http.Handler, name string, logger log.Logger) {
+	if h == nil {
+		h = handlers.NewCheckHandler(handlers.NewCheckHandlerConfiguration())
+		logger.Info().
+			Str("service", name).
+			Str("endpoint", pattern).
+			Msg("no probe provided, reverting to default (OK)")
+	}
+
+	mux.Handle(pattern, h)
+}
+
+//var probeHandler = func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }
+
 // NewService initializes a new debug service.
 func NewService(opts ...Option) *http.Server {
 	dopts := newOptions(opts...)
@@ -29,13 +45,8 @@ func NewService(opts ...Option) *http.Server {
 		promhttp.Handler(),
 	))
 
-	if dopts.Health != nil {
-		mux.Handle("/healthz", dopts.Health)
-	}
-
-	if dopts.Ready != nil {
-		mux.Handle("/readyz", dopts.Ready)
-	}
+	handleProbe(mux, "/healthz", dopts.Health, dopts.Name, dopts.Logger) // healthiness check
+	handleProbe(mux, "/readyz", dopts.Ready, dopts.Name, dopts.Logger)   // readiness check
 
 	if dopts.ConfigDump != nil {
 		mux.Handle("/config", dopts.ConfigDump)

diff --git a/services/activitylog/pkg/server/debug/server.go b/services/activitylog/pkg/server/debug/server.go
@@ -13,18 +13,12 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	healthHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger).
-			WithCheck("http reachability", checks.NewHTTPCheck(options.Config.HTTP.Addr)),
-	)
+	healthHandlerConfiguration := handlers.NewCheckHandlerConfiguration().
+		WithLogger(options.Logger).
+		WithCheck("http reachability", checks.NewHTTPCheck(options.Config.HTTP.Addr))
 
-	readyHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger).
-			WithCheck("nats reachability", checks.NewNatsCheck(options.Config.Events.Cluster)).
-			WithChecks(healthHandler.Checks()),
-	)
+	readyHandlerConfiguration := healthHandlerConfiguration.
+		WithCheck("nats reachability", checks.NewNatsCheck(options.Config.Events.Cluster))
 
 	return debug.NewService(
 		debug.Logger(options.Logger),
@@ -34,7 +28,7 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(healthHandler),
-		debug.Ready(readyHandler),
+		debug.Health(handlers.NewCheckHandler(healthHandlerConfiguration)),
+		debug.Ready(handlers.NewCheckHandler(readyHandlerConfiguration)),
 	), nil
 }
diff --git a/services/antivirus/pkg/server/debug/server.go b/services/antivirus/pkg/server/debug/server.go
@@ -17,27 +17,21 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	healthHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger),
-	)
-
-	readyHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger).
-			WithCheck("nats reachability", checks.NewNatsCheck(options.Config.Events.Cluster)).
-			WithCheck("antivirus reachability", func(ctx context.Context) error {
-				cfg := options.Config
-				switch cfg.Scanner.Type {
-				default:
-					return errors.New("no antivirus configured")
-				case "clamav":
-					return clamd.NewClamd(cfg.Scanner.ClamAV.Socket).Ping()
-				case "icap":
-					return checks.NewTCPCheck(cfg.Scanner.ICAP.URL)(ctx)
-				}
-			}),
-	)
+	readyHandlerConfiguration := handlers.NewCheckHandlerConfiguration().
+		WithLogger(options.Logger).
+		WithLogger(options.Logger).
+		WithCheck("nats reachability", checks.NewNatsCheck(options.Config.Events.Cluster)).
+		WithCheck("antivirus reachability", func(ctx context.Context) error {
+			cfg := options.Config
+			switch cfg.Scanner.Type {
+			default:
+				return errors.New("no antivirus configured")
+			case "clamav":
+				return clamd.NewClamd(cfg.Scanner.ClamAV.Socket).Ping()
+			case "icap":
+				return checks.NewTCPCheck(cfg.Scanner.ICAP.URL)(ctx)
+			}
+		})
 
 	return debug.NewService(
 		debug.Logger(options.Logger),
@@ -47,7 +41,6 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(healthHandler),
-		debug.Ready(readyHandler),
+		debug.Ready(handlers.NewCheckHandler(readyHandlerConfiguration)),
 	), nil
 }
diff --git a/services/app-provider/pkg/server/debug/server.go b/services/app-provider/pkg/server/debug/server.go
@@ -3,7 +3,6 @@ package debug
 import (
 	"net/http"
 
-	"github.com/owncloud/ocis/v2/ocis-pkg/handlers"
 	"github.com/owncloud/ocis/v2/ocis-pkg/service/debug"
 	"github.com/owncloud/ocis/v2/ocis-pkg/version"
 )
@@ -12,11 +11,6 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	checkHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger),
-	)
-
 	return debug.NewService(
 		debug.Logger(options.Logger),
 		debug.Name(options.Config.Service.Name),
@@ -25,8 +19,6 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(checkHandler),
-		debug.Ready(checkHandler),
 		//debug.CorsAllowedOrigins(options.Config.HTTP.CORS.AllowedOrigins),
 		//debug.CorsAllowedMethods(options.Config.HTTP.CORS.AllowedMethods),
 		//debug.CorsAllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),

diff --git a/services/app-registry/pkg/server/debug/server.go b/services/app-registry/pkg/server/debug/server.go
@@ -3,7 +3,6 @@ package debug
 import (
 	"net/http"
 
-	"github.com/owncloud/ocis/v2/ocis-pkg/handlers"
 	"github.com/owncloud/ocis/v2/ocis-pkg/service/debug"
 	"github.com/owncloud/ocis/v2/ocis-pkg/version"
 )
@@ -12,11 +11,6 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	checkHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger),
-	)
-
 	return debug.NewService(
 		debug.Logger(options.Logger),
 		debug.Name(options.Config.Service.Name),
@@ -25,8 +19,6 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(checkHandler),
-		debug.Ready(checkHandler),
 		//debug.CorsAllowedOrigins(options.Config.HTTP.CORS.AllowedOrigins),
 		//debug.CorsAllowedMethods(options.Config.HTTP.CORS.AllowedMethods),
 		//debug.CorsAllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),

diff --git a/services/audit/pkg/server/debug/server.go b/services/audit/pkg/server/debug/server.go
@@ -13,17 +13,9 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	checkHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger),
-	)
-
-	readyHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger).
-			WithCheck("nats reachability", checks.NewNatsCheck(options.Config.Events.Cluster)).
-			WithChecks(checkHandler.Checks()),
-	)
+	readyHandlerConfiguration := handlers.NewCheckHandlerConfiguration().
+		WithLogger(options.Logger).
+		WithCheck("nats reachability", checks.NewNatsCheck(options.Config.Events.Cluster))
 
 	return debug.NewService(
 		debug.Logger(options.Logger),
@@ -33,7 +25,6 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(checkHandler),
-		debug.Ready(readyHandler),
+		debug.Ready(handlers.NewCheckHandler(readyHandlerConfiguration)),
 	), nil
 }
diff --git a/services/auth-app/pkg/server/debug/server.go b/services/auth-app/pkg/server/debug/server.go
@@ -3,7 +3,6 @@ package debug
 import (
 	"net/http"
 
-	"github.com/owncloud/ocis/v2/ocis-pkg/handlers"
 	"github.com/owncloud/ocis/v2/ocis-pkg/service/debug"
 	"github.com/owncloud/ocis/v2/ocis-pkg/version"
 )
@@ -12,11 +11,6 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	checkHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger),
-	)
-
 	return debug.NewService(
 		debug.Logger(options.Logger),
 		debug.Name(options.Config.Service.Name),
@@ -25,8 +19,6 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(checkHandler),
-		debug.Ready(checkHandler),
 		//debug.CorsAllowedOrigins(options.Config.HTTP.CORS.AllowedOrigins),
 		//debug.CorsAllowedMethods(options.Config.HTTP.CORS.AllowedMethods),
 		//debug.CorsAllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),

diff --git a/services/auth-basic/pkg/server/debug/server.go b/services/auth-basic/pkg/server/debug/server.go
@@ -3,7 +3,6 @@ package debug
 import (
 	"net/http"
 
-	"github.com/owncloud/ocis/v2/ocis-pkg/handlers"
 	"github.com/owncloud/ocis/v2/ocis-pkg/service/debug"
 	"github.com/owncloud/ocis/v2/ocis-pkg/version"
 )
@@ -12,11 +11,6 @@ import (
 func Server(opts ...Option) (*http.Server, error) {
 	options := newOptions(opts...)
 
-	checkHandler := handlers.NewCheckHandler(
-		handlers.NewCheckHandlerConfiguration().
-			WithLogger(options.Logger),
-	)
-
 	return debug.NewService(
 		debug.Logger(options.Logger),
 		debug.Name(options.Config.Service.Name),
@@ -25,8 +19,6 @@ func Server(opts ...Option) (*http.Server, error) {
 		debug.Token(options.Config.Debug.Token),
 		debug.Pprof(options.Config.Debug.Pprof),
 		debug.Zpages(options.Config.Debug.Zpages),
-		debug.Health(checkHandler),
-		debug.Ready(checkHandler),
 		//debug.CorsAllowedOrigins(options.Config.HTTP.CORS.AllowedOrigins),
 		//debug.CorsAllowedMethods(options.Config.HTTP.CORS.AllowedMethods),
 		//debug.CorsAllowedHeaders(options.Config.HTTP.CORS.AllowedHeaders),