diff --git a/pkg/addon-operator/operator.go b/pkg/addon-operator/operator.go index fd4aef7e..cefd2b8c 100644 --- a/pkg/addon-operator/operator.go +++ b/pkg/addon-operator/operator.go @@ -680,8 +680,9 @@ func (op *AddonOperator) HandleConvergeModules(t sh_task.Task, logLabels map[str if op.ConvergeState.Phase == converge.WaitBeforeAll { logEntry.Infof("ConvergeModules: beforeAll hooks done, run modules") + var state *module_manager.ModulesState - state, handleErr := op.ModuleManager.RefreshEnabledState(t.GetLogLabels()) + state, handleErr = op.ModuleManager.RefreshEnabledState(t.GetLogLabels()) if handleErr == nil { // TODO disable hooks before was done in DiscoverModulesStateRefresh. Should we stick to this solution or disable events later during the handling each ModuleDelete task? // Disable events for disabled modules. @@ -950,14 +951,10 @@ func (op *AddonOperator) StartModuleManagerEventHandler() { // dynamically_enabled_extender case dynamic_extender.DynamicExtenderEvent: // we don't need to schedule any tasks as DynamicExtender is inherintelly : - graphStateChanged, err := op.ModuleManager.UpdateGraphState() - if err != nil { - eventLogEntry.Errorf("Couldn't update the graph's state: %v", err) - } + graphStateChanged := op.ModuleManager.RecalculateGraph() if graphStateChanged { // ConvergeModules may be in progress, Reset converge state. - op.ConvergeState.Phase = converge.StandBy convergeTask := converge.NewConvergeModulesTask( "ReloadAll-After-GlobalHookDynamicUpdate", converge.ReloadAllModules, @@ -965,13 +962,15 @@ func (op *AddonOperator) StartModuleManagerEventHandler() { ) firstTask := op.engine.TaskQueues.GetMain().GetFirst() if firstTask != nil && RemoveCurrentConvergeTasks(op.engine.TaskQueues.GetMain(), firstTask.GetId()) { - logEntry.Infof("ConvergeModules: global hook dynamic modification detected, restart current converge process (%s)", op.ConvergeState.Phase) + logEntry.Infof("ConvergeModules: global hook dynamic modification detected, restart current converge process (%s)", op.ConvergeState.Phase) op.engine.TaskQueues.GetMain().AddFirst(convergeTask) op.logTaskAdd(eventLogEntry, "DynamicExtender is updated, put first", convergeTask) } else { logEntry.Infof("ConvergeModules: global hook dynamic modification detected, rerun all modules required") op.engine.TaskQueues.GetMain().AddLast(convergeTask) } + // ConvergeModules may be in progress Reset converge state. + op.ConvergeState.Phase = converge.StandBy } // kube_config_extender @@ -988,12 +987,7 @@ func (op *AddonOperator) StartModuleManagerEventHandler() { } // Config is valid now, add task to update ModuleManager state. op.ModuleManager.SetKubeConfigValid(true) - - graphStateChanged, err := op.ModuleManager.UpdateGraphState() - if err != nil { - eventLogEntry.Errorf("Couldn't update the graph's state: %v", err) - continue - } + graphStateChanged := op.ModuleManager.RecalculateGraph() var ( kubeConfigTask sh_task.Task @@ -1018,8 +1012,6 @@ func (op *AddonOperator) StartModuleManagerEventHandler() { } if event.GlobalSectionChanged || graphStateChanged { - // ConvergeModules may be in progress Reset converge state. - op.ConvergeState.Phase = converge.StandBy convergeTask = converge.NewConvergeModulesTask( "ReloadAll-After-KubeConfigChange", converge.ReloadAllModules, @@ -1039,6 +1031,8 @@ func (op *AddonOperator) StartModuleManagerEventHandler() { logEntry.Infof("ConvergeModules: kube config modification detected, rerun all modules required") op.engine.TaskQueues.GetMain().AddLast(convergeTask) } + // ConvergeModules may be in progress Reset converge state. + op.ConvergeState.Phase = converge.StandBy } else { modulesToRerun := []string{} for _, moduleName := range event.ModuleValuesChanged { diff --git a/pkg/addon-operator/operator_test.go b/pkg/addon-operator/operator_test.go index 940456fb..da74a216 100644 --- a/pkg/addon-operator/operator_test.go +++ b/pkg/addon-operator/operator_test.go @@ -142,6 +142,7 @@ func assembleTestAddonOperator(t *testing.T, configPath string) (*AddonOperator, err = op.InitModuleManager() g.Expect(err).ShouldNot(HaveOccurred(), "Should init ModuleManager") + _ = op.ModuleManager.RecalculateGraph() return op, result } diff --git a/pkg/module_manager/models/modules/basic.go b/pkg/module_manager/models/modules/basic.go index 133e9948..5f1a27c7 100644 --- a/pkg/module_manager/models/modules/basic.go +++ b/pkg/module_manager/models/modules/basic.go @@ -379,21 +379,6 @@ func (bm *BasicModule) RunEnabledScript(tmpDir string, precedingEnabledModules [ logEntry := log.WithFields(utils.LabelsToLogFields(logLabels)) enabledScriptPath := filepath.Join(bm.Path, "enabled") - - f, err := os.Stat(enabledScriptPath) - if os.IsNotExist(err) { - logEntry.Debugf("MODULE '%s' is ENABLED. Enabled script doesn't exist!", bm.Name) - return true, nil - } else if err != nil { - logEntry.Errorf("Cannot stat enabled script '%s': %s", enabledScriptPath, err) - return false, err - } - - if !utils_file.IsFileExecutable(f) { - logEntry.Errorf("Found non-executable enabled script '%s'", enabledScriptPath) - return false, fmt.Errorf("non-executable enable script") - } - configValuesPath, err := bm.prepareConfigValuesJsonFile(tmpDir) if err != nil { logEntry.Errorf("Prepare CONFIG_VALUES_PATH file for '%s': %s", enabledScriptPath, err) diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index f938c86d..1a4dd552 100644 --- a/pkg/module_manager/module_manager.go +++ b/pkg/module_manager/module_manager.go @@ -399,15 +399,13 @@ func (mm *ModuleManager) RefreshEnabledState(logLabels map[string]string) (*Modu }) logEntry := log.WithFields(utils.LabelsToLogFields(refreshLogLabels)) - enabledModules, err := mm.moduleScheduler.GetEnabledModuleNames() + enabledModules, modulesDiff, err := mm.moduleScheduler.GetGraphState() if err != nil { return nil, err } logEntry.Infof("Enabled modules: %+v", enabledModules) - modulesDiff := mm.moduleScheduler.GleanGraphDiff() - var ( modulesToEnable []string modulesToDisable []string @@ -421,6 +419,7 @@ func (mm *ModuleManager) RefreshEnabledState(logLabels map[string]string) (*Modu } } modulesToDisable = utils.SortReverseByReference(modulesToDisable, mm.modules.NamesInOrder()) + modulesToEnable = utils.SortByReference(modulesToEnable, mm.modules.NamesInOrder()) logEntry.Debugf("Refresh state results:\n"+ " enabledModules: %v\n"+ @@ -808,9 +807,9 @@ func (mm *ModuleManager) applyEnabledPatch(enabledPatch utils.ValuesPatch, exten return nil } -// UpdateGraphState runs corresponding scheduler method that returns true if the graph's state has changed -func (mm *ModuleManager) UpdateGraphState() (bool, error) { - return mm.moduleScheduler.UpdateGraphState() +// RecalculateGraph runs corresponding scheduler method that returns true if the graph's state has changed +func (mm *ModuleManager) RecalculateGraph() bool { + return mm.moduleScheduler.RecalculateGraph() } // GlobalSynchronizationNeeded is true if there is at least one global diff --git a/pkg/module_manager/scheduler/extenders/script_enabled/script.go b/pkg/module_manager/scheduler/extenders/script_enabled/script.go index 99557266..8e7eb3f3 100644 --- a/pkg/module_manager/scheduler/extenders/script_enabled/script.go +++ b/pkg/module_manager/scheduler/extenders/script_enabled/script.go @@ -1,26 +1,43 @@ package script_enabled import ( + "errors" "fmt" "os" + "path/filepath" "sync" + log "github.com/sirupsen/logrus" + "github.com/flant/addon-operator/pkg/module_manager/scheduler/extenders" "github.com/flant/addon-operator/pkg/module_manager/scheduler/node" + utils_file "github.com/flant/shell-operator/pkg/utils/file" ) const ( Name extenders.ExtenderName = "ScriptEnabled" + + noEnabledScript scriptState = "NoEnabledScript" + nonExecutableScript scriptState = "NonExecutableScript" + statError scriptState = "StatError" ) +type scriptState string + type Extender struct { - tmpDir string - basicModules map[string]node.ModuleInterface + tmpDir string + basicModuleDescriptors map[string]moduleDescriptor l sync.RWMutex enabledModules []string } +type moduleDescriptor struct { + module node.ModuleInterface + scriptState scriptState + stateDescription string +} + func NewExtender(tmpDir string) (*Extender, error) { info, err := os.Stat(tmpDir) if err != nil { @@ -32,16 +49,37 @@ func NewExtender(tmpDir string) (*Extender, error) { } e := &Extender{ - basicModules: make(map[string]node.ModuleInterface), - enabledModules: make([]string, 0), - tmpDir: tmpDir, + basicModuleDescriptors: make(map[string]moduleDescriptor), + enabledModules: make([]string, 0), + tmpDir: tmpDir, } return e, nil } func (e *Extender) AddBasicModule(module node.ModuleInterface) { - e.basicModules[module.GetName()] = module + moduleD := moduleDescriptor{ + module: module, + } + + enabledScriptPath := filepath.Join(module.GetPath(), "enabled") + f, err := os.Stat(enabledScriptPath) + if err != nil { + if os.IsNotExist(err) { + moduleD.scriptState = noEnabledScript + log.Debugf("MODULE '%s' is ENABLED. Enabled script doesn't exist!", module.GetName()) + } else { + moduleD.scriptState = statError + moduleD.stateDescription = fmt.Sprintf("Cannot stat enabled script for '%s' module: %v", module.GetName(), err) + log.Errorf(moduleD.stateDescription) + } + } else { + if !utils_file.IsFileExecutable(f) { + moduleD.scriptState = nonExecutableScript + log.Warnf("Found non-executable enabled script for '%s' module - assuming enabled state", module.GetName()) + } + } + e.basicModuleDescriptors[module.GetName()] = moduleD } func (e *Extender) Name() extenders.ExtenderName { @@ -59,18 +97,37 @@ func (e *Extender) Reset() { } func (e *Extender) Filter(moduleName string) (*bool, error) { - if module, found := e.basicModules[moduleName]; found { - enabled, err := module.RunEnabledScript(e.tmpDir, e.enabledModules, map[string]string{"operator.component": "ModuleManager.Scheduler", "extender": "script_enabled"}) - if err != nil { - return nil, err + if moduleDescriptor, found := e.basicModuleDescriptors[moduleName]; found { + var err error + var enabled bool + + switch moduleDescriptor.scriptState { + case "": + enabled, err = moduleDescriptor.module.RunEnabledScript(e.tmpDir, e.enabledModules, map[string]string{"operator.component": "ModuleManager.Scheduler", "extender": "script_enabled"}) + if err != nil { + err = fmt.Errorf("Failed to execute '%s' module's enabled script: %v", moduleDescriptor.module.GetName(), err) + } + + case statError: + log.Errorf(moduleDescriptor.stateDescription) + enabled = false + err = errors.New(moduleDescriptor.stateDescription) + + case nonExecutableScript: + enabled = true + log.Warnf("Found non-executable enabled script for '%s' module - assuming enabled state", moduleDescriptor.module.GetName()) + + case noEnabledScript: + enabled = true + log.Debugf("MODULE '%s' is ENABLED. Enabled script doesn't exist!", moduleDescriptor.module.GetName()) } if enabled { e.l.Lock() - e.enabledModules = append(e.enabledModules, module.GetName()) + e.enabledModules = append(e.enabledModules, moduleDescriptor.module.GetName()) e.l.Unlock() } - return &enabled, nil + return &enabled, err } return nil, nil } diff --git a/pkg/module_manager/scheduler/extenders/script_enabled/script_test.go b/pkg/module_manager/scheduler/extenders/script_enabled/script_test.go index 8dcf79f8..5a727943 100644 --- a/pkg/module_manager/scheduler/extenders/script_enabled/script_test.go +++ b/pkg/module_manager/scheduler/extenders/script_enabled/script_test.go @@ -1,6 +1,8 @@ package script_enabled import ( + "errors" + "fmt" "os" "testing" @@ -22,40 +24,57 @@ func TestExtender(t *testing.T) { Name: "ingress-nginx", Order: 402, EnabledScriptResult: true, + Path: "./testdata/402-ingress-nginx/", }, { Name: "cert-manager", Order: 30, EnabledScriptResult: true, + Path: "./testdata/030-cert-manager/", }, { Name: "node-local-dns", Order: 20, EnabledScriptResult: true, + EnabledScriptErr: fmt.Errorf("Exit code 1"), + Path: "./testdata/020-node-local-dns/", }, { Name: "admission-policy-engine", Order: 15, EnabledScriptResult: false, + Path: "./testdata/015-admission-policy-engine/", }, { Name: "chrony", Order: 45, EnabledScriptResult: false, + Path: "./testdata/045-chrony/", }, } for _, m := range basicModules { e.AddBasicModule(m) enabled, err := e.Filter(m.Name) - assert.NoError(t, err) switch m.GetName() { - case "ingress-nginx", "cert-manager", "node-local-dns": - assert.Equal(t, *enabled, true) + case "ingress-nginx": + assert.Equal(t, true, *enabled) + assert.Equal(t, nil, err) + case "cert-manager": + assert.Equal(t, true, *enabled) + assert.Equal(t, nil, err) + case "node-local-dns": + assert.Equal(t, false, *enabled) + assert.Equal(t, errors.New("Failed to execute 'node-local-dns' module's enabled script: Exit code 1"), err) case "admission-policy-engine", "chrony": - assert.Equal(t, *enabled, false) + assert.Equal(t, false, *enabled) + assert.Equal(t, nil, err) } } - expected := []string{"ingress-nginx", "cert-manager", "node-local-dns"} - assert.Equal(t, e.enabledModules, expected) + + expected := []string{"ingress-nginx", "cert-manager"} + assert.Equal(t, expected, e.enabledModules) + + err = os.RemoveAll(tmp) + assert.NoError(t, err) } diff --git a/pkg/module_manager/scheduler/extenders/script_enabled/testdata/015-admission-policy-engine/enabled b/pkg/module_manager/scheduler/extenders/script_enabled/testdata/015-admission-policy-engine/enabled new file mode 100755 index 00000000..e69de29b diff --git a/pkg/module_manager/scheduler/extenders/script_enabled/testdata/020-node-local-dns/enabled b/pkg/module_manager/scheduler/extenders/script_enabled/testdata/020-node-local-dns/enabled new file mode 100755 index 00000000..e69de29b diff --git a/pkg/module_manager/scheduler/extenders/script_enabled/testdata/045-chrony/enabled b/pkg/module_manager/scheduler/extenders/script_enabled/testdata/045-chrony/enabled new file mode 100755 index 00000000..e69de29b diff --git a/pkg/module_manager/scheduler/extenders/script_enabled/testdata/402-ingress-nginx/enabled b/pkg/module_manager/scheduler/extenders/script_enabled/testdata/402-ingress-nginx/enabled new file mode 100644 index 00000000..8c3cbfc3 --- /dev/null +++ b/pkg/module_manager/scheduler/extenders/script_enabled/testdata/402-ingress-nginx/enabled @@ -0,0 +1,3 @@ +#!/bin/bash + +exit 0 diff --git a/pkg/module_manager/scheduler/node/node.go b/pkg/module_manager/scheduler/node/node.go index a934ba9d..9f909d95 100644 --- a/pkg/module_manager/scheduler/node/node.go +++ b/pkg/module_manager/scheduler/node/node.go @@ -8,11 +8,14 @@ type ModuleInterface interface { RunEnabledScript(string, []string, map[string]string) (bool, error) GetName() string GetOrder() uint32 + GetPath() string } type MockModule struct { EnabledScriptResult bool + EnabledScriptErr error Name string + Path string Order uint32 } @@ -20,11 +23,18 @@ func (m MockModule) GetName() string { return m.Name } +func (m MockModule) GetPath() string { + return m.Path +} + func (m MockModule) GetOrder() uint32 { return m.Order } func (m MockModule) RunEnabledScript(_ string, _ []string, _ map[string]string) (bool, error) { + if m.EnabledScriptErr != nil { + return false, m.EnabledScriptErr + } return m.EnabledScriptResult, nil } diff --git a/pkg/module_manager/scheduler/node/node_test.go b/pkg/module_manager/scheduler/node/node_test.go new file mode 100644 index 00000000..08f4369a --- /dev/null +++ b/pkg/module_manager/scheduler/node/node_test.go @@ -0,0 +1,25 @@ +package node + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewNode(t *testing.T) { + n := NewNode() + assert.Equal(t, &Node{}, n) + + m := MockModule{ + Name: "test-node", + Order: 32, + } + + n = NewNode().WithName("test-node").WithWeight(uint32(32)).WithType(ModuleType).WithModule(m) + assert.Equal(t, &Node{ + name: "test-node", + weight: 32, + typ: ModuleType, + module: m, + }, n) +} diff --git a/pkg/module_manager/scheduler/scheduler.go b/pkg/module_manager/scheduler/scheduler.go index f58df5f4..95656961 100644 --- a/pkg/module_manager/scheduler/scheduler.go +++ b/pkg/module_manager/scheduler/scheduler.go @@ -39,6 +39,13 @@ type Scheduler struct { enabledModules *[]string // storage for current module diff diff map[string]bool + // keeps all errors happened on last run + errList []string +} + +type vertexState struct { + enabled bool + updatedBy string } // NewScheduler returns a new instance of scheduler @@ -52,6 +59,7 @@ func NewScheduler(ctx context.Context) *Scheduler { extCh: make(chan extenders.ExtenderEvent, 1), dag: graph.New(nodeHash, graph.Directed(), graph.Acyclic()), diff: make(map[string]bool, 0), + errList: make([]string, 0), } } @@ -109,7 +117,7 @@ func (s *Scheduler) AddModuleVertex(module node.ModuleInterface) error { return err // parent not found - create it default: - parent = node.NewNode().WithName(vertex.GetWeight().String()).WithWeight(uint32(vertex.GetWeight())).WithType(node.WeightType) + parent := node.NewNode().WithName(vertex.GetWeight().String()).WithWeight(uint32(vertex.GetWeight())).WithType(node.WeightType) if err := s.AddWeightVertex(parent); err != nil { return err } @@ -322,110 +330,149 @@ 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 +// so that traversing the graph isn't required. func (s *Scheduler) GetEnabledModuleNames() ([]string, error) { - s.l.Lock() + if len(s.errList) > 0 { + return []string{}, fmt.Errorf("couldn't get enabled modules - graph in a faulty state: %s", strings.Join(s.errList, ",")) + } + if s.enabledModules != nil { - defer s.l.Unlock() return *s.enabledModules, nil } - enabledModules := make([]string, 0) - s.l.Unlock() - // run initial UpdateGraphState - _, err := s.UpdateGraphState() - if err != nil { - return nil, fmt.Errorf("couldn't get initial graph state: %v", err) - } + return []string{}, nil +} +// 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.errList isn't empty, we try to recalculate the graph in case there were some minor errors last time. +func (s *Scheduler) GetGraphState() ( /*enabled modules*/ []string /*modules diff*/, map[string]bool, error) { s.l.Lock() defer s.l.Unlock() - nodeNames, err := graph.StableTopologicalSort(s.dag, moduleSortFunc) - if err != nil { - return nil, fmt.Errorf("couldn't get the graph topological sorted view: %v", err) + var recalculateGraph bool + + // graph hasn't been initialized yet + if s.enabledModules == nil { + log.Warnf("Module Scheduler: graph hasn't been calculated yet") + recalculateGraph = true } - for _, name := range nodeNames { - if vertex, props, err := s.dag.VertexWithProperties(name); err == nil { - if props.Attributes["type"] == string(node.ModuleType) && vertex.GetState() { - enabledModules = append(enabledModules, name) - } - } else { - return enabledModules, fmt.Errorf("couldn't get %s vertex from the graph: %v", name, err) - } + if len(s.errList) > 0 { + log.Warnf("Module Scheduler: graph in a faulty state and will be recalculated: %s", strings.Join(s.errList, ",")) + recalculateGraph = true } - s.enabledModules = &enabledModules - return enabledModules, nil + if recalculateGraph { + _ = s.recalculateGraphState() + } + + if len(s.errList) > 0 || s.enabledModules == nil { + return nil, nil, fmt.Errorf("couldn't recalculate graph: %s", strings.Join(s.errList, ",")) + } + + return *s.enabledModules, s.gleanGraphDiff(), nil } -// UpdateGraphState cycles over all module-type vertices and applies all extenders to +// RecalculateGraph is a public version of recalculateGraphState() +func (s *Scheduler) RecalculateGraph() bool { + s.l.Lock() + defer s.l.Unlock() + return s.recalculateGraphState() +} + +// recalculateGraphState cycles over all module-type vertices and applies all extenders to // determine current states of the modules. Besides, it updates slice of all currently enabled modules. -// It returns true if the state of the graph has changed -func (s *Scheduler) UpdateGraphState() ( /* Graph's state has changed */ bool, error) { +// It returns true if the state of the graph has changed or if there were any errors during the run. +func (s *Scheduler) recalculateGraphState() /* Graph's state has changed */ bool { diff := make(map[string]bool, 0) + errList := make([]string, 0) enabledModules := make([]string, 0) - s.l.Lock() - defer s.l.Unlock() + names, err := graph.StableTopologicalSort(s.dag, moduleSortFunc) if err != nil { - return false, err + errList = append(errList, err.Error()) + s.errList = errList + return true } + // create a buffer to store all updates during upcoming run, the updates are applied if there is no errors during the rung + vBuf := make(map[string]*vertexState) + +outerCycle: for _, name := range names { vertex, props, err := s.dag.VertexWithProperties(name) if err != nil { - return false, fmt.Errorf("couldn't get %s vertex from the graph: %v", name, err) + errList = append(errList, fmt.Sprintf("couldn't get %s vertex from the graph: %v", name, err)) + break } if props.Attributes["type"] == string(node.ModuleType) { - previousState := vertex.GetState() - vertex.SetState(false) - vertex.SetUpdatedBy("") + moduleName := vertex.GetName() + vBuf[moduleName] = &vertexState{} for _, ex := range s.extenders { - // if ex is a shutter and the module is already disabled - there's no sense in running the extender - if ex.Name() == script_extender.Name && !vertex.GetState() { + if ex.Name() == script_extender.Name && !vBuf[moduleName].enabled { continue } - moduleStatus, err := ex.Filter(vertex.GetModule().GetName()) + moduleStatus, err := ex.Filter(moduleName) if err != nil { - return false, err + errList = append(errList, err.Error()) + break outerCycle } if moduleStatus != nil { if ex.Name() == script_extender.Name { - if !*moduleStatus && vertex.GetState() { - vertex.SetState(*moduleStatus) - vertex.SetUpdatedBy(string(ex.Name())) + if !*moduleStatus && vBuf[moduleName].enabled { + vBuf[moduleName].enabled = *moduleStatus + vBuf[moduleName].updatedBy = string(ex.Name()) } break } - - vertex.SetState(*moduleStatus) - vertex.SetUpdatedBy(string(ex.Name())) + vBuf[moduleName].enabled = *moduleStatus + vBuf[moduleName].updatedBy = string(ex.Name()) } } - if previousState != vertex.GetState() { - diff[vertex.GetName()] = vertex.GetState() + if vBuf[moduleName].enabled != vertex.GetState() { + diff[vertex.GetName()] = vBuf[moduleName].enabled } - if vertex.GetState() { + if vBuf[moduleName].enabled { enabledModules = append(enabledModules, name) } } } + // reset extenders' states if needed (mostly for enabled_script extender) for _, ex := range s.extenders { if re, ok := ex.(extenders.ResettableExtender); ok { re.Reset() } } - s.enabledModules = &enabledModules + if len(errList) > 0 { + s.errList = errList + log.Warnf("Module Scheduler: Graph converge failed with errors: %s", strings.Join(s.errList, ",")) + return true + } + // commit changes to the graph + for vertexName, state := range vBuf { + vertex, _, err := s.dag.VertexWithProperties(vertexName) + if err != nil { + errList = append(errList, fmt.Sprintf("couldn't get %s vertex from the graph: %v", vertexName, err)) + s.errList = errList + return true + } + vertex.SetState(state.enabled) + vertex.SetUpdatedBy(state.updatedBy) + } + + s.enabledModules = &enabledModules // merge the diff for module, newState := range diff { // if a new diff has an opposite state for the module, the module is deleted from the resulting diff @@ -439,15 +486,18 @@ func (s *Scheduler) UpdateGraphState() ( /* Graph's state has changed */ bool, e } } - s.printGraph() - return len(diff) > 0, nil + // reset any previous errors + s.errList = make([]string, 0) + log.Infof("Graph was successfully updated, diff: %v", s.diff) + + // TODO: provide access to the report via the operator's web server + // s.printGraph() + return len(diff) > 0 } -// GleanGraphDiff returns the diff value and resets diff storage -func (s *Scheduler) GleanGraphDiff() map[string]bool { - s.l.Lock() +// gleanGraphDiff returns modules diff list +func (s *Scheduler) gleanGraphDiff() map[string]bool { diff := s.diff s.diff = make(map[string]bool, 0) - s.l.Unlock() return diff } diff --git a/pkg/module_manager/scheduler/scheduler_test.go b/pkg/module_manager/scheduler/scheduler_test.go index d5f38e5d..51e49445 100644 --- a/pkg/module_manager/scheduler/scheduler_test.go +++ b/pkg/module_manager/scheduler/scheduler_test.go @@ -2,6 +2,7 @@ package scheduler import ( "context" + "errors" "os" "path/filepath" "testing" @@ -32,7 +33,93 @@ func (k kcmMock) KubeConfigEventCh() chan config.KubeConfigEvent { return make(chan config.KubeConfigEvent) } -func TestUpdateGraphState(t *testing.T) { +func TestGetEnabledModuleNames(t *testing.T) { + values := ` +# CE Bundle "Default" +ingressNginxEnabled: true +nodeLocalDnsEnabled: true +certManagerEnabled: true +` + basicModules := []*node.MockModule{ + { + Name: "cert-manager", + Order: 30, + EnabledScriptResult: true, + }, + { + Name: "node-local-dns", + Order: 20, + EnabledScriptResult: true, + }, + { + Name: "ingress-nginx", + Order: 402, + EnabledScriptResult: true, + EnabledScriptErr: errors.New("Exit code not 0"), + Path: "./testdata/402-ingress-nginx/", + }, + } + + tmp, err := os.MkdirTemp(t.TempDir(), "getEnabledTest") + require.NoError(t, err) + + s := NewScheduler(context.TODO()) + + valuesFile := filepath.Join(tmp, "values.yaml") + err = os.WriteFile(valuesFile, []byte(values), 0o644) + require.NoError(t, err) + + se, err := static.NewExtender(tmp) + assert.NoError(t, err) + + err = s.AddExtender(se) + assert.NoError(t, err) + + scripte, err := script_enabled.NewExtender(tmp) + assert.NoError(t, err) + err = s.AddExtender(scripte) + assert.NoError(t, err) + + for _, m := range basicModules { + err := s.AddModuleVertex(m) + assert.NoError(t, err) + scripte.AddBasicModule(m) + } + + err = s.ApplyExtenders("Static,ScriptEnabled") + require.NoError(t, err) + + _ = s.RecalculateGraph() + + enabledModules, err := s.GetEnabledModuleNames() + assert.Error(t, err) + + assert.Equal(t, []string{}, enabledModules) + + // finalize + err = os.RemoveAll(tmp) + assert.NoError(t, err) +} + +func TestAddModuleVertex(t *testing.T) { + s := NewScheduler(context.TODO()) + basicModule := &node.MockModule{ + Name: "ingress-nginx", + Order: 402, + } + + err := s.AddModuleVertex(basicModule) + assert.NoError(t, err) + + vertex, err := s.dag.Vertex(basicModule.GetName()) + assert.NoError(t, err) + + _, err = s.dag.Vertex(vertex.GetWeight().String()) + assert.NoError(t, err) + assert.Equal(t, false, vertex.GetState()) +} + +func TestRecalculateGraph(t *testing.T) { values := ` # Default global values section # todo remove duplicate config values they should be in global-hooks/openapi/config-values.yaml only @@ -71,6 +158,7 @@ l2LoadBalancerEnabled: false Name: "ingress-nginx", Order: 402, EnabledScriptResult: true, + Path: "./testdata/402-ingress-nginx/", }, { Name: "cert-manager", @@ -96,16 +184,19 @@ l2LoadBalancerEnabled: false Name: "foo-bar", Order: 133, EnabledScriptResult: false, + Path: "./testdata/133-foo-bar/", }, { Name: "flant-integration", Order: 450, EnabledScriptResult: false, + Path: "./testdata/450-flant-integration/", }, { Name: "monitoring-applications", Order: 340, EnabledScriptResult: false, + Path: "./testdata/340-monitoring-applications/", }, { Name: "l2-load-balancer", @@ -143,7 +234,6 @@ l2LoadBalancerEnabled: false tmp, err := os.MkdirTemp(t.TempDir(), "values-test") require.NoError(t, err) valuesFile := filepath.Join(tmp, "values.yaml") - err = os.WriteFile(valuesFile, []byte(values), 0o644) require.NoError(t, err) @@ -156,10 +246,10 @@ l2LoadBalancerEnabled: false err = s.ApplyExtenders("Static") require.NoError(t, err) - updated, err := s.UpdateGraphState() - assert.NoError(t, err) + updated := s.RecalculateGraph() assert.Equal(t, true, updated) - diff := s.GleanGraphDiff() + _, diff, err := s.GetGraphState() + assert.NoError(t, err) expected := map[string]bool{ "admission-policy-engine/Static": true, @@ -193,15 +283,12 @@ l2LoadBalancerEnabled: false assert.Equal(t, expectedDiff, diff) de := dynamically_enabled.NewExtender() - go func() { //nolint:revive for range s.EventCh() { } }() - assert.NoError(t, err) - err = s.AddExtender(de) assert.NoError(t, err) @@ -211,10 +298,10 @@ l2LoadBalancerEnabled: false de.UpdateStatus("l2-load-balancer", "add", true) de.UpdateStatus("node-local-dns", "remove", true) de.UpdateStatus("openstack-cloud-provider", "add", true) - updated, err = s.UpdateGraphState() - assert.NoError(t, err) + updated = s.RecalculateGraph() assert.Equal(t, true, updated) - diff = s.GleanGraphDiff() + _, diff, err = s.GetGraphState() + assert.NoError(t, err) expected = map[string]bool{ "admission-policy-engine/Static": true, @@ -261,10 +348,10 @@ l2LoadBalancerEnabled: false de.UpdateStatus("node-local-dns", "add", true) - updated, err = s.UpdateGraphState() - assert.NoError(t, err) + updated = s.RecalculateGraph() assert.Equal(t, true, updated) - diff = s.GleanGraphDiff() + _, diff, err = s.GetGraphState() + assert.NoError(t, err) expected = map[string]bool{ "admission-policy-engine/Static": true, @@ -307,10 +394,10 @@ l2LoadBalancerEnabled: false err = s.ApplyExtenders("Static,DynamicallyEnabled,KubeConfig,ScriptEnabled") require.NoError(t, err) - updated, err = s.UpdateGraphState() - assert.NoError(t, err) + updated = s.RecalculateGraph() assert.Equal(t, true, updated) - diff = s.GleanGraphDiff() + _, diff, err = s.GetGraphState() + assert.NoError(t, err) expected = map[string]bool{ "admission-policy-engine/Static": true, @@ -340,17 +427,76 @@ l2LoadBalancerEnabled: false assert.Equal(t, expectedDiff, diff) de.UpdateStatus("openstack-cloud-provider", "add", false) - + de.UpdateStatus("ingress-nginx", "add", true) de.UpdateStatus("node-local-dns", "add", true) - updated, err = s.UpdateGraphState() - assert.NoError(t, err) + updated = s.RecalculateGraph() assert.Equal(t, true, updated) - updated, err = s.UpdateGraphState() - assert.NoError(t, err) + updated = s.RecalculateGraph() assert.Equal(t, false, updated) + _, diff, err = s.GetGraphState() + assert.NoError(t, err) + + expected = map[string]bool{ + "admission-policy-engine/Static": true, + "cert-manager/KubeConfig": true, + "chrony/KubeConfig": false, + "node-local-dns/DynamicallyEnabled": true, + "foo-bar/ScriptEnabled": false, + "flant-integration/ScriptEnabled": false, + "monitoring-applications/ScriptEnabled": false, + "ingress-nginx/DynamicallyEnabled": true, + "l2-load-balancer/DynamicallyEnabled": true, + "openstack-cloud-provider/DynamicallyEnabled": false, + "echo/KubeConfig": true, + "prometheus/KubeConfig": true, + "prometheus-crd/KubeConfig": true, + } + + expectedDiff = map[string]bool{ + "openstack-cloud-provider": false, + "ingress-nginx": true, + } + + summary, err = s.PrintSummary() + assert.NoError(t, err) + + assert.Equal(t, expected, summary) + assert.Equal(t, expectedDiff, diff) + + basicModules[0].EnabledScriptErr = errors.New("Exit code not 0") + scripte.AddBasicModule(basicModules[0]) + + updated = s.RecalculateGraph() + assert.Equal(t, true, updated) + + _, diff, err = s.GetGraphState() + assert.Error(t, err) + + expected = map[string]bool{ + "admission-policy-engine/Static": true, + "cert-manager/KubeConfig": true, + "chrony/KubeConfig": false, + "node-local-dns/DynamicallyEnabled": true, + "foo-bar/ScriptEnabled": false, + "flant-integration/ScriptEnabled": false, + "monitoring-applications/ScriptEnabled": false, + "ingress-nginx/DynamicallyEnabled": true, + "l2-load-balancer/DynamicallyEnabled": true, + "openstack-cloud-provider/DynamicallyEnabled": false, + "echo/KubeConfig": true, + "prometheus/KubeConfig": true, + "prometheus-crd/KubeConfig": true, + } + + expectedDiff = nil + + assert.Equal(t, expected, summary) + assert.Equal(t, expectedDiff, diff) + assert.Equal(t, []string{"Failed to execute 'ingress-nginx' module's enabled script: Exit code not 0"}, s.errList) + s.printGraph() err = os.RemoveAll(tmp) diff --git a/pkg/module_manager/scheduler/testdata/133-foo-bar/enabled b/pkg/module_manager/scheduler/testdata/133-foo-bar/enabled new file mode 100755 index 00000000..e69de29b diff --git a/pkg/module_manager/scheduler/testdata/340-monitoring-applications/enabled b/pkg/module_manager/scheduler/testdata/340-monitoring-applications/enabled new file mode 100755 index 00000000..e69de29b diff --git a/pkg/module_manager/scheduler/testdata/402-ingress-nginx/enabled b/pkg/module_manager/scheduler/testdata/402-ingress-nginx/enabled new file mode 100755 index 00000000..e69de29b diff --git a/pkg/module_manager/scheduler/testdata/450-flant-integration/enabled b/pkg/module_manager/scheduler/testdata/450-flant-integration/enabled new file mode 100755 index 00000000..e69de29b