Skip to content

Commit 6c00d38

Browse files
ldmonsterldmonster
committed
fix nil, add error handling, fix logger
Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
1 parent 6cc8378 commit 6c00d38

File tree

3 files changed

+55
-39
lines changed

3 files changed

+55
-39
lines changed

‎pkg/addon-operator/debug_server.go‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package addon_operator
22

33
import (
44
"bytes"
5+
"errors"
56
"fmt"
67
"image/png"
78
"net/http"
@@ -54,12 +55,12 @@ func (op *AddonOperator) RegisterDebugGraphRoutes(dbgSrv *debug.Server) {
5455
dbgSrv.RegisterHandler(http.MethodGet, "/graph", func(_ *http.Request) (interface{}, error) {
5556
image, err := op.ModuleManager.GetGraphImage()
5657
if err != nil {
57-
return nil, fmt.Errorf("couldn't get graph's image")
58+
return nil, fmt.Errorf("couldn't get graph's image: %w", err)
5859
}
5960

6061
buf := new(bytes.Buffer)
6162
if err = png.Encode(buf, image); err != nil {
62-
return nil, fmt.Errorf("couldn't encode graph's image")
63+
return nil, fmt.Errorf("couldn't encode graph's image: %w", err)
6364
}
6465

6566
return buf.String(), nil
@@ -126,13 +127,19 @@ func (op *AddonOperator) RegisterDebugModuleRoutes(dbgSrv *debug.Server) {
126127

127128
m := op.ModuleManager.GetModule(modName)
128129
if m == nil {
129-
return nil, fmt.Errorf("Module not found")
130+
return nil, fmt.Errorf("module not found")
130131
}
131132

132133
deps := &modules.HelmModuleDependencies{
133134
HelmClientFactory: op.Helm,
134135
}
136+
135137
hm, err := modules.NewHelmModule(m, op.DefaultNamespace, op.ModuleManager.TempDir, deps, nil, modules.WithLogger(op.Logger.Named("helm-module")))
138+
// if module is not helm, success empty result
139+
if err != nil && errors.Is(err, modules.ErrModuleIsNotHelm) {
140+
return nil, nil
141+
}
142+
136143
if err != nil {
137144
return nil, fmt.Errorf("failed to create helm module: %w", err)
138145
}
@@ -145,7 +152,7 @@ func (op *AddonOperator) RegisterDebugModuleRoutes(dbgSrv *debug.Server) {
145152

146153
m := op.ModuleManager.GetModule(modName)
147154
if m == nil {
148-
return nil, fmt.Errorf("Unknown module %s", modName)
155+
return nil, fmt.Errorf("unknown module %s", modName)
149156
}
150157

151158
return m.GetValuesPatches(), nil
@@ -172,7 +179,7 @@ func (op *AddonOperator) RegisterDebugModuleRoutes(dbgSrv *debug.Server) {
172179

173180
m := op.ModuleManager.GetModule(modName)
174181
if m == nil {
175-
return nil, fmt.Errorf("Module not found")
182+
return nil, fmt.Errorf("module not found")
176183
}
177184

178185
mHooks := m.GetHooks()
@@ -203,7 +210,7 @@ func (op *AddonOperator) RegisterDiscoveryRoute(dbgSrv *debug.Server) {
203210

204211
err := chi.Walk(dbgSrv.Router, walkFn)
205212
if err != nil {
206-
return nil, err
213+
return nil, fmt.Errorf("chi walk: %w", err)
207214
}
208215

209216
return buf, nil

‎pkg/module_manager/models/modules/helm.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package modules
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"log/slog"
78
"os"
@@ -61,6 +62,8 @@ type HelmModuleDependencies struct {
6162
HelmValuesValidator HelmValuesValidator
6263
}
6364

65+
var ErrModuleIsNotHelm = errors.New("module is not a helm")
66+
6467
// NewHelmModule build HelmModule from the Module templates and values + global values
6568
func NewHelmModule(bm *BasicModule, namespace string, tmpDir string, deps *HelmModuleDependencies, validator HelmValuesValidator, opts ...ModuleOption) (*HelmModule, error) {
6669
moduleValues := bm.GetValues(false)
@@ -96,7 +99,7 @@ func NewHelmModule(bm *BasicModule, namespace string, tmpDir string, deps *HelmM
9699
if !isHelm {
97100
hm.logger.Info("module has neither Chart.yaml nor templates/ dir, is't not a helm chart",
98101
slog.String("name", bm.Name))
99-
return nil, nil
102+
return nil, ErrModuleIsNotHelm
100103
}
101104

102105
return hm, nil
@@ -353,7 +356,7 @@ func (hm *HelmModule) PrepareValuesYamlFile() (string, error) {
353356
return "", err
354357
}
355358

356-
log.Debug("Prepared module helm values info",
359+
hm.logger.Debug("Prepared module helm values info",
357360
slog.String("moduleName", hm.name),
358361
slog.String("values", hm.values.DebugString()))
359362

‎pkg/module_manager/module_manager.go‎

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package module_manager
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"image"
78
"log/slog"
@@ -191,7 +192,7 @@ func (mm *ModuleManager) GetGlobal() *modules.GlobalModule {
191192
func (mm *ModuleManager) ApplyNewKubeConfigValues(kubeConfig *config.KubeConfig, globalValuesChanged bool) error {
192193
if kubeConfig == nil {
193194
// have no idea, how it could be, just skip run
194-
log.Warn("No KubeConfig is set")
195+
mm.logger.Warn("No KubeConfig is set")
195196
return nil
196197
}
197198

@@ -213,7 +214,7 @@ func (mm *ModuleManager) ApplyNewKubeConfigValues(kubeConfig *config.KubeConfig,
213214
newGlobalValues, ok := valuesMap[mm.global.GetName()]
214215
if ok {
215216
if globalValuesChanged {
216-
log.Debug("Applying global values",
217+
mm.logger.Debug("Applying global values",
217218
slog.String("values", fmt.Sprintf("%v", newGlobalValues)))
218219
mm.global.SaveConfigValues(newGlobalValues)
219220
}
@@ -228,7 +229,7 @@ func (mm *ModuleManager) ApplyNewKubeConfigValues(kubeConfig *config.KubeConfig,
228229
}
229230

230231
if mod.GetConfigValues(false).Checksum() != values.Checksum() {
231-
log.Debug("Applying values to module",
232+
mm.logger.Debug("Applying values to module",
232233
slog.String("moduleName", moduleName),
233234
slog.String("values", fmt.Sprintf("%v", values)),
234235
slog.String("oldValues", fmt.Sprintf("%v", mod.GetConfigValues(false))))
@@ -298,7 +299,7 @@ func (mm *ModuleManager) warnAboutUnknownModules(kubeConfig *config.KubeConfig)
298299
}
299300
}
300301
if len(unknownNames) > 0 {
301-
log.Warn("KubeConfigManager has values for unknown modules",
302+
mm.logger.Warn("KubeConfigManager has values for unknown modules",
302303
slog.Any("modules", unknownNames))
303304
}
304305
}
@@ -404,7 +405,7 @@ func (mm *ModuleManager) RefreshStateFromHelmReleases(logLabels map[string]strin
404405
if err != nil {
405406
return nil, err
406407
}
407-
log.Debug("Following releases found",
408+
mm.logger.Debug("Following releases found",
408409
slog.Any("modules", releasedModules))
409410

410411
return mm.stateFromHelmReleases(releasedModules), nil
@@ -423,7 +424,7 @@ func (mm *ModuleManager) stateFromHelmReleases(releases []string) *ModulesState
423424
purge = utils.SortReverse(purge)
424425

425426
if len(purge) > 0 {
426-
log.Info("Modules to purge found",
427+
mm.logger.Info("Modules to purge found",
427428
slog.Any("modules", purge))
428429
}
429430

@@ -443,7 +444,7 @@ func (mm *ModuleManager) SetGlobalDiscoveryAPIVersions(apiVersions []string) {
443444
switch mm.moduleLoader.(type) {
444445
case *fs.FileSystemLoader:
445446
default:
446-
log.Debug("non-default module loader detected - applying apiVersions patch")
447+
mm.logger.Debug("non-default module loader detected - applying apiVersions patch")
447448
mm.global.SetAvailableAPIVersions(apiVersions)
448449
}
449450
}
@@ -606,6 +607,7 @@ func (mm *ModuleManager) DeleteModule(moduleName string, logLabels map[string]st
606607
MetricsStorage: mm.dependencies.MetricStorage,
607608
HelmValuesValidator: schemaStorage,
608609
}
610+
609611
helmModule, _ := modules.NewHelmModule(ml, mm.defaultNamespace, mm.TempDir, &hmdeps, schemaStorage, modules.WithLogger(mm.logger.Named("helm-module")))
610612
if helmModule != nil {
611613
releaseExists, err := mm.dependencies.Helm.NewClient(mm.logger, deleteLogLabels).IsReleaseExists(ml.GetName())
@@ -622,14 +624,14 @@ func (mm *ModuleManager) DeleteModule(moduleName string, logLabels map[string]st
622624
// Chart and release are existed, so run helm delete command
623625
err := mm.dependencies.Helm.NewClient(mm.logger, deleteLogLabels).DeleteRelease(ml.GetName())
624626
if err != nil {
625-
return err
627+
return fmt.Errorf("create helm client: %w", err)
626628
}
627629
}
628630
}
629631

630632
err := ml.RunHooksByBinding(AfterDeleteHelm, deleteLogLabels)
631633
if err != nil {
632-
return err
634+
return fmt.Errorf("run hooks by bindng: %w", err)
633635
}
634636

635637
// Cleanup state.
@@ -664,7 +666,7 @@ func (mm *ModuleManager) RunModule(moduleName string, logLabels map[string]strin
664666
err = bm.RunHooksByBinding(BeforeHelm, logLabels)
665667
treg.End()
666668
if err != nil {
667-
return false, err
669+
return false, fmt.Errorf("run hooks by binding: %w", err)
668670
}
669671

670672
treg = trace.StartRegion(context.Background(), "ModuleRun-HelmPhase-helm")
@@ -675,17 +677,19 @@ func (mm *ModuleManager) RunModule(moduleName string, logLabels map[string]strin
675677
MetricsStorage: mm.dependencies.MetricStorage,
676678
HelmValuesValidator: schemaStorage,
677679
}
680+
678681
helmModule, err := modules.NewHelmModule(bm, mm.defaultNamespace, mm.TempDir, deps, schemaStorage, modules.WithLogger(mm.logger.Named("helm-module")))
679-
if err != nil {
680-
return false, err
682+
if err != nil && !errors.Is(err, modules.ErrModuleIsNotHelm) {
683+
return false, fmt.Errorf("helm module create: %w", err)
681684
}
682-
if helmModule != nil {
683-
// could be nil, if it doesn't contain helm chart
685+
686+
if err == nil {
684687
err = helmModule.RunHelmInstall(logLabels)
685688
}
689+
686690
treg.End()
687-
if err != nil {
688-
return false, err
691+
if err != nil && !errors.Is(err, modules.ErrModuleIsNotHelm) {
692+
return false, fmt.Errorf("run helm install: %w", err)
689693
}
690694

691695
oldValues := bm.GetValues(false)
@@ -694,7 +698,7 @@ func (mm *ModuleManager) RunModule(moduleName string, logLabels map[string]strin
694698
err = bm.RunHooksByBinding(AfterHelm, logLabels)
695699
treg.End()
696700
if err != nil {
697-
return false, err
701+
return false, fmt.Errorf("run hooks by binding: %w", err)
698702
}
699703

700704
newValues := bm.GetValues(false)
@@ -745,7 +749,7 @@ func (mm *ModuleManager) HandleGlobalEnableKubernetesBindings(hookName string, c
745749
}
746750
})
747751
if err != nil {
748-
return err
752+
return fmt.Errorf("handle enable kubernetes bindings: %w", err)
749753
}
750754

751755
return nil
@@ -763,7 +767,7 @@ func (mm *ModuleManager) HandleModuleEnableKubernetesBindings(moduleName string,
763767
}
764768
})
765769
if err != nil {
766-
return err
770+
return fmt.Errorf("handle enable kubernetes bindings: %w", err)
767771
}
768772
}
769773

@@ -865,19 +869,19 @@ func (mm *ModuleManager) applyEnabledPatch(enabledPatch utils.ValuesPatch, exten
865869
modName = utils.ModuleNameFromValuesKey(modName)
866870
v, err := utils.ModuleEnabledValue(op.Value)
867871
if err != nil {
868-
return fmt.Errorf("apply enabled patch operation '%s' for %s: %v", op.Op, op.Path, err)
872+
return fmt.Errorf("apply enabled patch operation '%s' for %s: %w", op.Op, op.Path, err)
869873
}
870874
switch op.Op {
871875
case "add":
872-
log.Debug("apply dynamic enable",
876+
mm.logger.Debug("apply dynamic enable",
873877
slog.String("module", modName),
874878
slog.Bool("value", *v))
875879
case "remove":
876-
log.Debug("apply dynamic enable: module removed from dynamic enable",
880+
mm.logger.Debug("apply dynamic enable: module removed from dynamic enable",
877881
slog.String("module", modName))
878882
}
879883
extender.UpdateStatus(modName, op.Op, *v)
880-
log.Info("dynamically enabled module status change",
884+
mm.logger.Info("dynamically enabled module status change",
881885
slog.String("module", modName),
882886
slog.String("operation", op.Op),
883887
slog.Bool("state", *v))
@@ -946,7 +950,7 @@ func (mm *ModuleManager) ApplyBindingActions(moduleHook *hooks.ModuleHook, bindi
946950
// Recreate monitor. Synchronization phase is ignored, kubernetes events are allowed.
947951
err := moduleHook.GetHookController().UpdateMonitor(monitorCfg.Metadata.MonitorId, action.Kind, action.ApiVersion)
948952
if err != nil {
949-
return err
953+
return fmt.Errorf("update monitor: %w", err)
950954
}
951955
}
952956
return nil
@@ -1019,12 +1023,12 @@ func (mm *ModuleManager) RunModuleWithNewOpenAPISchema(moduleName, moduleSource,
10191023

10201024
basicModule, err := mm.moduleLoader.LoadModule(moduleSource, modulePath)
10211025
if err != nil {
1022-
return err
1026+
return fmt.Errorf("load module: %w", err)
10231027
}
10241028

10251029
err = currentModule.ApplyNewSchemaStorage(basicModule.GetSchemaStorage())
10261030
if err != nil {
1027-
return err
1031+
return fmt.Errorf("apply new schema storage: %w", err)
10281032
}
10291033

10301034
if mm.IsModuleEnabled(moduleName) {
@@ -1249,16 +1253,18 @@ func queueHasPendingModuleDeleteTask(q *queue.TaskQueue, moduleName string) bool
12491253
// registerModules load all available modules from modules directory.
12501254
func (mm *ModuleManager) registerModules(scriptEnabledExtender *script_extender.Extender) error {
12511255
if mm.ModulesDir == "" {
1252-
log.Warn("Empty modules directory is passed! No modules to load.")
1256+
mm.logger.Warn("empty modules directory is passed, no modules to load")
1257+
12531258
return nil
12541259
}
12551260

12561261
if mm.moduleLoader == nil {
1257-
log.Error("no module loader set")
1262+
mm.logger.Error("no module loader set")
1263+
12581264
return fmt.Errorf("no module loader set")
12591265
}
12601266

1261-
log.Debug("Search and register modules")
1267+
mm.logger.Debug("search and register modules")
12621268

12631269
mods, err := mm.moduleLoader.LoadModules()
12641270
if err != nil {
@@ -1278,7 +1284,7 @@ func (mm *ModuleManager) registerModules(scriptEnabledExtender *script_extender.
12781284

12791285
for _, mod := range mods {
12801286
if set.Has(mod.GetName()) {
1281-
log.Warn("module is not registered, because it has a duplicate",
1287+
mm.logger.Warn("module is not registered, because it has a duplicate",
12821288
slog.String("module", mod.GetName()),
12831289
slog.String("path", mod.GetPath()))
12841290
continue
@@ -1299,7 +1305,7 @@ func (mm *ModuleManager) registerModules(scriptEnabledExtender *script_extender.
12991305
})
13001306
}
13011307

1302-
log.Debug("Found modules",
1308+
mm.logger.Debug("Found modules",
13031309
slog.Any("modules", set.NamesInOrder()))
13041310

13051311
mm.modules = set

0 commit comments

Comments
 (0)