[modules] 'Enabled' scripts add reason (#636)

Signed-off-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
Co-authored-by: Sinelnikov Michail <mikhail.sinelnikov@flant.com>
Co-authored-by: Pavel Okhlopkov <36456348+ldmonster@users.noreply.github.com>

Author: 3 people

docs/src/LIFECYCLE-STEPS.md

@@ -89,8 +89,10 @@ Startup steps:
           - '{moduleName}' section in ConfigMap
           - patched with patches saved from previous module hooks
     - output
-      - 'enabled' state ($MODULE_ENABLED_RESULT temporary file)
-        - if hook return "false", hook state is disabled
+      - **module status**
+        - `$MODULE_ENABLED_RESULT` - *required* file, script writes `true` or `false`
+          - if `$MODULE_ENABLED_RESULT` contains `false`, the module is considered disabled
+        - `$MODULE_ENABLED_REASON` - *optional* file, the script can put a human-readable reason string
   - no 'enabled' script
     - merged 'enabled' state is used
   - create 3 lists

docs/src/LIFECYCLE.md

@@ -88,7 +88,11 @@ As a result of a 'module discovery' process, the tasks for the execution of all
 
 ### Enabled script
 
-A script or an executable file that returns the status of the module. The script has access to the module values in `$VALUES_PATH` and `$CONFIG_VALUES_PATH` files, more details about the values are available [here](VALUES.md#using-the-values-in-enabled-scripts). The variable `$MODULE_ENABLED_RESULT` passes the path to the file into which the script should write the module status: `true` or `false`.
+A script or an executable file that returns the status of the module. The script has access to the module values in `$VALUES_PATH` and `$CONFIG_VALUES_PATH` files, more details about the values are available [here](VALUES.md#using-the-values-in-enabled-scripts).
+
+The addon-operator exports two environment variables:
+**`$MODULE_ENABLED_RESULT`** — *required* path to a temporary file; Write `true` or `false` here.
+**`$MODULE_ENABLED_REASON`** — *optional* path to another file; write a **explanation** when the script decides to disable the module.  
 
 Below is an example of the `enabled` script that disables the module when parameter `param2` is set to "stopMePlease".
 
@@ -99,6 +103,7 @@ param2=$(jq -r '.simpleModule.param2' $VALUES_PATH)
 
 if [[ $param2 == "stopMePlease" ]] ; then
   echo "false" > $MODULE_ENABLED_RESULT
+  echo "Module disabled: param2 == stopMePlease" > "$MODULE_ENABLED_REASON"
 else
   echo "true" > $MODULE_ENABLED_RESULT
 fi

pkg/module_manager/models/modules/basic.go

@@ -762,6 +762,26 @@ func (bm *BasicModule) RunEnabledScript(ctx context.Context, tmpDir string, prec
 		}
 	}()
 
+	reasonFilePath, err := bm.prepareModuleEnabledReasonFile(tmpDir)
+	if err != nil {
+		logEntry.Error("Prepare MODULE_ENABLED_REASON file",
+			slog.String("path", reasonFilePath),
+			log.Err(err))
+		return false, err
+	}
+	defer func() {
+		if bm.keepTemporaryHookFiles {
+			return
+		}
+		err := os.Remove(reasonFilePath)
+		if err != nil {
+			bm.logger.With("module", bm.GetName()).
+				Error("Remove tmp file",
+					slog.String("path", enabledResultFilePath),
+					log.Err(err))
+		}
+	}()
+
 	logEntry.Debug("Execute enabled script",
 		slog.String("path", enabledScriptPath),
 		slog.Any("modules", precedingEnabledModules))
@@ -771,6 +791,7 @@ func (bm *BasicModule) RunEnabledScript(ctx context.Context, tmpDir string, prec
 	envs = append(envs, fmt.Sprintf("CONFIG_VALUES_PATH=%s", configValuesPath))
 	envs = append(envs, fmt.Sprintf("VALUES_PATH=%s", valuesPath))
 	envs = append(envs, fmt.Sprintf("MODULE_ENABLED_RESULT=%s", enabledResultFilePath))
+	envs = append(envs, fmt.Sprintf("MODULE_ENABLED_REASON=%s", reasonFilePath))
 
 	if err := bm.AssembleEnvironmentForModule(environmentmanager.EnabledScriptEnvironment); err != nil {
 		return false, fmt.Errorf("Assemble %q module's environment: %w", bm.GetName(), err)
@@ -821,8 +842,21 @@ func (bm *BasicModule) RunEnabledScript(ctx context.Context, tmpDir string, prec
 	logEntry.Info("Enabled script run successful",
 		slog.Bool("result", moduleEnabled),
 		slog.String("status", result))
+	var reason string
+	if !moduleEnabled {
+		reason, err = bm.readModuleEnabledReason(reasonFilePath)
+		if err != nil {
+			logEntry.Error("Read enabled result",
+				slog.String("path", enabledScriptPath),
+				log.Err(err))
+			return false, fmt.Errorf("bad enabled result")
+		}
+	}
 	bm.l.Lock()
 	bm.state.enabledScriptResult = &moduleEnabled
+	if reason != "" {
+		bm.state.enabledScriptReason = &reason
+	}
 	bm.l.Unlock()
 	return moduleEnabled, nil
 }
@@ -840,6 +874,14 @@ func (bm *BasicModule) prepareModuleEnabledResultFile(tmpdir string) (string, er
 	return path, nil
 }
 
+func (bm *BasicModule) prepareModuleEnabledReasonFile(tmpdir string) (string, error) {
+	path := filepath.Join(tmpdir, fmt.Sprintf("%s.module-enabled-reason", bm.GetName()))
+	if err := utils.CreateEmptyWritableFile(path); err != nil {
+		return "", err
+	}
+	return path, nil
+}
+
 func (bm *BasicModule) readModuleEnabledResult(filePath string) (bool, error) {
 	data, err := os.ReadFile(filePath)
 	if err != nil {
@@ -858,6 +900,14 @@ func (bm *BasicModule) readModuleEnabledResult(filePath string) (bool, error) {
 	return false, fmt.Errorf("expected 'true' or 'false', got '%s'", value)
 }
 
+func (bm *BasicModule) readModuleEnabledReason(path string) (string, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return "", fmt.Errorf("cannot read %s: %w", path, err)
+	}
+	return strings.TrimSpace(string(data)), nil
+}
+
 // VALUES_PATH
 func (bm *BasicModule) prepareValuesJsonFileWith(tmpdir string, values utils.Values) (string, error) {
 	data, err := values.JsonBytes()
@@ -1220,6 +1270,13 @@ func (bm *BasicModule) GetEnabledScriptResult() *bool {
 	return bm.state.enabledScriptResult
 }
 
+// GetEnabledScriptReason returns a string why the module is disabled
+func (bm *BasicModule) GetEnabledScriptReason() *string {
+	bm.l.RLock()
+	defer bm.l.RUnlock()
+	return bm.state.enabledScriptReason
+}
+
 // GetLastHookError get error of the last executed hook
 func (bm *BasicModule) GetLastHookError() error {
 	bm.l.RLock()
@@ -1358,4 +1415,5 @@ type moduleState struct {
 	hookErrors           map[string]error
 	synchronizationState *SynchronizationState
 	enabledScriptResult  *bool
+	enabledScriptReason  *string
 }

pkg/module_manager/models/modules/basic_test.go

@@ -234,3 +234,69 @@ fi
 	err = IsFileBatchHook("moduleName", hookPath, fileInfo)
 	require.NoError(t, err)
 }
+
+func stubDeps(logger *log.Logger) *hooks.HookExecutionDependencyContainer {
+	st := metric_storage.NewMetricStorage(context.TODO(), "addon_operator_", false, logger)
+	return &hooks.HookExecutionDependencyContainer{
+		HookMetricsStorage: st,
+		MetricStorage:      st,
+		KubeObjectPatcher:  objectpatch.NewObjectPatcher(nil, logger),
+		GlobalValuesGetter: &noopValuesGetter{},
+		KubeConfigManager:  &noopConfigManager{},
+	}
+}
+
+func TestRunEnabledScriptTrue(t *testing.T) {
+	tmpModuleDir := t.TempDir()
+	enabledPath := filepath.Join(tmpModuleDir, "enabled")
+
+	require.NoError(t, os.WriteFile(enabledPath, []byte(`#!/bin/sh
+echo "true" > "$MODULE_ENABLED_RESULT"
+exit 0
+`), 0o755))
+
+	bm, err := NewBasicModule("example", tmpModuleDir, 1, utils.Values{}, nil, nil)
+	require.NoError(t, err)
+
+	logger := log.NewLogger()
+	bm.WithLogger(logger)
+	bm.WithDependencies(stubDeps(logger))
+
+	ok, err := bm.RunEnabledScript(context.TODO(), t.TempDir(), nil, map[string]string{})
+	require.NoError(t, err)
+	require.True(t, ok, "Module should be enabled")
+
+	require.NotNil(t, bm.GetEnabledScriptResult())
+	require.True(t, *bm.GetEnabledScriptResult())
+	r := bm.GetEnabledScriptReason()
+	require.Nil(t, r)
+}
+
+func TestRunEnabledScriptFalseWithReason(t *testing.T) {
+	tmpModuleDir := t.TempDir()
+	enabledPath := filepath.Join(tmpModuleDir, "enabled")
+
+	require.NoError(t, os.WriteFile(enabledPath, []byte(`#!/bin/sh
+echo "false" > "$MODULE_ENABLED_RESULT"
+echo "Kubernetes version is too low" > "$MODULE_ENABLED_REASON"
+exit 0
+`), 0o755))
+
+	bm, err := NewBasicModule("example", tmpModuleDir, 1, utils.Values{}, nil, nil)
+	require.NoError(t, err)
+
+	logger := log.NewLogger()
+	bm.WithLogger(logger)
+	bm.WithDependencies(stubDeps(logger))
+
+	ok, err := bm.RunEnabledScript(context.TODO(), t.TempDir(), nil, map[string]string{})
+	require.NoError(t, err)
+	require.False(t, ok, "Module should be disabled")
+
+	require.NotNil(t, bm.GetEnabledScriptResult())
+	require.False(t, *bm.GetEnabledScriptResult())
+
+	erGetter := bm.GetEnabledScriptReason()
+	require.NotNil(t, erGetter)
+	require.Equal(t, "Kubernetes version is too low", *erGetter)
+}