amends

Signed-off-by: Mikhail Scherba <mikhail.scherba@flant.com>

pkg/module_manager/scheduler/extenders/dynamically_enabled/dynamic.go

@@ -52,9 +52,8 @@ func (e *Extender) UpdateStatus(moduleName, operation string, value bool) {
 func (e *Extender) sendNotify() {
 	if e.notifyCh != nil {
 		e.notifyCh <- extenders.ExtenderEvent{
-			ExtenderName: Name,
-			EncapsulatedEvent: DynamicExtenderEvent{
-			},
+			ExtenderName:      Name,
+			EncapsulatedEvent: DynamicExtenderEvent{},
 		}
 	}
 }
@@ -77,6 +76,3 @@ func (e *Extender) Filter(moduleName string, _ map[string]string) (*bool, error)
 func (e *Extender) SetNotifyChannel(_ context.Context, ch chan extenders.ExtenderEvent) {
 	e.notifyCh = ch
 }
-
-func (e *Extender) Order() {
-}

pkg/module_manager/scheduler/extenders/extenders.go

@@ -16,7 +16,6 @@ type Extender interface {
 	Name() ExtenderName
 	// Filter returns the result of applying the extender
 	Filter(moduleName string, logLabels map[string]string) (*bool, error)
-
 }
 
 type NotificationExtender interface {
@@ -29,3 +28,11 @@ type ResettableExtender interface {
 	// Reset resets the extender's cache
 	Reset()
 }
+
+// Type of extenders that can only disable an enabled module if some requirement isn't met.
+// By design, it makes sense to run terminators in the end of filtering because terminators can't be overridden by other extenders.
+// For example, enabled scripts extender.
+type TerminatingExtender interface {
+	// Just a signature to match extenders
+	IsTerminator()
+}

pkg/module_manager/scheduler/extenders/kube_config/kube_config.go

@@ -37,9 +37,6 @@ func (e Extender) Filter(moduleName string, _ map[string]string) (*bool, error)
 	return e.kubeConfigManager.IsModuleEnabled(moduleName), nil
 }
 
-func (e Extender) Order() {
-}
-
 func (e *Extender) sendNotify(kubeConfigEvent config.KubeConfigEvent) {
 	if e.notifyCh != nil {
 		e.notifyCh <- extenders.ExtenderEvent{

pkg/module_manager/scheduler/extenders/script_enabled/script.go

@@ -13,6 +13,8 @@ import (
 	"github.com/flant/addon-operator/pkg/module_manager/scheduler/node"
 	"github.com/flant/addon-operator/pkg/utils"
 	utils_file "github.com/flant/shell-operator/pkg/utils/file"
+
+	"k8s.io/utils/pointer"
 )
 
 const (
@@ -100,41 +102,41 @@ func (e *Extender) Reset() {
 func (e *Extender) Filter(moduleName string, logLabels map[string]string) (*bool, error) {
 	if moduleDescriptor, found := e.basicModuleDescriptors[moduleName]; found {
 		var err error
-		var enabled bool
+		var enabled *bool
 
 		switch moduleDescriptor.scriptState {
 		case "":
+			var isEnabled bool
 			refreshLogLabels := utils.MergeLabels(logLabels, map[string]string{
 				"extender": "ScriptEnabled",
 			})
-			enabled, err = moduleDescriptor.module.RunEnabledScript(e.tmpDir, e.enabledModules, refreshLogLabels)
+			isEnabled, err = moduleDescriptor.module.RunEnabledScript(e.tmpDir, e.enabledModules, refreshLogLabels)
 			if err != nil {
 				err = fmt.Errorf("Failed to execute '%s' module's enabled script: %v", moduleDescriptor.module.GetName(), err)
 			}
+			enabled = &isEnabled
 
 		case statError:
 			log.Errorf(moduleDescriptor.stateDescription)
-			enabled = false
+			enabled = pointer.Bool(false)
 			err = errors.New(moduleDescriptor.stateDescription)
 
 		case nonExecutableScript:
-			enabled = nil
 			log.Warnf("Found non-executable enabled script for '%s' module - assuming enabled state", moduleDescriptor.module.GetName())
 
 		case noEnabledScript:
-			enabled = nil
 			log.Debugf("MODULE '%s' is ENABLED. Enabled script doesn't exist!", moduleDescriptor.module.GetName())
 		}
 
-		if enabled {
+		if enabled != nil && *enabled {
 			e.l.Lock()
 			e.enabledModules = append(e.enabledModules, moduleDescriptor.module.GetName())
 			e.l.Unlock()
 		}
-		return &enabled, err
+		return enabled, err
 	}
 	return nil, nil
 }
 
-func (e *Extender) Order() {
+func (e *Extender) IsTerminator() {
 }

pkg/module_manager/scheduler/extenders/script_enabled/script_test.go

@@ -9,28 +9,37 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/flant/addon-operator/pkg/module_manager/scheduler/node"
+	node_mock "github.com/flant/addon-operator/pkg/module_manager/scheduler/node/mock"
 )
 
 func TestExtender(t *testing.T) {
+	var boolNilP *bool
 	tmp, err := os.MkdirTemp(t.TempDir(), "values-test")
 	require.NoError(t, err)
 
 	e, err := NewExtender(tmp)
 	require.NoError(t, err)
 
-	basicModules := []*node.MockModule{
+	basicModules := []*node_mock.MockModule{
 		{
 			Name:                "ingress-nginx",
 			Order:               402,
 			EnabledScriptResult: true,
-			Path:                "./testdata/402-ingress-nginx/",
+			// no executable bit
+			Path: "./testdata/402-ingress-nginx/",
 		},
 		{
 			Name:                "cert-manager",
 			Order:               30,
 			EnabledScriptResult: true,
-			Path:                "./testdata/030-cert-manager/",
+			// no enabled script
+			Path: "./testdata/030-cert-manager/",
+		},
+		{
+			Name:                "foo-bar",
+			Order:               31,
+			EnabledScriptResult: true,
+			Path:                "./testdata/031-foo-bar/",
 		},
 		{
 			Name:                "node-local-dns",
@@ -58,11 +67,14 @@ func TestExtender(t *testing.T) {
 		e.AddBasicModule(m)
 		enabled, err := e.Filter(m.Name, logLabels)
 		switch m.GetName() {
-		case "ingress-nginx":
+		case "foo-bar":
 			assert.Equal(t, true, *enabled)
 			assert.Equal(t, nil, err)
+		case "ingress-nginx":
+			assert.Equal(t, boolNilP, enabled)
+			assert.Equal(t, nil, err)
 		case "cert-manager":
-			assert.Equal(t, true, *enabled)
+			assert.Equal(t, boolNilP, enabled)
 			assert.Equal(t, nil, err)
 		case "node-local-dns":
 			assert.Equal(t, false, *enabled)
@@ -73,7 +85,7 @@ func TestExtender(t *testing.T) {
 		}
 	}
 
-	expected := []string{"ingress-nginx", "cert-manager"}
+	expected := []string{"foo-bar"}
 	assert.Equal(t, expected, e.enabledModules)
 
 	err = os.RemoveAll(tmp)

pkg/module_manager/scheduler/extenders/static/static.go

@@ -81,6 +81,3 @@ func (e Extender) Filter(moduleName string, _ map[string]string) (*bool, error)
 
 	return nil, nil
 }
-
-func (e Extender) Order() {
-}

pkg/module_manager/scheduler/node/mock/node_mock.go

@@ -0,0 +1,28 @@
+package node_mock
+
+type MockModule struct {
+	EnabledScriptResult bool
+	EnabledScriptErr    error
+	Name                string
+	Path                string
+	Order               uint32
+}
+
+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
+}

pkg/module_manager/scheduler/node/node.go

@@ -11,33 +11,6 @@ type ModuleInterface interface {
 	GetPath() string
 }
 
-type MockModule struct {
-	EnabledScriptResult bool
-	EnabledScriptErr    error
-	Name                string
-	Path                string
-	Order               uint32
-}
-
-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
-}
-
 type NodeType string
 
 type NodeWeight uint32

pkg/module_manager/scheduler/node/node_test.go

@@ -4,13 +4,15 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	node_mock "github.com/flant/addon-operator/pkg/module_manager/scheduler/node/mock"
 )
 
 func TestNewNode(t *testing.T) {
 	n := NewNode()
 	assert.Equal(t, &Node{}, n)
 
-	m := MockModule{
+	m := node_mock.MockModule{
 		Name:  "test-node",
 		Order: 32,
 	}

pkg/module_manager/scheduler/scheduler.go

@@ -23,10 +23,10 @@ import (
 )
 
 var defaultAppliedExtenders = []extenders.ExtenderName{
-  static_extender.Name,
-  dynamic_extender.Name,
-  kube_config_extender.Name,
-  script_extender.Name,
+	static_extender.Name,
+	dynamic_extender.Name,
+	kube_config_extender.Name,
+	script_extender.Name,
 }
 
 type Scheduler struct {
@@ -439,7 +439,8 @@ outerCycle:
 			vBuf[moduleName] = &vertexState{}
 
 			for _, ex := range s.extenders {
-				if ex.Name() == script_extender.Name && !vBuf[moduleName].enabled {
+				// if current extender is a terminating one and by this point the module is already disabled - there's little sense in checking against a terminator
+				if _, ok := ex.(extenders.TerminatingExtender); ok && !vBuf[moduleName].enabled {
 					continue
 				}
 
@@ -450,7 +451,8 @@ outerCycle:
 				}
 
 				if moduleStatus != nil {
-					if ex.Name() == script_extender.Name {
+					// if current extender is a terminating one and it says to disable - stop cycling over remaining extenders and disable the module
+					if _, ok := ex.(extenders.TerminatingExtender); ok {
 						if !*moduleStatus && vBuf[moduleName].enabled {
 							vBuf[moduleName].enabled = *moduleStatus
 							vBuf[moduleName].updatedBy = string(ex.Name())

pkg/module_manager/scheduler/scheduler_test.go

@@ -15,7 +15,7 @@ import (
 	"github.com/flant/addon-operator/pkg/module_manager/scheduler/extenders/kube_config"
 	"github.com/flant/addon-operator/pkg/module_manager/scheduler/extenders/script_enabled"
 	"github.com/flant/addon-operator/pkg/module_manager/scheduler/extenders/static"
-	"github.com/flant/addon-operator/pkg/module_manager/scheduler/node"
+	node_mock "github.com/flant/addon-operator/pkg/module_manager/scheduler/node/mock"
 )
 
 type kcmMock struct {
@@ -40,7 +40,7 @@ func TestFilter(t *testing.T) {
 ingressNginxEnabled: true
 nodeLocalDnsEnabled: false
 `
-	basicModules := []*node.MockModule{
+	basicModules := []*node_mock.MockModule{
 		{
 			Name:  "cert-manager",
 			Order: 30,
@@ -113,7 +113,7 @@ nodeLocalDnsEnabled: true
 certManagerEnabled: true
 `
 	logLabels := map[string]string{"source": "TestGetEnabledModuleNames"}
-	basicModules := []*node.MockModule{
+	basicModules := []*node_mock.MockModule{
 		{
 			Name:                "cert-manager",
 			Order:               30,
@@ -174,7 +174,7 @@ certManagerEnabled: true
 
 func TestAddModuleVertex(t *testing.T) {
 	s := NewScheduler(context.TODO())
-	basicModule := &node.MockModule{
+	basicModule := &node_mock.MockModule{
 		Name:  "ingress-nginx",
 		Order: 402,
 	}
@@ -226,7 +226,7 @@ l2LoadBalancerEnabled: false
 
 `
 	logLabels := map[string]string{"source": "TestRecalculateGraph"}
-	basicModules := []*node.MockModule{
+	basicModules := []*node_mock.MockModule{
 		{
 			Name:                "ingress-nginx",
 			Order:               402,