From d9ce4c561e755c6488d84a316dc7fabf4f048852 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 12:26:10 +0300 Subject: [PATCH 01/25] refactor: replace map with sync.Map for VaryingInformers in monitor Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor.go | 83 ++++++++++++++++--------- pkg/kube_events_manager/monitor_test.go | 24 ++++--- 2 files changed, 69 insertions(+), 38 deletions(-) diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 27aaa469..134c7f87 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "sort" + "sync" "github.com/deckhouse/deckhouse/pkg/log" @@ -35,7 +36,7 @@ type monitor struct { // Namespace informer to get new namespaces NamespaceInformer *namespaceInformer // map of dynamically starting informers - VaryingInformers map[string][]*resourceInformer + VaryingInformers sync.Map eventCb func(kemtypes.KubeEvent) eventsEnabled bool @@ -62,7 +63,7 @@ func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage Config: config, eventCb: eventCb, ResourceInformers: make([]*resourceInformer, 0), - VaryingInformers: make(map[string][]*resourceInformer), + VaryingInformers: sync.Map{}, cancelForNs: make(map[string]context.CancelFunc), staticNamespaces: make(map[string]bool), logger: logger, @@ -119,25 +120,25 @@ func (m *monitor) CreateInformers() error { return } // ignore already started informers - _, ok := m.VaryingInformers[nsName] + _, ok := m.VaryingInformers.Load(nsName) if ok { return } logEntry.Info("got ns, create dynamic ResourceInformers", slog.String("name", nsName)) - var err error - m.VaryingInformers[nsName], err = m.CreateInformersForNamespace(nsName) + varyingInformers, err := m.CreateInformersForNamespace(nsName) if err != nil { logEntry.Error("create ResourceInformers for ns", slog.String("name", nsName), log.Err(err)) } + m.VaryingInformers.Store(nsName, varyingInformers) var ctx context.Context ctx, m.cancelForNs[nsName] = context.WithCancel(m.ctx) - for _, informer := range m.VaryingInformers[nsName] { + for _, informer := range varyingInformers { informer.withContext(ctx) if m.eventsEnabled { informer.enableKubeEventCb() @@ -164,7 +165,7 @@ func (m *monitor) CreateInformers() error { // TODO wait - delete(m.VaryingInformers, nsName) + m.VaryingInformers.Delete(nsName) delete(m.cancelForNs, nsName) }, ) @@ -179,13 +180,13 @@ func (m *monitor) CreateInformers() error { continue } - var err error - m.VaryingInformers[nsName], err = m.CreateInformersForNamespace(nsName) + varyingInformers, err := m.CreateInformersForNamespace(nsName) if err != nil { logEntry.Error("create ResourceInformers for ns", slog.String("name", nsName), log.Err(err)) } + m.VaryingInformers.Store(nsName, varyingInformers) } } @@ -200,11 +201,14 @@ func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { objects = append(objects, informer.getCachedObjects()...) } - for nsName := range m.VaryingInformers { - for _, informer := range m.VaryingInformers[nsName] { - objects = append(objects, informer.getCachedObjects()...) + m.VaryingInformers.Range(func(key, value any) bool { + if value, ok := value.([]*resourceInformer); ok { + for _, informer := range value { + objects = append(objects, informer.getCachedObjects()...) + } } - } + return true + }) // Sort objects by namespace and name sort.Sort(kemtypes.ByNamespaceAndName(objects)) @@ -218,11 +222,15 @@ func (m *monitor) EnableKubeEventCb() { for _, informer := range m.ResourceInformers { informer.enableKubeEventCb() } - for nsName := range m.VaryingInformers { - for _, informer := range m.VaryingInformers[nsName] { - informer.enableKubeEventCb() + // Execute eventCb for events accumulated during "Synchronization" phase. + m.VaryingInformers.Range(func(key, value any) bool { + if value, ok := value.([]*resourceInformer); ok { + for _, informer := range value { + informer.enableKubeEventCb() + } } - } + return true + }) // Enable events for future VaryingInformers. m.eventsEnabled = true } @@ -269,14 +277,21 @@ func (m *monitor) Start(parentCtx context.Context) { informer.start() } - for nsName := range m.VaryingInformers { + m.VaryingInformers.Range(func(key, value any) bool { + nsName, ok := key.(string) + if !ok { + return false + } var ctx context.Context ctx, m.cancelForNs[nsName] = context.WithCancel(m.ctx) - for _, informer := range m.VaryingInformers[nsName] { - informer.withContext(ctx) - informer.start() + if value, ok := value.([]*resourceInformer); ok { + for _, informer := range value { + informer.withContext(ctx) + informer.start() + } } - } + return true + }) if m.NamespaceInformer != nil { m.NamespaceInformer.withContext(m.ctx) @@ -299,11 +314,14 @@ func (m *monitor) PauseHandleEvents() { informer.pauseHandleEvents() } - for _, informers := range m.VaryingInformers { - for _, informer := range informers { - informer.pauseHandleEvents() + m.VaryingInformers.Range(func(key, value any) bool { + if value, ok := value.([]*resourceInformer); ok { + for _, informer := range value { + informer.pauseHandleEvents() + } } - } + return true + }) if m.NamespaceInformer != nil { m.NamespaceInformer.pauseHandleEvents() @@ -319,12 +337,15 @@ func (m *monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObj last.add(informer.getCachedObjectsInfoIncrement()) } - for nsName := range m.VaryingInformers { - for _, informer := range m.VaryingInformers[nsName] { - total.add(informer.getCachedObjectsInfo()) - last.add(informer.getCachedObjectsInfoIncrement()) + m.VaryingInformers.Range(func(key, value any) bool { + if value, ok := value.([]*resourceInformer); ok { + for _, informer := range value { + total.add(informer.getCachedObjectsInfo()) + last.add(informer.getCachedObjectsInfoIncrement()) + } } - } + return true + }) return total, last } diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index 4cbba90c..56d5242e 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -5,12 +5,13 @@ import ( "fmt" "testing" - "github.com/deckhouse/deckhouse/pkg/log" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/flant/kube-client/fake" "github.com/flant/kube-client/manifest" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" @@ -72,8 +73,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { createNsWithLabels(fc, "test-ns-1", map[string]string{"test-label": ""}) // Wait until informers appears. - g.Eventually(mon.VaryingInformers, "5s", "10ms"). - Should(HaveKey("test-ns-1"), "Should create informer for new namespace") + g.Eventually(func() bool { + _, exists := mon.VaryingInformers.Load("test-ns-1") + return exists + }, "5s", "10ms"). + Should(BeTrue(), "Should create informer for new namespace") createCM(fc, "test-ns-1", testCM("cm-1")) @@ -110,8 +114,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { createNsWithLabels(fc, "test-ns-2", map[string]string{"test-label": ""}) // Monitor should create new configmap informer for new namespace. - g.Eventually(mon.VaryingInformers, "5s", "10ms"). - Should(HaveKey("test-ns-2"), "Should create informer for ns/test-ns-2") + g.Eventually(func() bool { + _, exists := mon.VaryingInformers.Load("test-ns-2") + return exists + }, "5s", "10ms"). + Should(BeTrue(), "Should create informer for ns/test-ns-2") // Create new ConfigMap after Synchronization. createCM(fc, "test-ns-2", testCM("cm-2-1")) @@ -129,8 +136,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { createNsWithLabels(fc, "test-ns-non-matched", map[string]string{"non-matched-label": ""}) // Monitor should create new configmap informer for new namespace. - g.Eventually(mon.VaryingInformers, "5s", "10ms"). - ShouldNot(HaveKey("test-ns-non-matched"), "Should not create informer for non-mathed Namespace") + g.Eventually(func() bool { + _, exists := mon.VaryingInformers.Load("test-ns-non-matched") + return exists + }, "5s", "10ms"). + ShouldNot(BeTrue(), "Should not create informer for non-mathed Namespace") } func createNsWithLabels(fc *fake.Cluster, name string, labels map[string]string) { From 8b8fb60aff87d1dd13fcb2be0e11e881f8c531e3 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 13:03:32 +0300 Subject: [PATCH 02/25] fix: correct locking mechanism in BindingContextController to prevent double start Signed-off-by: Evsyukov Denis --- test/hook/context/generator.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 3dbe9314..d34f4a52 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -66,8 +66,6 @@ func NewBindingContextController(config string, logger *log.Logger, version ...f b.KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(ctx, b.fakeCluster.Client, b.logger.Named("kube-events-manager")) b.KubeEventsManager.WithMetricStorage(metricstorage.NewMetricStorage(ctx, "metrics-prefix", false, log.NewNop())) - // Re-create factory to drop informers created using different b.fakeCluster.Client. - kubeeventsmanager.DefaultFactoryStore = kubeeventsmanager.NewFactoryStore() b.ScheduleManager = schedulemanager.NewScheduleManager(ctx, b.logger.Named("schedule-manager")) @@ -93,11 +91,12 @@ func (b *BindingContextController) RegisterCRD(group, version, kind string, name // Run generates binding contexts for hook tests func (b *BindingContextController) Run(initialState string) (GeneratedBindingContexts, error) { b.mu.Lock() - defer b.mu.Unlock() - if b.started { - return GeneratedBindingContexts{}, fmt.Errorf("attempt to runner started runner, it cannot be started twice") + b.mu.Unlock() + return GeneratedBindingContexts{}, fmt.Errorf("attempt to start an already started runner, it cannot be started twice") } + b.started = true + b.mu.Unlock() err := b.Controller.SetInitialState(initialState) if err != nil { @@ -131,7 +130,6 @@ func (b *BindingContextController) Run(initialState string) (GeneratedBindingCon } b.HookCtrl.UnlockKubernetesEvents() - b.started = true time.Sleep(50 * time.Millisecond) return cc.CombinedAndUpdated(b.HookCtrl) From 5e9bf838fe3b16dd735713900776f5e309393fae Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 13:07:52 +0300 Subject: [PATCH 03/25] fix: add mutex for thread-safe access to objsFromEvents in monitor tests Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor_test.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index 56d5242e..e5d84747 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -3,6 +3,7 @@ package kubeeventsmanager import ( "context" "fmt" + "sync" "testing" . "github.com/onsi/gomega" @@ -39,6 +40,7 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { }, } objsFromEvents := make([]string, 0) + var objsMutex sync.Mutex metricStorage := metric.NewStorageMock(t) metricStorage.HistogramObserveMock.Set(func(metric string, value float64, labels map[string]string, buckets []float64) { @@ -56,7 +58,9 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { metricStorage.GaugeSetMock.When("{PREFIX}kube_snapshot_objects", 3, map[string]string(nil)).Then() mon := NewMonitor(context.Background(), fc.Client, metricStorage, monitorCfg, func(ev kemtypes.KubeEvent) { + objsMutex.Lock() objsFromEvents = append(objsFromEvents, snapshotResourceIDs(ev.Objects)...) + objsMutex.Unlock() }, log.NewNop()) // Start monitor. @@ -96,7 +100,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { mon.EnableKubeEventCb() // Should catch 2 events for cm-2 and cm-3. - g.Eventually(func() []string { return objsFromEvents }, "6s", "10ms"). + g.Eventually(func() []string { + objsMutex.Lock() + defer objsMutex.Unlock() + return objsFromEvents + }, "6s", "10ms"). Should(SatisfyAll( ContainElement("test-ns-1/ConfigMap/cm-2"), ContainElement("test-ns-1/ConfigMap/cm-3"), @@ -129,8 +137,12 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { }, "5s", "10ms").Should(ContainElement("test-ns-2/ConfigMap/cm-2-1"), "Should update snapshot on new ConfigMap after Synchronization") // Should catch event for cm-2-1. - g.Eventually(func() []string { return objsFromEvents }, "5s", "10ms"). - Should(ContainElement("test-ns-2/ConfigMap/cm-2-1"), "Should fire KubeEvent for new ConfigMap after Synchronization", objsFromEvents) + g.Eventually(func() []string { + objsMutex.Lock() + defer objsMutex.Unlock() + return objsFromEvents + }, "5s", "10ms"). + Should(ContainElement("test-ns-2/ConfigMap/cm-2-1"), "Should fire KubeEvent for new ConfigMap after Synchronization") // Add non-matched Namespace. createNsWithLabels(fc, "test-ns-non-matched", map[string]string{"non-matched-label": ""}) From 0873339572f7e2a24cf6c80cbb45f104ff517f77 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 13:17:54 +0300 Subject: [PATCH 04/25] refactor: simplify Range function parameters in VaryingInformers Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 134c7f87..8534b8a5 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -201,7 +201,7 @@ func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { objects = append(objects, informer.getCachedObjects()...) } - m.VaryingInformers.Range(func(key, value any) bool { + m.VaryingInformers.Range(func(_, value any) bool { if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { objects = append(objects, informer.getCachedObjects()...) @@ -223,7 +223,7 @@ func (m *monitor) EnableKubeEventCb() { informer.enableKubeEventCb() } // Execute eventCb for events accumulated during "Synchronization" phase. - m.VaryingInformers.Range(func(key, value any) bool { + m.VaryingInformers.Range(func(_, value any) bool { if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { informer.enableKubeEventCb() @@ -280,7 +280,7 @@ func (m *monitor) Start(parentCtx context.Context) { m.VaryingInformers.Range(func(key, value any) bool { nsName, ok := key.(string) if !ok { - return false + return true } var ctx context.Context ctx, m.cancelForNs[nsName] = context.WithCancel(m.ctx) @@ -314,7 +314,7 @@ func (m *monitor) PauseHandleEvents() { informer.pauseHandleEvents() } - m.VaryingInformers.Range(func(key, value any) bool { + m.VaryingInformers.Range(func(_, value any) bool { if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { informer.pauseHandleEvents() @@ -337,7 +337,7 @@ func (m *monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObj last.add(informer.getCachedObjectsInfoIncrement()) } - m.VaryingInformers.Range(func(key, value any) bool { + m.VaryingInformers.Range(func(_, value any) bool { if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { total.add(informer.getCachedObjectsInfo()) From 163443fd8f545fc18f4a2420f221f58c00b4517e Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 13:18:18 +0300 Subject: [PATCH 05/25] refactor: remove duplicate log import in monitor_test.go Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index e5d84747..fd6f873d 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -6,13 +6,12 @@ import ( "sync" "testing" + "github.com/deckhouse/deckhouse/pkg/log" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/deckhouse/deckhouse/pkg/log" - "github.com/flant/kube-client/fake" "github.com/flant/kube-client/manifest" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" From 78f56f45ada8e4ef6589f85861cfb3065ac9c305 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 16:47:23 +0300 Subject: [PATCH 06/25] refactor: change return type of NewKubeEventsManager and NewMonitor to interfaces, replace maps with sync.Map for concurrency safety Signed-off-by: Evsyukov Denis --- .../kube_events_manager.go | 2 +- pkg/kube_events_manager/monitor.go | 39 ++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/pkg/kube_events_manager/kube_events_manager.go b/pkg/kube_events_manager/kube_events_manager.go index 14a381c2..00a8e10d 100644 --- a/pkg/kube_events_manager/kube_events_manager.go +++ b/pkg/kube_events_manager/kube_events_manager.go @@ -47,7 +47,7 @@ type kubeEventsManager struct { var _ KubeEventsManager = (*kubeEventsManager)(nil) // NewKubeEventsManager returns an implementation of KubeEventsManager. -func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) *kubeEventsManager { +func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) KubeEventsManager { cctx, cancel := context.WithCancel(ctx) em := &kubeEventsManager{ ctx: cctx, diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 8534b8a5..7afad00a 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -41,9 +41,9 @@ type monitor struct { eventCb func(kemtypes.KubeEvent) eventsEnabled bool // Index of namespaces statically defined in monitor configuration - staticNamespaces map[string]bool + staticNamespaces sync.Map - cancelForNs map[string]context.CancelFunc + cancelForNs sync.Map ctx context.Context cancel context.CancelFunc @@ -52,7 +52,7 @@ type monitor struct { logger *log.Logger } -func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) *monitor { +func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) Monitor { cctx, cancel := context.WithCancel(ctx) return &monitor{ @@ -64,8 +64,8 @@ func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage eventCb: eventCb, ResourceInformers: make([]*resourceInformer, 0), VaryingInformers: sync.Map{}, - cancelForNs: make(map[string]context.CancelFunc), - staticNamespaces: make(map[string]bool), + cancelForNs: sync.Map{}, + staticNamespaces: sync.Map{}, logger: logger, } } @@ -99,7 +99,7 @@ func (m *monitor) CreateInformers() error { // This list of informers is static. for _, nsName := range nsNames { if nsName != "" { - m.staticNamespaces[nsName] = true + m.staticNamespaces.Store(nsName, true) } informers, err := m.CreateInformersForNamespace(nsName) if err != nil { @@ -116,12 +116,11 @@ func (m *monitor) CreateInformers() error { func(nsName string) { // Added/Modified event: check, create and run informers for Ns // ignore event if namespace is already has static ResourceInformers - if _, ok := m.staticNamespaces[nsName]; ok { + if _, ok := m.staticNamespaces.Load(nsName); ok { return } // ignore already started informers - _, ok := m.VaryingInformers.Load(nsName) - if ok { + if _, ok := m.VaryingInformers.Load(nsName); ok { return } @@ -135,8 +134,8 @@ func (m *monitor) CreateInformers() error { } m.VaryingInformers.Store(nsName, varyingInformers) - var ctx context.Context - ctx, m.cancelForNs[nsName] = context.WithCancel(m.ctx) + ctx, cancelForNs := context.WithCancel(m.ctx) + m.cancelForNs.Store(nsName, cancelForNs) for _, informer := range varyingInformers { informer.withContext(ctx) @@ -151,22 +150,24 @@ func (m *monitor) CreateInformers() error { logEntry.Info("deleted ns, stop dynamic ResourceInformers", slog.String("name", nsName)) // ignore statically specified namespaces - if _, ok := m.staticNamespaces[nsName]; ok { + if _, ok := m.staticNamespaces.Load(nsName); ok { return } // ignore already stopped informers - _, ok := m.cancelForNs[nsName] - if !ok { + if _, ok := m.cancelForNs.Load(nsName); !ok { return } - m.cancelForNs[nsName]() + fn, _ := m.cancelForNs.Load(nsName) + if fn, ok := fn.(context.CancelFunc); ok { + fn() + } // TODO wait m.VaryingInformers.Delete(nsName) - delete(m.cancelForNs, nsName) + m.cancelForNs.Delete(nsName) }, ) if err != nil { @@ -176,7 +177,7 @@ func (m *monitor) CreateInformers() error { logEntry.Info("got ns, create dynamic ResourceInformers", slog.String("name", nsName)) // ignore event if namespace is already has static ResourceInformers - if _, ok := m.staticNamespaces[nsName]; ok { + if _, ok := m.staticNamespaces.Load(nsName); ok { continue } @@ -282,8 +283,8 @@ func (m *monitor) Start(parentCtx context.Context) { if !ok { return true } - var ctx context.Context - ctx, m.cancelForNs[nsName] = context.WithCancel(m.ctx) + ctx, cancelForNs := context.WithCancel(m.ctx) + m.cancelForNs.Store(nsName, cancelForNs) if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { informer.withContext(ctx) From 7531e0b4b935560dc5830b92e2870ead33d40004 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 17:03:10 +0300 Subject: [PATCH 07/25] refactor: change KubeEventsManager fields to pointers for consistency and improved memory management Signed-off-by: Evsyukov Denis --- pkg/hook/controller/hook_controller.go | 6 +- .../kubernetes_bindings_controller.go | 69 +++++++------------ .../schedule_bindings_controller.go | 35 +++------- pkg/hook/hook_manager.go | 4 +- .../kube_events_manager.go | 43 ++++-------- pkg/kube_events_manager/monitor.go | 35 ++++------ pkg/shell-operator/manager_events_handler.go | 4 +- pkg/shell-operator/operator.go | 2 +- test/hook/context/generator.go | 2 +- test/hook/context/state.go | 2 +- .../kube_event_manager_test.go | 2 +- 11 files changed, 71 insertions(+), 133 deletions(-) diff --git a/pkg/hook/controller/hook_controller.go b/pkg/hook/controller/hook_controller.go index b332ccbe..be07d886 100644 --- a/pkg/hook/controller/hook_controller.go +++ b/pkg/hook/controller/hook_controller.go @@ -40,8 +40,8 @@ func NewHookController() *HookController { } type HookController struct { - KubernetesController KubernetesBindingsController - ScheduleController ScheduleBindingsController + KubernetesController *KubernetesBindingsController + ScheduleController *ScheduleBindingsController AdmissionController *AdmissionBindingsController ConversionController *ConversionBindingsController kubernetesBindings []htypes.OnKubernetesEventConfig @@ -53,7 +53,7 @@ type HookController struct { logger *log.Logger } -func (hc *HookController) InitKubernetesBindings(bindings []htypes.OnKubernetesEventConfig, kubeEventMgr kubeeventsmanager.KubeEventsManager, logger *log.Logger) { +func (hc *HookController) InitKubernetesBindings(bindings []htypes.OnKubernetesEventConfig, kubeEventMgr *kubeeventsmanager.KubeEventsManager, logger *log.Logger) { if len(bindings) == 0 { return } diff --git a/pkg/hook/controller/kubernetes_bindings_controller.go b/pkg/hook/controller/kubernetes_bindings_controller.go index 3eed31da..21329e89 100644 --- a/pkg/hook/controller/kubernetes_bindings_controller.go +++ b/pkg/hook/controller/kubernetes_bindings_controller.go @@ -20,33 +20,13 @@ type KubernetesBindingToMonitorLink struct { BindingConfig htypes.OnKubernetesEventConfig } -// KubernetesBindingsController handles kubernetes bindings for one hook. -type KubernetesBindingsController interface { - WithKubernetesBindings([]htypes.OnKubernetesEventConfig) - WithKubeEventsManager(kubeeventsmanager.KubeEventsManager) - EnableKubernetesBindings() ([]BindingExecutionInfo, error) - UpdateMonitor(monitorId string, kind, apiVersion string) error - UnlockEvents() - UnlockEventsFor(monitorID string) - StopMonitors() - CanHandleEvent(kubeEvent kemtypes.KubeEvent) bool - HandleEvent(kubeEvent kemtypes.KubeEvent) BindingExecutionInfo - BindingNames() []string - - SnapshotsFrom(bindingNames ...string) map[string][]kemtypes.ObjectAndFilterResult - SnapshotsFor(bindingName string) []kemtypes.ObjectAndFilterResult - Snapshots() map[string][]kemtypes.ObjectAndFilterResult - SnapshotsInfo() []string - SnapshotsDump() map[string]interface{} -} - -// kubernetesHooksController is a main implementation of KubernetesHooksController -type kubernetesBindingsController struct { +// KubernetesHooksController is a main implementation of KubernetesHooksController +type KubernetesBindingsController struct { // bindings configurations KubernetesBindings []htypes.OnKubernetesEventConfig // dependencies - kubeEventsManager kubeeventsmanager.KubeEventsManager + kubeEventsManager *kubeeventsmanager.KubeEventsManager logger *log.Logger @@ -55,29 +35,26 @@ type kubernetesBindingsController struct { BindingMonitorLinks map[string]*KubernetesBindingToMonitorLink } -// kubernetesHooksController should implement the KubernetesHooksController -var _ KubernetesBindingsController = &kubernetesBindingsController{} - // NewKubernetesBindingsController returns an implementation of KubernetesBindingsController -var NewKubernetesBindingsController = func(logger *log.Logger) *kubernetesBindingsController { - return &kubernetesBindingsController{ +var NewKubernetesBindingsController = func(logger *log.Logger) *KubernetesBindingsController { + return &KubernetesBindingsController{ BindingMonitorLinks: make(map[string]*KubernetesBindingToMonitorLink), logger: logger, } } -func (c *kubernetesBindingsController) WithKubernetesBindings(bindings []htypes.OnKubernetesEventConfig) { +func (c *KubernetesBindingsController) WithKubernetesBindings(bindings []htypes.OnKubernetesEventConfig) { c.KubernetesBindings = bindings } -func (c *kubernetesBindingsController) WithKubeEventsManager(kubeEventsManager kubeeventsmanager.KubeEventsManager) { +func (c *KubernetesBindingsController) WithKubeEventsManager(kubeEventsManager *kubeeventsmanager.KubeEventsManager) { c.kubeEventsManager = kubeEventsManager } // EnableKubernetesBindings adds a monitor for each 'kubernetes' binding. This method // returns an array of BindingExecutionInfo to help construct initial tasks to run hooks. // Informers in each monitor are started immediately to keep up the "fresh" state of object caches. -func (c *kubernetesBindingsController) EnableKubernetesBindings() ([]BindingExecutionInfo, error) { +func (c *KubernetesBindingsController) EnableKubernetesBindings() ([]BindingExecutionInfo, error) { res := make([]BindingExecutionInfo, 0) for _, config := range c.KubernetesBindings { @@ -102,7 +79,7 @@ func (c *kubernetesBindingsController) EnableKubernetesBindings() ([]BindingExec return res, nil } -func (c *kubernetesBindingsController) UpdateMonitor(monitorId string, kind, apiVersion string) error { +func (c *KubernetesBindingsController) UpdateMonitor(monitorId string, kind, apiVersion string) error { // Find binding for monitorId link, ok := c.getBindingMonitorLinksById(monitorId) if !ok { @@ -153,7 +130,7 @@ func (c *kubernetesBindingsController) UpdateMonitor(monitorId string, kind, api } // UnlockEvents turns on eventCb for all monitors to emit events after Synchronization. -func (c *kubernetesBindingsController) UnlockEvents() { +func (c *KubernetesBindingsController) UnlockEvents() { c.iterateBindingMonitorLinks(func(monitorID string) bool { m := c.kubeEventsManager.GetMonitor(monitorID) m.EnableKubeEventCb() @@ -162,7 +139,7 @@ func (c *kubernetesBindingsController) UnlockEvents() { } // UnlockEventsFor turns on eventCb for matched monitor to emit events after Synchronization. -func (c *kubernetesBindingsController) UnlockEventsFor(monitorID string) { +func (c *KubernetesBindingsController) UnlockEventsFor(monitorID string) { m := c.kubeEventsManager.GetMonitor(monitorID) if m == nil { log.Warn("monitor was not found", slog.String("monitorID", monitorID)) @@ -173,14 +150,14 @@ func (c *kubernetesBindingsController) UnlockEventsFor(monitorID string) { // StopMonitors stops all monitors for the hook. // TODO handle error! -func (c *kubernetesBindingsController) StopMonitors() { +func (c *KubernetesBindingsController) StopMonitors() { c.iterateBindingMonitorLinks(func(monitorID string) bool { _ = c.kubeEventsManager.StopMonitor(monitorID) return false }) } -func (c *kubernetesBindingsController) CanHandleEvent(kubeEvent kemtypes.KubeEvent) bool { +func (c *KubernetesBindingsController) CanHandleEvent(kubeEvent kemtypes.KubeEvent) bool { var canHandleEvent bool c.iterateBindingMonitorLinks(func(monitorID string) bool { @@ -194,7 +171,7 @@ func (c *kubernetesBindingsController) CanHandleEvent(kubeEvent kemtypes.KubeEve return canHandleEvent } -func (c *kubernetesBindingsController) iterateBindingMonitorLinks(doFn func(monitorID string) bool) { +func (c *KubernetesBindingsController) iterateBindingMonitorLinks(doFn func(monitorID string) bool) { c.l.RLock() for monitorID := range c.BindingMonitorLinks { if exit := doFn(monitorID); exit { @@ -204,14 +181,14 @@ func (c *kubernetesBindingsController) iterateBindingMonitorLinks(doFn func(moni c.l.RUnlock() } -func (c *kubernetesBindingsController) getBindingMonitorLinksById(monitorId string) (*KubernetesBindingToMonitorLink, bool) { +func (c *KubernetesBindingsController) getBindingMonitorLinksById(monitorId string) (*KubernetesBindingToMonitorLink, bool) { c.l.RLock() link, found := c.BindingMonitorLinks[monitorId] c.l.RUnlock() return link, found } -func (c *kubernetesBindingsController) setBindingMonitorLinks(monitorId string, link *KubernetesBindingToMonitorLink) { +func (c *KubernetesBindingsController) setBindingMonitorLinks(monitorId string, link *KubernetesBindingToMonitorLink) { c.l.Lock() c.BindingMonitorLinks[monitorId] = link c.l.Unlock() @@ -219,7 +196,7 @@ func (c *kubernetesBindingsController) setBindingMonitorLinks(monitorId string, // HandleEvent receives event from KubeEventManager and returns a BindingExecutionInfo // to help create a new task to run a hook. -func (c *kubernetesBindingsController) HandleEvent(kubeEvent kemtypes.KubeEvent) BindingExecutionInfo { +func (c *KubernetesBindingsController) HandleEvent(kubeEvent kemtypes.KubeEvent) BindingExecutionInfo { link, hasKey := c.getBindingMonitorLinksById(kubeEvent.MonitorId) if !hasKey { log.Error("Possible bug!!! Unknown kube event: no such monitor id registered", slog.String("monitorID", kubeEvent.MonitorId)) @@ -243,7 +220,7 @@ func (c *kubernetesBindingsController) HandleEvent(kubeEvent kemtypes.KubeEvent) return bInfo } -func (c *kubernetesBindingsController) BindingNames() []string { +func (c *KubernetesBindingsController) BindingNames() []string { names := []string{} for _, binding := range c.KubernetesBindings { names = append(names, binding.BindingName) @@ -253,7 +230,7 @@ func (c *kubernetesBindingsController) BindingNames() []string { // SnapshotsFor returns snapshot for single onKubernetes binding. // It finds a monitorId for a binding name and returns an array of objects. -func (c *kubernetesBindingsController) SnapshotsFor(bindingName string) []kemtypes.ObjectAndFilterResult { +func (c *KubernetesBindingsController) SnapshotsFor(bindingName string) []kemtypes.ObjectAndFilterResult { for _, binding := range c.KubernetesBindings { if bindingName == binding.BindingName { monitorID := binding.Monitor.Metadata.MonitorId @@ -269,7 +246,7 @@ func (c *kubernetesBindingsController) SnapshotsFor(bindingName string) []kemtyp // SnapshotsFrom returns snapshot for several binding names. // It finds a monitorId for each binding name and get its Snapshot, // then returns a map of object arrays for each binding name. -func (c *kubernetesBindingsController) SnapshotsFrom(bindingNames ...string) map[string][]kemtypes.ObjectAndFilterResult { +func (c *KubernetesBindingsController) SnapshotsFrom(bindingNames ...string) map[string][]kemtypes.ObjectAndFilterResult { res := map[string][]kemtypes.ObjectAndFilterResult{} for _, bindingName := range bindingNames { @@ -285,11 +262,11 @@ func (c *kubernetesBindingsController) SnapshotsFrom(bindingNames ...string) map return res } -func (c *kubernetesBindingsController) Snapshots() map[string][]kemtypes.ObjectAndFilterResult { +func (c *KubernetesBindingsController) Snapshots() map[string][]kemtypes.ObjectAndFilterResult { return c.SnapshotsFrom(c.BindingNames()...) } -func (c *kubernetesBindingsController) SnapshotsInfo() []string { +func (c *KubernetesBindingsController) SnapshotsInfo() []string { infos := make([]string, 0) for _, binding := range c.KubernetesBindings { monitorID := binding.Monitor.Metadata.MonitorId @@ -314,7 +291,7 @@ func (c *kubernetesBindingsController) SnapshotsInfo() []string { return infos } -func (c *kubernetesBindingsController) SnapshotsDump() map[string]interface{} { +func (c *KubernetesBindingsController) SnapshotsDump() map[string]interface{} { dumps := make(map[string]interface{}) for _, binding := range c.KubernetesBindings { monitorID := binding.Monitor.Metadata.MonitorId diff --git a/pkg/hook/controller/schedule_bindings_controller.go b/pkg/hook/controller/schedule_bindings_controller.go index 187a19a9..9295c90b 100644 --- a/pkg/hook/controller/schedule_bindings_controller.go +++ b/pkg/hook/controller/schedule_bindings_controller.go @@ -8,7 +8,7 @@ import ( schedulemanager "github.com/flant/shell-operator/pkg/schedule_manager" ) -// A link between a hook and a kube monitor +// ScheduleBindingToCrontabLink a link between a hook and a kube monitor type ScheduleBindingToCrontabLink struct { BindingName string Crontab string @@ -19,18 +19,8 @@ type ScheduleBindingToCrontabLink struct { Group string } -// ScheduleBindingsController handles schedule bindings for one hook. -type ScheduleBindingsController interface { - WithScheduleBindings([]htypes.ScheduleConfig) - WithScheduleManager(schedulemanager.ScheduleManager) - EnableScheduleBindings() - DisableScheduleBindings() - CanHandleEvent(crontab string) bool - HandleEvent(crontab string) []BindingExecutionInfo -} - -// scheduleHooksController is a main implementation of KubernetesHooksController -type scheduleBindingsController struct { +// ScheduleBindingsController is a main implementation of KubernetesHooksController +type ScheduleBindingsController struct { // dependencies scheduleManager schedulemanager.ScheduleManager @@ -42,27 +32,24 @@ type scheduleBindingsController struct { ScheduleBindings []htypes.ScheduleConfig } -// kubernetesHooksController should implement the KubernetesHooksController -var _ ScheduleBindingsController = &scheduleBindingsController{} - // NewScheduleBindingsController returns an implementation of ScheduleBindingsController -var NewScheduleBindingsController = func() *scheduleBindingsController { - return &scheduleBindingsController{ +var NewScheduleBindingsController = func() *ScheduleBindingsController { + return &ScheduleBindingsController{ ScheduleLinks: make(map[string]*ScheduleBindingToCrontabLink), } } -func (c *scheduleBindingsController) WithScheduleBindings(bindings []htypes.ScheduleConfig) { +func (c *ScheduleBindingsController) WithScheduleBindings(bindings []htypes.ScheduleConfig) { c.ScheduleBindings = bindings } -func (c *scheduleBindingsController) WithScheduleManager(scheduleManager schedulemanager.ScheduleManager) { +func (c *ScheduleBindingsController) WithScheduleManager(scheduleManager schedulemanager.ScheduleManager) { c.l.Lock() c.scheduleManager = scheduleManager c.l.Unlock() } -func (c *scheduleBindingsController) CanHandleEvent(crontab string) bool { +func (c *ScheduleBindingsController) CanHandleEvent(crontab string) bool { c.l.RLock() defer c.l.RUnlock() for _, link := range c.ScheduleLinks { @@ -73,7 +60,7 @@ func (c *scheduleBindingsController) CanHandleEvent(crontab string) bool { return false } -func (c *scheduleBindingsController) HandleEvent(crontab string) []BindingExecutionInfo { +func (c *ScheduleBindingsController) HandleEvent(crontab string) []BindingExecutionInfo { res := []BindingExecutionInfo{} c.l.RLock() @@ -102,7 +89,7 @@ func (c *scheduleBindingsController) HandleEvent(crontab string) []BindingExecut return res } -func (c *scheduleBindingsController) EnableScheduleBindings() { +func (c *ScheduleBindingsController) EnableScheduleBindings() { c.l.Lock() for _, config := range c.ScheduleBindings { c.ScheduleLinks[config.ScheduleEntry.Id] = &ScheduleBindingToCrontabLink{ @@ -118,7 +105,7 @@ func (c *scheduleBindingsController) EnableScheduleBindings() { c.l.Unlock() } -func (c *scheduleBindingsController) DisableScheduleBindings() { +func (c *ScheduleBindingsController) DisableScheduleBindings() { c.l.Lock() for _, config := range c.ScheduleBindings { c.scheduleManager.Remove(config.ScheduleEntry) diff --git a/pkg/hook/hook_manager.go b/pkg/hook/hook_manager.go index 2fd9f178..d3cb88e0 100644 --- a/pkg/hook/hook_manager.go +++ b/pkg/hook/hook_manager.go @@ -29,7 +29,7 @@ type Manager struct { // dependencies workingDir string tempDir string - kubeEventsManager kubeeventsmanager.KubeEventsManager + kubeEventsManager *kubeeventsmanager.KubeEventsManager scheduleManager schedulemanager.ScheduleManager conversionWebhookManager *conversion.WebhookManager admissionWebhookManager *admission.WebhookManager @@ -52,7 +52,7 @@ type Manager struct { type ManagerConfig struct { WorkingDir string TempDir string - Kmgr kubeeventsmanager.KubeEventsManager + Kmgr *kubeeventsmanager.KubeEventsManager Smgr schedulemanager.ScheduleManager Wmgr *admission.WebhookManager Cmgr *conversion.WebhookManager diff --git a/pkg/kube_events_manager/kube_events_manager.go b/pkg/kube_events_manager/kube_events_manager.go index 00a8e10d..7a41ec7e 100644 --- a/pkg/kube_events_manager/kube_events_manager.go +++ b/pkg/kube_events_manager/kube_events_manager.go @@ -14,20 +14,8 @@ import ( "github.com/flant/shell-operator/pkg/metric" ) -type KubeEventsManager interface { - WithMetricStorage(mstor metric.Storage) - AddMonitor(monitorConfig *MonitorConfig) error - HasMonitor(monitorID string) bool - GetMonitor(monitorID string) Monitor - StartMonitor(monitorID string) - StopMonitor(monitorID string) error - - Ch() chan kemtypes.KubeEvent - PauseHandleEvents() -} - -// kubeEventsManager is a main implementation of KubeEventsManager. -type kubeEventsManager struct { +// KubeEventsManager is a main implementation of KubeEventsManager. +type KubeEventsManager struct { // channel to emit KubeEvent objects KubeEventCh chan kemtypes.KubeEvent @@ -38,37 +26,34 @@ type kubeEventsManager struct { metricStorage metric.Storage m sync.RWMutex - Monitors map[string]Monitor + Monitors map[string]*Monitor logger *log.Logger } -// kubeEventsManager should implement KubeEventsManager. -var _ KubeEventsManager = (*kubeEventsManager)(nil) - // NewKubeEventsManager returns an implementation of KubeEventsManager. -func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) KubeEventsManager { +func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) *KubeEventsManager { cctx, cancel := context.WithCancel(ctx) - em := &kubeEventsManager{ + em := &KubeEventsManager{ ctx: cctx, cancel: cancel, KubeClient: client, m: sync.RWMutex{}, - Monitors: make(map[string]Monitor), + Monitors: make(map[string]*Monitor), KubeEventCh: make(chan kemtypes.KubeEvent, 1), logger: logger, } return em } -func (mgr *kubeEventsManager) WithMetricStorage(mstor metric.Storage) { +func (mgr *KubeEventsManager) WithMetricStorage(mstor metric.Storage) { mgr.metricStorage = mstor } // AddMonitor creates a monitor with informers and return a KubeEvent with existing objects. // TODO cleanup informers in case of error // TODO use Context to stop informers -func (mgr *kubeEventsManager) AddMonitor(monitorConfig *MonitorConfig) error { +func (mgr *KubeEventsManager) AddMonitor(monitorConfig *MonitorConfig) error { log.Debug("Add MONITOR", slog.String("config", fmt.Sprintf("%+v", monitorConfig))) monitor := NewMonitor( @@ -96,7 +81,7 @@ func (mgr *kubeEventsManager) AddMonitor(monitorConfig *MonitorConfig) error { } // HasMonitor returns true if there is a monitor with monitorID. -func (mgr *kubeEventsManager) HasMonitor(monitorID string) bool { +func (mgr *KubeEventsManager) HasMonitor(monitorID string) bool { mgr.m.RLock() _, has := mgr.Monitors[monitorID] mgr.m.RUnlock() @@ -104,14 +89,14 @@ func (mgr *kubeEventsManager) HasMonitor(monitorID string) bool { } // GetMonitor returns monitor by its ID. -func (mgr *kubeEventsManager) GetMonitor(monitorID string) Monitor { +func (mgr *KubeEventsManager) GetMonitor(monitorID string) *Monitor { mgr.m.RLock() defer mgr.m.RUnlock() return mgr.Monitors[monitorID] } // StartMonitor starts all informers for the monitor. -func (mgr *kubeEventsManager) StartMonitor(monitorID string) { +func (mgr *KubeEventsManager) StartMonitor(monitorID string) { mgr.m.RLock() monitor := mgr.Monitors[monitorID] mgr.m.RUnlock() @@ -119,7 +104,7 @@ func (mgr *kubeEventsManager) StartMonitor(monitorID string) { } // StopMonitor stops monitor and removes it from the index. -func (mgr *kubeEventsManager) StopMonitor(monitorID string) error { +func (mgr *KubeEventsManager) StopMonitor(monitorID string) error { mgr.m.RLock() monitor, ok := mgr.Monitors[monitorID] mgr.m.RUnlock() @@ -133,14 +118,14 @@ func (mgr *kubeEventsManager) StopMonitor(monitorID string) error { } // Ch returns a channel to receive KubeEvent objects. -func (mgr *kubeEventsManager) Ch() chan kemtypes.KubeEvent { +func (mgr *KubeEventsManager) Ch() chan kemtypes.KubeEvent { return mgr.KubeEventCh } // PauseHandleEvents set flags for all informers to ignore incoming events. // Useful for shutdown without panicking. // Calling cancel() leads to a race and panicking, see https://github.com/kubernetes/kubernetes/issues/59822 -func (mgr *kubeEventsManager) PauseHandleEvents() { +func (mgr *KubeEventsManager) PauseHandleEvents() { mgr.m.RLock() defer mgr.m.RUnlock() for _, monitor := range mgr.Monitors { diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 7afad00a..5c326004 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -15,19 +15,8 @@ import ( utils "github.com/flant/shell-operator/pkg/utils/labels" ) -type Monitor interface { - CreateInformers() error - Start(context.Context) - Stop() - PauseHandleEvents() - Snapshot() []kemtypes.ObjectAndFilterResult - EnableKubeEventCb() - GetConfig() *MonitorConfig - SnapshotOperations() (total *CachedObjectsInfo, last *CachedObjectsInfo) -} - // Monitor holds informers for resources and a namespace informer -type monitor struct { +type Monitor struct { Name string Config *MonitorConfig KubeClient *klient.Client @@ -52,10 +41,10 @@ type monitor struct { logger *log.Logger } -func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) Monitor { +func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) *Monitor { cctx, cancel := context.WithCancel(ctx) - return &monitor{ + return &Monitor{ ctx: cctx, cancel: cancel, KubeClient: client, @@ -70,7 +59,7 @@ func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage } } -func (m *monitor) GetConfig() *MonitorConfig { +func (m *Monitor) GetConfig() *MonitorConfig { return m.Config } @@ -79,7 +68,7 @@ func (m *monitor) GetConfig() *MonitorConfig { // If MonitorConfig.NamespaceSelector.MatchNames is defined, then // multiple informers are created for each namespace. // If no NamespaceSelector defined, then one informer is created. -func (m *monitor) CreateInformers() error { +func (m *Monitor) CreateInformers() error { logEntry := utils.EnrichLoggerWithLabels(m.logger, m.Config.Metadata.LogLabels). With("binding.name", m.Config.Metadata.DebugName) @@ -195,7 +184,7 @@ func (m *monitor) CreateInformers() error { } // Snapshot returns all existed objects from all created informers -func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { +func (m *Monitor) Snapshot() []kemtypes.ObjectAndFilterResult { objects := make([]kemtypes.ObjectAndFilterResult, 0) for _, informer := range m.ResourceInformers { @@ -219,7 +208,7 @@ func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { // EnableKubeEventCb allows execution of event callback for all informers. // Also executes eventCb for events accumulated during "Synchronization" phase. -func (m *monitor) EnableKubeEventCb() { +func (m *Monitor) EnableKubeEventCb() { for _, informer := range m.ResourceInformers { informer.enableKubeEventCb() } @@ -240,7 +229,7 @@ func (m *monitor) EnableKubeEventCb() { // it is only one informer. If matchName is specified, then multiple informers are created. // // If namespace is empty, then informer is bounded to all namespaces. -func (m *monitor) CreateInformersForNamespace(namespace string) ([]*resourceInformer, error) { +func (m *Monitor) CreateInformersForNamespace(namespace string) ([]*resourceInformer, error) { informers := make([]*resourceInformer, 0) cfg := &resourceInformerConfig{ client: m.KubeClient, @@ -270,7 +259,7 @@ func (m *monitor) CreateInformersForNamespace(namespace string) ([]*resourceInfo } // Start calls Run on all informers. -func (m *monitor) Start(parentCtx context.Context) { +func (m *Monitor) Start(parentCtx context.Context) { m.ctx, m.cancel = context.WithCancel(parentCtx) for _, informer := range m.ResourceInformers { @@ -301,7 +290,7 @@ func (m *monitor) Start(parentCtx context.Context) { } // Stop stops all informers -func (m *monitor) Stop() { +func (m *Monitor) Stop() { if m.cancel != nil { m.cancel() } @@ -310,7 +299,7 @@ func (m *monitor) Stop() { // PauseHandleEvents set flags for all informers to ignore incoming events. // Useful for shutdown without panicking. // Calling cancel() leads to a race and panicking, see https://github.com/kubernetes/kubernetes/issues/59822 -func (m *monitor) PauseHandleEvents() { +func (m *Monitor) PauseHandleEvents() { for _, informer := range m.ResourceInformers { informer.pauseHandleEvents() } @@ -329,7 +318,7 @@ func (m *monitor) PauseHandleEvents() { } } -func (m *monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObjectsInfo /*last*/) { +func (m *Monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObjectsInfo /*last*/) { total := &CachedObjectsInfo{} last := &CachedObjectsInfo{} diff --git a/pkg/shell-operator/manager_events_handler.go b/pkg/shell-operator/manager_events_handler.go index 5d790edf..b4c154fe 100644 --- a/pkg/shell-operator/manager_events_handler.go +++ b/pkg/shell-operator/manager_events_handler.go @@ -15,7 +15,7 @@ import ( type managerEventsHandlerConfig struct { tqs *queue.TaskQueueSet - mgr kubeeventsmanager.KubeEventsManager + mgr *kubeeventsmanager.KubeEventsManager smgr schedulemanager.ScheduleManager logger *log.Logger @@ -25,7 +25,7 @@ type ManagerEventsHandler struct { ctx context.Context cancel context.CancelFunc - kubeEventsManager kubeeventsmanager.KubeEventsManager + kubeEventsManager *kubeeventsmanager.KubeEventsManager scheduleManager schedulemanager.ScheduleManager kubeEventCb func(kubeEvent kemtypes.KubeEvent) []task.Task diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index aa3d5da8..914cabcb 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -48,7 +48,7 @@ type ShellOperator struct { ObjectPatcher *objectpatch.ObjectPatcher ScheduleManager schedulemanager.ScheduleManager - KubeEventsManager kubeeventsmanager.KubeEventsManager + KubeEventsManager *kubeeventsmanager.KubeEventsManager TaskQueues *queue.TaskQueueSet diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index d34f4a52..ceaf48fb 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -35,7 +35,7 @@ type BindingContextController struct { HookConfig string Controller *StateController - KubeEventsManager kubeeventsmanager.KubeEventsManager + KubeEventsManager *kubeeventsmanager.KubeEventsManager ScheduleManager schedulemanager.ScheduleManager fakeCluster *fake.Cluster diff --git a/test/hook/context/state.go b/test/hook/context/state.go index d99a965d..4045845f 100644 --- a/test/hook/context/state.go +++ b/test/hook/context/state.go @@ -24,7 +24,7 @@ type StateController struct { } // NewStateController creates controller to apply state changes -func NewStateController(fc *fake.Cluster, ev kubeeventsmanager.KubeEventsManager) *StateController { +func NewStateController(fc *fake.Cluster, ev *kubeeventsmanager.KubeEventsManager) *StateController { c := &StateController{ CurrentState: make(map[string]manifest.Manifest), fakeCluster: fc, diff --git a/test/integration/kube_event_manager/kube_event_manager_test.go b/test/integration/kube_event_manager/kube_event_manager_test.go index b2813d44..b29315cf 100644 --- a/test/integration/kube_event_manager/kube_event_manager_test.go +++ b/test/integration/kube_event_manager/kube_event_manager_test.go @@ -24,7 +24,7 @@ func Test(t *testing.T) { } var _ = Describe("Binding 'kubernetes' with kind 'Pod' should emit KubeEvent objects", func() { - var KubeEventsManager kubeeventsmanager.KubeEventsManager + var KubeEventsManager *kubeeventsmanager.KubeEventsManager BeforeEach(func() { KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(context.Background(), KubeClient, log.NewNop()) From eaa1dd07f79e7c7f988530d557139f9e1c4461ae Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 17:16:57 +0300 Subject: [PATCH 08/25] refactor: change ScheduleManager fields to pointers for consistency and improved memory management Signed-off-by: Evsyukov Denis --- pkg/hook/controller/hook_controller.go | 2 +- .../schedule_bindings_controller.go | 4 +-- pkg/hook/hook_manager.go | 4 +-- pkg/schedule_manager/schedule_manager.go | 26 ++++++------------- pkg/shell-operator/manager_events_handler.go | 4 +-- pkg/shell-operator/operator.go | 2 +- test/hook/context/generator.go | 2 +- 7 files changed, 17 insertions(+), 27 deletions(-) diff --git a/pkg/hook/controller/hook_controller.go b/pkg/hook/controller/hook_controller.go index be07d886..070f05ec 100644 --- a/pkg/hook/controller/hook_controller.go +++ b/pkg/hook/controller/hook_controller.go @@ -66,7 +66,7 @@ func (hc *HookController) InitKubernetesBindings(bindings []htypes.OnKubernetesE hc.logger = logger } -func (hc *HookController) InitScheduleBindings(bindings []htypes.ScheduleConfig, scheduleMgr schedulemanager.ScheduleManager) { +func (hc *HookController) InitScheduleBindings(bindings []htypes.ScheduleConfig, scheduleMgr *schedulemanager.ScheduleManager) { if len(bindings) == 0 { return } diff --git a/pkg/hook/controller/schedule_bindings_controller.go b/pkg/hook/controller/schedule_bindings_controller.go index 9295c90b..c61afbfb 100644 --- a/pkg/hook/controller/schedule_bindings_controller.go +++ b/pkg/hook/controller/schedule_bindings_controller.go @@ -22,7 +22,7 @@ type ScheduleBindingToCrontabLink struct { // ScheduleBindingsController is a main implementation of KubernetesHooksController type ScheduleBindingsController struct { // dependencies - scheduleManager schedulemanager.ScheduleManager + scheduleManager *schedulemanager.ScheduleManager l sync.RWMutex // All hooks with 'kubernetes' bindings @@ -43,7 +43,7 @@ func (c *ScheduleBindingsController) WithScheduleBindings(bindings []htypes.Sche c.ScheduleBindings = bindings } -func (c *ScheduleBindingsController) WithScheduleManager(scheduleManager schedulemanager.ScheduleManager) { +func (c *ScheduleBindingsController) WithScheduleManager(scheduleManager *schedulemanager.ScheduleManager) { c.l.Lock() c.scheduleManager = scheduleManager c.l.Unlock() diff --git a/pkg/hook/hook_manager.go b/pkg/hook/hook_manager.go index d3cb88e0..45491d16 100644 --- a/pkg/hook/hook_manager.go +++ b/pkg/hook/hook_manager.go @@ -30,7 +30,7 @@ type Manager struct { workingDir string tempDir string kubeEventsManager *kubeeventsmanager.KubeEventsManager - scheduleManager schedulemanager.ScheduleManager + scheduleManager *schedulemanager.ScheduleManager conversionWebhookManager *conversion.WebhookManager admissionWebhookManager *admission.WebhookManager @@ -53,7 +53,7 @@ type ManagerConfig struct { WorkingDir string TempDir string Kmgr *kubeeventsmanager.KubeEventsManager - Smgr schedulemanager.ScheduleManager + Smgr *schedulemanager.ScheduleManager Wmgr *admission.WebhookManager Cmgr *conversion.WebhookManager diff --git a/pkg/schedule_manager/schedule_manager.go b/pkg/schedule_manager/schedule_manager.go index b62be1a1..8f7dc06d 100644 --- a/pkg/schedule_manager/schedule_manager.go +++ b/pkg/schedule_manager/schedule_manager.go @@ -10,20 +10,12 @@ import ( smtypes "github.com/flant/shell-operator/pkg/schedule_manager/types" ) -type ScheduleManager interface { - Stop() - Start() - Add(entry smtypes.ScheduleEntry) - Remove(entry smtypes.ScheduleEntry) - Ch() chan string -} - type CronEntry struct { EntryID cron.EntryID Ids map[string]bool } -type scheduleManager struct { +type ScheduleManager struct { ctx context.Context cancel context.CancelFunc cron *cron.Cron @@ -33,11 +25,9 @@ type scheduleManager struct { logger *log.Logger } -var _ ScheduleManager = &scheduleManager{} - -func NewScheduleManager(ctx context.Context, logger *log.Logger) *scheduleManager { +func NewScheduleManager(ctx context.Context, logger *log.Logger) *ScheduleManager { cctx, cancel := context.WithCancel(ctx) - sm := &scheduleManager{ + sm := &ScheduleManager{ ctx: cctx, cancel: cancel, ScheduleCh: make(chan string, 1), @@ -49,7 +39,7 @@ func NewScheduleManager(ctx context.Context, logger *log.Logger) *scheduleManage return sm } -func (sm *scheduleManager) Stop() { +func (sm *ScheduleManager) Stop() { if sm.cancel != nil { sm.cancel() } @@ -58,7 +48,7 @@ func (sm *scheduleManager) Stop() { // Add create entry for crontab and id and start scheduled function. // Crontab string should be validated with cron.Parse // function before pass to Add. -func (sm *scheduleManager) Add(newEntry smtypes.ScheduleEntry) { +func (sm *ScheduleManager) Add(newEntry smtypes.ScheduleEntry) { logEntry := sm.logger.With("operator.component", "scheduleManager") cronEntry, hasCronEntry := sm.Entries[newEntry.Crontab] @@ -89,7 +79,7 @@ func (sm *scheduleManager) Add(newEntry smtypes.ScheduleEntry) { } } -func (sm *scheduleManager) Remove(delEntry smtypes.ScheduleEntry) { +func (sm *ScheduleManager) Remove(delEntry smtypes.ScheduleEntry) { cronEntry, hasCronEntry := sm.Entries[delEntry.Crontab] // Nothing to Remove @@ -115,7 +105,7 @@ func (sm *scheduleManager) Remove(delEntry smtypes.ScheduleEntry) { } } -func (sm *scheduleManager) Start() { +func (sm *ScheduleManager) Start() { sm.cron.Start() go func() { <-sm.ctx.Done() @@ -123,6 +113,6 @@ func (sm *scheduleManager) Start() { }() } -func (sm *scheduleManager) Ch() chan string { +func (sm *ScheduleManager) Ch() chan string { return sm.ScheduleCh } diff --git a/pkg/shell-operator/manager_events_handler.go b/pkg/shell-operator/manager_events_handler.go index b4c154fe..8a5f9354 100644 --- a/pkg/shell-operator/manager_events_handler.go +++ b/pkg/shell-operator/manager_events_handler.go @@ -16,7 +16,7 @@ import ( type managerEventsHandlerConfig struct { tqs *queue.TaskQueueSet mgr *kubeeventsmanager.KubeEventsManager - smgr schedulemanager.ScheduleManager + smgr *schedulemanager.ScheduleManager logger *log.Logger } @@ -26,7 +26,7 @@ type ManagerEventsHandler struct { cancel context.CancelFunc kubeEventsManager *kubeeventsmanager.KubeEventsManager - scheduleManager schedulemanager.ScheduleManager + scheduleManager *schedulemanager.ScheduleManager kubeEventCb func(kubeEvent kemtypes.KubeEvent) []task.Task scheduleCb func(crontab string) []task.Task diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index 914cabcb..a40521ed 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -47,7 +47,7 @@ type ShellOperator struct { KubeClient *klient.Client ObjectPatcher *objectpatch.ObjectPatcher - ScheduleManager schedulemanager.ScheduleManager + ScheduleManager *schedulemanager.ScheduleManager KubeEventsManager *kubeeventsmanager.KubeEventsManager TaskQueues *queue.TaskQueueSet diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index ceaf48fb..67681d73 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -36,7 +36,7 @@ type BindingContextController struct { Controller *StateController KubeEventsManager *kubeeventsmanager.KubeEventsManager - ScheduleManager schedulemanager.ScheduleManager + ScheduleManager *schedulemanager.ScheduleManager fakeCluster *fake.Cluster From 00844308e38181605f110e5f7b5a5455549014b6 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Tue, 8 Apr 2025 17:18:28 +0300 Subject: [PATCH 09/25] refactor: rename resourceInformer to ResourceInformer for consistency Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor.go | 18 ++++---- pkg/kube_events_manager/resource_informer.go | 44 ++++++++++---------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 5c326004..9e1c9d71 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -21,7 +21,7 @@ type Monitor struct { Config *MonitorConfig KubeClient *klient.Client // Static list of informers - ResourceInformers []*resourceInformer + ResourceInformers []*ResourceInformer // Namespace informer to get new namespaces NamespaceInformer *namespaceInformer // map of dynamically starting informers @@ -51,7 +51,7 @@ func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage metricStorage: mstor, Config: config, eventCb: eventCb, - ResourceInformers: make([]*resourceInformer, 0), + ResourceInformers: make([]*ResourceInformer, 0), VaryingInformers: sync.Map{}, cancelForNs: sync.Map{}, staticNamespaces: sync.Map{}, @@ -192,7 +192,7 @@ func (m *Monitor) Snapshot() []kemtypes.ObjectAndFilterResult { } m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { + if value, ok := value.([]*ResourceInformer); ok { for _, informer := range value { objects = append(objects, informer.getCachedObjects()...) } @@ -214,7 +214,7 @@ func (m *Monitor) EnableKubeEventCb() { } // Execute eventCb for events accumulated during "Synchronization" phase. m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { + if value, ok := value.([]*ResourceInformer); ok { for _, informer := range value { informer.enableKubeEventCb() } @@ -229,8 +229,8 @@ func (m *Monitor) EnableKubeEventCb() { // it is only one informer. If matchName is specified, then multiple informers are created. // // If namespace is empty, then informer is bounded to all namespaces. -func (m *Monitor) CreateInformersForNamespace(namespace string) ([]*resourceInformer, error) { - informers := make([]*resourceInformer, 0) +func (m *Monitor) CreateInformersForNamespace(namespace string) ([]*ResourceInformer, error) { + informers := make([]*ResourceInformer, 0) cfg := &resourceInformerConfig{ client: m.KubeClient, mstor: m.metricStorage, @@ -274,7 +274,7 @@ func (m *Monitor) Start(parentCtx context.Context) { } ctx, cancelForNs := context.WithCancel(m.ctx) m.cancelForNs.Store(nsName, cancelForNs) - if value, ok := value.([]*resourceInformer); ok { + if value, ok := value.([]*ResourceInformer); ok { for _, informer := range value { informer.withContext(ctx) informer.start() @@ -305,7 +305,7 @@ func (m *Monitor) PauseHandleEvents() { } m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { + if value, ok := value.([]*ResourceInformer); ok { for _, informer := range value { informer.pauseHandleEvents() } @@ -328,7 +328,7 @@ func (m *Monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObj } m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { + if value, ok := value.([]*ResourceInformer); ok { for _, informer := range value { total.add(informer.getCachedObjectsInfo()) last.add(informer.getCachedObjectsInfoIncrement()) diff --git a/pkg/kube_events_manager/resource_informer.go b/pkg/kube_events_manager/resource_informer.go index 443877fd..5e294c28 100644 --- a/pkg/kube_events_manager/resource_informer.go +++ b/pkg/kube_events_manager/resource_informer.go @@ -23,7 +23,7 @@ import ( "github.com/flant/shell-operator/pkg/utils/measure" ) -type resourceInformer struct { +type ResourceInformer struct { id string KubeClient *klient.Client Monitor *MonitorConfig @@ -54,7 +54,7 @@ type resourceInformer struct { eventCbEnabled bool eventBufLock sync.Mutex - // TODO resourceInformer should be stoppable (think of deleted namespaces and disabled modules in addon-operator) + // TODO ResourceInformer should be stoppable (think of deleted namespaces and disabled modules in addon-operator) ctx context.Context cancel context.CancelFunc @@ -66,7 +66,7 @@ type resourceInformer struct { logger *log.Logger } -// resourceInformer should implement ResourceInformer +// ResourceInformer should implement ResourceInformer type resourceInformerConfig struct { client *klient.Client mstor metric.Storage @@ -76,8 +76,8 @@ type resourceInformerConfig struct { logger *log.Logger } -func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *resourceInformer { - informer := &resourceInformer{ +func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *ResourceInformer { + informer := &ResourceInformer{ id: uuid.Must(uuid.NewV4()).String(), KubeClient: cfg.client, metricStorage: cfg.mstor, @@ -95,17 +95,17 @@ func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *resource return informer } -func (ei *resourceInformer) withContext(ctx context.Context) { +func (ei *ResourceInformer) withContext(ctx context.Context) { ei.ctx, ei.cancel = context.WithCancel(ctx) } -func (ei *resourceInformer) putEvent(ev kemtypes.KubeEvent) { +func (ei *ResourceInformer) putEvent(ev kemtypes.KubeEvent) { if ei.eventCb != nil { ei.eventCb(ev) } } -func (ei *resourceInformer) createSharedInformer() error { +func (ei *ResourceInformer) createSharedInformer() error { var err error // discover GroupVersionResource for informer @@ -153,7 +153,7 @@ func (ei *resourceInformer) createSharedInformer() error { } // Snapshot returns all cached objects for this informer -func (ei *resourceInformer) getCachedObjects() []kemtypes.ObjectAndFilterResult { +func (ei *ResourceInformer) getCachedObjects() []kemtypes.ObjectAndFilterResult { ei.cacheLock.RLock() res := make([]kemtypes.ObjectAndFilterResult, 0) for _, obj := range ei.cachedObjects { @@ -170,7 +170,7 @@ func (ei *resourceInformer) getCachedObjects() []kemtypes.ObjectAndFilterResult return res } -func (ei *resourceInformer) enableKubeEventCb() { +func (ei *ResourceInformer) enableKubeEventCb() { ei.eventBufLock.Lock() defer ei.eventBufLock.Unlock() if ei.eventCbEnabled { @@ -186,7 +186,7 @@ func (ei *resourceInformer) enableKubeEventCb() { // loadExistedObjects get a list of existed objects in namespace that match selectors and // fills Checksum map with checksums of existing objects. -func (ei *resourceInformer) loadExistedObjects() error { +func (ei *ResourceInformer) loadExistedObjects() error { defer trace.StartRegion(context.Background(), "loadExistedObjects").End() objList, err := ei.KubeClient.Dynamic(). @@ -260,23 +260,23 @@ func (ei *resourceInformer) loadExistedObjects() error { return nil } -func (ei *resourceInformer) OnAdd(obj interface{}, _ bool) { +func (ei *ResourceInformer) OnAdd(obj interface{}, _ bool) { ei.handleWatchEvent(obj, kemtypes.WatchEventAdded) } -func (ei *resourceInformer) OnUpdate(_, newObj interface{}) { +func (ei *ResourceInformer) OnUpdate(_, newObj interface{}) { ei.handleWatchEvent(newObj, kemtypes.WatchEventModified) } -func (ei *resourceInformer) OnDelete(obj interface{}) { +func (ei *ResourceInformer) OnDelete(obj interface{}) { ei.handleWatchEvent(obj, kemtypes.WatchEventDeleted) } // HandleKubeEvent register object in cache. Pass object to callback if object's checksum is changed. // TODO refactor: pass KubeEvent as argument // TODO add delay to merge Added and Modified events (node added and then labels applied — one hook run on Added+Modified is enough) -// func (ei *resourceInformer) HandleKubeEvent(obj *unstructured.Unstructured, objectId string, filterResult string, newChecksum string, eventType WatchEventType) { -func (ei *resourceInformer) handleWatchEvent(object interface{}, eventType kemtypes.WatchEventType) { +// func (ei *ResourceInformer) HandleKubeEvent(obj *unstructured.Unstructured, objectId string, filterResult string, newChecksum string, eventType WatchEventType) { +func (ei *ResourceInformer) handleWatchEvent(object interface{}, eventType kemtypes.WatchEventType) { // check if stop if ei.stopped { log.Debug("received WATCH for stopped informer", @@ -411,7 +411,7 @@ func (ei *resourceInformer) handleWatchEvent(object interface{}, eventType kemty } } -func (ei *resourceInformer) adjustFieldSelector(selector *kemtypes.FieldSelector, objName string) *kemtypes.FieldSelector { +func (ei *ResourceInformer) adjustFieldSelector(selector *kemtypes.FieldSelector, objName string) *kemtypes.FieldSelector { var selectorCopy *kemtypes.FieldSelector if selector != nil { @@ -440,7 +440,7 @@ func (ei *resourceInformer) adjustFieldSelector(selector *kemtypes.FieldSelector return selectorCopy } -func (ei *resourceInformer) shouldFireEvent(checkEvent kemtypes.WatchEventType) bool { +func (ei *ResourceInformer) shouldFireEvent(checkEvent kemtypes.WatchEventType) bool { for _, event := range ei.Monitor.EventTypes { if event == checkEvent { return true @@ -449,7 +449,7 @@ func (ei *resourceInformer) shouldFireEvent(checkEvent kemtypes.WatchEventType) return false } -func (ei *resourceInformer) start() { +func (ei *ResourceInformer) start() { log.Debug("RUN resource informer", slog.String("debugName", ei.Monitor.Metadata.DebugName)) go func() { @@ -470,20 +470,20 @@ func (ei *resourceInformer) start() { log.Debug("informer is ready", slog.String("debugName", ei.Monitor.Metadata.DebugName)) } -func (ei *resourceInformer) pauseHandleEvents() { +func (ei *ResourceInformer) pauseHandleEvents() { log.Debug("PAUSE resource informer", slog.String("debugName", ei.Monitor.Metadata.DebugName)) ei.stopped = true } // CachedObjectsInfo returns info accumulated from start. -func (ei *resourceInformer) getCachedObjectsInfo() CachedObjectsInfo { +func (ei *ResourceInformer) getCachedObjectsInfo() CachedObjectsInfo { ei.cacheLock.RLock() defer ei.cacheLock.RUnlock() return *ei.cachedObjectsInfo } // getCachedObjectsInfoIncrement returns info accumulated from last call and clean it. -func (ei *resourceInformer) getCachedObjectsInfoIncrement() CachedObjectsInfo { +func (ei *ResourceInformer) getCachedObjectsInfoIncrement() CachedObjectsInfo { ei.cacheLock.Lock() defer ei.cacheLock.Unlock() info := *ei.cachedObjectsIncrement From 764fe201c3994daf94fb0d9df70e33f2264a212f Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 08:49:24 +0300 Subject: [PATCH 10/25] test: enable race detection in test execution Signed-off-by: Evsyukov Denis --- .github/workflows/tests.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ec5f21db..a0004140 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -45,5 +45,6 @@ jobs: export GOOS=linux go test \ + --race \ -tags test \ ./cmd/... ./pkg/... ./test/utils From 9feaca0fcb71c4978de767f47412df7c2d47d151 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 08:55:14 +0300 Subject: [PATCH 11/25] refactor: replace boolean flag with atomic.Bool for thread-safe state management Signed-off-by: Evsyukov Denis --- test/hook/context/generator.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 67681d73..77d3b975 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "sync" + "sync/atomic" "time" "github.com/deckhouse/deckhouse/pkg/log" @@ -41,7 +42,7 @@ type BindingContextController struct { fakeCluster *fake.Cluster mu sync.Mutex - started bool + started atomic.Bool logger *log.Logger } @@ -91,12 +92,10 @@ func (b *BindingContextController) RegisterCRD(group, version, kind string, name // Run generates binding contexts for hook tests func (b *BindingContextController) Run(initialState string) (GeneratedBindingContexts, error) { b.mu.Lock() - if b.started { - b.mu.Unlock() + defer b.mu.Unlock() + if b.started.Load() { return GeneratedBindingContexts{}, fmt.Errorf("attempt to start an already started runner, it cannot be started twice") } - b.started = true - b.mu.Unlock() err := b.Controller.SetInitialState(initialState) if err != nil { @@ -130,6 +129,7 @@ func (b *BindingContextController) Run(initialState string) (GeneratedBindingCon } b.HookCtrl.UnlockKubernetesEvents() + b.started.Swap(true) time.Sleep(50 * time.Millisecond) return cc.CombinedAndUpdated(b.HookCtrl) From dbb73441a15c2cebb2504599fd210fb22e1a1d51 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 09:45:08 +0300 Subject: [PATCH 12/25] refactor: initialize ScheduleManager and KubeEventsManager as pointers for improved memory management Signed-off-by: Evsyukov Denis --- pkg/shell-operator/operator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index a40521ed..854725c1 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -6,10 +6,11 @@ import ( "log/slog" "time" - "github.com/deckhouse/deckhouse/pkg/log" "github.com/gofrs/uuid/v5" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "github.com/deckhouse/deckhouse/pkg/log" + klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/hook" bindingcontext "github.com/flant/shell-operator/pkg/hook/binding_context" @@ -88,6 +89,9 @@ func NewShellOperator(ctx context.Context, opts ...Option) *ShellOperator { so.logger = log.NewLogger(log.Options{}).Named("shell-operator") } + so.ScheduleManager = &schedulemanager.ScheduleManager{} + so.KubeEventsManager = &kubeeventsmanager.KubeEventsManager{} + return so } From a488189256e6d70241fb7275bb806d987f8e25d4 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 09:50:44 +0300 Subject: [PATCH 13/25] refactor: replace Swap with Store for boolean state management and add error handling for missing monitors Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/kube_events_manager.go | 7 ++++++- pkg/shell-operator/operator.go | 3 +-- test/hook/context/generator.go | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/kube_events_manager/kube_events_manager.go b/pkg/kube_events_manager/kube_events_manager.go index 7a41ec7e..6249b8ad 100644 --- a/pkg/kube_events_manager/kube_events_manager.go +++ b/pkg/kube_events_manager/kube_events_manager.go @@ -98,7 +98,12 @@ func (mgr *KubeEventsManager) GetMonitor(monitorID string) *Monitor { // StartMonitor starts all informers for the monitor. func (mgr *KubeEventsManager) StartMonitor(monitorID string) { mgr.m.RLock() - monitor := mgr.Monitors[monitorID] + monitor, ok := mgr.Monitors[monitorID] + if !ok { + mgr.m.RUnlock() + mgr.logger.Error("Monitor not found", slog.String("monitorID", monitorID)) + return + } mgr.m.RUnlock() monitor.Start(mgr.ctx) } diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index 854725c1..87db5f4c 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -6,11 +6,10 @@ import ( "log/slog" "time" + "github.com/deckhouse/deckhouse/pkg/log" "github.com/gofrs/uuid/v5" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "github.com/deckhouse/deckhouse/pkg/log" - klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/hook" bindingcontext "github.com/flant/shell-operator/pkg/hook/binding_context" diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 77d3b975..4795c453 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -129,7 +129,7 @@ func (b *BindingContextController) Run(initialState string) (GeneratedBindingCon } b.HookCtrl.UnlockKubernetesEvents() - b.started.Swap(true) + b.started.Store(true) time.Sleep(50 * time.Millisecond) return cc.CombinedAndUpdated(b.HookCtrl) From 7a105057487e1bd4fec3634e9369fc9ab5ef082b Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 10:46:53 +0300 Subject: [PATCH 14/25] refactor: change pointer receivers to value receivers for KubeEventsManager and ScheduleManager for consistency Signed-off-by: Evsyukov Denis --- pkg/hook/controller/hook_controller.go | 11 ++-- .../kubernetes_bindings_controller.go | 66 ++++++++++++------- .../schedule_bindings_controller.go | 32 +++++---- pkg/hook/hook_manager.go | 11 ++-- .../kube_events_manager.go | 45 +++++++------ pkg/kube_events_manager/monitor.go | 51 ++++++++------ pkg/kube_events_manager/resource_informer.go | 47 ++++++------- pkg/schedule_manager/schedule_manager.go | 26 +++++--- pkg/shell-operator/manager_events_handler.go | 8 +-- pkg/shell-operator/operator.go | 10 ++- test/hook/context/generator.go | 4 +- test/hook/context/state.go | 2 +- .../kube_event_manager_test.go | 6 +- 13 files changed, 188 insertions(+), 131 deletions(-) diff --git a/pkg/hook/controller/hook_controller.go b/pkg/hook/controller/hook_controller.go index 070f05ec..4a8292c9 100644 --- a/pkg/hook/controller/hook_controller.go +++ b/pkg/hook/controller/hook_controller.go @@ -1,9 +1,10 @@ package controller import ( - "github.com/deckhouse/deckhouse/pkg/log" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "github.com/deckhouse/deckhouse/pkg/log" + bctx "github.com/flant/shell-operator/pkg/hook/binding_context" htypes "github.com/flant/shell-operator/pkg/hook/types" kubeeventsmanager "github.com/flant/shell-operator/pkg/kube_events_manager" @@ -40,8 +41,8 @@ func NewHookController() *HookController { } type HookController struct { - KubernetesController *KubernetesBindingsController - ScheduleController *ScheduleBindingsController + KubernetesController KubernetesBindingsController + ScheduleController ScheduleBindingsController AdmissionController *AdmissionBindingsController ConversionController *ConversionBindingsController kubernetesBindings []htypes.OnKubernetesEventConfig @@ -53,7 +54,7 @@ type HookController struct { logger *log.Logger } -func (hc *HookController) InitKubernetesBindings(bindings []htypes.OnKubernetesEventConfig, kubeEventMgr *kubeeventsmanager.KubeEventsManager, logger *log.Logger) { +func (hc *HookController) InitKubernetesBindings(bindings []htypes.OnKubernetesEventConfig, kubeEventMgr kubeeventsmanager.KubeEventsManager, logger *log.Logger) { if len(bindings) == 0 { return } @@ -66,7 +67,7 @@ func (hc *HookController) InitKubernetesBindings(bindings []htypes.OnKubernetesE hc.logger = logger } -func (hc *HookController) InitScheduleBindings(bindings []htypes.ScheduleConfig, scheduleMgr *schedulemanager.ScheduleManager) { +func (hc *HookController) InitScheduleBindings(bindings []htypes.ScheduleConfig, scheduleMgr schedulemanager.ScheduleManager) { if len(bindings) == 0 { return } diff --git a/pkg/hook/controller/kubernetes_bindings_controller.go b/pkg/hook/controller/kubernetes_bindings_controller.go index 21329e89..07a726e1 100644 --- a/pkg/hook/controller/kubernetes_bindings_controller.go +++ b/pkg/hook/controller/kubernetes_bindings_controller.go @@ -20,13 +20,33 @@ type KubernetesBindingToMonitorLink struct { BindingConfig htypes.OnKubernetesEventConfig } -// KubernetesHooksController is a main implementation of KubernetesHooksController -type KubernetesBindingsController struct { +// KubernetesBindingsController handles kubernetes bindings for one hook. +type KubernetesBindingsController interface { + WithKubernetesBindings([]htypes.OnKubernetesEventConfig) + WithKubeEventsManager(kubeeventsmanager.KubeEventsManager) + EnableKubernetesBindings() ([]BindingExecutionInfo, error) + UpdateMonitor(monitorId string, kind, apiVersion string) error + UnlockEvents() + UnlockEventsFor(monitorID string) + StopMonitors() + CanHandleEvent(kubeEvent kemtypes.KubeEvent) bool + HandleEvent(kubeEvent kemtypes.KubeEvent) BindingExecutionInfo + BindingNames() []string + + SnapshotsFrom(bindingNames ...string) map[string][]kemtypes.ObjectAndFilterResult + SnapshotsFor(bindingName string) []kemtypes.ObjectAndFilterResult + Snapshots() map[string][]kemtypes.ObjectAndFilterResult + SnapshotsInfo() []string + SnapshotsDump() map[string]interface{} +} + +// kubernetesHooksController is a main implementation of KubernetesHooksController +type kubernetesBindingsController struct { // bindings configurations KubernetesBindings []htypes.OnKubernetesEventConfig // dependencies - kubeEventsManager *kubeeventsmanager.KubeEventsManager + kubeEventsManager kubeeventsmanager.KubeEventsManager logger *log.Logger @@ -36,25 +56,25 @@ type KubernetesBindingsController struct { } // NewKubernetesBindingsController returns an implementation of KubernetesBindingsController -var NewKubernetesBindingsController = func(logger *log.Logger) *KubernetesBindingsController { - return &KubernetesBindingsController{ +var NewKubernetesBindingsController = func(logger *log.Logger) KubernetesBindingsController { + return &kubernetesBindingsController{ BindingMonitorLinks: make(map[string]*KubernetesBindingToMonitorLink), logger: logger, } } -func (c *KubernetesBindingsController) WithKubernetesBindings(bindings []htypes.OnKubernetesEventConfig) { +func (c *kubernetesBindingsController) WithKubernetesBindings(bindings []htypes.OnKubernetesEventConfig) { c.KubernetesBindings = bindings } -func (c *KubernetesBindingsController) WithKubeEventsManager(kubeEventsManager *kubeeventsmanager.KubeEventsManager) { +func (c *kubernetesBindingsController) WithKubeEventsManager(kubeEventsManager kubeeventsmanager.KubeEventsManager) { c.kubeEventsManager = kubeEventsManager } // EnableKubernetesBindings adds a monitor for each 'kubernetes' binding. This method // returns an array of BindingExecutionInfo to help construct initial tasks to run hooks. // Informers in each monitor are started immediately to keep up the "fresh" state of object caches. -func (c *KubernetesBindingsController) EnableKubernetesBindings() ([]BindingExecutionInfo, error) { +func (c *kubernetesBindingsController) EnableKubernetesBindings() ([]BindingExecutionInfo, error) { res := make([]BindingExecutionInfo, 0) for _, config := range c.KubernetesBindings { @@ -79,7 +99,7 @@ func (c *KubernetesBindingsController) EnableKubernetesBindings() ([]BindingExec return res, nil } -func (c *KubernetesBindingsController) UpdateMonitor(monitorId string, kind, apiVersion string) error { +func (c *kubernetesBindingsController) UpdateMonitor(monitorId string, kind, apiVersion string) error { // Find binding for monitorId link, ok := c.getBindingMonitorLinksById(monitorId) if !ok { @@ -130,7 +150,7 @@ func (c *KubernetesBindingsController) UpdateMonitor(monitorId string, kind, api } // UnlockEvents turns on eventCb for all monitors to emit events after Synchronization. -func (c *KubernetesBindingsController) UnlockEvents() { +func (c *kubernetesBindingsController) UnlockEvents() { c.iterateBindingMonitorLinks(func(monitorID string) bool { m := c.kubeEventsManager.GetMonitor(monitorID) m.EnableKubeEventCb() @@ -139,7 +159,7 @@ func (c *KubernetesBindingsController) UnlockEvents() { } // UnlockEventsFor turns on eventCb for matched monitor to emit events after Synchronization. -func (c *KubernetesBindingsController) UnlockEventsFor(monitorID string) { +func (c *kubernetesBindingsController) UnlockEventsFor(monitorID string) { m := c.kubeEventsManager.GetMonitor(monitorID) if m == nil { log.Warn("monitor was not found", slog.String("monitorID", monitorID)) @@ -150,14 +170,14 @@ func (c *KubernetesBindingsController) UnlockEventsFor(monitorID string) { // StopMonitors stops all monitors for the hook. // TODO handle error! -func (c *KubernetesBindingsController) StopMonitors() { +func (c *kubernetesBindingsController) StopMonitors() { c.iterateBindingMonitorLinks(func(monitorID string) bool { _ = c.kubeEventsManager.StopMonitor(monitorID) return false }) } -func (c *KubernetesBindingsController) CanHandleEvent(kubeEvent kemtypes.KubeEvent) bool { +func (c *kubernetesBindingsController) CanHandleEvent(kubeEvent kemtypes.KubeEvent) bool { var canHandleEvent bool c.iterateBindingMonitorLinks(func(monitorID string) bool { @@ -171,7 +191,7 @@ func (c *KubernetesBindingsController) CanHandleEvent(kubeEvent kemtypes.KubeEve return canHandleEvent } -func (c *KubernetesBindingsController) iterateBindingMonitorLinks(doFn func(monitorID string) bool) { +func (c *kubernetesBindingsController) iterateBindingMonitorLinks(doFn func(monitorID string) bool) { c.l.RLock() for monitorID := range c.BindingMonitorLinks { if exit := doFn(monitorID); exit { @@ -181,14 +201,14 @@ func (c *KubernetesBindingsController) iterateBindingMonitorLinks(doFn func(moni c.l.RUnlock() } -func (c *KubernetesBindingsController) getBindingMonitorLinksById(monitorId string) (*KubernetesBindingToMonitorLink, bool) { +func (c *kubernetesBindingsController) getBindingMonitorLinksById(monitorId string) (*KubernetesBindingToMonitorLink, bool) { c.l.RLock() link, found := c.BindingMonitorLinks[monitorId] c.l.RUnlock() return link, found } -func (c *KubernetesBindingsController) setBindingMonitorLinks(monitorId string, link *KubernetesBindingToMonitorLink) { +func (c *kubernetesBindingsController) setBindingMonitorLinks(monitorId string, link *KubernetesBindingToMonitorLink) { c.l.Lock() c.BindingMonitorLinks[monitorId] = link c.l.Unlock() @@ -196,7 +216,7 @@ func (c *KubernetesBindingsController) setBindingMonitorLinks(monitorId string, // HandleEvent receives event from KubeEventManager and returns a BindingExecutionInfo // to help create a new task to run a hook. -func (c *KubernetesBindingsController) HandleEvent(kubeEvent kemtypes.KubeEvent) BindingExecutionInfo { +func (c *kubernetesBindingsController) HandleEvent(kubeEvent kemtypes.KubeEvent) BindingExecutionInfo { link, hasKey := c.getBindingMonitorLinksById(kubeEvent.MonitorId) if !hasKey { log.Error("Possible bug!!! Unknown kube event: no such monitor id registered", slog.String("monitorID", kubeEvent.MonitorId)) @@ -220,7 +240,7 @@ func (c *KubernetesBindingsController) HandleEvent(kubeEvent kemtypes.KubeEvent) return bInfo } -func (c *KubernetesBindingsController) BindingNames() []string { +func (c *kubernetesBindingsController) BindingNames() []string { names := []string{} for _, binding := range c.KubernetesBindings { names = append(names, binding.BindingName) @@ -230,7 +250,7 @@ func (c *KubernetesBindingsController) BindingNames() []string { // SnapshotsFor returns snapshot for single onKubernetes binding. // It finds a monitorId for a binding name and returns an array of objects. -func (c *KubernetesBindingsController) SnapshotsFor(bindingName string) []kemtypes.ObjectAndFilterResult { +func (c *kubernetesBindingsController) SnapshotsFor(bindingName string) []kemtypes.ObjectAndFilterResult { for _, binding := range c.KubernetesBindings { if bindingName == binding.BindingName { monitorID := binding.Monitor.Metadata.MonitorId @@ -246,7 +266,7 @@ func (c *KubernetesBindingsController) SnapshotsFor(bindingName string) []kemtyp // SnapshotsFrom returns snapshot for several binding names. // It finds a monitorId for each binding name and get its Snapshot, // then returns a map of object arrays for each binding name. -func (c *KubernetesBindingsController) SnapshotsFrom(bindingNames ...string) map[string][]kemtypes.ObjectAndFilterResult { +func (c *kubernetesBindingsController) SnapshotsFrom(bindingNames ...string) map[string][]kemtypes.ObjectAndFilterResult { res := map[string][]kemtypes.ObjectAndFilterResult{} for _, bindingName := range bindingNames { @@ -262,11 +282,11 @@ func (c *KubernetesBindingsController) SnapshotsFrom(bindingNames ...string) map return res } -func (c *KubernetesBindingsController) Snapshots() map[string][]kemtypes.ObjectAndFilterResult { +func (c *kubernetesBindingsController) Snapshots() map[string][]kemtypes.ObjectAndFilterResult { return c.SnapshotsFrom(c.BindingNames()...) } -func (c *KubernetesBindingsController) SnapshotsInfo() []string { +func (c *kubernetesBindingsController) SnapshotsInfo() []string { infos := make([]string, 0) for _, binding := range c.KubernetesBindings { monitorID := binding.Monitor.Metadata.MonitorId @@ -291,7 +311,7 @@ func (c *KubernetesBindingsController) SnapshotsInfo() []string { return infos } -func (c *KubernetesBindingsController) SnapshotsDump() map[string]interface{} { +func (c *kubernetesBindingsController) SnapshotsDump() map[string]interface{} { dumps := make(map[string]interface{}) for _, binding := range c.KubernetesBindings { monitorID := binding.Monitor.Metadata.MonitorId diff --git a/pkg/hook/controller/schedule_bindings_controller.go b/pkg/hook/controller/schedule_bindings_controller.go index c61afbfb..e4d00fc9 100644 --- a/pkg/hook/controller/schedule_bindings_controller.go +++ b/pkg/hook/controller/schedule_bindings_controller.go @@ -19,10 +19,20 @@ type ScheduleBindingToCrontabLink struct { Group string } -// ScheduleBindingsController is a main implementation of KubernetesHooksController -type ScheduleBindingsController struct { +// ScheduleBindingsController handles schedule bindings for one hook. +type ScheduleBindingsController interface { + WithScheduleBindings([]htypes.ScheduleConfig) + WithScheduleManager(schedulemanager.ScheduleManager) + EnableScheduleBindings() + DisableScheduleBindings() + CanHandleEvent(crontab string) bool + HandleEvent(crontab string) []BindingExecutionInfo +} + +// scheduleBindingsController is a main implementation of KubernetesHooksController +type scheduleBindingsController struct { // dependencies - scheduleManager *schedulemanager.ScheduleManager + scheduleManager schedulemanager.ScheduleManager l sync.RWMutex // All hooks with 'kubernetes' bindings @@ -33,23 +43,23 @@ type ScheduleBindingsController struct { } // NewScheduleBindingsController returns an implementation of ScheduleBindingsController -var NewScheduleBindingsController = func() *ScheduleBindingsController { - return &ScheduleBindingsController{ +var NewScheduleBindingsController = func() ScheduleBindingsController { + return &scheduleBindingsController{ ScheduleLinks: make(map[string]*ScheduleBindingToCrontabLink), } } -func (c *ScheduleBindingsController) WithScheduleBindings(bindings []htypes.ScheduleConfig) { +func (c *scheduleBindingsController) WithScheduleBindings(bindings []htypes.ScheduleConfig) { c.ScheduleBindings = bindings } -func (c *ScheduleBindingsController) WithScheduleManager(scheduleManager *schedulemanager.ScheduleManager) { +func (c *scheduleBindingsController) WithScheduleManager(scheduleManager schedulemanager.ScheduleManager) { c.l.Lock() c.scheduleManager = scheduleManager c.l.Unlock() } -func (c *ScheduleBindingsController) CanHandleEvent(crontab string) bool { +func (c *scheduleBindingsController) CanHandleEvent(crontab string) bool { c.l.RLock() defer c.l.RUnlock() for _, link := range c.ScheduleLinks { @@ -60,7 +70,7 @@ func (c *ScheduleBindingsController) CanHandleEvent(crontab string) bool { return false } -func (c *ScheduleBindingsController) HandleEvent(crontab string) []BindingExecutionInfo { +func (c *scheduleBindingsController) HandleEvent(crontab string) []BindingExecutionInfo { res := []BindingExecutionInfo{} c.l.RLock() @@ -89,7 +99,7 @@ func (c *ScheduleBindingsController) HandleEvent(crontab string) []BindingExecut return res } -func (c *ScheduleBindingsController) EnableScheduleBindings() { +func (c *scheduleBindingsController) EnableScheduleBindings() { c.l.Lock() for _, config := range c.ScheduleBindings { c.ScheduleLinks[config.ScheduleEntry.Id] = &ScheduleBindingToCrontabLink{ @@ -105,7 +115,7 @@ func (c *ScheduleBindingsController) EnableScheduleBindings() { c.l.Unlock() } -func (c *ScheduleBindingsController) DisableScheduleBindings() { +func (c *scheduleBindingsController) DisableScheduleBindings() { c.l.Lock() for _, config := range c.ScheduleBindings { c.scheduleManager.Remove(config.ScheduleEntry) diff --git a/pkg/hook/hook_manager.go b/pkg/hook/hook_manager.go index 45491d16..1bff773a 100644 --- a/pkg/hook/hook_manager.go +++ b/pkg/hook/hook_manager.go @@ -10,9 +10,10 @@ import ( "sort" "strings" - "github.com/deckhouse/deckhouse/pkg/log" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/flant/shell-operator/pkg/app" "github.com/flant/shell-operator/pkg/executor" "github.com/flant/shell-operator/pkg/hook/controller" @@ -29,8 +30,8 @@ type Manager struct { // dependencies workingDir string tempDir string - kubeEventsManager *kubeeventsmanager.KubeEventsManager - scheduleManager *schedulemanager.ScheduleManager + kubeEventsManager kubeeventsmanager.KubeEventsManager + scheduleManager schedulemanager.ScheduleManager conversionWebhookManager *conversion.WebhookManager admissionWebhookManager *admission.WebhookManager @@ -52,8 +53,8 @@ type Manager struct { type ManagerConfig struct { WorkingDir string TempDir string - Kmgr *kubeeventsmanager.KubeEventsManager - Smgr *schedulemanager.ScheduleManager + Kmgr kubeeventsmanager.KubeEventsManager + Smgr schedulemanager.ScheduleManager Wmgr *admission.WebhookManager Cmgr *conversion.WebhookManager diff --git a/pkg/kube_events_manager/kube_events_manager.go b/pkg/kube_events_manager/kube_events_manager.go index 6249b8ad..5b0a815f 100644 --- a/pkg/kube_events_manager/kube_events_manager.go +++ b/pkg/kube_events_manager/kube_events_manager.go @@ -14,8 +14,20 @@ import ( "github.com/flant/shell-operator/pkg/metric" ) +type KubeEventsManager interface { + WithMetricStorage(mstor metric.Storage) + AddMonitor(monitorConfig *MonitorConfig) error + HasMonitor(monitorID string) bool + GetMonitor(monitorID string) Monitor + StartMonitor(monitorID string) + StopMonitor(monitorID string) error + + Ch() chan kemtypes.KubeEvent + PauseHandleEvents() +} + // KubeEventsManager is a main implementation of KubeEventsManager. -type KubeEventsManager struct { +type kubeEventsManager struct { // channel to emit KubeEvent objects KubeEventCh chan kemtypes.KubeEvent @@ -26,34 +38,34 @@ type KubeEventsManager struct { metricStorage metric.Storage m sync.RWMutex - Monitors map[string]*Monitor + Monitors map[string]Monitor logger *log.Logger } // NewKubeEventsManager returns an implementation of KubeEventsManager. -func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) *KubeEventsManager { +func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) KubeEventsManager { cctx, cancel := context.WithCancel(ctx) - em := &KubeEventsManager{ + em := &kubeEventsManager{ ctx: cctx, cancel: cancel, KubeClient: client, m: sync.RWMutex{}, - Monitors: make(map[string]*Monitor), + Monitors: make(map[string]Monitor), KubeEventCh: make(chan kemtypes.KubeEvent, 1), logger: logger, } return em } -func (mgr *KubeEventsManager) WithMetricStorage(mstor metric.Storage) { +func (mgr *kubeEventsManager) WithMetricStorage(mstor metric.Storage) { mgr.metricStorage = mstor } // AddMonitor creates a monitor with informers and return a KubeEvent with existing objects. // TODO cleanup informers in case of error // TODO use Context to stop informers -func (mgr *KubeEventsManager) AddMonitor(monitorConfig *MonitorConfig) error { +func (mgr *kubeEventsManager) AddMonitor(monitorConfig *MonitorConfig) error { log.Debug("Add MONITOR", slog.String("config", fmt.Sprintf("%+v", monitorConfig))) monitor := NewMonitor( @@ -81,7 +93,7 @@ func (mgr *KubeEventsManager) AddMonitor(monitorConfig *MonitorConfig) error { } // HasMonitor returns true if there is a monitor with monitorID. -func (mgr *KubeEventsManager) HasMonitor(monitorID string) bool { +func (mgr *kubeEventsManager) HasMonitor(monitorID string) bool { mgr.m.RLock() _, has := mgr.Monitors[monitorID] mgr.m.RUnlock() @@ -89,27 +101,22 @@ func (mgr *KubeEventsManager) HasMonitor(monitorID string) bool { } // GetMonitor returns monitor by its ID. -func (mgr *KubeEventsManager) GetMonitor(monitorID string) *Monitor { +func (mgr *kubeEventsManager) GetMonitor(monitorID string) Monitor { mgr.m.RLock() defer mgr.m.RUnlock() return mgr.Monitors[monitorID] } // StartMonitor starts all informers for the monitor. -func (mgr *KubeEventsManager) StartMonitor(monitorID string) { +func (mgr *kubeEventsManager) StartMonitor(monitorID string) { mgr.m.RLock() - monitor, ok := mgr.Monitors[monitorID] - if !ok { - mgr.m.RUnlock() - mgr.logger.Error("Monitor not found", slog.String("monitorID", monitorID)) - return - } + monitor := mgr.Monitors[monitorID] mgr.m.RUnlock() monitor.Start(mgr.ctx) } // StopMonitor stops monitor and removes it from the index. -func (mgr *KubeEventsManager) StopMonitor(monitorID string) error { +func (mgr *kubeEventsManager) StopMonitor(monitorID string) error { mgr.m.RLock() monitor, ok := mgr.Monitors[monitorID] mgr.m.RUnlock() @@ -123,14 +130,14 @@ func (mgr *KubeEventsManager) StopMonitor(monitorID string) error { } // Ch returns a channel to receive KubeEvent objects. -func (mgr *KubeEventsManager) Ch() chan kemtypes.KubeEvent { +func (mgr *kubeEventsManager) Ch() chan kemtypes.KubeEvent { return mgr.KubeEventCh } // PauseHandleEvents set flags for all informers to ignore incoming events. // Useful for shutdown without panicking. // Calling cancel() leads to a race and panicking, see https://github.com/kubernetes/kubernetes/issues/59822 -func (mgr *KubeEventsManager) PauseHandleEvents() { +func (mgr *kubeEventsManager) PauseHandleEvents() { mgr.m.RLock() defer mgr.m.RUnlock() for _, monitor := range mgr.Monitors { diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 9e1c9d71..7afad00a 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -15,13 +15,24 @@ import ( utils "github.com/flant/shell-operator/pkg/utils/labels" ) +type Monitor interface { + CreateInformers() error + Start(context.Context) + Stop() + PauseHandleEvents() + Snapshot() []kemtypes.ObjectAndFilterResult + EnableKubeEventCb() + GetConfig() *MonitorConfig + SnapshotOperations() (total *CachedObjectsInfo, last *CachedObjectsInfo) +} + // Monitor holds informers for resources and a namespace informer -type Monitor struct { +type monitor struct { Name string Config *MonitorConfig KubeClient *klient.Client // Static list of informers - ResourceInformers []*ResourceInformer + ResourceInformers []*resourceInformer // Namespace informer to get new namespaces NamespaceInformer *namespaceInformer // map of dynamically starting informers @@ -41,17 +52,17 @@ type Monitor struct { logger *log.Logger } -func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) *Monitor { +func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) Monitor { cctx, cancel := context.WithCancel(ctx) - return &Monitor{ + return &monitor{ ctx: cctx, cancel: cancel, KubeClient: client, metricStorage: mstor, Config: config, eventCb: eventCb, - ResourceInformers: make([]*ResourceInformer, 0), + ResourceInformers: make([]*resourceInformer, 0), VaryingInformers: sync.Map{}, cancelForNs: sync.Map{}, staticNamespaces: sync.Map{}, @@ -59,7 +70,7 @@ func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage } } -func (m *Monitor) GetConfig() *MonitorConfig { +func (m *monitor) GetConfig() *MonitorConfig { return m.Config } @@ -68,7 +79,7 @@ func (m *Monitor) GetConfig() *MonitorConfig { // If MonitorConfig.NamespaceSelector.MatchNames is defined, then // multiple informers are created for each namespace. // If no NamespaceSelector defined, then one informer is created. -func (m *Monitor) CreateInformers() error { +func (m *monitor) CreateInformers() error { logEntry := utils.EnrichLoggerWithLabels(m.logger, m.Config.Metadata.LogLabels). With("binding.name", m.Config.Metadata.DebugName) @@ -184,7 +195,7 @@ func (m *Monitor) CreateInformers() error { } // Snapshot returns all existed objects from all created informers -func (m *Monitor) Snapshot() []kemtypes.ObjectAndFilterResult { +func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { objects := make([]kemtypes.ObjectAndFilterResult, 0) for _, informer := range m.ResourceInformers { @@ -192,7 +203,7 @@ func (m *Monitor) Snapshot() []kemtypes.ObjectAndFilterResult { } m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*ResourceInformer); ok { + if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { objects = append(objects, informer.getCachedObjects()...) } @@ -208,13 +219,13 @@ func (m *Monitor) Snapshot() []kemtypes.ObjectAndFilterResult { // EnableKubeEventCb allows execution of event callback for all informers. // Also executes eventCb for events accumulated during "Synchronization" phase. -func (m *Monitor) EnableKubeEventCb() { +func (m *monitor) EnableKubeEventCb() { for _, informer := range m.ResourceInformers { informer.enableKubeEventCb() } // Execute eventCb for events accumulated during "Synchronization" phase. m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*ResourceInformer); ok { + if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { informer.enableKubeEventCb() } @@ -229,8 +240,8 @@ func (m *Monitor) EnableKubeEventCb() { // it is only one informer. If matchName is specified, then multiple informers are created. // // If namespace is empty, then informer is bounded to all namespaces. -func (m *Monitor) CreateInformersForNamespace(namespace string) ([]*ResourceInformer, error) { - informers := make([]*ResourceInformer, 0) +func (m *monitor) CreateInformersForNamespace(namespace string) ([]*resourceInformer, error) { + informers := make([]*resourceInformer, 0) cfg := &resourceInformerConfig{ client: m.KubeClient, mstor: m.metricStorage, @@ -259,7 +270,7 @@ func (m *Monitor) CreateInformersForNamespace(namespace string) ([]*ResourceInfo } // Start calls Run on all informers. -func (m *Monitor) Start(parentCtx context.Context) { +func (m *monitor) Start(parentCtx context.Context) { m.ctx, m.cancel = context.WithCancel(parentCtx) for _, informer := range m.ResourceInformers { @@ -274,7 +285,7 @@ func (m *Monitor) Start(parentCtx context.Context) { } ctx, cancelForNs := context.WithCancel(m.ctx) m.cancelForNs.Store(nsName, cancelForNs) - if value, ok := value.([]*ResourceInformer); ok { + if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { informer.withContext(ctx) informer.start() @@ -290,7 +301,7 @@ func (m *Monitor) Start(parentCtx context.Context) { } // Stop stops all informers -func (m *Monitor) Stop() { +func (m *monitor) Stop() { if m.cancel != nil { m.cancel() } @@ -299,13 +310,13 @@ func (m *Monitor) Stop() { // PauseHandleEvents set flags for all informers to ignore incoming events. // Useful for shutdown without panicking. // Calling cancel() leads to a race and panicking, see https://github.com/kubernetes/kubernetes/issues/59822 -func (m *Monitor) PauseHandleEvents() { +func (m *monitor) PauseHandleEvents() { for _, informer := range m.ResourceInformers { informer.pauseHandleEvents() } m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*ResourceInformer); ok { + if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { informer.pauseHandleEvents() } @@ -318,7 +329,7 @@ func (m *Monitor) PauseHandleEvents() { } } -func (m *Monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObjectsInfo /*last*/) { +func (m *monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObjectsInfo /*last*/) { total := &CachedObjectsInfo{} last := &CachedObjectsInfo{} @@ -328,7 +339,7 @@ func (m *Monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObj } m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*ResourceInformer); ok { + if value, ok := value.([]*resourceInformer); ok { for _, informer := range value { total.add(informer.getCachedObjectsInfo()) last.add(informer.getCachedObjectsInfoIncrement()) diff --git a/pkg/kube_events_manager/resource_informer.go b/pkg/kube_events_manager/resource_informer.go index 5e294c28..4defeae2 100644 --- a/pkg/kube_events_manager/resource_informer.go +++ b/pkg/kube_events_manager/resource_informer.go @@ -8,13 +8,14 @@ import ( "sync" "time" - "github.com/deckhouse/deckhouse/pkg/log" "github.com/gofrs/uuid/v5" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/cache" + "github.com/deckhouse/deckhouse/pkg/log" + klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/app" "github.com/flant/shell-operator/pkg/filter/jq" @@ -23,7 +24,7 @@ import ( "github.com/flant/shell-operator/pkg/utils/measure" ) -type ResourceInformer struct { +type resourceInformer struct { id string KubeClient *klient.Client Monitor *MonitorConfig @@ -54,7 +55,7 @@ type ResourceInformer struct { eventCbEnabled bool eventBufLock sync.Mutex - // TODO ResourceInformer should be stoppable (think of deleted namespaces and disabled modules in addon-operator) + // TODO resourceInformer should be stoppable (think of deleted namespaces and disabled modules in addon-operator) ctx context.Context cancel context.CancelFunc @@ -66,7 +67,7 @@ type ResourceInformer struct { logger *log.Logger } -// ResourceInformer should implement ResourceInformer +// resourceInformer should implement resourceInformer type resourceInformerConfig struct { client *klient.Client mstor metric.Storage @@ -76,8 +77,8 @@ type resourceInformerConfig struct { logger *log.Logger } -func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *ResourceInformer { - informer := &ResourceInformer{ +func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *resourceInformer { + informer := &resourceInformer{ id: uuid.Must(uuid.NewV4()).String(), KubeClient: cfg.client, metricStorage: cfg.mstor, @@ -95,17 +96,17 @@ func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *Resource return informer } -func (ei *ResourceInformer) withContext(ctx context.Context) { +func (ei *resourceInformer) withContext(ctx context.Context) { ei.ctx, ei.cancel = context.WithCancel(ctx) } -func (ei *ResourceInformer) putEvent(ev kemtypes.KubeEvent) { +func (ei *resourceInformer) putEvent(ev kemtypes.KubeEvent) { if ei.eventCb != nil { ei.eventCb(ev) } } -func (ei *ResourceInformer) createSharedInformer() error { +func (ei *resourceInformer) createSharedInformer() error { var err error // discover GroupVersionResource for informer @@ -153,7 +154,7 @@ func (ei *ResourceInformer) createSharedInformer() error { } // Snapshot returns all cached objects for this informer -func (ei *ResourceInformer) getCachedObjects() []kemtypes.ObjectAndFilterResult { +func (ei *resourceInformer) getCachedObjects() []kemtypes.ObjectAndFilterResult { ei.cacheLock.RLock() res := make([]kemtypes.ObjectAndFilterResult, 0) for _, obj := range ei.cachedObjects { @@ -170,7 +171,7 @@ func (ei *ResourceInformer) getCachedObjects() []kemtypes.ObjectAndFilterResult return res } -func (ei *ResourceInformer) enableKubeEventCb() { +func (ei *resourceInformer) enableKubeEventCb() { ei.eventBufLock.Lock() defer ei.eventBufLock.Unlock() if ei.eventCbEnabled { @@ -186,7 +187,7 @@ func (ei *ResourceInformer) enableKubeEventCb() { // loadExistedObjects get a list of existed objects in namespace that match selectors and // fills Checksum map with checksums of existing objects. -func (ei *ResourceInformer) loadExistedObjects() error { +func (ei *resourceInformer) loadExistedObjects() error { defer trace.StartRegion(context.Background(), "loadExistedObjects").End() objList, err := ei.KubeClient.Dynamic(). @@ -260,23 +261,23 @@ func (ei *ResourceInformer) loadExistedObjects() error { return nil } -func (ei *ResourceInformer) OnAdd(obj interface{}, _ bool) { +func (ei *resourceInformer) OnAdd(obj interface{}, _ bool) { ei.handleWatchEvent(obj, kemtypes.WatchEventAdded) } -func (ei *ResourceInformer) OnUpdate(_, newObj interface{}) { +func (ei *resourceInformer) OnUpdate(_, newObj interface{}) { ei.handleWatchEvent(newObj, kemtypes.WatchEventModified) } -func (ei *ResourceInformer) OnDelete(obj interface{}) { +func (ei *resourceInformer) OnDelete(obj interface{}) { ei.handleWatchEvent(obj, kemtypes.WatchEventDeleted) } // HandleKubeEvent register object in cache. Pass object to callback if object's checksum is changed. // TODO refactor: pass KubeEvent as argument // TODO add delay to merge Added and Modified events (node added and then labels applied — one hook run on Added+Modified is enough) -// func (ei *ResourceInformer) HandleKubeEvent(obj *unstructured.Unstructured, objectId string, filterResult string, newChecksum string, eventType WatchEventType) { -func (ei *ResourceInformer) handleWatchEvent(object interface{}, eventType kemtypes.WatchEventType) { +// func (ei *resourceInformer) HandleKubeEvent(obj *unstructured.Unstructured, objectId string, filterResult string, newChecksum string, eventType WatchEventType) { +func (ei *resourceInformer) handleWatchEvent(object interface{}, eventType kemtypes.WatchEventType) { // check if stop if ei.stopped { log.Debug("received WATCH for stopped informer", @@ -411,7 +412,7 @@ func (ei *ResourceInformer) handleWatchEvent(object interface{}, eventType kemty } } -func (ei *ResourceInformer) adjustFieldSelector(selector *kemtypes.FieldSelector, objName string) *kemtypes.FieldSelector { +func (ei *resourceInformer) adjustFieldSelector(selector *kemtypes.FieldSelector, objName string) *kemtypes.FieldSelector { var selectorCopy *kemtypes.FieldSelector if selector != nil { @@ -440,7 +441,7 @@ func (ei *ResourceInformer) adjustFieldSelector(selector *kemtypes.FieldSelector return selectorCopy } -func (ei *ResourceInformer) shouldFireEvent(checkEvent kemtypes.WatchEventType) bool { +func (ei *resourceInformer) shouldFireEvent(checkEvent kemtypes.WatchEventType) bool { for _, event := range ei.Monitor.EventTypes { if event == checkEvent { return true @@ -449,7 +450,7 @@ func (ei *ResourceInformer) shouldFireEvent(checkEvent kemtypes.WatchEventType) return false } -func (ei *ResourceInformer) start() { +func (ei *resourceInformer) start() { log.Debug("RUN resource informer", slog.String("debugName", ei.Monitor.Metadata.DebugName)) go func() { @@ -470,20 +471,20 @@ func (ei *ResourceInformer) start() { log.Debug("informer is ready", slog.String("debugName", ei.Monitor.Metadata.DebugName)) } -func (ei *ResourceInformer) pauseHandleEvents() { +func (ei *resourceInformer) pauseHandleEvents() { log.Debug("PAUSE resource informer", slog.String("debugName", ei.Monitor.Metadata.DebugName)) ei.stopped = true } // CachedObjectsInfo returns info accumulated from start. -func (ei *ResourceInformer) getCachedObjectsInfo() CachedObjectsInfo { +func (ei *resourceInformer) getCachedObjectsInfo() CachedObjectsInfo { ei.cacheLock.RLock() defer ei.cacheLock.RUnlock() return *ei.cachedObjectsInfo } // getCachedObjectsInfoIncrement returns info accumulated from last call and clean it. -func (ei *ResourceInformer) getCachedObjectsInfoIncrement() CachedObjectsInfo { +func (ei *resourceInformer) getCachedObjectsInfoIncrement() CachedObjectsInfo { ei.cacheLock.Lock() defer ei.cacheLock.Unlock() info := *ei.cachedObjectsIncrement diff --git a/pkg/schedule_manager/schedule_manager.go b/pkg/schedule_manager/schedule_manager.go index 8f7dc06d..8e70fdee 100644 --- a/pkg/schedule_manager/schedule_manager.go +++ b/pkg/schedule_manager/schedule_manager.go @@ -4,9 +4,10 @@ import ( "context" "log/slog" - "github.com/deckhouse/deckhouse/pkg/log" "gopkg.in/robfig/cron.v2" + "github.com/deckhouse/deckhouse/pkg/log" + smtypes "github.com/flant/shell-operator/pkg/schedule_manager/types" ) @@ -15,7 +16,14 @@ type CronEntry struct { Ids map[string]bool } -type ScheduleManager struct { +type ScheduleManager interface { + Stop() + Start() + Add(entry smtypes.ScheduleEntry) + Remove(entry smtypes.ScheduleEntry) + Ch() chan string +} +type scheduleManager struct { ctx context.Context cancel context.CancelFunc cron *cron.Cron @@ -25,9 +33,9 @@ type ScheduleManager struct { logger *log.Logger } -func NewScheduleManager(ctx context.Context, logger *log.Logger) *ScheduleManager { +func NewScheduleManager(ctx context.Context, logger *log.Logger) ScheduleManager { cctx, cancel := context.WithCancel(ctx) - sm := &ScheduleManager{ + sm := &scheduleManager{ ctx: cctx, cancel: cancel, ScheduleCh: make(chan string, 1), @@ -39,7 +47,7 @@ func NewScheduleManager(ctx context.Context, logger *log.Logger) *ScheduleManage return sm } -func (sm *ScheduleManager) Stop() { +func (sm *scheduleManager) Stop() { if sm.cancel != nil { sm.cancel() } @@ -48,7 +56,7 @@ func (sm *ScheduleManager) Stop() { // Add create entry for crontab and id and start scheduled function. // Crontab string should be validated with cron.Parse // function before pass to Add. -func (sm *ScheduleManager) Add(newEntry smtypes.ScheduleEntry) { +func (sm *scheduleManager) Add(newEntry smtypes.ScheduleEntry) { logEntry := sm.logger.With("operator.component", "scheduleManager") cronEntry, hasCronEntry := sm.Entries[newEntry.Crontab] @@ -79,7 +87,7 @@ func (sm *ScheduleManager) Add(newEntry smtypes.ScheduleEntry) { } } -func (sm *ScheduleManager) Remove(delEntry smtypes.ScheduleEntry) { +func (sm *scheduleManager) Remove(delEntry smtypes.ScheduleEntry) { cronEntry, hasCronEntry := sm.Entries[delEntry.Crontab] // Nothing to Remove @@ -105,7 +113,7 @@ func (sm *ScheduleManager) Remove(delEntry smtypes.ScheduleEntry) { } } -func (sm *ScheduleManager) Start() { +func (sm *scheduleManager) Start() { sm.cron.Start() go func() { <-sm.ctx.Done() @@ -113,6 +121,6 @@ func (sm *ScheduleManager) Start() { }() } -func (sm *ScheduleManager) Ch() chan string { +func (sm *scheduleManager) Ch() chan string { return sm.ScheduleCh } diff --git a/pkg/shell-operator/manager_events_handler.go b/pkg/shell-operator/manager_events_handler.go index 8a5f9354..5d790edf 100644 --- a/pkg/shell-operator/manager_events_handler.go +++ b/pkg/shell-operator/manager_events_handler.go @@ -15,8 +15,8 @@ import ( type managerEventsHandlerConfig struct { tqs *queue.TaskQueueSet - mgr *kubeeventsmanager.KubeEventsManager - smgr *schedulemanager.ScheduleManager + mgr kubeeventsmanager.KubeEventsManager + smgr schedulemanager.ScheduleManager logger *log.Logger } @@ -25,8 +25,8 @@ type ManagerEventsHandler struct { ctx context.Context cancel context.CancelFunc - kubeEventsManager *kubeeventsmanager.KubeEventsManager - scheduleManager *schedulemanager.ScheduleManager + kubeEventsManager kubeeventsmanager.KubeEventsManager + scheduleManager schedulemanager.ScheduleManager kubeEventCb func(kubeEvent kemtypes.KubeEvent) []task.Task scheduleCb func(crontab string) []task.Task diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index 87db5f4c..26321ce6 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -6,10 +6,11 @@ import ( "log/slog" "time" - "github.com/deckhouse/deckhouse/pkg/log" "github.com/gofrs/uuid/v5" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "github.com/deckhouse/deckhouse/pkg/log" + klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/hook" bindingcontext "github.com/flant/shell-operator/pkg/hook/binding_context" @@ -47,8 +48,8 @@ type ShellOperator struct { KubeClient *klient.Client ObjectPatcher *objectpatch.ObjectPatcher - ScheduleManager *schedulemanager.ScheduleManager - KubeEventsManager *kubeeventsmanager.KubeEventsManager + ScheduleManager schedulemanager.ScheduleManager + KubeEventsManager kubeeventsmanager.KubeEventsManager TaskQueues *queue.TaskQueueSet @@ -88,9 +89,6 @@ func NewShellOperator(ctx context.Context, opts ...Option) *ShellOperator { so.logger = log.NewLogger(log.Options{}).Named("shell-operator") } - so.ScheduleManager = &schedulemanager.ScheduleManager{} - so.KubeEventsManager = &kubeeventsmanager.KubeEventsManager{} - return so } diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 4795c453..331fd4af 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -36,8 +36,8 @@ type BindingContextController struct { HookConfig string Controller *StateController - KubeEventsManager *kubeeventsmanager.KubeEventsManager - ScheduleManager *schedulemanager.ScheduleManager + KubeEventsManager kubeeventsmanager.KubeEventsManager + ScheduleManager schedulemanager.ScheduleManager fakeCluster *fake.Cluster diff --git a/test/hook/context/state.go b/test/hook/context/state.go index 4045845f..d99a965d 100644 --- a/test/hook/context/state.go +++ b/test/hook/context/state.go @@ -24,7 +24,7 @@ type StateController struct { } // NewStateController creates controller to apply state changes -func NewStateController(fc *fake.Cluster, ev *kubeeventsmanager.KubeEventsManager) *StateController { +func NewStateController(fc *fake.Cluster, ev kubeeventsmanager.KubeEventsManager) *StateController { c := &StateController{ CurrentState: make(map[string]manifest.Manifest), fakeCluster: fc, diff --git a/test/integration/kube_event_manager/kube_event_manager_test.go b/test/integration/kube_event_manager/kube_event_manager_test.go index b29315cf..779742b5 100644 --- a/test/integration/kube_event_manager/kube_event_manager_test.go +++ b/test/integration/kube_event_manager/kube_event_manager_test.go @@ -11,12 +11,12 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/deckhouse/deckhouse/pkg/log" "github.com/flant/shell-operator/pkg/app" kubeeventsmanager "github.com/flant/shell-operator/pkg/kube_events_manager" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" - . "github.com/flant/shell-operator/test/integration/suite" testutils "github.com/flant/shell-operator/test/utils" + + "github.com/deckhouse/deckhouse/pkg/log" ) func Test(t *testing.T) { @@ -24,7 +24,7 @@ func Test(t *testing.T) { } var _ = Describe("Binding 'kubernetes' with kind 'Pod' should emit KubeEvent objects", func() { - var KubeEventsManager *kubeeventsmanager.KubeEventsManager + var KubeEventsManager kubeeventsmanager.KubeEventsManager BeforeEach(func() { KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(context.Background(), KubeClient, log.NewNop()) From 933ef2d458cf11496bfc48108f006db63a37d725 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 10:51:45 +0300 Subject: [PATCH 15/25] refactor: replace assert with require in tests for improved error handling Signed-off-by: Evsyukov Denis --- .../kube_events_manager_test.go | 9 ++++--- pkg/kube_events_manager/monitor_test.go | 24 ++----------------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/pkg/kube_events_manager/kube_events_manager_test.go b/pkg/kube_events_manager/kube_events_manager_test.go index 0d4c92ed..43bd3669 100644 --- a/pkg/kube_events_manager/kube_events_manager_test.go +++ b/pkg/kube_events_manager/kube_events_manager_test.go @@ -6,14 +6,16 @@ import ( "testing" "time" - "github.com/deckhouse/deckhouse/pkg/log" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/version" fakediscovery "k8s.io/client-go/discovery/fake" + "github.com/deckhouse/deckhouse/pkg/log" + klient "github.com/flant/kube-client/client" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" ) @@ -70,9 +72,7 @@ func Test_MainKubeEventsManager_Run(t *testing.T) { err := mgr.AddMonitor(monitor) - if assert.NoError(t, err) { - assert.Len(t, mgr.Monitors, 1) - } + require.NoError(t, err) } // FIXME: sometimes fails, skip for now. @@ -139,7 +139,6 @@ func Test_MainKubeEventsManager_HandleEvents(t *testing.T) { // Init() replacement mgr := NewKubeEventsManager(ctx, kubeClient, log.NewNop()) - mgr.KubeEventCh = make(chan kemtypes.KubeEvent, 10) // monitor with 3 namespaces and 4 object names and all event types monitor := &MonitorConfig{ diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index fd6f873d..31236679 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -6,12 +6,13 @@ import ( "sync" "testing" - "github.com/deckhouse/deckhouse/pkg/log" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/flant/kube-client/fake" "github.com/flant/kube-client/manifest" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" @@ -75,13 +76,6 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { // create new ns with matching labels and then create new ConfigMap. createNsWithLabels(fc, "test-ns-1", map[string]string{"test-label": ""}) - // Wait until informers appears. - g.Eventually(func() bool { - _, exists := mon.VaryingInformers.Load("test-ns-1") - return exists - }, "5s", "10ms"). - Should(BeTrue(), "Should create informer for new namespace") - createCM(fc, "test-ns-1", testCM("cm-1")) // Should update snapshot with new objects. @@ -120,13 +114,6 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { // Create new ns with labels and cm there. createNsWithLabels(fc, "test-ns-2", map[string]string{"test-label": ""}) - // Monitor should create new configmap informer for new namespace. - g.Eventually(func() bool { - _, exists := mon.VaryingInformers.Load("test-ns-2") - return exists - }, "5s", "10ms"). - Should(BeTrue(), "Should create informer for ns/test-ns-2") - // Create new ConfigMap after Synchronization. createCM(fc, "test-ns-2", testCM("cm-2-1")) @@ -145,13 +132,6 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { // Add non-matched Namespace. createNsWithLabels(fc, "test-ns-non-matched", map[string]string{"non-matched-label": ""}) - - // Monitor should create new configmap informer for new namespace. - g.Eventually(func() bool { - _, exists := mon.VaryingInformers.Load("test-ns-non-matched") - return exists - }, "5s", "10ms"). - ShouldNot(BeTrue(), "Should not create informer for non-mathed Namespace") } func createNsWithLabels(fc *fake.Cluster, name string, labels map[string]string) { From 7a5d34353553fd9a33972ac5eb1a4d37a1371ec6 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 11:02:07 +0300 Subject: [PATCH 16/25] refactor: change function return types to pointers for KubeEventsManager and ScheduleManager for consistency Signed-off-by: Evsyukov Denis --- pkg/hook/controller/hook_controller.go | 3 +-- .../controller/kubernetes_bindings_controller.go | 5 ++++- .../controller/schedule_bindings_controller.go | 5 ++++- pkg/hook/hook_manager.go | 3 +-- pkg/kube_events_manager/kube_events_manager.go | 7 +++++-- .../kube_events_manager_test.go | 9 +++++---- pkg/kube_events_manager/monitor.go | 4 +++- pkg/kube_events_manager/monitor_test.go | 3 +-- pkg/kube_events_manager/resource_informer.go | 5 ++--- pkg/schedule_manager/schedule_manager.go | 15 +++++++++------ pkg/shell-operator/operator.go | 3 +-- 11 files changed, 36 insertions(+), 26 deletions(-) diff --git a/pkg/hook/controller/hook_controller.go b/pkg/hook/controller/hook_controller.go index 4a8292c9..b332ccbe 100644 --- a/pkg/hook/controller/hook_controller.go +++ b/pkg/hook/controller/hook_controller.go @@ -1,9 +1,8 @@ package controller import ( - v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "github.com/deckhouse/deckhouse/pkg/log" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" bctx "github.com/flant/shell-operator/pkg/hook/binding_context" htypes "github.com/flant/shell-operator/pkg/hook/types" diff --git a/pkg/hook/controller/kubernetes_bindings_controller.go b/pkg/hook/controller/kubernetes_bindings_controller.go index 07a726e1..f7d6e38e 100644 --- a/pkg/hook/controller/kubernetes_bindings_controller.go +++ b/pkg/hook/controller/kubernetes_bindings_controller.go @@ -55,8 +55,11 @@ type kubernetesBindingsController struct { BindingMonitorLinks map[string]*KubernetesBindingToMonitorLink } +// kubernetesHooksController should implement the KubernetesHooksController +var _ KubernetesBindingsController = (*kubernetesBindingsController)(nil) + // NewKubernetesBindingsController returns an implementation of KubernetesBindingsController -var NewKubernetesBindingsController = func(logger *log.Logger) KubernetesBindingsController { +var NewKubernetesBindingsController = func(logger *log.Logger) *kubernetesBindingsController { return &kubernetesBindingsController{ BindingMonitorLinks: make(map[string]*KubernetesBindingToMonitorLink), logger: logger, diff --git a/pkg/hook/controller/schedule_bindings_controller.go b/pkg/hook/controller/schedule_bindings_controller.go index e4d00fc9..de595358 100644 --- a/pkg/hook/controller/schedule_bindings_controller.go +++ b/pkg/hook/controller/schedule_bindings_controller.go @@ -42,8 +42,11 @@ type scheduleBindingsController struct { ScheduleBindings []htypes.ScheduleConfig } +// kubernetesHooksController should implement the KubernetesHooksController +var _ ScheduleBindingsController = (*scheduleBindingsController)(nil) + // NewScheduleBindingsController returns an implementation of ScheduleBindingsController -var NewScheduleBindingsController = func() ScheduleBindingsController { +var NewScheduleBindingsController = func() *scheduleBindingsController { return &scheduleBindingsController{ ScheduleLinks: make(map[string]*ScheduleBindingToCrontabLink), } diff --git a/pkg/hook/hook_manager.go b/pkg/hook/hook_manager.go index 1bff773a..2fd9f178 100644 --- a/pkg/hook/hook_manager.go +++ b/pkg/hook/hook_manager.go @@ -10,9 +10,8 @@ import ( "sort" "strings" - v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "github.com/deckhouse/deckhouse/pkg/log" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "github.com/flant/shell-operator/pkg/app" "github.com/flant/shell-operator/pkg/executor" diff --git a/pkg/kube_events_manager/kube_events_manager.go b/pkg/kube_events_manager/kube_events_manager.go index 5b0a815f..14a381c2 100644 --- a/pkg/kube_events_manager/kube_events_manager.go +++ b/pkg/kube_events_manager/kube_events_manager.go @@ -26,7 +26,7 @@ type KubeEventsManager interface { PauseHandleEvents() } -// KubeEventsManager is a main implementation of KubeEventsManager. +// kubeEventsManager is a main implementation of KubeEventsManager. type kubeEventsManager struct { // channel to emit KubeEvent objects KubeEventCh chan kemtypes.KubeEvent @@ -43,8 +43,11 @@ type kubeEventsManager struct { logger *log.Logger } +// kubeEventsManager should implement KubeEventsManager. +var _ KubeEventsManager = (*kubeEventsManager)(nil) + // NewKubeEventsManager returns an implementation of KubeEventsManager. -func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) KubeEventsManager { +func NewKubeEventsManager(ctx context.Context, client *klient.Client, logger *log.Logger) *kubeEventsManager { cctx, cancel := context.WithCancel(ctx) em := &kubeEventsManager{ ctx: cctx, diff --git a/pkg/kube_events_manager/kube_events_manager_test.go b/pkg/kube_events_manager/kube_events_manager_test.go index 43bd3669..0d4c92ed 100644 --- a/pkg/kube_events_manager/kube_events_manager_test.go +++ b/pkg/kube_events_manager/kube_events_manager_test.go @@ -6,16 +6,14 @@ import ( "testing" "time" + "github.com/deckhouse/deckhouse/pkg/log" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/version" fakediscovery "k8s.io/client-go/discovery/fake" - "github.com/deckhouse/deckhouse/pkg/log" - klient "github.com/flant/kube-client/client" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" ) @@ -72,7 +70,9 @@ func Test_MainKubeEventsManager_Run(t *testing.T) { err := mgr.AddMonitor(monitor) - require.NoError(t, err) + if assert.NoError(t, err) { + assert.Len(t, mgr.Monitors, 1) + } } // FIXME: sometimes fails, skip for now. @@ -139,6 +139,7 @@ func Test_MainKubeEventsManager_HandleEvents(t *testing.T) { // Init() replacement mgr := NewKubeEventsManager(ctx, kubeClient, log.NewNop()) + mgr.KubeEventCh = make(chan kemtypes.KubeEvent, 10) // monitor with 3 namespaces and 4 object names and all event types monitor := &MonitorConfig{ diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 7afad00a..a063100e 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -52,7 +52,9 @@ type monitor struct { logger *log.Logger } -func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) Monitor { +var _ Monitor = (*monitor)(nil) + +func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) *monitor { cctx, cancel := context.WithCancel(ctx) return &monitor{ diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index 31236679..760b40f9 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -6,13 +6,12 @@ import ( "sync" "testing" + "github.com/deckhouse/deckhouse/pkg/log" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/deckhouse/deckhouse/pkg/log" - "github.com/flant/kube-client/fake" "github.com/flant/kube-client/manifest" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" diff --git a/pkg/kube_events_manager/resource_informer.go b/pkg/kube_events_manager/resource_informer.go index 4defeae2..443877fd 100644 --- a/pkg/kube_events_manager/resource_informer.go +++ b/pkg/kube_events_manager/resource_informer.go @@ -8,14 +8,13 @@ import ( "sync" "time" + "github.com/deckhouse/deckhouse/pkg/log" "github.com/gofrs/uuid/v5" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/cache" - "github.com/deckhouse/deckhouse/pkg/log" - klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/app" "github.com/flant/shell-operator/pkg/filter/jq" @@ -67,7 +66,7 @@ type resourceInformer struct { logger *log.Logger } -// resourceInformer should implement resourceInformer +// resourceInformer should implement ResourceInformer type resourceInformerConfig struct { client *klient.Client mstor metric.Storage diff --git a/pkg/schedule_manager/schedule_manager.go b/pkg/schedule_manager/schedule_manager.go index 8e70fdee..705c0265 100644 --- a/pkg/schedule_manager/schedule_manager.go +++ b/pkg/schedule_manager/schedule_manager.go @@ -11,11 +11,6 @@ import ( smtypes "github.com/flant/shell-operator/pkg/schedule_manager/types" ) -type CronEntry struct { - EntryID cron.EntryID - Ids map[string]bool -} - type ScheduleManager interface { Stop() Start() @@ -23,6 +18,12 @@ type ScheduleManager interface { Remove(entry smtypes.ScheduleEntry) Ch() chan string } + +type CronEntry struct { + EntryID cron.EntryID + Ids map[string]bool +} + type scheduleManager struct { ctx context.Context cancel context.CancelFunc @@ -33,7 +34,9 @@ type scheduleManager struct { logger *log.Logger } -func NewScheduleManager(ctx context.Context, logger *log.Logger) ScheduleManager { +var _ ScheduleManager = (*scheduleManager)(nil) + +func NewScheduleManager(ctx context.Context, logger *log.Logger) *scheduleManager { cctx, cancel := context.WithCancel(ctx) sm := &scheduleManager{ ctx: cctx, diff --git a/pkg/shell-operator/operator.go b/pkg/shell-operator/operator.go index 26321ce6..aa3d5da8 100644 --- a/pkg/shell-operator/operator.go +++ b/pkg/shell-operator/operator.go @@ -6,11 +6,10 @@ import ( "log/slog" "time" + "github.com/deckhouse/deckhouse/pkg/log" "github.com/gofrs/uuid/v5" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "github.com/deckhouse/deckhouse/pkg/log" - klient "github.com/flant/kube-client/client" "github.com/flant/shell-operator/pkg/hook" bindingcontext "github.com/flant/shell-operator/pkg/hook/binding_context" From 388196741c8d21b58d0ec83713e3078ee023b9e6 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 11:09:45 +0300 Subject: [PATCH 17/25] refactor: add integration test cases for namespace informers in monitor tests Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor_test.go | 12 ++++++++++++ pkg/schedule_manager/schedule_manager.go | 3 +-- .../kube_event_manager/kube_event_manager_test.go | 3 ++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index 760b40f9..32710d55 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -75,6 +75,10 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { // create new ns with matching labels and then create new ConfigMap. createNsWithLabels(fc, "test-ns-1", map[string]string{"test-label": ""}) + // Wait until informers appears. + g.Eventually(&mon.VaryingInformers, "5s", "10ms"). + Should(HaveKey("test-ns-1"), "Should create informer for new namespace") + createCM(fc, "test-ns-1", testCM("cm-1")) // Should update snapshot with new objects. @@ -113,6 +117,10 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { // Create new ns with labels and cm there. createNsWithLabels(fc, "test-ns-2", map[string]string{"test-label": ""}) + // Monitor should create new configmap informer for new namespace. + g.Eventually(&mon.VaryingInformers, "5s", "10ms"). + Should(HaveKey("test-ns-2"), "Should create informer for ns/test-ns-2") + // Create new ConfigMap after Synchronization. createCM(fc, "test-ns-2", testCM("cm-2-1")) @@ -131,6 +139,10 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { // Add non-matched Namespace. createNsWithLabels(fc, "test-ns-non-matched", map[string]string{"non-matched-label": ""}) + + // Monitor should create new configmap informer for new namespace. + g.Eventually(&mon.VaryingInformers, "5s", "10ms"). + ShouldNot(HaveKey("test-ns-non-matched"), "Should not create informer for non-mathed Namespace") } func createNsWithLabels(fc *fake.Cluster, name string, labels map[string]string) { diff --git a/pkg/schedule_manager/schedule_manager.go b/pkg/schedule_manager/schedule_manager.go index 705c0265..fb765b25 100644 --- a/pkg/schedule_manager/schedule_manager.go +++ b/pkg/schedule_manager/schedule_manager.go @@ -4,9 +4,8 @@ import ( "context" "log/slog" - "gopkg.in/robfig/cron.v2" - "github.com/deckhouse/deckhouse/pkg/log" + "gopkg.in/robfig/cron.v2" smtypes "github.com/flant/shell-operator/pkg/schedule_manager/types" ) diff --git a/test/integration/kube_event_manager/kube_event_manager_test.go b/test/integration/kube_event_manager/kube_event_manager_test.go index 779742b5..c15727eb 100644 --- a/test/integration/kube_event_manager/kube_event_manager_test.go +++ b/test/integration/kube_event_manager/kube_event_manager_test.go @@ -14,7 +14,8 @@ import ( "github.com/flant/shell-operator/pkg/app" kubeeventsmanager "github.com/flant/shell-operator/pkg/kube_events_manager" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" - testutils "github.com/flant/shell-operator/test/utils" + . "github.com/flant/shell-operator/test/integration/suite" + "github.com/deckhouse/deckhouse/pkg/log" ) From 3e00eac37163d1ec4092df6c9eb1320658af72bd Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 11:12:04 +0300 Subject: [PATCH 18/25] refactor: update informer checks in monitor tests for improved clarity and consistency Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor_test.go | 24 +++++++++++++------ .../kube_event_manager_test.go | 5 ++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index 32710d55..ad25399c 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -6,12 +6,13 @@ import ( "sync" "testing" - "github.com/deckhouse/deckhouse/pkg/log" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/deckhouse/deckhouse/pkg/log" + "github.com/flant/kube-client/fake" "github.com/flant/kube-client/manifest" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" @@ -76,8 +77,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { createNsWithLabels(fc, "test-ns-1", map[string]string{"test-label": ""}) // Wait until informers appears. - g.Eventually(&mon.VaryingInformers, "5s", "10ms"). - Should(HaveKey("test-ns-1"), "Should create informer for new namespace") + g.Eventually(func() bool { + _, ok := mon.VaryingInformers.Load("test-ns-1") + return ok + }, "5s", "10ms"). + Should(BeTrue(), "Should create informer for new namespace") createCM(fc, "test-ns-1", testCM("cm-1")) @@ -118,8 +122,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { createNsWithLabels(fc, "test-ns-2", map[string]string{"test-label": ""}) // Monitor should create new configmap informer for new namespace. - g.Eventually(&mon.VaryingInformers, "5s", "10ms"). - Should(HaveKey("test-ns-2"), "Should create informer for ns/test-ns-2") + g.Eventually(func() bool { + _, ok := mon.VaryingInformers.Load("test-ns-2") + return ok + }, "5s", "10ms"). + Should(BeTrue(), "Should create informer for ns/test-ns-2") // Create new ConfigMap after Synchronization. createCM(fc, "test-ns-2", testCM("cm-2-1")) @@ -141,8 +148,11 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { createNsWithLabels(fc, "test-ns-non-matched", map[string]string{"non-matched-label": ""}) // Monitor should create new configmap informer for new namespace. - g.Eventually(&mon.VaryingInformers, "5s", "10ms"). - ShouldNot(HaveKey("test-ns-non-matched"), "Should not create informer for non-mathed Namespace") + g.Eventually(func() bool { + _, ok := mon.VaryingInformers.Load("test-ns-non-matched") + return ok + }, "5s", "10ms"). + ShouldNot(BeTrue(), "Should not create informer for non-mathed Namespace") } func createNsWithLabels(fc *fake.Cluster, name string, labels map[string]string) { diff --git a/test/integration/kube_event_manager/kube_event_manager_test.go b/test/integration/kube_event_manager/kube_event_manager_test.go index c15727eb..b2813d44 100644 --- a/test/integration/kube_event_manager/kube_event_manager_test.go +++ b/test/integration/kube_event_manager/kube_event_manager_test.go @@ -11,13 +11,12 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/deckhouse/deckhouse/pkg/log" "github.com/flant/shell-operator/pkg/app" kubeeventsmanager "github.com/flant/shell-operator/pkg/kube_events_manager" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" . "github.com/flant/shell-operator/test/integration/suite" - - - "github.com/deckhouse/deckhouse/pkg/log" + testutils "github.com/flant/shell-operator/test/utils" ) func Test(t *testing.T) { From 4484a577f517e2a77587e30962f0615890a11ddb Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 11:18:26 +0300 Subject: [PATCH 19/25] refactor: reset factory store in KubeEventsManager to ensure consistent informer creation Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor_test.go | 3 +-- test/hook/context/generator.go | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index ad25399c..39a6b59e 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -6,13 +6,12 @@ import ( "sync" "testing" + "github.com/deckhouse/deckhouse/pkg/log" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/deckhouse/deckhouse/pkg/log" - "github.com/flant/kube-client/fake" "github.com/flant/kube-client/manifest" kemtypes "github.com/flant/shell-operator/pkg/kube_events_manager/types" diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 331fd4af..ca732dbe 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -67,6 +67,10 @@ func NewBindingContextController(config string, logger *log.Logger, version ...f b.KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(ctx, b.fakeCluster.Client, b.logger.Named("kube-events-manager")) b.KubeEventsManager.WithMetricStorage(metricstorage.NewMetricStorage(ctx, "metrics-prefix", false, log.NewNop())) + // Re-create factory to drop informers created using different b.fakeCluster.Client. + b.mu.Lock() + kubeeventsmanager.DefaultFactoryStore = kubeeventsmanager.NewFactoryStore() + b.mu.Unlock() b.ScheduleManager = schedulemanager.NewScheduleManager(ctx, b.logger.Named("schedule-manager")) From 0c351b0b2693d5bee654587ee0712bbaf103d30d Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 16:23:00 +0300 Subject: [PATCH 20/25] refactor: replace sync.Map with custom types for varying informers and cancellation functions Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor.go | 128 ++++++++++++++++++++--------- test/hook/context/generator.go | 4 +- 2 files changed, 91 insertions(+), 41 deletions(-) diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index a063100e..a438a915 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -36,14 +36,14 @@ type monitor struct { // Namespace informer to get new namespaces NamespaceInformer *namespaceInformer // map of dynamically starting informers - VaryingInformers sync.Map + VaryingInformers varyingInformers eventCb func(kemtypes.KubeEvent) eventsEnabled bool // Index of namespaces statically defined in monitor configuration staticNamespaces sync.Map - cancelForNs sync.Map + cancelForNs cancelForNs ctx context.Context cancel context.CancelFunc @@ -52,6 +52,74 @@ type monitor struct { logger *log.Logger } +type varyingInformers struct { + value sync.Map +} + +func (v *varyingInformers) Store(key string, value []*resourceInformer) { + v.value.Store(key, value) +} + +func (v *varyingInformers) Load(key string) ([]*resourceInformer, bool) { + value, ok := v.value.Load(key) + if !ok { + return nil, false + } + if value, ok := value.([]*resourceInformer); ok { + return value, true + } + return nil, false +} + +func (v *varyingInformers) Range(f func(key string, value []*resourceInformer) bool) { + v.value.Range(func(key, value any) bool { + if key, ok := key.(string); ok { + if value, ok := value.([]*resourceInformer); ok { + return f(key, value) + } + } + return true + }) +} + +func (v *varyingInformers) Delete(key string) { + v.value.Delete(key) +} + +type cancelForNs struct { + value sync.Map +} + +func (c *cancelForNs) Store(key string, value context.CancelFunc) { + c.value.Store(key, value) +} + +func (c *cancelForNs) Load(key string) (context.CancelFunc, bool) { + value, ok := c.value.Load(key) + if !ok { + return nil, false + } + if value, ok := value.(context.CancelFunc); ok { + return value, true + } + return nil, false +} + +func (c *cancelForNs) Range(f func(key string, value context.CancelFunc) bool) { + c.value.Range(func(key, value any) bool { + if key, ok := key.(string); ok { + if value, ok := value.(context.CancelFunc); ok { + return f(key, value) + } + } + return true + }) +} + +func (c *cancelForNs) Delete(key string) { + c.value.Delete(key) +} + var _ Monitor = (*monitor)(nil) func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage, config *MonitorConfig, eventCb func(kemtypes.KubeEvent), logger *log.Logger) *monitor { @@ -65,8 +133,8 @@ func NewMonitor(ctx context.Context, client *klient.Client, mstor metric.Storage Config: config, eventCb: eventCb, ResourceInformers: make([]*resourceInformer, 0), - VaryingInformers: sync.Map{}, - cancelForNs: sync.Map{}, + VaryingInformers: varyingInformers{value: sync.Map{}}, + cancelForNs: cancelForNs{value: sync.Map{}}, staticNamespaces: sync.Map{}, logger: logger, } @@ -162,9 +230,7 @@ func (m *monitor) CreateInformers() error { } fn, _ := m.cancelForNs.Load(nsName) - if fn, ok := fn.(context.CancelFunc); ok { - fn() - } + fn() // TODO wait @@ -204,11 +270,9 @@ func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { objects = append(objects, informer.getCachedObjects()...) } - m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { - for _, informer := range value { - objects = append(objects, informer.getCachedObjects()...) - } + m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + for _, informer := range value { + objects = append(objects, informer.getCachedObjects()...) } return true }) @@ -226,11 +290,9 @@ func (m *monitor) EnableKubeEventCb() { informer.enableKubeEventCb() } // Execute eventCb for events accumulated during "Synchronization" phase. - m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { - for _, informer := range value { - informer.enableKubeEventCb() - } + m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + for _, informer := range value { + informer.enableKubeEventCb() } return true }) @@ -280,18 +342,12 @@ func (m *monitor) Start(parentCtx context.Context) { informer.start() } - m.VaryingInformers.Range(func(key, value any) bool { - nsName, ok := key.(string) - if !ok { - return true - } + m.VaryingInformers.Range(func(nsName string, value []*resourceInformer) bool { ctx, cancelForNs := context.WithCancel(m.ctx) m.cancelForNs.Store(nsName, cancelForNs) - if value, ok := value.([]*resourceInformer); ok { - for _, informer := range value { - informer.withContext(ctx) - informer.start() - } + for _, informer := range value { + informer.withContext(ctx) + informer.start() } return true }) @@ -317,11 +373,9 @@ func (m *monitor) PauseHandleEvents() { informer.pauseHandleEvents() } - m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { - for _, informer := range value { - informer.pauseHandleEvents() - } + m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + for _, informer := range value { + informer.pauseHandleEvents() } return true }) @@ -340,12 +394,10 @@ func (m *monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObj last.add(informer.getCachedObjectsInfoIncrement()) } - m.VaryingInformers.Range(func(_, value any) bool { - if value, ok := value.([]*resourceInformer); ok { - for _, informer := range value { - total.add(informer.getCachedObjectsInfo()) - last.add(informer.getCachedObjectsInfoIncrement()) - } + m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + for _, informer := range value { + total.add(informer.getCachedObjectsInfo()) + last.add(informer.getCachedObjectsInfoIncrement()) } return true }) diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index ca732dbe..57a300cb 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -68,9 +68,7 @@ func NewBindingContextController(config string, logger *log.Logger, version ...f b.KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(ctx, b.fakeCluster.Client, b.logger.Named("kube-events-manager")) b.KubeEventsManager.WithMetricStorage(metricstorage.NewMetricStorage(ctx, "metrics-prefix", false, log.NewNop())) // Re-create factory to drop informers created using different b.fakeCluster.Client. - b.mu.Lock() - kubeeventsmanager.DefaultFactoryStore = kubeeventsmanager.NewFactoryStore() - b.mu.Unlock() + // kubeeventsmanager.DefaultFactoryStore = kubeeventsmanager.NewFactoryStore() b.ScheduleManager = schedulemanager.NewScheduleManager(ctx, b.logger.Named("schedule-manager")) From 6d1822987dc4d0c66bdf517f3e2a8d1cbe545a96 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Wed, 9 Apr 2025 16:38:00 +0300 Subject: [PATCH 21/25] refactor: introduce RangeValue method for varyingInformers to simplify range operations Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index a438a915..30229a0a 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -82,6 +82,15 @@ func (v *varyingInformers) Range(f func(key string, value []*resourceInformer) b }) } +func (v *varyingInformers) RangeValue(f func(value []*resourceInformer)) { + v.value.Range(func(_, value any) bool { + if value, ok := value.([]*resourceInformer); ok { + f(value) + } + return true + }) +} + func (v *varyingInformers) Delete(key string) { v.value.Delete(key) } @@ -270,11 +279,10 @@ func (m *monitor) Snapshot() []kemtypes.ObjectAndFilterResult { objects = append(objects, informer.getCachedObjects()...) } - m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + m.VaryingInformers.RangeValue(func(value []*resourceInformer) { for _, informer := range value { objects = append(objects, informer.getCachedObjects()...) } - return true }) // Sort objects by namespace and name @@ -290,11 +298,10 @@ func (m *monitor) EnableKubeEventCb() { informer.enableKubeEventCb() } // Execute eventCb for events accumulated during "Synchronization" phase. - m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + m.VaryingInformers.RangeValue(func(value []*resourceInformer) { for _, informer := range value { informer.enableKubeEventCb() } - return true }) // Enable events for future VaryingInformers. m.eventsEnabled = true @@ -373,11 +380,10 @@ func (m *monitor) PauseHandleEvents() { informer.pauseHandleEvents() } - m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + m.VaryingInformers.RangeValue(func(value []*resourceInformer) { for _, informer := range value { informer.pauseHandleEvents() } - return true }) if m.NamespaceInformer != nil { @@ -394,12 +400,11 @@ func (m *monitor) SnapshotOperations() (*CachedObjectsInfo /*total*/, *CachedObj last.add(informer.getCachedObjectsInfoIncrement()) } - m.VaryingInformers.Range(func(_ string, value []*resourceInformer) bool { + m.VaryingInformers.RangeValue(func(value []*resourceInformer) { for _, informer := range value { total.add(informer.getCachedObjectsInfo()) last.add(informer.getCachedObjectsInfoIncrement()) } - return true }) return total, last From 6c02343e27bbd13ba68cfa1dfb468773bb0d816c Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Thu, 10 Apr 2025 08:59:47 +0300 Subject: [PATCH 22/25] refactor: improve cancellation function handling in monitor by checking existence before invocation Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/monitor.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/kube_events_manager/monitor.go b/pkg/kube_events_manager/monitor.go index 30229a0a..da11c384 100644 --- a/pkg/kube_events_manager/monitor.go +++ b/pkg/kube_events_manager/monitor.go @@ -238,8 +238,9 @@ func (m *monitor) CreateInformers() error { return } - fn, _ := m.cancelForNs.Load(nsName) - fn() + if fn, ok := m.cancelForNs.Load(nsName); ok { + fn() + } // TODO wait From f27e013336cf7de488af549abe518a28c6048964 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Thu, 10 Apr 2025 10:52:30 +0300 Subject: [PATCH 23/25] refactor: remove global factory store initialization and use instance-specific factory store in resource informer Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/factory.go | 9 +-------- pkg/kube_events_manager/resource_informer.go | 6 ++++-- test/hook/context/generator.go | 2 -- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/pkg/kube_events_manager/factory.go b/pkg/kube_events_manager/factory.go index 35bfd528..7c646353 100644 --- a/pkg/kube_events_manager/factory.go +++ b/pkg/kube_events_manager/factory.go @@ -15,14 +15,7 @@ import ( "k8s.io/client-go/tools/cache" ) -var ( - DefaultFactoryStore *FactoryStore - DefaultSyncTime = 100 * time.Millisecond -) - -func init() { - DefaultFactoryStore = NewFactoryStore() -} +var DefaultSyncTime = 100 * time.Millisecond type FactoryIndex struct { GVR schema.GroupVersionResource diff --git a/pkg/kube_events_manager/resource_informer.go b/pkg/kube_events_manager/resource_informer.go index 443877fd..ac926c0f 100644 --- a/pkg/kube_events_manager/resource_informer.go +++ b/pkg/kube_events_manager/resource_informer.go @@ -37,6 +37,7 @@ type resourceInformer struct { GroupVersionResource schema.GroupVersionResource ListOptions metav1.ListOptions + defaultFactoryStore *FactoryStore // A cache of objects and filterResults. It is a part of the Monitor's snapshot. cachedObjects map[string]*kemtypes.ObjectAndFilterResult cacheLock sync.RWMutex @@ -91,6 +92,7 @@ func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *resource cachedObjectsInfo: &CachedObjectsInfo{}, cachedObjectsIncrement: &CachedObjectsInfo{}, logger: cfg.logger, + defaultFactoryStore: NewFactoryStore(), } return informer } @@ -455,13 +457,13 @@ func (ei *resourceInformer) start() { go func() { if ei.ctx != nil { <-ei.ctx.Done() - DefaultFactoryStore.Stop(ei.id, ei.FactoryIndex) + ei.defaultFactoryStore.Stop(ei.id, ei.FactoryIndex) } }() // TODO: separate handler and informer errorHandler := newWatchErrorHandler(ei.Monitor.Metadata.DebugName, ei.Monitor.Kind, ei.Monitor.Metadata.LogLabels, ei.metricStorage, ei.logger.Named("watch-error-handler")) - err := DefaultFactoryStore.Start(ei.ctx, ei.id, ei.KubeClient.Dynamic(), ei.FactoryIndex, ei, errorHandler) + err := ei.defaultFactoryStore.Start(ei.ctx, ei.id, ei.KubeClient.Dynamic(), ei.FactoryIndex, ei, errorHandler) if err != nil { ei.Monitor.Logger.Error("cache is not synced for informer", slog.String("debugName", ei.Monitor.Metadata.DebugName)) return diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 57a300cb..331fd4af 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -67,8 +67,6 @@ func NewBindingContextController(config string, logger *log.Logger, version ...f b.KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(ctx, b.fakeCluster.Client, b.logger.Named("kube-events-manager")) b.KubeEventsManager.WithMetricStorage(metricstorage.NewMetricStorage(ctx, "metrics-prefix", false, log.NewNop())) - // Re-create factory to drop informers created using different b.fakeCluster.Client. - // kubeeventsmanager.DefaultFactoryStore = kubeeventsmanager.NewFactoryStore() b.ScheduleManager = schedulemanager.NewScheduleManager(ctx, b.logger.Named("schedule-manager")) From 6596696750cc086d3821f88f2bda516541578f20 Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Thu, 10 Apr 2025 12:02:07 +0300 Subject: [PATCH 24/25] refactor: initialize default factory store and reset in KubeEventsManager for consistent informer management Signed-off-by: Evsyukov Denis --- pkg/kube_events_manager/factory.go | 15 ++++++++++++++- pkg/kube_events_manager/resource_informer.go | 6 ++---- test/hook/context/generator.go | 2 ++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/pkg/kube_events_manager/factory.go b/pkg/kube_events_manager/factory.go index 7c646353..a52ebab4 100644 --- a/pkg/kube_events_manager/factory.go +++ b/pkg/kube_events_manager/factory.go @@ -15,7 +15,14 @@ import ( "k8s.io/client-go/tools/cache" ) -var DefaultSyncTime = 100 * time.Millisecond +var ( + DefaultFactoryStore *FactoryStore + DefaultSyncTime = 100 * time.Millisecond +) + +func init() { + DefaultFactoryStore = NewFactoryStore() +} type FactoryIndex struct { GVR schema.GroupVersionResource @@ -42,6 +49,12 @@ func NewFactoryStore() *FactoryStore { } } +func (c *FactoryStore) Reset() { + c.mu.Lock() + defer c.mu.Unlock() + c.data = make(map[FactoryIndex]Factory) +} + func (c *FactoryStore) add(index FactoryIndex, f dynamicinformer.DynamicSharedInformerFactory) { ctx, cancel := context.WithCancel(context.Background()) c.data[index] = Factory{ diff --git a/pkg/kube_events_manager/resource_informer.go b/pkg/kube_events_manager/resource_informer.go index ac926c0f..443877fd 100644 --- a/pkg/kube_events_manager/resource_informer.go +++ b/pkg/kube_events_manager/resource_informer.go @@ -37,7 +37,6 @@ type resourceInformer struct { GroupVersionResource schema.GroupVersionResource ListOptions metav1.ListOptions - defaultFactoryStore *FactoryStore // A cache of objects and filterResults. It is a part of the Monitor's snapshot. cachedObjects map[string]*kemtypes.ObjectAndFilterResult cacheLock sync.RWMutex @@ -92,7 +91,6 @@ func newResourceInformer(ns, name string, cfg *resourceInformerConfig) *resource cachedObjectsInfo: &CachedObjectsInfo{}, cachedObjectsIncrement: &CachedObjectsInfo{}, logger: cfg.logger, - defaultFactoryStore: NewFactoryStore(), } return informer } @@ -457,13 +455,13 @@ func (ei *resourceInformer) start() { go func() { if ei.ctx != nil { <-ei.ctx.Done() - ei.defaultFactoryStore.Stop(ei.id, ei.FactoryIndex) + DefaultFactoryStore.Stop(ei.id, ei.FactoryIndex) } }() // TODO: separate handler and informer errorHandler := newWatchErrorHandler(ei.Monitor.Metadata.DebugName, ei.Monitor.Kind, ei.Monitor.Metadata.LogLabels, ei.metricStorage, ei.logger.Named("watch-error-handler")) - err := ei.defaultFactoryStore.Start(ei.ctx, ei.id, ei.KubeClient.Dynamic(), ei.FactoryIndex, ei, errorHandler) + err := DefaultFactoryStore.Start(ei.ctx, ei.id, ei.KubeClient.Dynamic(), ei.FactoryIndex, ei, errorHandler) if err != nil { ei.Monitor.Logger.Error("cache is not synced for informer", slog.String("debugName", ei.Monitor.Metadata.DebugName)) return diff --git a/test/hook/context/generator.go b/test/hook/context/generator.go index 331fd4af..88179259 100644 --- a/test/hook/context/generator.go +++ b/test/hook/context/generator.go @@ -67,6 +67,8 @@ func NewBindingContextController(config string, logger *log.Logger, version ...f b.KubeEventsManager = kubeeventsmanager.NewKubeEventsManager(ctx, b.fakeCluster.Client, b.logger.Named("kube-events-manager")) b.KubeEventsManager.WithMetricStorage(metricstorage.NewMetricStorage(ctx, "metrics-prefix", false, log.NewNop())) + // Re-create factory to drop informers created using different b.fakeCluster.Client. + kubeeventsmanager.DefaultFactoryStore.Reset() b.ScheduleManager = schedulemanager.NewScheduleManager(ctx, b.logger.Named("schedule-manager")) From ebc9ab1146f27d693959d228805e2aa4eb19c9a2 Mon Sep 17 00:00:00 2001 From: Yuriy Losev Date: Thu, 10 Apr 2025 16:20:06 +0400 Subject: [PATCH 25/25] Update pkg/kube_events_manager/monitor_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/kube_events_manager/monitor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kube_events_manager/monitor_test.go b/pkg/kube_events_manager/monitor_test.go index 39a6b59e..c261c0ec 100644 --- a/pkg/kube_events_manager/monitor_test.go +++ b/pkg/kube_events_manager/monitor_test.go @@ -151,7 +151,7 @@ func Test_Monitor_should_handle_dynamic_ns_events(t *testing.T) { _, ok := mon.VaryingInformers.Load("test-ns-non-matched") return ok }, "5s", "10ms"). - ShouldNot(BeTrue(), "Should not create informer for non-mathed Namespace") + ShouldNot(BeTrue(), "Should not create informer for non-matched Namespace") } func createNsWithLabels(fc *fake.Cluster, name string, labels map[string]string) {