From c913b018d4d36fa3a7ca91839e692e07fff7b611 Mon Sep 17 00:00:00 2001 From: Craig Peterson <192540+captncraig@users.noreply.github.com> Date: Fri, 18 Aug 2023 11:21:21 -0600 Subject: [PATCH] prometheus.operator.*, simplify cluster update logic. (#4852) * prometheus.operator.* simplify logic for cluster updates * remove uneeded extra variable --- .../prometheus/operator/common/component.go | 38 ++++++------------- component/prometheus/operator/types.go | 5 --- component/prometheus/operator/types_test.go | 17 --------- 3 files changed, 12 insertions(+), 48 deletions(-) diff --git a/component/prometheus/operator/common/component.go b/component/prometheus/operator/common/component.go index d174b01aa8a3..c258c519a604 100644 --- a/component/prometheus/operator/common/component.go +++ b/component/prometheus/operator/common/component.go @@ -51,7 +51,6 @@ func (c *Component) Run(ctx context.Context) error { } }() - var runningConfig *operator.Arguments c.reportHealth(nil) errChan := make(chan error, 1) for { @@ -64,28 +63,19 @@ func (c *Component) Run(ctx context.Context) error { case err := <-errChan: c.reportHealth(err) case <-c.onUpdate: - c.mut.Lock() - nextConfig := c.config - // only restart crd manager if our config has changed. - // NOT on cluster changes. - if !nextConfig.Equals(runningConfig) { - runningConfig = nextConfig - manager := newCrdManager(c.opts, c.opts.Logger, nextConfig, c.kind) - c.manager = manager - if cancel != nil { - cancel() - } - innerCtx, cancel = context.WithCancel(ctx) - go func() { - if err := manager.Run(innerCtx); err != nil { - level.Error(c.opts.Logger).Log("msg", "error running crd manager", "err", err) - errChan <- err - } - }() - } else { - c.manager.ClusteringUpdated() + manager := newCrdManager(c.opts, c.opts.Logger, c.config, c.kind) + c.manager = manager + if cancel != nil { + cancel() } + innerCtx, cancel = context.WithCancel(ctx) + go func() { + if err := manager.Run(innerCtx); err != nil { + level.Error(c.opts.Logger).Log("msg", "error running crd manager", "err", err) + errChan <- err + } + }() c.mut.Unlock() } } @@ -115,11 +105,7 @@ func (c *Component) NotifyClusterChange() { return // no-op } - // Schedule a reload so targets get redistributed. - select { - case c.onUpdate <- struct{}{}: - default: - } + c.manager.ClusteringUpdated() } // DebugInfo returns debug information for this component. diff --git a/component/prometheus/operator/types.go b/component/prometheus/operator/types.go index e1d652c92fb4..441c77e3e4eb 100644 --- a/component/prometheus/operator/types.go +++ b/component/prometheus/operator/types.go @@ -1,7 +1,6 @@ package operator import ( - "reflect" "time" "github.com/grafana/agent/component/common/config" @@ -30,10 +29,6 @@ type Arguments struct { RelabelConfigs []*flow_relabel.Config `river:"rule,block,optional"` } -func (a *Arguments) Equals(b *Arguments) bool { - return reflect.DeepEqual(a, b) -} - // Clustering holds values that configure clustering-specific behavior. type Clustering struct { // TODO(@tpaschalis) Move this block to a shared place for all components using clustering. diff --git a/component/prometheus/operator/types_test.go b/component/prometheus/operator/types_test.go index 202a6fe9faef..5dc7f934bf05 100644 --- a/component/prometheus/operator/types_test.go +++ b/component/prometheus/operator/types_test.go @@ -27,20 +27,3 @@ func TestRiverUnmarshal(t *testing.T) { err := river.Unmarshal([]byte(exampleRiverConfig), &args) require.NoError(t, err) } - -func TestEqual(t *testing.T) { - a := Arguments{ - Namespaces: []string{"my-app"}, - Clustering: Clustering{Enabled: true}, - } - b := Arguments{ - Namespaces: []string{"my-app"}, - Clustering: Clustering{Enabled: true}, - } - c := Arguments{ - Namespaces: []string{"my-app", "other-app"}, - Clustering: Clustering{Enabled: false}, - } - require.True(t, a.Equals(&b)) - require.False(t, a.Equals(&c)) -}