|
| 1 | +package modules |
| 2 | + |
| 3 | +import ( |
| 4 | + "context" |
| 5 | + "errors" |
| 6 | + "os" |
| 7 | + "path/filepath" |
| 8 | + "sync" |
| 9 | + "testing" |
| 10 | + |
| 11 | + "github.com/deckhouse/deckhouse/pkg/log" |
| 12 | + "github.com/stretchr/testify/assert" |
| 13 | + "github.com/stretchr/testify/require" |
| 14 | + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" |
| 15 | + |
| 16 | + gohook "github.com/flant/addon-operator/pkg/module_manager/go_hook" |
| 17 | + "github.com/flant/addon-operator/pkg/module_manager/models/hooks" |
| 18 | + "github.com/flant/addon-operator/pkg/module_manager/models/hooks/kind" |
| 19 | + "github.com/flant/addon-operator/pkg/utils" |
| 20 | + "github.com/flant/shell-operator/pkg/hook/config" |
| 21 | + "github.com/flant/shell-operator/pkg/hook/controller" |
| 22 | + sh_op_types "github.com/flant/shell-operator/pkg/hook/types" |
| 23 | +) |
| 24 | + |
| 25 | +// newKubeGoHook returns a go hook with a single OnKubernetesEvent binding. |
| 26 | +// Config is precompiled, so no binary is executed on InitializeHookConfig. |
| 27 | +func newKubeGoHook(name string) *kind.GoHook { |
| 28 | + gh := kind.NewGoHook(&gohook.HookConfig{ |
| 29 | + Kubernetes: []gohook.KubernetesConfig{ |
| 30 | + { |
| 31 | + Name: "pods", |
| 32 | + ApiVersion: "v1", |
| 33 | + Kind: "Pod", |
| 34 | + FilterFunc: func(_ *unstructured.Unstructured) (gohook.FilterResult, error) { |
| 35 | + return nil, nil |
| 36 | + }, |
| 37 | + }, |
| 38 | + }, |
| 39 | + Logger: log.NewNop(), |
| 40 | + }, func(_ context.Context, _ *gohook.HookInput) error { return nil }) |
| 41 | + |
| 42 | + gh.AddMetadata(&gohook.HookMetadata{Name: name, Path: "/hooks/" + name}) |
| 43 | + |
| 44 | + return gh |
| 45 | +} |
| 46 | + |
| 47 | +// newInitializedModuleHook wraps a go hook into ModuleHook and loads its config, |
| 48 | +// so GetHookConfig().Bindings() returns OnKubernetesEvent. |
| 49 | +func newInitializedModuleHook(t *testing.T, name string) *hooks.ModuleHook { |
| 50 | + t.Helper() |
| 51 | + |
| 52 | + mh := hooks.NewModuleHook(newKubeGoHook(name)) |
| 53 | + require.NoError(t, mh.InitializeHookConfig()) |
| 54 | + |
| 55 | + return mh |
| 56 | +} |
| 57 | + |
| 58 | +// TestAddHookIsIdempotentPerBinding proves the byBinding duplication bug: |
| 59 | +// re-registering a hook with the same name must replace the old entry, |
| 60 | +// not append a second one (Trigger A root cause). |
| 61 | +func TestAddHookIsIdempotentPerBinding(t *testing.T) { |
| 62 | + hs := newHooksStorage() |
| 63 | + |
| 64 | + first := newInitializedModuleHook(t, "hooks/license") |
| 65 | + // A retry creates a fresh object for the same hook (searchModuleHooks |
| 66 | + // calls NewModuleHook on every attempt). |
| 67 | + second := newInitializedModuleHook(t, "hooks/license") |
| 68 | + |
| 69 | + hs.AddHook(first) |
| 70 | + hs.AddHook(second) |
| 71 | + |
| 72 | + byName := hs.getHookByName("hooks/license") |
| 73 | + require.Same(t, second, byName, "byName keeps the most recent object") |
| 74 | + |
| 75 | + byBinding := hs.getHooks(sh_op_types.OnKubernetesEvent) |
| 76 | + require.Len(t, byBinding, 1, |
| 77 | + "byBinding must hold exactly one entry per hook name after re-registration") |
| 78 | + require.Same(t, second, byBinding[0], |
| 79 | + "byBinding entry must be the most recently registered object") |
| 80 | +} |
| 81 | + |
| 82 | +// flakyGoHook fails config loading a given number of times. |
| 83 | +// Simulates the transient exec failure (ECHILD) that aborted hook |
| 84 | +// registration mid-loop in production. |
| 85 | +type flakyGoHook struct { |
| 86 | + *kind.GoHook |
| 87 | + |
| 88 | + mu sync.Mutex |
| 89 | + failures int |
| 90 | +} |
| 91 | + |
| 92 | +func (f *flakyGoHook) GetConfigForModule(moduleKind string) (*config.HookConfig, error) { |
| 93 | + f.mu.Lock() |
| 94 | + defer f.mu.Unlock() |
| 95 | + |
| 96 | + if f.failures > 0 { |
| 97 | + f.failures-- |
| 98 | + return nil, errors.New("exec file '/hooks/go-hooks-bin': waitid: no child processes") |
| 99 | + } |
| 100 | + |
| 101 | + return f.GoHook.GetConfigForModule(moduleKind) |
| 102 | +} |
| 103 | + |
| 104 | +// TestRegistrationRetryLeavesNoOrphanHooks reproduces the production incident: |
| 105 | +// attempt 1 publishes some hooks and fails mid-loop, attempt 2 re-registers |
| 106 | +// fresh objects and attaches controllers only to them. Every hook visible via |
| 107 | +// GetHooks must have a controller afterwards - a nil one is exactly the value |
| 108 | +// that crashed the operator with SIGSEGV. |
| 109 | +func TestRegistrationRetryLeavesNoOrphanHooks(t *testing.T) { |
| 110 | + logger := log.NewNop() |
| 111 | + |
| 112 | + bm, err := NewBasicModule("flant-integration", t.TempDir(), 1, utils.Values{}, nil, nil) |
| 113 | + require.NoError(t, err) |
| 114 | + bm.WithDependencies(stubDeps(logger)) |
| 115 | + |
| 116 | + // Attempt 1: the second hook fails on config load, registerHooks aborts. |
| 117 | + // The first hook is already published to byBinding without a controller. |
| 118 | + okHook1 := hooks.NewModuleHook(newKubeGoHook("hooks/ok")) |
| 119 | + failingHook1 := hooks.NewModuleHook(&flakyGoHook{GoHook: newKubeGoHook("hooks/license"), failures: 1}) |
| 120 | + |
| 121 | + err = bm.registerHooks([]*hooks.ModuleHook{okHook1, failingHook1}, logger) |
| 122 | + require.Error(t, err, "attempt 1 must fail on the failing hook") |
| 123 | + |
| 124 | + // Attempt 2: ModuleRun retry - fresh hook objects, the failure is gone. |
| 125 | + okHook2 := hooks.NewModuleHook(newKubeGoHook("hooks/ok")) |
| 126 | + failingHook2 := hooks.NewModuleHook(&flakyGoHook{GoHook: newKubeGoHook("hooks/license")}) |
| 127 | + |
| 128 | + err = bm.registerHooks([]*hooks.ModuleHook{okHook2, failingHook2}, logger) |
| 129 | + require.NoError(t, err, "attempt 2 must succeed") |
| 130 | + |
| 131 | + // RegisterModuleHooks attaches controllers only to hooks returned by the |
| 132 | + // successful attempt, then raises the readiness flag. |
| 133 | + for _, hk := range []*hooks.ModuleHook{okHook2, failingHook2} { |
| 134 | + hk.WithHookController(controller.NewHookController()) |
| 135 | + } |
| 136 | + bm.SetHooksControllersReady() |
| 137 | + |
| 138 | + // Invariant: controllersReady == true implies every hook in byBinding has |
| 139 | + // a controller. |
| 140 | + kubeHooks := bm.GetHooks(sh_op_types.OnKubernetesEvent) |
| 141 | + for _, hk := range kubeHooks { |
| 142 | + assert.NotNilf(t, hk.GetHookController(), |
| 143 | + "hook %q has no controller: stale duplicate left by failed attempt 1", hk.GetName()) |
| 144 | + } |
| 145 | + require.Len(t, kubeHooks, 2, |
| 146 | + "byBinding must contain one entry per hook, without stale duplicates") |
| 147 | +} |
| 148 | + |
| 149 | +// TestControllersReadyFlagIsRaceFree proves the data race on controllersReady: |
| 150 | +// the flag is written by the registration goroutine and read by the kube-events |
| 151 | +// dispatcher without synchronization. Run with -race. |
| 152 | +func TestControllersReadyFlagIsRaceFree(t *testing.T) { |
| 153 | + bm, err := NewBasicModule("race-flag", t.TempDir(), 1, utils.Values{}, nil, nil) |
| 154 | + require.NoError(t, err) |
| 155 | + |
| 156 | + var wg sync.WaitGroup |
| 157 | + wg.Add(2) |
| 158 | + |
| 159 | + go func() { |
| 160 | + defer wg.Done() |
| 161 | + for range 1000 { |
| 162 | + bm.SetHooksControllersReady() |
| 163 | + } |
| 164 | + }() |
| 165 | + |
| 166 | + go func() { |
| 167 | + defer wg.Done() |
| 168 | + for range 1000 { |
| 169 | + _ = bm.HooksControllersReady() |
| 170 | + } |
| 171 | + }() |
| 172 | + |
| 173 | + wg.Wait() |
| 174 | + require.True(t, bm.HooksControllersReady()) |
| 175 | +} |
| 176 | + |
| 177 | +// TestConcurrentRegisterHooksIsSafe proves Trigger B entry: two ModuleRun |
| 178 | +// workers can both pass the `registered` check before either sets it, so the |
| 179 | +// module is registered twice (duplicates + data race on the flag). Run with -race. |
| 180 | +func TestConcurrentRegisterHooksIsSafe(t *testing.T) { |
| 181 | + moduleDir := t.TempDir() |
| 182 | + hooksDir := filepath.Join(moduleDir, "hooks") |
| 183 | + require.NoError(t, os.MkdirAll(hooksDir, 0o755)) |
| 184 | + require.NoError(t, os.WriteFile(filepath.Join(hooksDir, "startup.sh"), []byte(`#!/bin/sh |
| 185 | +if [ "$1" = "--config" ]; then |
| 186 | + echo '{"configVersion":"v1","onStartup":10}' |
| 187 | + exit 0 |
| 188 | +fi |
| 189 | +exit 0 |
| 190 | +`), 0o755)) |
| 191 | + |
| 192 | + logger := log.NewNop() |
| 193 | + |
| 194 | + bm, err := NewBasicModule("concurrent", moduleDir, 1, utils.Values{}, nil, nil) |
| 195 | + require.NoError(t, err) |
| 196 | + bm.WithDependencies(stubDeps(logger)) |
| 197 | + |
| 198 | + var wg sync.WaitGroup |
| 199 | + for range 2 { |
| 200 | + wg.Add(1) |
| 201 | + go func() { |
| 202 | + defer wg.Done() |
| 203 | + _, _ = bm.RegisterHooks(logger) |
| 204 | + }() |
| 205 | + } |
| 206 | + wg.Wait() |
| 207 | + |
| 208 | + require.Len(t, bm.GetHooks(sh_op_types.OnStartup), 1, |
| 209 | + "concurrent registration must not produce duplicate hooks") |
| 210 | +} |
0 commit comments