From ba6914170c7c707c108b977c97c19e0a9b52f62e Mon Sep 17 00:00:00 2001 From: adel121 Date: Mon, 9 Dec 2024 11:13:32 +0100 Subject: [PATCH] use a hybrid health check for wlm kubeapiserver collector --- .../core/healthprobe/impl/healthprobe_test.go | 3 ++- .../internal/kubeapiserver/kubeapiserver.go | 2 +- pkg/status/health/global.go | 14 ++++++----- pkg/status/health/health.go | 23 ++++++++----------- pkg/status/health/health_test.go | 6 ++--- pkg/status/health/options.go | 14 +++++++++++ pkg/util/containers/filter.go | 20 ++++++++++++++++ 7 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 pkg/status/health/options.go diff --git a/comp/core/healthprobe/impl/healthprobe_test.go b/comp/core/healthprobe/impl/healthprobe_test.go index 0dc9b5d271bada..7774e420437654 100644 --- a/comp/core/healthprobe/impl/healthprobe_test.go +++ b/comp/core/healthprobe/impl/healthprobe_test.go @@ -13,11 +13,12 @@ import ( "net/http/httptest" "testing" + "github.com/stretchr/testify/assert" + healthprobeComponent "github.com/DataDog/datadog-agent/comp/core/healthprobe/def" logmock "github.com/DataDog/datadog-agent/comp/core/log/mock" compdef "github.com/DataDog/datadog-agent/comp/def" "github.com/DataDog/datadog-agent/pkg/status/health" - "github.com/stretchr/testify/assert" ) func TestServer(t *testing.T) { diff --git a/comp/core/workloadmeta/collectors/internal/kubeapiserver/kubeapiserver.go b/comp/core/workloadmeta/collectors/internal/kubeapiserver/kubeapiserver.go index 0666db1755b75e..7def82fc21d9bc 100644 --- a/comp/core/workloadmeta/collectors/internal/kubeapiserver/kubeapiserver.go +++ b/comp/core/workloadmeta/collectors/internal/kubeapiserver/kubeapiserver.go @@ -224,7 +224,7 @@ func runStartupCheck(ctx context.Context, stores []*reflectorStore) { // There is no way to ensure liveness correctly as it would need to be plugged inside the // inner loop of Reflector. // However, we add Startup when we got at least some data. - startupHealthCheck := health.RegisterStartup(componentName) + startupHealthCheck := health.RegisterReadiness(componentName, health.Once) // Checked synced, in its own scope to cleanly un-reference the syncTimer { diff --git a/pkg/status/health/global.go b/pkg/status/health/global.go index 32af425cbe111e..25dccf8d14c197 100644 --- a/pkg/status/health/global.go +++ b/pkg/status/health/global.go @@ -12,21 +12,23 @@ import ( var readinessAndLivenessCatalog = newCatalog() var readinessOnlyCatalog = newCatalog() -var startupOnlyCatalog = newStartupCatalog() +var startupOnlyCatalog = newCatalog() // RegisterReadiness registers a component for readiness check with the default 30 seconds timeout, returns a token -func RegisterReadiness(name string) *Handle { - return readinessOnlyCatalog.register(name) +func RegisterReadiness(name string, options ...Option) *Handle { + return readinessOnlyCatalog.register(name, options...) } // RegisterLiveness registers a component for liveness check with the default 30 seconds timeout, returns a token -func RegisterLiveness(name string) *Handle { - return readinessAndLivenessCatalog.register(name) +func RegisterLiveness(name string, options ...Option) *Handle { + return readinessAndLivenessCatalog.register(name, options...) } // RegisterStartup registers a component for startup check, returns a token func RegisterStartup(name string) *Handle { - return startupOnlyCatalog.register(name) + // Startup health checks are registered with Once option because, by design, they should stop being checked + // once they are marked as healthy once + return startupOnlyCatalog.register(name, Once) } // Deregister a component from the healthcheck diff --git a/pkg/status/health/health.go b/pkg/status/health/health.go index 7e3e3327bb794b..0b1a0e8a69c966 100644 --- a/pkg/status/health/health.go +++ b/pkg/status/health/health.go @@ -29,33 +29,25 @@ type component struct { name string healthChan chan time.Time healthy bool + // if set to true, once the check is healthy, we mark it as healthy forever and we stop checking it + once bool } type catalog struct { sync.RWMutex components map[*Handle]*component latestRun time.Time - startup bool } func newCatalog() *catalog { return &catalog{ components: make(map[*Handle]*component), latestRun: time.Now(), // Start healthy - startup: false, - } -} - -func newStartupCatalog() *catalog { - return &catalog{ - components: make(map[*Handle]*component), - latestRun: time.Now(), // Start healthy - startup: true, } } // register a component with the default 30 seconds timeout, returns a token -func (c *catalog) register(name string) *Handle { +func (c *catalog) register(name string, options ...Option) *Handle { c.Lock() defer c.Unlock() @@ -68,6 +60,11 @@ func (c *catalog) register(name string) *Handle { healthChan: make(chan time.Time, bufferSize), healthy: false, } + + for _, option := range options { + option(component) + } + h := &Handle{ C: component.healthChan, } @@ -107,8 +104,8 @@ func (c *catalog) pingComponents(healthDeadline time.Time) bool { c.Lock() defer c.Unlock() for _, component := range c.components { - // In startup mode, we skip already healthy components. - if c.startup && component.healthy { + // We skip components that are registered to be skipped once they pass once + if component.healthy && component.once { continue } select { diff --git a/pkg/status/health/health_test.go b/pkg/status/health/health_test.go index f1d934fb157e5b..37a57773a1d738 100644 --- a/pkg/status/health/health_test.go +++ b/pkg/status/health/health_test.go @@ -124,9 +124,9 @@ func TestGetHealthy(t *testing.T) { assert.Len(t, status.Unhealthy, 0) } -func TestStartupCatalog(t *testing.T) { - cat := newStartupCatalog() - token := cat.register("test1") +func TestCatalogWithOnceComponent(t *testing.T) { + cat := newCatalog() + token := cat.register("test1", Once) // Start unhealthy status := cat.getStatus() diff --git a/pkg/status/health/options.go b/pkg/status/health/options.go new file mode 100644 index 00000000000000..47d508eab00709 --- /dev/null +++ b/pkg/status/health/options.go @@ -0,0 +1,14 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2016-present Datadog, Inc. + +// Package health implements the internal healthcheck +package health + +type Option func(*component) + +// Once has the effect of not checking the health of a component once it has been marked healthy once +func Once(c *component) { + c.once = true +} diff --git a/pkg/util/containers/filter.go b/pkg/util/containers/filter.go index 1e2ab52934e776..f72cf25ddb1165 100644 --- a/pkg/util/containers/filter.go +++ b/pkg/util/containers/filter.go @@ -107,6 +107,26 @@ type Filter struct { var sharedFilter *Filter +/* + +^p1/p2/p3/p4/.../pn:tag@hash$ + +name@sha:.... +name:tag + + +Case 1: pattern already includes a colon +^name:tag$ --> doesn't work for image.Name, but works for image.RawName +^name:tag@sha:hash --> works for image.Name and image.RawName (should be kept as it is) + +Case 2: pattern contains no colons +Means that only image name is specified (no tag, no digest) +^name --> works for image.Name and image.RawName (should be kept as it is) +^name$ --> works for image.Name, but not for image.RawName (should be altered to ^name(:|$) ) + + +*/ + func parseFilters(filters []string) (imageFilters, nameFilters, namespaceFilters []*regexp.Regexp, filterErrs []string, err error) { var filterWarnings []string for _, filter := range filters {