Skip to content

Commit f1a8d76

Browse files
fix: execute all container hooks targeting the same pod/container (#8356)
When multiple container hooks were configured in deploy.kubectl.hooks.after targeting the same pod and container, only the first hook would execute. The shared visitedContainers map used a key of phase:podName:containerName, causing subsequent hooks for the same container to be silently skipped. Fix by including the hook index in the visitedContainers key so each hook definition independently tracks which containers it has processed. Fixes #8356 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fde4323 commit f1a8d76

2 files changed

Lines changed: 49 additions & 5 deletions

File tree

pkg/skaffold/hooks/deploy.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func (r deployRunner) run(ctx context.Context, out io.Writer, hooks []latest.Dep
109109
output.Default.Fprintln(out, fmt.Sprintf("Starting %s hooks...", phase))
110110
}
111111
env := r.getEnv(manifestsNs)
112-
for _, h := range hooks {
112+
for i, h := range hooks {
113113
if h.HostHook != nil {
114114
hook := hostHook{*h.HostHook, env}
115115
if err := hook.run(ctx, nil, out); err != nil && !errors.Is(err, &Skip{}) {
@@ -119,7 +119,7 @@ func (r deployRunner) run(ctx context.Context, out io.Writer, hooks []latest.Dep
119119
hook := containerHook{
120120
cfg: latest.ContainerHook{Command: h.ContainerHook.Command},
121121
cli: r.cli,
122-
selector: filterContainersSelector(r.visitedContainers, phase, namePatternSelector(h.ContainerHook.PodName, h.ContainerHook.ContainerName)),
122+
selector: filterContainersSelector(r.visitedContainers, phase, i, namePatternSelector(h.ContainerHook.PodName, h.ContainerHook.ContainerName)),
123123
namespaces: *r.namespaces,
124124
formatter: r.formatter,
125125
}
@@ -134,10 +134,12 @@ func (r deployRunner) run(ctx context.Context, out io.Writer, hooks []latest.Dep
134134
return nil
135135
}
136136

137-
// filterContainersSelector filters the containers that have already been processed from a previous deploy iteration
138-
func filterContainersSelector(visitedContainers *sync.Map, phase phase, selector containerSelector) containerSelector {
137+
// filterContainersSelector filters the containers that have already been processed from a previous deploy iteration.
138+
// The hookIndex parameter differentiates between distinct hook items so that multiple hooks targeting the same
139+
// container are each allowed to execute.
140+
func filterContainersSelector(visitedContainers *sync.Map, phase phase, hookIndex int, selector containerSelector) containerSelector {
139141
return func(p corev1.Pod, c corev1.Container) (bool, error) {
140-
key := fmt.Sprintf("%s:%s:%s", phase, p.GetName(), c.Name)
142+
key := fmt.Sprintf("%s:%d:%s:%s", phase, hookIndex, p.GetName(), c.Name)
141143
if _, found := visitedContainers.LoadOrStore(key, struct{}{}); found {
142144
return false, nil
143145
}

pkg/skaffold/hooks/deploy_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,48 @@ func TestDeployHooks(t *testing.T) {
113113
})
114114
}
115115

116+
func TestMultipleContainerHooksOnSamePod(t *testing.T) {
117+
testutil.Run(t, "TestMultipleContainerHooksOnSamePod", func(t *testutil.T) {
118+
hooks := latest.DeployHooks{
119+
PostHooks: []latest.DeployHookItem{
120+
{
121+
ContainerHook: &latest.NamedContainerHook{
122+
ContainerHook: latest.ContainerHook{
123+
Command: []string{"echo", "container-hook-0"},
124+
},
125+
PodName: "pod1",
126+
ContainerName: "container1",
127+
},
128+
},
129+
{
130+
ContainerHook: &latest.NamedContainerHook{
131+
ContainerHook: latest.ContainerHook{
132+
Command: []string{"echo", "container-hook-1"},
133+
},
134+
PodName: "pod1",
135+
ContainerName: "container1",
136+
},
137+
},
138+
},
139+
}
140+
141+
namespaces := []string{"np1", "np2"}
142+
opts := NewDeployEnvOpts("run_id", testKubeContext, namespaces)
143+
formatter := func(corev1.Pod, corev1.ContainerStatus, func() bool) log.Formatter { return mockLogFormatter{} }
144+
runner := NewDeployRunner(&kubectl.CLI{KubeContext: testKubeContext}, hooks, &namespaces, formatter, opts, nil)
145+
146+
t.Override(&util.DefaultExecCommand,
147+
testutil.CmdRunWithOutput("kubectl --context context1 exec pod1 --namespace np1 -c container1 -- echo container-hook-0", "container-hook-0").
148+
AndRunWithOutput("kubectl --context context1 exec pod1 --namespace np1 -c container1 -- echo container-hook-1", "container-hook-1"))
149+
t.Override(&kubernetesclient.Client, fakeKubernetesClient)
150+
var out bytes.Buffer
151+
err := runner.RunPostHooks(context.Background(), &out)
152+
t.CheckNoError(err)
153+
t.CheckContains("container-hook-0", out.String())
154+
t.CheckContains("container-hook-1", out.String())
155+
})
156+
}
157+
116158
func TestNewCloudRunDeployRunnerHooksMapping(t *testing.T) {
117159
tests := []struct {
118160
description string

0 commit comments

Comments
 (0)