Skip to content

Commit

Permalink
remove excessive error args
Browse files Browse the repository at this point in the history 
Signed-off-by: Mikhail Scherba <mikhail.scherba@flant.com>
  • Loading branch information
@miklezzzz
miklezzzz committed Jun 24, 2024
1 parent d7c9e38 commit a191481
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 55 deletions.
12 changes: 2 additions & 10 deletions pkg/addon-operator/debug_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down Expand Up @@ -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
Expand Down
12 changes: 2 additions & 10 deletions pkg/addon-operator/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
})
Expand All @@ -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(),
Expand Down Expand Up @@ -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
})
Expand Down
31 changes: 12 additions & 19 deletions pkg/module_manager/module_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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) {
Expand All @@ -752,18 +750,15 @@ 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 {
gh := mm.GetGlobalHook(hookName)
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)
Expand All @@ -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) {
Expand Down
27 changes: 16 additions & 11 deletions pkg/module_manager/scheduler/scheduler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
7 changes: 2 additions & 5 deletions pkg/module_manager/scheduler/scheduler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit a191481

Please sign in to comment.