From fa509801a1f02887917f0d442d4b641df63355a3 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Mon, 6 Feb 2023 12:01:53 +0300 Subject: [PATCH 1/6] feat: load modules from multiple paths, support symlink as module - Resolve symlinks in modules directory, use symlink name to determine module name and order - Support multiple paths to search for modules. - Support directories without number prefix as module dirs. Default order is 1. - Remove '3 digits prefix' restriction. - Load values.yaml from all paths. Signed-off-by: Ivan Mikheykin --- MODULES.md | 30 ++- RUNNING.md | 10 +- pkg/app/app.go | 14 +- pkg/module_manager/module.go | 94 ++------- pkg/module_manager/module_loader.go | 197 ++++++++++++++++++ pkg/module_manager/module_loader_test.go | 61 ++++++ pkg/module_manager/module_manager.go | 38 ++-- pkg/module_manager/module_manager_test.go | 50 ++--- pkg/module_manager/module_set.go | 90 ++++++++ pkg/module_manager/module_set_test.go | 50 +++++ .../testdata/module_loader/dir1/values.yaml | 5 + .../testdata/module_loader/dir2/values.yaml | 7 + .../testdata/module_loader/dir3/values.yaml | 8 + pkg/utils/values_test.go | 41 ++++ 14 files changed, 554 insertions(+), 141 deletions(-) create mode 100644 pkg/module_manager/module_loader.go create mode 100644 pkg/module_manager/module_loader_test.go create mode 100644 pkg/module_manager/module_set.go create mode 100644 pkg/module_manager/module_set_test.go create mode 100644 pkg/module_manager/testdata/module_loader/dir1/values.yaml create mode 100644 pkg/module_manager/testdata/module_loader/dir2/values.yaml create mode 100644 pkg/module_manager/testdata/module_loader/dir3/values.yaml diff --git a/MODULES.md b/MODULES.md index 699089c2..c988ab21 100644 --- a/MODULES.md +++ b/MODULES.md @@ -1,26 +1,34 @@ # Module structure -A module is a directory with files. Addon-operator searches for the modules directories in `/modules` or in the path specified by the $MODULES_DIR variable. The module has the same name as the corresponding directory excluding the numeric prefix. +A module is a directory with files. Addon-operator searches for the modules directories in `/modules` or in the paths specified by the $MODULES_DIR variable. The module has the same name as the corresponding directory excluding the numeric prefix. -The file structure of the module’s directory: +An example of the file structure of the module: ``` /modules/001-simple-module -├── .helmignore -├── Chart.yaml -├── enabled ├── hooks -│ └── module-hooks.sh -├── README.md +│ ├── module-hook-1.sh +│ ├── ... +│ └── module-hook-N.sh +├── openapi +│ ├── config-values.yaml +│ └── values.yaml ├── templates +│ ├── config-maps.yaml +│ ├── ... │ └── daemon-set.yaml +├── enabled +├── README.md +├── .helmignore +├── Chart.yaml └── values.yaml ``` -- `hooks` — a directory with hooks; -- `enabled` — a script that gets the status of module (is it enabled or not). See the [modules discovery](LIFECYCLE.md#modules-discovery) process; -- `Chart.yaml`, `.helmignore`, `templates` — a Helm chart files; -- `README.md` — a file with the module description; +- `hooks` — a directory with hooks. +- `openapi` — [OpenAPI schemas](VALUES.md) for config values and for helm values. +- `enabled` — a script that gets the status of module (is it enabled or not). See the [modules discovery](LIFECYCLE.md#modules-discovery) process. +- `Chart.yaml`, `.helmignore`, `templates` — a Helm chart files. +- `README.md` — an optional file with the module description. - `values.yaml` – default values for chart in a [YAML format](VALUES.md). The name of this module is `simple-module`. values.yaml should contain a section `simpleModule` and a `simpleModuleEnabled` flag (see [VALUES](VALUES.md#values-storage)). diff --git a/RUNNING.md b/RUNNING.md index 15afbe40..debcb923 100644 --- a/RUNNING.md +++ b/RUNNING.md @@ -2,13 +2,15 @@ ## Environment variables -**MODULES_DIR** — a directory where modules are located. - **GLOBAL_HOOKS_DIR** — a directory with global hook files. -**ADDON_OPERATOR_NAMESPACE** — a required parameter with namespace where Addon-operator is deployed. +**MODULES_DIR** — paths separated by colon where modules are located. + +**UNNUMBERED_MODULE_ORDER** — a default order for modules without numbered prefix. + +**ADDON_OPERATOR_NAMESPACE** — a required parameter with namespace where Addon-operator is deployed. -**ADDON_OPERATOR_CONFIG_MAP** — a name of ConfigMap to store values. Default is `addon-operator`. +**ADDON_OPERATOR_CONFIG_MAP** — a name of ConfigMap to store values. Default is `addon-operator`. Namespace and config map name are used to watch for ConfigMap changes. diff --git a/pkg/app/app.go b/pkg/app/app.go index e37d0810..28a2abd1 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -31,22 +31,28 @@ var ( GlobalHooksDir = "global-hooks" ModulesDir = "modules" + + UnnumberedModuleOrder = 1 ) const ( - DefaultGlobalHooksDir = "global-hooks" - DefaultModulesDir = "modules" DefaultTempDir = "/tmp/addon-operator" DefaultDebugUnixSocket = "/var/run/addon-operator/debug.socket" ) // DefineStartCommandFlags init global flags with default values func DefineStartCommandFlags(kpApp *kingpin.Application, cmd *kingpin.CmdClause) { - cmd.Flag("modules-dir", "a path where to search for module directories"). + cmd.Flag("modules-dir", "paths where to search for module directories"). Envar("MODULES_DIR"). - Default(DefaultModulesDir). + Default(ModulesDir). StringVar(&ModulesDir) + // TODO Delete this setting after refactoring module dependencies machinery. + cmd.Flag("unnumbered-modules-order", "default order for modules without numbered prefix in name"). + Envar("UNNUMBERED_MODULES_ORDER"). + Default(strconv.Itoa(UnnumberedModuleOrder)). + IntVar(&UnnumberedModuleOrder) + cmd.Flag("global-hooks-dir", "a path where to search for global hook files (and OpenAPI schemas)"). Envar("GLOBAL_HOOKS_DIR"). Default(GlobalHooksDir). diff --git a/pkg/module_manager/module.go b/pkg/module_manager/module.go index 759319cf..49f38bf9 100644 --- a/pkg/module_manager/module.go +++ b/pkg/module_manager/module.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "runtime/trace" "strings" "time" @@ -33,8 +32,9 @@ import ( ) type Module struct { - Name string - Path string + Name string // MODULE_NAME env + Path string // MODULE_DIR env + Order int // MODULE_ORDER env // module values from modules/values.yaml file CommonStaticConfig *utils.ModuleConfig // module values from modules//values.yaml @@ -47,10 +47,11 @@ type Module struct { helm *helm.ClientFactory } -func NewModule(name, path string) *Module { +func NewModule(name string, path string, order int) *Module { return &Module{ Name: name, Path: path, + Order: order, State: NewModuleState(), } } @@ -885,61 +886,30 @@ func (m *Module) runEnabledScript(precedingEnabledModules []string, logLabels ma return moduleEnabled, nil } -var ValidModuleNameRe = regexp.MustCompile(`^[0-9][0-9][0-9]-(.*)$`) - -func SearchModules(modulesDir string) (modules []*Module, err error) { - if modulesDir == "" { - log.Warnf("Modules directory path is empty! No modules to load.") - return nil, nil - } - - files, err := os.ReadDir(modulesDir) // returns a list of modules sorted by filename - if err != nil { - return nil, fmt.Errorf("list modules directory '%s': %s", modulesDir, err) +// RegisterModules load all available modules from modules directory. +func (mm *moduleManager) RegisterModules() error { + if mm.ModulesDir == "" { + log.Warnf("Empty modules directory is passed! No modules to load.") + return nil } - badModulesDirs := make([]string, 0) - modules = make([]*Module, 0) - - for _, file := range files { - if !file.IsDir() { - continue - } - matchRes := ValidModuleNameRe.FindStringSubmatch(file.Name()) - if matchRes != nil { - moduleName := matchRes[1] - modulePath := filepath.Join(modulesDir, file.Name()) - module := NewModule(moduleName, modulePath) - modules = append(modules, module) - } else { - badModulesDirs = append(badModulesDirs, filepath.Join(modulesDir, file.Name())) - } - } + log.Debug("Search and register modules") - if len(badModulesDirs) > 0 { - return nil, fmt.Errorf("modules directory contains directories not matched ValidModuleRegex '%s': %s", ValidModuleNameRe, strings.Join(badModulesDirs, ", ")) + // load global and modules common static values from modules/values.yaml + commonStaticValues, err := LoadCommonStaticValues(mm.ModulesDir) + if err != nil { + return fmt.Errorf("load common values for modules: %s", err) } - - return -} - -// RegisterModules load all available modules from modules directory -// FIXME: Only 000-name modules are loaded, allow non-prefixed modules. -func (mm *moduleManager) RegisterModules() error { - log.Debug("Search and register modules") + mm.commonStaticValues = commonStaticValues modules, err := SearchModules(mm.ModulesDir) if err != nil { return err } - log.Debugf("Found %d modules", len(modules)) - // load global and modules common static values from modules/values.yaml - if err := mm.loadCommonStaticValues(); err != nil { - return fmt.Errorf("load common values for modules: %s", err) - } + log.Debugf("Found modules: %v", modules.NamesInOrder()) - for _, module := range modules { + for _, module := range modules.List() { logEntry := log.WithField("module", module.Name) module.WithModuleManager(mm) @@ -953,9 +923,6 @@ func (mm *moduleManager) RegisterModules() error { return fmt.Errorf("bad module values") } - mm.allModulesByName[module.Name] = module - mm.allModulesNamesInOrder = append(mm.allModulesNamesInOrder, module.Name) - // Load validation schemas openAPIPath := filepath.Join(module.Path, "openapi") configBytes, valuesBytes, err := ReadOpenAPIFiles(openAPIPath) @@ -975,6 +942,7 @@ func (mm *moduleManager) RegisterModules() error { logEntry.Infof("Module from '%s'. %s", module.Path, mm.ValuesValidator.SchemaStorage.ModuleSchemasDescription(module.ValuesKey())) } + mm.modules = modules return nil } @@ -1008,30 +976,6 @@ func (m *Module) loadStaticValues() (err error) { return nil } -func (mm *moduleManager) loadCommonStaticValues() error { - valuesPath := filepath.Join(mm.ModulesDir, "values.yaml") - if _, err := os.Stat(valuesPath); os.IsNotExist(err) { - log.Debugf("No common static values file: %s", err) - return nil - } - - valuesYaml, err := os.ReadFile(valuesPath) - if err != nil { - return fmt.Errorf("load common values file '%s': %s", valuesPath, err) - } - - values, err := utils.NewValuesFromBytes(valuesYaml) - if err != nil { - return err - } - - mm.commonStaticValues = values - - log.Debugf("Successfully load common static values:\n%s", mm.commonStaticValues.DebugString()) - - return nil -} - func dumpData(filePath string, data []byte) error { err := os.WriteFile(filePath, data, 0o644) if err != nil { diff --git a/pkg/module_manager/module_loader.go b/pkg/module_manager/module_loader.go new file mode 100644 index 00000000..2eae87bf --- /dev/null +++ b/pkg/module_manager/module_loader.go @@ -0,0 +1,197 @@ +package module_manager + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + + "github.com/flant/addon-operator/pkg/app" + "github.com/flant/addon-operator/pkg/utils" + log "github.com/sirupsen/logrus" +) + +func SearchModules(modulesDirs string) (*ModuleSet, error) { + paths := splitToPaths(modulesDirs) + modules := new(ModuleSet) + for _, path := range paths { + modulesInDir, err := findModulesInDir(path) + if err != nil { + return nil, err + } + + // Add only "new" modules. Modules from first directories are in top priority as commands in $PATH. + for _, module := range modulesInDir { + if !modules.Has(module.Name) { + modules.Add(module) + } + } + } + + return modules, nil +} + +const PathsSeparator = ":" + +func splitToPaths(dir string) []string { + res := make([]string, 0) + paths := strings.Split(dir, PathsSeparator) + for _, path := range paths { + if path == "" { + continue + } + res = append(res, path) + } + return res +} + +func findModulesInDir(modulesDir string) ([]*Module, error) { + dirEntries, err := os.ReadDir(modulesDir) + if err != nil && os.IsNotExist(err) { + return nil, fmt.Errorf("path '%s' not exists", modulesDir) + } + if err != nil { + return nil, fmt.Errorf("list modules directory '%s': %s", modulesDir, err) + } + + modules := make([]*Module, 0) + for _, dirEntry := range dirEntries { + name, absPath, err := resolveDirEntry(modulesDir, dirEntry) + if err != nil { + return nil, err + } + // Skip non-directories. + if name == "" { + continue + } + + module, err := moduleFromDirName(name, absPath) + if err != nil { + return nil, err + } + + modules = append(modules, module) + } + + return modules, nil +} + +func resolveDirEntry(dirPath string, entry os.DirEntry) (string, string, error) { + name := entry.Name() + absPath := filepath.Join(dirPath, name) + + if entry.IsDir() { + return name, absPath, nil + } + // Check if entry is a symlink to a directory. + targetPath, err := resolveSymlinkToDir(dirPath, entry) + if err != nil { + return "", "", fmt.Errorf("resolve '%s' as a possible symlink: %v", absPath, err) + } + if targetPath != "" { + return name, targetPath, nil + } + + if name != "values.yaml" { + log.Warnf("Ignore '%s' while searching for modules", absPath) + } + return "", "", nil +} + +func resolveSymlinkToDir(dirPath string, entry os.DirEntry) (string, error) { + info, err := entry.Info() + if err != nil { + return "", err + } + targetDirPath, isTargetDir, err := utils.SymlinkInfo(filepath.Join(dirPath, info.Name()), info) + if err != nil { + return "", err + } + + if isTargetDir { + return targetDirPath, nil + } + + return "", nil +} + +// ValidModuleNameRe defines a valid module name. It may have a number prefix: it is an order of the module. +var ValidModuleNameRe = regexp.MustCompile(`^(([0-9]+)-)?(.+)$`) + +const ( + ModuleOrderIdx = 2 + ModuleNameIdx = 3 +) + +// moduleFromDirName returns Module instance filled with name, order and its absolute path. +func moduleFromDirName(dirName string, absPath string) (*Module, error) { + matchRes := ValidModuleNameRe.FindStringSubmatch(dirName) + if matchRes == nil { + return nil, fmt.Errorf("'%s' is invalid name for module: should match regex '%s'", dirName, ValidModuleNameRe.String()) + } + name := matchRes[ModuleNameIdx] + + // Check if name is consistent for conversions between kebab-case and camelCase. + valuesKey := utils.ModuleNameToValuesKey(name) + restoredName := utils.ModuleNameFromValuesKey(valuesKey) + if name != restoredName { + return nil, fmt.Errorf("'%s' name should be in kebab-case and be restorable from camelCase: consider renaming to '%s'", name, restoredName) + } + + module := NewModule(matchRes[ModuleNameIdx], + absPath, + parseIntOrDefault(matchRes[ModuleOrderIdx], app.UnnumberedModuleOrder), + ) + + return module, nil +} + +func parseIntOrDefault(num string, defaultValue int) int { + val, err := strconv.ParseInt(num, 10, 31) + if err != nil { + return defaultValue + } + return int(val) +} + +// LoadCommonStaticValues finds values.yaml files in all specified directories. +func LoadCommonStaticValues(modulesDirs string) (utils.Values, error) { + paths := splitToPaths(modulesDirs) + + res := make(map[string]interface{}) + for _, path := range paths { + valuesPath := filepath.Join(path, "values.yaml") + values, err := loadValuesFromFile(valuesPath) + if err != nil { + return nil, err + } + + for k, v := range values { + if _, ok := res[k]; !ok { + res[k] = v + } + } + } + + return res, nil +} + +func loadValuesFromFile(valuesPath string) (utils.Values, error) { + valuesYaml, err := os.ReadFile(valuesPath) + if err != nil && os.IsNotExist(err) { + log.Debugf("No common static values file '%s': %v", valuesPath, err) + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("load common values file '%s': %s", valuesPath, err) + } + + values, err := utils.NewValuesFromBytes(valuesYaml) + if err != nil { + return nil, err + } + + return values, nil +} diff --git a/pkg/module_manager/module_loader_test.go b/pkg/module_manager/module_loader_test.go new file mode 100644 index 00000000..3627bb2c --- /dev/null +++ b/pkg/module_manager/module_loader_test.go @@ -0,0 +1,61 @@ +package module_manager + +import ( + "testing" + + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" +) + +func TestDirWithSymlinks(t *testing.T) { + g := NewWithT(t) + dir := "testdata/module_loader/dir1" + + mods, err := SearchModules(dir) + g.Expect(err).ShouldNot(HaveOccurred()) + + g.Expect(mods.Has("module-one")).Should(BeTrue()) + g.Expect(mods.Has("module-two")).Should(BeTrue()) + + vals, err := LoadCommonStaticValues(dir) + g.Expect(err).ShouldNot(HaveOccurred(), "should load common values") + g.Expect(vals).Should(MatchAllKeys(Keys{ + "moduleOne": MatchAllKeys(Keys{ + "param1": Equal("val1"), + "param2": Equal("val2"), + }), + "moduleTwo": MatchAllKeys(Keys{ + "param1": Equal("val1"), + }), + }), "should load values for module-one and module-two") +} + +func TestLoadMultiDir(t *testing.T) { + g := NewWithT(t) + dirs := "testdata/module_loader/dir2:testdata/module_loader/dir3" + + mods, err := SearchModules(dirs) + g.Expect(err).ShouldNot(HaveOccurred()) + + g.Expect(mods.Has("module-one")).ShouldNot(BeTrue(), "should not load module-one") + g.Expect(mods.Has("module-two")).ShouldNot(BeTrue()) + + g.Expect(mods.Has("mod-one")).Should(BeTrue(), "should load module-one as mod-one") + g.Expect(mods.Has("mod-two")).Should(BeTrue(), "should load module-one as mod-two") + + vals, err := LoadCommonStaticValues(dirs) + g.Expect(err).ShouldNot(HaveOccurred(), "should load common values") + g.Expect(vals).Should(MatchAllKeys(Keys{ + "modOne": MatchAllKeys(Keys{ + "param1": Equal("val2"), + "param2": Equal("val2"), + }), + "modTwo": MatchAllKeys(Keys{ + "param1": Equal("val2"), + "param2": Equal("val2"), + }), + "moduleThree": MatchAllKeys(Keys{ + "param1": Equal("val3"), + }), + }), "should load values for mod-one and mod-two from dir2 and values for module-three from dir3") +} diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index 3c9328dd..7bc044d8 100644 --- a/pkg/module_manager/module_manager.go +++ b/pkg/module_manager/module_manager.go @@ -145,11 +145,8 @@ type moduleManager struct { hookMetricStorage *metric_storage.MetricStorage ValuesValidator *validation.ValuesValidator - // Index of all modules in modules directory. Key is module name. - allModulesByName map[string]*Module - - // Ordered list of all modules names for ordered iterations of allModulesByName. - allModulesNamesInOrder []string + // All known modules from specified directories ($MODULES_DIR) + modules *ModuleSet // TODO new layer of values for *Enabled values // commonStaticEnabledValues utils.Values // modules/values.yaml @@ -210,8 +207,7 @@ func NewModuleManager() *moduleManager { return &moduleManager{ ValuesValidator: validation.NewValuesValidator(), - allModulesByName: make(map[string]*Module), - allModulesNamesInOrder: make([]string, 0), + modules: new(ModuleSet), enabledModulesByConfig: make(map[string]struct{}), enabledModules: make([]string, 0), dynamicEnabled: make(map[string]*bool), @@ -342,7 +338,7 @@ func (mm *moduleManager) HandleNewKubeConfig(kubeConfig *kube_config_manager.Kub // Full reload if enabled flags are changed. isEnabledChanged := false - for moduleName := range mm.allModulesByName { + for _, moduleName := range mm.modules.NamesInOrder() { // Current module state. _, wasEnabled := mm.enabledModulesByConfig[moduleName] _, isEnabled := newEnabledByConfig[moduleName] @@ -432,7 +428,7 @@ func (mm *moduleManager) warnAboutUnknownModules(kubeConfig *kube_config_manager unknownNames := make([]string, 0) for moduleName := range kubeConfig.Modules { - if _, isKnown := mm.allModulesByName[moduleName]; !isKnown { + if !mm.modules.Has(moduleName) { unknownNames = append(unknownNames, moduleName) } } @@ -450,11 +446,11 @@ func (mm *moduleManager) warnAboutUnknownModules(kubeConfig *kube_config_manager func (mm *moduleManager) calculateEnabledModulesByConfig(config *kube_config_manager.KubeConfig) map[string]struct{} { enabledByConfig := make(map[string]struct{}) - for moduleName, module := range mm.allModulesByName { + for _, module := range mm.modules.List() { var kubeConfigEnabled *bool var kubeConfigEnabledStr string if config != nil { - if kubeConfig, hasKubeConfig := config.Modules[moduleName]; hasKubeConfig { + if kubeConfig, hasKubeConfig := config.Modules[module.Name]; hasKubeConfig { kubeConfigEnabled = kubeConfig.IsEnabled kubeConfigEnabledStr = kubeConfig.GetEnabled() } @@ -467,7 +463,7 @@ func (mm *moduleManager) calculateEnabledModulesByConfig(config *kube_config_man ) if isEnabled { - enabledByConfig[moduleName] = struct{}{} + enabledByConfig[module.Name] = struct{}{} } log.Debugf("enabledByConfig: module '%s' enabled flags: common '%v', static '%v', kubeConfig '%v', result: '%v'", @@ -491,7 +487,7 @@ func (mm *moduleManager) calculateEnabledModulesWithDynamic(enabledByConfig map[ log.Debugf("calculateEnabled: dynamicEnabled is %s", mm.DumpDynamicEnabled()) enabled := make([]string, 0) - for _, moduleName := range mm.allModulesNamesInOrder { + for _, moduleName := range mm.modules.NamesInOrder() { _, isEnabledByConfig := enabledByConfig[moduleName] isEnabled := mergeEnabled( @@ -622,7 +618,7 @@ func (mm *moduleManager) stateFromHelmReleases(releases []string) *ModulesState // Filter out known modules. enabledModules := make([]string, 0) - for _, modName := range mm.allModulesNamesInOrder { + for _, modName := range mm.modules.NamesInOrder() { // Remove known module to detect unknown ones. if _, has := releasesMap[modName]; has { // Treat known module as enabled module. @@ -685,7 +681,7 @@ func (mm *moduleManager) RefreshEnabledState(logLabels map[string]string) (*Modu // Disabled modules are that present in the list of currently enabled modules // but not present in the list after running enabled scripts disabledModules := utils.ListSubtract(mm.enabledModules, enabledModules) - disabledModules = utils.SortReverseByReference(disabledModules, mm.allModulesNamesInOrder) + disabledModules = utils.SortReverseByReference(disabledModules, mm.modules.NamesInOrder()) logEntry.Debugf("Refresh state results:\n"+ " mm.enabledModulesByConfig: %v\n"+ @@ -709,17 +705,15 @@ func (mm *moduleManager) RefreshEnabledState(logLabels map[string]string) (*Modu } func (mm *moduleManager) GetModule(name string) *Module { - module, exist := mm.allModulesByName[name] - if exist { - return module - } else { - log.Errorf("Possible bug!!! GetModule: no module '%s' in ModuleManager indexes", name) - return nil + if mm.modules.Has(name) { + return mm.modules.Get(name) } + log.Errorf("Possible bug!!! GetModule: no module '%s' in ModuleManager indexes", name) + return nil } func (mm *moduleManager) GetModuleNames() []string { - return mm.allModulesNamesInOrder + return mm.modules.NamesInOrder() } func (mm *moduleManager) GetEnabledModuleNames() []string { diff --git a/pkg/module_manager/module_manager_test.go b/pkg/module_manager/module_manager_test.go index f0a9855e..96f17c75 100644 --- a/pkg/module_manager/module_manager_test.go +++ b/pkg/module_manager/module_manager_test.go @@ -165,14 +165,14 @@ func Test_ModuleManager_LoadValuesInInit(t *testing.T) { }, } - assert.Contains(t, mm.allModulesByName, "with-values-1") - assert.Contains(t, mm.allModulesByName, "with-values-2") + assert.True(t, mm.modules.Has("with-values-1")) + assert.True(t, mm.modules.Has("with-values-2")) - with1 := mm.allModulesByName["with-values-1"] + with1 := mm.modules.Get("with-values-1") assert.NotNil(t, with1.StaticConfig) assert.Equal(t, modWithValues1Expected, with1.StaticConfig.Values) - with2 := mm.allModulesByName["with-values-2"] + with2 := mm.modules.Get("with-values-2") assert.NotNil(t, with2.StaticConfig) assert.Equal(t, modWithValues2Expected, with2.StaticConfig.Values) }, @@ -184,9 +184,9 @@ func Test_ModuleManager_LoadValuesInInit(t *testing.T) { func() { assert.Len(t, mm.commonStaticValues, 0) assert.Len(t, mm.commonStaticValues.Global(), 0) - assert.Len(t, mm.allModulesByName, 1) - assert.NotNil(t, mm.allModulesByName["module"].CommonStaticConfig) - assert.NotNil(t, mm.allModulesByName["module"].StaticConfig) + assert.Len(t, mm.modules.List(), 1) + assert.NotNil(t, mm.modules.Get("module").CommonStaticConfig) + assert.NotNil(t, mm.modules.Get("module").StaticConfig) }, }, { @@ -195,14 +195,14 @@ func Test_ModuleManager_LoadValuesInInit(t *testing.T) { func() { assert.Len(t, mm.commonStaticValues, 4) assert.Len(t, mm.commonStaticValues.Global(), 1) - assert.Len(t, mm.allModulesByName, 4) + assert.Len(t, mm.modules.List(), 4) - assert.Contains(t, mm.allModulesByName, "with-values-1") - assert.Contains(t, mm.allModulesByName, "with-values-2") - assert.Contains(t, mm.allModulesByName, "without-values") - assert.Contains(t, mm.allModulesByName, "with-kube-values") + assert.True(t, mm.modules.Has("with-values-1")) + assert.True(t, mm.modules.Has("with-values-2")) + assert.True(t, mm.modules.Has("without-values")) + assert.True(t, mm.modules.Has("with-kube-values")) - with1 := mm.allModulesByName["with-values-1"] + with1 := mm.modules.Get("with-values-1") assert.NotNil(t, with1.CommonStaticConfig) assert.NotNil(t, with1.StaticConfig) assert.Equal(t, "with-values-1", with1.CommonStaticConfig.ModuleName) @@ -223,12 +223,12 @@ func Test_ModuleManager_LoadValuesInInit(t *testing.T) { assert.NotContains(t, mm.kubeModulesConfigValues, "with-values-1") // all modules should have CommonStaticConfig and StaticConfig - assert.NotNil(t, mm.allModulesByName["with-values-2"].CommonStaticConfig) - assert.NotNil(t, mm.allModulesByName["with-values-2"].StaticConfig) - assert.NotNil(t, mm.allModulesByName["without-values"].CommonStaticConfig) - assert.NotNil(t, mm.allModulesByName["without-values"].StaticConfig) - assert.NotNil(t, mm.allModulesByName["with-kube-values"].CommonStaticConfig) - assert.NotNil(t, mm.allModulesByName["with-kube-values"].StaticConfig) + assert.NotNil(t, mm.modules.Get("with-values-2").CommonStaticConfig) + assert.NotNil(t, mm.modules.Get("with-values-2").StaticConfig) + assert.NotNil(t, mm.modules.Get("without-values").CommonStaticConfig) + assert.NotNil(t, mm.modules.Get("without-values").StaticConfig) + assert.NotNil(t, mm.modules.Get("with-kube-values").CommonStaticConfig) + assert.NotNil(t, mm.modules.Get("with-kube-values").StaticConfig) fmt.Printf("kubeModulesConfigValues: %#v\n", mm.kubeModulesConfigValues) @@ -263,11 +263,11 @@ func Test_ModuleManager_LoadValues_ApplyDefaults(t *testing.T) { // assert.Len(t, mm.commonStaticValues, 1) // assert.Len(t, mm.commonStaticValues.Global(), 1) - assert.Len(t, res.moduleManager.allModulesByName, 1) + assert.Len(t, res.moduleManager.modules.List(), 1) - assert.Contains(t, res.moduleManager.allModulesByName, "module-one") + assert.True(t, res.moduleManager.modules.Has("module-one")) - modOne := res.moduleManager.allModulesByName["module-one"] + modOne := res.moduleManager.modules.Get("module-one") assert.NotNil(t, modOne.CommonStaticConfig) assert.NotNil(t, modOne.StaticConfig) assert.Equal(t, "module-one", modOne.CommonStaticConfig.ModuleName) @@ -311,8 +311,8 @@ func Test_ModuleManager_LoadValues_ApplyDefaults(t *testing.T) { func Test_ModuleManager_Get_Module(t *testing.T) { mm, res := initModuleManager(t, "get__module") - programmaticModule := &Module{Name: "programmatic-module"} - res.moduleManager.allModulesByName["programmatic-module"] = programmaticModule + programmaticModule := &Module{Name: "programmatic-module", Order: 10} + res.moduleManager.modules.Add(programmaticModule) var module *Module @@ -468,7 +468,7 @@ func Test_ModuleManager_Get_ModuleHooksInOrder(t *testing.T) { "after-helm-binding-hooks", AfterHelm, func() { - assert.Len(t, res.moduleManager.allModulesByName, 1) + // assert.Len(t, res.moduleManager.allModulesByName, 1) assert.Len(t, res.moduleManager.modulesHooksOrderByName, 1) expectedOrder := []string{ diff --git a/pkg/module_manager/module_set.go b/pkg/module_manager/module_set.go new file mode 100644 index 00000000..12824179 --- /dev/null +++ b/pkg/module_manager/module_set.go @@ -0,0 +1,90 @@ +package module_manager + +import ( + "sort" + "sync" +) + +type ModuleSet struct { + modules map[string]*Module + orderedNames []string + lck sync.RWMutex +} + +func (s *ModuleSet) Add(modules ...*Module) { + if len(modules) == 0 { + return + } + + s.lck.Lock() + defer s.lck.Unlock() + + if s.modules == nil { + s.modules = make(map[string]*Module) + } + + for _, module := range modules { + // Invalidate ordered names cache. + if _, ok := s.modules[module.Name]; ok { + s.orderedNames = nil + } + s.modules[module.Name] = module + } +} + +func (s *ModuleSet) Get(name string) *Module { + s.lck.RLock() + defer s.lck.RUnlock() + return s.modules[name] +} + +func (s *ModuleSet) List() []*Module { + s.lck.Lock() + defer s.lck.Unlock() + list := make([]*Module, 0, len(s.modules)) + for _, name := range s.namesInOrder() { + list = append(list, s.modules[name]) + } + return list +} + +func (s *ModuleSet) NamesInOrder() []string { + s.lck.Lock() + defer s.lck.Unlock() + return s.namesInOrder() +} + +func (s *ModuleSet) namesInOrder() []string { + if s.orderedNames == nil { + s.orderedNames = sortModuleNames(s.modules) + } + return s.orderedNames +} + +func (s *ModuleSet) Has(name string) bool { + s.lck.RLock() + defer s.lck.RUnlock() + _, ok := s.modules[name] + return ok +} + +func sortModuleNames(modules map[string]*Module) []string { + // Get modules array. + mods := make([]*Module, 0, len(modules)) + for _, mod := range modules { + mods = append(mods, mod) + } + // Sort by order and name + sort.Slice(mods, func(i, j int) bool { + if mods[i].Order != mods[j].Order { + return mods[i].Order < mods[j].Order + } + return mods[i].Name < mods[j].Name + }) + // return names array. + names := make([]string, 0, len(modules)) + for _, mod := range mods { + names = append(names, mod.Name) + } + return names +} diff --git a/pkg/module_manager/module_set_test.go b/pkg/module_manager/module_set_test.go new file mode 100644 index 00000000..13de7830 --- /dev/null +++ b/pkg/module_manager/module_set_test.go @@ -0,0 +1,50 @@ +package module_manager + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestModuleSet(t *testing.T) { + g := NewWithT(t) + ms := new(ModuleSet) + + ms.Add(&Module{ + Name: "module-two", + Order: 10, + }) + ms.Add(&Module{ + Name: "module-one", + Order: 5, + }) + ms.Add(&Module{ + Name: "module-three-two", + Order: 15, + }) + ms.Add(&Module{ + Name: "module-four", + Order: 1, + }) + ms.Add(&Module{ + Name: "module-three-one", + Order: 15, + }) + // "overriden" module + ms.Add(&Module{ + Name: "module-four", + Order: 20, + }) + + expectNames := []string{ + "module-one", + "module-two", + "module-three-one", + "module-three-two", + "module-four", + } + + g.Expect(ms.NamesInOrder()).Should(Equal(expectNames)) + g.Expect(ms.Has("module-four")).Should(BeTrue(), "should have module-four") + g.Expect(ms.Get("module-four").Order).Should(Equal(20), "should have module-four with order:20") +} diff --git a/pkg/module_manager/testdata/module_loader/dir1/values.yaml b/pkg/module_manager/testdata/module_loader/dir1/values.yaml new file mode 100644 index 00000000..5dbcfe1e --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir1/values.yaml @@ -0,0 +1,5 @@ +moduleOne: + param1: val1 + param2: val2 +moduleTwo: + param1: val1 diff --git a/pkg/module_manager/testdata/module_loader/dir2/values.yaml b/pkg/module_manager/testdata/module_loader/dir2/values.yaml new file mode 100644 index 00000000..4cc435a8 --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir2/values.yaml @@ -0,0 +1,7 @@ +modOne: + param1: val2 + param2: val2 + +modTwo: + param1: val2 + param2: val2 diff --git a/pkg/module_manager/testdata/module_loader/dir3/values.yaml b/pkg/module_manager/testdata/module_loader/dir3/values.yaml new file mode 100644 index 00000000..dcb2a1a0 --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir3/values.yaml @@ -0,0 +1,8 @@ +modOne: + param1: val3 + param2: val3 +modTwo: + param1: val3 + param2: val3 +moduleThree: + param1: val3 \ No newline at end of file diff --git a/pkg/utils/values_test.go b/pkg/utils/values_test.go index 6578508a..e8a1f9f5 100644 --- a/pkg/utils/values_test.go +++ b/pkg/utils/values_test.go @@ -182,3 +182,44 @@ paramArr: g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(values).To(Equal(expected)) } + +func TestNameValuesKeyNameConsistence(t *testing.T) { + tests := []struct { + origName string + expectValuesKey string + expectName string + }{ + { + "module", + "module", + "module", + }, + { + "module-one", + "moduleOne", + "module-one", + }, + { + "module-one-0-1", + "moduleOne01", + "module-one-0-1", + }, + { + // Inconsistent module name! + "module_one", + "moduleOne", + "module-one", // Real expectation in "module_one", same as module name. + }, + } + + for _, test := range tests { + name := fmt.Sprintf("%s -> %s -> %s", test.origName, test.expectValuesKey, test.expectName) + t.Run(name, func(t *testing.T) { + g := NewWithT(t) + valuesKey := ModuleNameToValuesKey(test.origName) + moduleName := ModuleNameFromValuesKey(valuesKey) + newName := fmt.Sprintf("%s -> %s -> %s", test.origName, valuesKey, moduleName) + g.Expect(name).Should(Equal(newName), "should convert values key to original module name") + }) + } +} From 0c21278e40052da83d781d311d54c5b52f8f7914 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Mon, 6 Feb 2023 12:08:09 +0300 Subject: [PATCH 2/6] ++ Signed-off-by: Ivan Mikheykin --- pkg/module_manager/module_set_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/module_manager/module_set_test.go b/pkg/module_manager/module_set_test.go index 13de7830..b7f6b653 100644 --- a/pkg/module_manager/module_set_test.go +++ b/pkg/module_manager/module_set_test.go @@ -30,7 +30,7 @@ func TestModuleSet(t *testing.T) { Name: "module-three-one", Order: 15, }) - // "overriden" module + // "overridden" module ms.Add(&Module{ Name: "module-four", Order: 20, From b667311351ac89604e3ea66d46e014a4d1a678cf Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Mon, 6 Feb 2023 17:28:02 +0300 Subject: [PATCH 3/6] ++ gitkeep for empty dirs Signed-off-by: Ivan Mikheykin --- .../testdata/module_loader/modules/001-module-one/.gitkeep | 5 +++++ .../testdata/module_loader/modules/002-module-two/.gitkeep | 0 .../testdata/module_loader/modules/003-module-three/.gitkeep | 0 3 files changed, 5 insertions(+) create mode 100644 pkg/module_manager/testdata/module_loader/modules/001-module-one/.gitkeep create mode 100644 pkg/module_manager/testdata/module_loader/modules/002-module-two/.gitkeep create mode 100644 pkg/module_manager/testdata/module_loader/modules/003-module-three/.gitkeep diff --git a/pkg/module_manager/testdata/module_loader/modules/001-module-one/.gitkeep b/pkg/module_manager/testdata/module_loader/modules/001-module-one/.gitkeep new file mode 100644 index 00000000..a246fee1 --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/modules/001-module-one/.gitkeep @@ -0,0 +1,5 @@ +global: + a: 1 + b: 2 + c: 3 + d: ["a", "b", "c"] diff --git a/pkg/module_manager/testdata/module_loader/modules/002-module-two/.gitkeep b/pkg/module_manager/testdata/module_loader/modules/002-module-two/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/pkg/module_manager/testdata/module_loader/modules/003-module-three/.gitkeep b/pkg/module_manager/testdata/module_loader/modules/003-module-three/.gitkeep new file mode 100644 index 00000000..e69de29b From 60209f81874af029b1ba21f781200824cb15fc97 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Tue, 7 Feb 2023 15:14:55 +0300 Subject: [PATCH 4/6] ++ symlinks --- .../module_loader/dir1/001-module-one | 1 + .../module_loader/dir1/002-module-two | 1 + .../testdata/module_loader/dir2/012-mod-two | 1 + .../testdata/module_loader/dir2/100-mod-one | 1 + pkg/values/validation/validator_test.go | 36 +++++++++++++++++++ 5 files changed, 40 insertions(+) create mode 120000 pkg/module_manager/testdata/module_loader/dir1/001-module-one create mode 120000 pkg/module_manager/testdata/module_loader/dir1/002-module-two create mode 120000 pkg/module_manager/testdata/module_loader/dir2/012-mod-two create mode 120000 pkg/module_manager/testdata/module_loader/dir2/100-mod-one diff --git a/pkg/module_manager/testdata/module_loader/dir1/001-module-one b/pkg/module_manager/testdata/module_loader/dir1/001-module-one new file mode 120000 index 00000000..f5a46dfb --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir1/001-module-one @@ -0,0 +1 @@ +../modules/001-module-one \ No newline at end of file diff --git a/pkg/module_manager/testdata/module_loader/dir1/002-module-two b/pkg/module_manager/testdata/module_loader/dir1/002-module-two new file mode 120000 index 00000000..54f69ccb --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir1/002-module-two @@ -0,0 +1 @@ +../modules/002-module-two \ No newline at end of file diff --git a/pkg/module_manager/testdata/module_loader/dir2/012-mod-two b/pkg/module_manager/testdata/module_loader/dir2/012-mod-two new file mode 120000 index 00000000..54f69ccb --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir2/012-mod-two @@ -0,0 +1 @@ +../modules/002-module-two \ No newline at end of file diff --git a/pkg/module_manager/testdata/module_loader/dir2/100-mod-one b/pkg/module_manager/testdata/module_loader/dir2/100-mod-one new file mode 120000 index 00000000..f5a46dfb --- /dev/null +++ b/pkg/module_manager/testdata/module_loader/dir2/100-mod-one @@ -0,0 +1 @@ +../modules/001-module-one \ No newline at end of file diff --git a/pkg/values/validation/validator_test.go b/pkg/values/validation/validator_test.go index 2c8e0010..870861c6 100644 --- a/pkg/values/validation/validator_test.go +++ b/pkg/values/validation/validator_test.go @@ -164,3 +164,39 @@ properties: g.Expect(mErr.Error()).Should(ContainSubstring("3 errors occurred")) g.Expect(mErr.Error()).Should(ContainSubstring("forbidden property")) } + +func Test_Validate_MultiplyOfInt(t *testing.T) { + g := NewWithT(t) + + var err error + var moduleValues utils.Values + moduleValues, err = utils.NewValuesFromBytes([]byte(` +moduleName: + sampling: 100 +# sampling2: 56.105 +`)) + g.Expect(err).ShouldNot(HaveOccurred()) + + configSchemaYaml := ` +type: object +additionalProperties: false +properties: + sampling: + type: number + minimum: 0.01 + maximum: 100.0 + multipleOf: 0.01 + sampling2: + type: number + minimum: 0.01 + maximum: 100.0 + multipleOf: 0.01 +` + v := NewValuesValidator() + + err = v.SchemaStorage.AddModuleValuesSchemas("moduleName", []byte(configSchemaYaml), nil) + g.Expect(err).ShouldNot(HaveOccurred()) + + mErr := v.ValidateModuleConfigValues("moduleName", moduleValues) + g.Expect(mErr).ShouldNot(HaveOccurred()) +} From d7e1c58c19cb750d5a6c852228c1fdcdd6918195 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Wed, 8 Feb 2023 11:53:12 +0300 Subject: [PATCH 5/6] ++ english Co-authored-by: Eugene Shevchenko --- pkg/module_manager/module_loader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/module_manager/module_loader.go b/pkg/module_manager/module_loader.go index 2eae87bf..08d364cf 100644 --- a/pkg/module_manager/module_loader.go +++ b/pkg/module_manager/module_loader.go @@ -50,10 +50,10 @@ func splitToPaths(dir string) []string { func findModulesInDir(modulesDir string) ([]*Module, error) { dirEntries, err := os.ReadDir(modulesDir) if err != nil && os.IsNotExist(err) { - return nil, fmt.Errorf("path '%s' not exists", modulesDir) + return nil, fmt.Errorf("path '%s' does not exist", modulesDir) } if err != nil { - return nil, fmt.Errorf("list modules directory '%s': %s", modulesDir, err) + return nil, fmt.Errorf("listing modules directory '%s': %s", modulesDir, err) } modules := make([]*Module, 0) From 34a3afcdb91ceead03866c42fc2bf0f29d8cdae8 Mon Sep 17 00:00:00 2001 From: Ivan Mikheykin Date: Wed, 8 Feb 2023 12:20:50 +0300 Subject: [PATCH 6/6] ++ remove some 'Possible bug' messages, use const for "values.yaml", explain data type for UNNUMBERED_MODULE_ORDER Signed-off-by: Ivan Mikheykin --- RUNNING.md | 2 +- pkg/module_manager/module.go | 6 +++--- pkg/module_manager/module_loader.go | 21 +++++++++++-------- pkg/module_manager/module_manager.go | 15 ++----------- .../testdata/module_loader/dir3/values.yaml | 2 +- 5 files changed, 19 insertions(+), 27 deletions(-) diff --git a/RUNNING.md b/RUNNING.md index debcb923..9c562134 100644 --- a/RUNNING.md +++ b/RUNNING.md @@ -6,7 +6,7 @@ **MODULES_DIR** — paths separated by colon where modules are located. -**UNNUMBERED_MODULE_ORDER** — a default order for modules without numbered prefix. +**UNNUMBERED_MODULE_ORDER** — an integer number to use as the default order for modules without numbered prefix. **ADDON_OPERATOR_NAMESPACE** — a required parameter with namespace where Addon-operator is deployed. diff --git a/pkg/module_manager/module.go b/pkg/module_manager/module.go index 49f38bf9..7ee1257c 100644 --- a/pkg/module_manager/module.go +++ b/pkg/module_manager/module.go @@ -953,13 +953,13 @@ func (m *Module) loadStaticValues() (err error) { if err != nil { return err } - log.Debugf("module %s common static values: %s", m.Name, m.CommonStaticConfig.Values.DebugString()) + log.Debugf("module %s static values in common file: %s", m.Name, m.CommonStaticConfig.Values.DebugString()) - valuesYamlPath := filepath.Join(m.Path, "values.yaml") + valuesYamlPath := filepath.Join(m.Path, ValuesFileName) if _, err := os.Stat(valuesYamlPath); os.IsNotExist(err) { m.StaticConfig = utils.NewModuleConfig(m.Name) - log.Debugf("module %s is static disabled: no values.yaml exists", m.Name) + log.Debugf("module %s has no static values", m.Name) return nil } diff --git a/pkg/module_manager/module_loader.go b/pkg/module_manager/module_loader.go index 08d364cf..18d4f8c4 100644 --- a/pkg/module_manager/module_loader.go +++ b/pkg/module_manager/module_loader.go @@ -13,6 +13,11 @@ import ( log "github.com/sirupsen/logrus" ) +const ( + PathsSeparator = ":" + ValuesFileName = "values.yaml" +) + func SearchModules(modulesDirs string) (*ModuleSet, error) { paths := splitToPaths(modulesDirs) modules := new(ModuleSet) @@ -33,8 +38,6 @@ func SearchModules(modulesDirs string) (*ModuleSet, error) { return modules, nil } -const PathsSeparator = ":" - func splitToPaths(dir string) []string { res := make([]string, 0) paths := strings.Split(dir, PathsSeparator) @@ -94,7 +97,7 @@ func resolveDirEntry(dirPath string, entry os.DirEntry) (string, string, error) return name, targetPath, nil } - if name != "values.yaml" { + if name != ValuesFileName { log.Warnf("Ignore '%s' while searching for modules", absPath) } return "", "", nil @@ -162,8 +165,7 @@ func LoadCommonStaticValues(modulesDirs string) (utils.Values, error) { res := make(map[string]interface{}) for _, path := range paths { - valuesPath := filepath.Join(path, "values.yaml") - values, err := loadValuesFromFile(valuesPath) + values, err := loadValuesFileFromDir(path) if err != nil { return nil, err } @@ -178,14 +180,15 @@ func LoadCommonStaticValues(modulesDirs string) (utils.Values, error) { return res, nil } -func loadValuesFromFile(valuesPath string) (utils.Values, error) { - valuesYaml, err := os.ReadFile(valuesPath) +func loadValuesFileFromDir(dir string) (utils.Values, error) { + valuesFilePath := filepath.Join(dir, ValuesFileName) + valuesYaml, err := os.ReadFile(valuesFilePath) if err != nil && os.IsNotExist(err) { - log.Debugf("No common static values file '%s': %v", valuesPath, err) + log.Debugf("No common static values file '%s': %v", valuesFilePath, err) return nil, nil } if err != nil { - return nil, fmt.Errorf("load common values file '%s': %s", valuesPath, err) + return nil, fmt.Errorf("load common values file '%s': %s", valuesFilePath, err) } values, err := utils.NewValuesFromBytes(valuesYaml) diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index 7bc044d8..e9b2d7a6 100644 --- a/pkg/module_manager/module_manager.go +++ b/pkg/module_manager/module_manager.go @@ -705,11 +705,7 @@ func (mm *moduleManager) RefreshEnabledState(logLabels map[string]string) (*Modu } func (mm *moduleManager) GetModule(name string) *Module { - if mm.modules.Has(name) { - return mm.modules.Get(name) - } - log.Errorf("Possible bug!!! GetModule: no module '%s' in ModuleManager indexes", name) - return nil + return mm.modules.Get(name) } func (mm *moduleManager) GetModuleNames() []string { @@ -730,13 +726,7 @@ func (mm *moduleManager) IsModuleEnabled(moduleName string) bool { } func (mm *moduleManager) GetGlobalHook(name string) *GlobalHook { - globalHook, exist := mm.globalHooksByName[name] - if exist { - return globalHook - } else { - log.Errorf("Possible bug!!! GetGlobalHook: no global hook '%s' in ModuleManager indexes", name) - return nil - } + return mm.globalHooksByName[name] } func (mm *moduleManager) GetModuleHook(name string) *ModuleHook { @@ -749,7 +739,6 @@ func (mm *moduleManager) GetModuleHook(name string) *ModuleHook { } } } - log.Errorf("Possible bug!!! GetModuleHook: no module hook '%s' in ModuleManager indexes", name) return nil } diff --git a/pkg/module_manager/testdata/module_loader/dir3/values.yaml b/pkg/module_manager/testdata/module_loader/dir3/values.yaml index dcb2a1a0..4b79a8a1 100644 --- a/pkg/module_manager/testdata/module_loader/dir3/values.yaml +++ b/pkg/module_manager/testdata/module_loader/dir3/values.yaml @@ -5,4 +5,4 @@ modTwo: param1: val3 param2: val3 moduleThree: - param1: val3 \ No newline at end of file + param1: val3