diff --git a/pkg/addon-operator/debug_server.go b/pkg/addon-operator/debug_server.go index f43ed10c..15d3d664 100644 --- a/pkg/addon-operator/debug_server.go +++ b/pkg/addon-operator/debug_server.go @@ -50,10 +50,7 @@ func (op *AddonOperator) RegisterDebugGlobalRoutes(dbgSrv *debug.Server) { func (op *AddonOperator) RegisterDebugModuleRoutes(dbgSrv *debug.Server) { dbgSrv.RegisterHandler(http.MethodGet, "/module/list.{format:(json|yaml|text)}", func(_ *http.Request) (interface{}, error) { - mods, err := op.ModuleManager.GetEnabledModuleNames() - if err != nil { - return map[string][]string{}, err - } + mods := op.ModuleManager.GetEnabledModuleNames() sort.Strings(mods) return map[string][]string{"enabledModules": mods}, nil }) @@ -139,12 +136,7 @@ func (op *AddonOperator) RegisterDebugModuleRoutes(dbgSrv *debug.Server) { dbgSrv.RegisterHandler(http.MethodGet, "/module/resource-monitor.{format:(json|yaml)}", func(_ *http.Request) (interface{}, error) { dump := map[string]interface{}{} - mods, err := op.ModuleManager.GetEnabledModuleNames() - if err != nil { - return dump, err - } - - for _, moduleName := range mods { + for _, moduleName := range op.ModuleManager.GetEnabledModuleNames() { if !op.HelmResourcesManager.HasMonitor(moduleName) { dump[moduleName] = "No monitor" continue diff --git a/pkg/addon-operator/operator.go b/pkg/addon-operator/operator.go index 97fc3cd4..609621d8 100644 --- a/pkg/addon-operator/operator.go +++ b/pkg/addon-operator/operator.go @@ -313,7 +313,7 @@ func (op *AddonOperator) RegisterManagerEventsHandlers() { logEntry.Debugf("Create tasks for 'schedule' event '%s'", crontab) var tasks []sh_task.Task - err := op.ModuleManager.HandleScheduleEvent(crontab, + op.ModuleManager.HandleScheduleEvent(crontab, func(globalHook *hooks.GlobalHook, info controller.BindingExecutionInfo) { if !op.allowHandleScheduleEvent(globalHook) { return @@ -371,10 +371,6 @@ func (op *AddonOperator) RegisterManagerEventsHandlers() { tasks = append(tasks, newTask) }) - if err != nil { - logEntry.Errorf("handle schedule event '%s': %s", crontab, err) - return []sh_task.Task{} - } return tasks }) @@ -388,7 +384,7 @@ func (op *AddonOperator) RegisterManagerEventsHandlers() { logEntry.Debugf("Create tasks for 'kubernetes' event '%s'", kubeEvent.String()) var tasks []sh_task.Task - err := op.ModuleManager.HandleKubeEvent(kubeEvent, + op.ModuleManager.HandleKubeEvent(kubeEvent, func(globalHook *hooks.GlobalHook, info controller.BindingExecutionInfo) { hookLabels := utils.MergeLabels(logLabels, map[string]string{ "hook": globalHook.GetName(), @@ -442,10 +438,6 @@ func (op *AddonOperator) RegisterManagerEventsHandlers() { tasks = append(tasks, newTask) }) - if err != nil { - logEntry.Errorf("handle kube event '%s': %s", kubeEvent.String(), err) - return []sh_task.Task{} - } return tasks }) diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index 88769f68..b9eaf681 100644 --- a/pkg/module_manager/module_manager.go +++ b/pkg/module_manager/module_manager.go @@ -371,16 +371,13 @@ func (mm *ModuleManager) RefreshStateFromHelmReleases(logLabels map[string]strin return nil, err } - return mm.stateFromHelmReleases(releasedModules) + return mm.stateFromHelmReleases(releasedModules), nil } // stateFromHelmReleases calculates modules to purge from Helm releases. -func (mm *ModuleManager) stateFromHelmReleases(releases []string) (*ModulesState, error) { +func (mm *ModuleManager) stateFromHelmReleases(releases []string) *ModulesState { releasesMap := utils.ListToMapStringStruct(releases) - enabledModules, err := mm.GetEnabledModuleNames() - if err != nil { - return nil, err - } + enabledModules := mm.GetEnabledModuleNames() for _, moduleName := range enabledModules { delete(releasesMap, moduleName) @@ -393,7 +390,7 @@ func (mm *ModuleManager) stateFromHelmReleases(releases []string) (*ModulesState return &ModulesState{ ModulesToPurge: purge, - }, nil + } } // RefreshEnabledState gets current diff of the graph and forms ModuleState @@ -453,7 +450,8 @@ func (mm *ModuleManager) GetModuleNames() []string { return mm.modules.NamesInOrder() } -func (mm *ModuleManager) GetEnabledModuleNames() ([]string, error) { +// GetEnabledModuleNames runs corresponding method of the module scheduler +func (mm *ModuleManager) GetEnabledModuleNames() []string { return mm.moduleScheduler.GetEnabledModuleNames() } @@ -626,8 +624,8 @@ func (mm *ModuleManager) RunModuleHook(moduleName, hookName string, binding Bind return ml.RunHookByName(hookName, binding, bindingContext, logLabels) } -func (mm *ModuleManager) HandleKubeEvent(kubeEvent KubeEvent, createGlobalTaskFn func(*hooks.GlobalHook, controller.BindingExecutionInfo), createModuleTaskFn func(*modules.BasicModule, *hooks.ModuleHook, controller.BindingExecutionInfo)) error { - return mm.LoopByBinding(OnKubernetesEvent, func(gh *hooks.GlobalHook, m *modules.BasicModule, mh *hooks.ModuleHook) { +func (mm *ModuleManager) HandleKubeEvent(kubeEvent KubeEvent, createGlobalTaskFn func(*hooks.GlobalHook, controller.BindingExecutionInfo), createModuleTaskFn func(*modules.BasicModule, *hooks.ModuleHook, controller.BindingExecutionInfo)) { + mm.LoopByBinding(OnKubernetesEvent, func(gh *hooks.GlobalHook, m *modules.BasicModule, mh *hooks.ModuleHook) { defer func() { if err := recover(); err != nil { logEntry := log.WithField("function", "HandleKubeEvent").WithField("event", "OnKubernetesEvent") @@ -730,8 +728,8 @@ func (mm *ModuleManager) DisableModuleHooks(moduleName string) { mm.SetModulePhaseAndNotify(ml, modules.HooksDisabled) } -func (mm *ModuleManager) HandleScheduleEvent(crontab string, createGlobalTaskFn func(*hooks.GlobalHook, controller.BindingExecutionInfo), createModuleTaskFn func(*modules.BasicModule, *hooks.ModuleHook, controller.BindingExecutionInfo)) error { - return mm.LoopByBinding(Schedule, func(gh *hooks.GlobalHook, m *modules.BasicModule, mh *hooks.ModuleHook) { +func (mm *ModuleManager) HandleScheduleEvent(crontab string, createGlobalTaskFn func(*hooks.GlobalHook, controller.BindingExecutionInfo), createModuleTaskFn func(*modules.BasicModule, *hooks.ModuleHook, controller.BindingExecutionInfo)) { + mm.LoopByBinding(Schedule, func(gh *hooks.GlobalHook, m *modules.BasicModule, mh *hooks.ModuleHook) { if gh != nil { if gh.GetHookController().CanHandleScheduleEvent(crontab) { gh.GetHookController().HandleScheduleEvent(crontab, func(info controller.BindingExecutionInfo) { @@ -752,7 +750,7 @@ func (mm *ModuleManager) HandleScheduleEvent(crontab string, createGlobalTaskFn }) } -func (mm *ModuleManager) LoopByBinding(binding BindingType, fn func(gh *hooks.GlobalHook, m *modules.BasicModule, mh *hooks.ModuleHook)) error { +func (mm *ModuleManager) LoopByBinding(binding BindingType, fn func(gh *hooks.GlobalHook, m *modules.BasicModule, mh *hooks.ModuleHook)) { globalHooks := mm.GetGlobalHooksInOrder(binding) for _, hookName := range globalHooks { @@ -760,10 +758,7 @@ func (mm *ModuleManager) LoopByBinding(binding BindingType, fn func(gh *hooks.Gl fn(gh, nil, nil) } - mods, err := mm.moduleScheduler.GetEnabledModuleNames() - if err != nil { - return err - } + mods := mm.moduleScheduler.GetEnabledModuleNames() for _, moduleName := range mods { m := mm.GetModule(moduleName) @@ -777,8 +772,6 @@ func (mm *ModuleManager) LoopByBinding(binding BindingType, fn func(gh *hooks.Gl fn(nil, m, mh) } } - - return nil } func (mm *ModuleManager) runDynamicEnabledLoop(extender *dynamic_extender.Extender) { diff --git a/pkg/module_manager/scheduler/scheduler.go b/pkg/module_manager/scheduler/scheduler.go index 06382883..c0e7ac1f 100644 --- a/pkg/module_manager/scheduler/scheduler.go +++ b/pkg/module_manager/scheduler/scheduler.go @@ -332,23 +332,28 @@ func (s *Scheduler) IsModuleEnabled(moduleName string) bool { // GetEnabledModuleNames returns a list of all enabled module-type vertices from s.enabledModules // so that traversing the graph isn't required. -func (s *Scheduler) GetEnabledModuleNames() ([]string, error) { - if len(s.errList) > 0 { - return []string{}, fmt.Errorf("couldn't get enabled modules - graph in a faulty state: %s", strings.Join(s.errList, ",")) - } +func (s *Scheduler) GetEnabledModuleNames() []string { + s.l.Lock() + defer s.l.Unlock() + return s.getEnabledModuleNames() +} - if s.enabledModules != nil { - return *s.enabledModules, nil +func (s *Scheduler) getEnabledModuleNames() []string { + if s.enabledModules == nil { + return []string{} } - return []string{}, nil + enabledModules := make([]string, len(*s.enabledModules)) + copy(enabledModules, *s.enabledModules) + + return enabledModules } // GetGraphState returns: // * list of enabled modules if not nil // * current modules diff // * error if any -// if s.enabledModules is nil, we infer that the graph hasn't been calculatet yet and run RecalculateGraph for the first time. +// if s.enabledModules is nil, we infer that the graph hasn't been calculated yet and run RecalculateGraph for the first time. // if s.errList isn't empty, we try to recalculate the graph in case there were some minor errors last time. func (s *Scheduler) GetGraphState(logLabels map[string]string) ( /*enabled modules*/ []string /*modules diff*/, map[string]bool, error) { var recalculateGraph bool @@ -371,11 +376,11 @@ func (s *Scheduler) GetGraphState(logLabels map[string]string) ( /*enabled modul _, _ = s.recalculateGraphState(logLabels) } - if len(s.errList) > 0 || s.enabledModules == nil { - return nil, nil, fmt.Errorf("couldn't recalculate graph: %s", strings.Join(s.errList, ",")) + if len(s.errList) > 0 { + return s.getEnabledModuleNames(), nil, fmt.Errorf("couldn't recalculate graph: %s", strings.Join(s.errList, ",")) } - return *s.enabledModules, s.gleanGraphDiff(), nil + return s.getEnabledModuleNames(), s.gleanGraphDiff(), nil } // RecalculateGraph is a public version of recalculateGraphState() diff --git a/pkg/module_manager/scheduler/scheduler_test.go b/pkg/module_manager/scheduler/scheduler_test.go index 9dbcff16..b5f4efa7 100644 --- a/pkg/module_manager/scheduler/scheduler_test.go +++ b/pkg/module_manager/scheduler/scheduler_test.go @@ -81,8 +81,7 @@ nodeLocalDnsEnabled: false logLabels := map[string]string{"source": "TestFilter"} _, _ = s.RecalculateGraph(logLabels) - enabledModules, err := s.GetEnabledModuleNames() - assert.NoError(t, err) + enabledModules := s.GetEnabledModuleNames() assert.Equal(t, []string{"ingress-nginx"}, enabledModules) filter, err := s.Filter(static.Name, "ingress-nginx", logLabels) @@ -165,9 +164,7 @@ certManagerEnabled: true _, _ = s.RecalculateGraph(logLabels) - enabledModules, err := s.GetEnabledModuleNames() - assert.Error(t, err) - + enabledModules := s.GetEnabledModuleNames() assert.Equal(t, []string{}, enabledModules) // finalize