Skip to content

Commit 1c3944b

Browse files
renzodavid9renzodavid9
authored
feat: inject namespace from rendered manifests in post deploy hooks (#9090)
* feat: inject namespace from rendered manifests in post deploy hooks * fix: change unit test according to new logic * test: integration test for post deploy hook namespaces values
1 parent 9fd7101 commit 1c3944b

File tree

8 files changed

+161
-59
lines changed

8 files changed

+161
-59
lines changed

‎integration/deploy_hooks_test.go

+78
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
Copyright 2023 The Skaffold Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package integration
18+
19+
import (
20+
"fmt"
21+
"testing"
22+
23+
"github.com/GoogleContainerTools/skaffold/v2/integration/skaffold"
24+
"github.com/GoogleContainerTools/skaffold/v2/testutil"
25+
)
26+
27+
func TestPostDeployHooksNamespaces(t *testing.T) {
28+
MarkIntegrationTest(t, CanRunWithoutGcp)
29+
30+
tests := []struct {
31+
description string
32+
dir string
33+
configFile string
34+
resourceFile string
35+
}{
36+
{
37+
description: "set SKAFFOLD_NAMESPACES from manifests with kubectl deployer",
38+
configFile: `apiVersion: skaffold/v4beta6
39+
kind: Config
40+
manifests:
41+
rawYaml:
42+
- pod.yaml
43+
44+
deploy:
45+
kubectl:
46+
hooks:
47+
after:
48+
- host:
49+
command: ["sh", "-c", "echo $SKAFFOLD_NAMESPACES"]
50+
`,
51+
resourceFile: `apiVersion: v1
52+
kind: Pod
53+
metadata:
54+
name: getting-started
55+
namespace: %v
56+
spec:
57+
containers:
58+
- name: getting-started
59+
image: us-central1-docker.pkg.dev/k8s-skaffold/testing/skaffold-example
60+
`,
61+
},
62+
}
63+
64+
for _, test := range tests {
65+
t.Run(test.description, func(t *testing.T) {
66+
ns, _ := SetupNamespace(t)
67+
expectedOutput := fmt.Sprintf("Starting post-deploy hooks...\ndefault,%v", ns.Name)
68+
resourceWithNs := fmt.Sprintf(test.resourceFile, ns.Name)
69+
70+
dir := testutil.NewTempDir(t)
71+
dir.Write("skaffold.yaml", test.configFile)
72+
dir.Write("pod.yaml", resourceWithNs)
73+
out, err := skaffold.Run().InDir(dir.Root()).RunWithCombinedOutput(t)
74+
testutil.CheckError(t, false, err)
75+
testutil.CheckContains(t, expectedOutput, string(out))
76+
})
77+
}
78+
}

‎pkg/skaffold/deploy/helm/helm.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ type Deployer struct {
9797
namespace string
9898
configFile string
9999

100-
namespaces *[]string
100+
namespaces *[]string
101+
manifestsNamespaces *[]string
101102

102103
// packaging temporary directory, used for predictable test output
103104
pkgTmpDir string
@@ -166,6 +167,9 @@ func NewDeployer(ctx context.Context, cfg Config, labeller *label.DefaultLabelle
166167
RuntimeType: artifact.RuntimeType,
167168
})
168169
}
170+
171+
manifestsNamespaces := []string{}
172+
169173
return &Deployer{
170174
configName: configName,
171175
LegacyHelmDeploy: h,
@@ -177,7 +181,8 @@ func NewDeployer(ctx context.Context, cfg Config, labeller *label.DefaultLabelle
177181
logger: logger,
178182
statusMonitor: component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces, customResourceSelectors),
179183
syncer: component.NewSyncer(kubectl, &namespaces, logger.GetFormatter()),
180-
hookRunner: hooks.NewDeployRunner(kubectl, h.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces)),
184+
manifestsNamespaces: &manifestsNamespaces,
185+
hookRunner: hooks.NewDeployRunner(kubectl, h.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces), &manifestsNamespaces),
181186
originalImages: ogImages,
182187
kubeContext: cfg.GetKubeContext(),
183188
kubeConfig: cfg.GetKubeConfig(),
@@ -302,6 +307,7 @@ func (h *Deployer) Deploy(ctx context.Context, out io.Writer, builds []graph.Art
302307

303308
h.TrackBuildArtifacts(builds, deployedImages)
304309
h.trackNamespaces(namespaces)
310+
*h.manifestsNamespaces = namespaces
305311
return nil
306312
}
307313

‎pkg/skaffold/deploy/helm/helm_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1393,7 +1393,7 @@ func TestHelmHooks(t *testing.T) {
13931393
for _, test := range tests {
13941394
testutil.Run(t, test.description, func(t *testutil.T) {
13951395
t.Override(&util.DefaultExecCommand, testutil.CmdRunWithOutput("helm version --client", version31))
1396-
t.Override(&hooks.NewDeployRunner, func(*ctl.CLI, latest.DeployHooks, *[]string, logger.Formatter, hooks.DeployEnvOpts) hooks.Runner {
1396+
t.Override(&hooks.NewDeployRunner, func(*ctl.CLI, latest.DeployHooks, *[]string, logger.Formatter, hooks.DeployEnvOpts, *[]string) hooks.Runner {
13971397
return test.runner
13981398
})
13991399

‎pkg/skaffold/deploy/kubectl/kubectl.go

+43-38
Original file line numberDiff line numberDiff line change
@@ -58,25 +58,26 @@ type Deployer struct {
5858

5959
*latest.KubectlDeploy
6060

61-
accessor access.Accessor
62-
imageLoader loader.ImageLoader
63-
logger k8slogger.Logger
64-
debugger debug.Debugger
65-
statusMonitor kstatus.Monitor
66-
syncer sync.Syncer
67-
hookRunner hooks.Runner
68-
originalImages []graph.Artifact // the set of images parsed from the Deployer's manifest set
69-
localImages []graph.Artifact // the set of images marked as "local" by the Runner
70-
podSelector *kubernetes.ImageList
71-
hydratedManifests []string
72-
workingDir string
73-
globalConfig string
74-
defaultRepo *string
75-
multiLevelRepo *bool
76-
kubectl CLI
77-
insecureRegistries map[string]bool
78-
labeller *label.DefaultLabeller
79-
namespaces *[]string
61+
accessor access.Accessor
62+
imageLoader loader.ImageLoader
63+
logger k8slogger.Logger
64+
debugger debug.Debugger
65+
statusMonitor kstatus.Monitor
66+
syncer sync.Syncer
67+
hookRunner hooks.Runner
68+
originalImages []graph.Artifact // the set of images parsed from the Deployer's manifest set
69+
localImages []graph.Artifact // the set of images marked as "local" by the Runner
70+
podSelector *kubernetes.ImageList
71+
hydratedManifests []string
72+
workingDir string
73+
globalConfig string
74+
defaultRepo *string
75+
multiLevelRepo *bool
76+
kubectl CLI
77+
insecureRegistries map[string]bool
78+
labeller *label.DefaultLabeller
79+
namespaces *[]string
80+
manifestsNamespaces *[]string
8081

8182
transformableAllowlist map[apimachinery.GroupKind]latest.ResourceFilter
8283
transformableDenylist map[apimachinery.GroupKind]latest.ResourceFilter
@@ -121,26 +122,29 @@ func NewDeployer(cfg Config, labeller *label.DefaultLabeller, d *latest.KubectlD
121122
})
122123
}
123124

125+
manifestsNamespaces := []string{}
126+
124127
return &Deployer{
125-
originalImages: ogImages,
126-
configName: configName,
127-
KubectlDeploy: d,
128-
podSelector: podSelector,
129-
namespaces: &namespaces,
130-
accessor: component.NewAccessor(cfg, cfg.GetKubeContext(), kubectl.CLI, podSelector, labeller, &namespaces),
131-
debugger: component.NewDebugger(cfg.Mode(), podSelector, &namespaces, cfg.GetKubeContext()),
132-
imageLoader: component.NewImageLoader(cfg, kubectl.CLI),
133-
logger: logger,
134-
statusMonitor: component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces, customResourceSelectors),
135-
syncer: component.NewSyncer(kubectl.CLI, &namespaces, logger.GetFormatter()),
136-
hookRunner: hooks.NewDeployRunner(kubectl.CLI, d.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces)),
137-
workingDir: cfg.GetWorkingDir(),
138-
globalConfig: cfg.GlobalConfig(),
139-
defaultRepo: cfg.DefaultRepo(),
140-
multiLevelRepo: cfg.MultiLevelRepo(),
141-
kubectl: kubectl,
142-
insecureRegistries: cfg.GetInsecureRegistries(),
143-
labeller: labeller,
128+
originalImages: ogImages,
129+
configName: configName,
130+
KubectlDeploy: d,
131+
podSelector: podSelector,
132+
namespaces: &namespaces,
133+
accessor: component.NewAccessor(cfg, cfg.GetKubeContext(), kubectl.CLI, podSelector, labeller, &namespaces),
134+
debugger: component.NewDebugger(cfg.Mode(), podSelector, &namespaces, cfg.GetKubeContext()),
135+
imageLoader: component.NewImageLoader(cfg, kubectl.CLI),
136+
logger: logger,
137+
statusMonitor: component.NewMonitor(cfg, cfg.GetKubeContext(), labeller, &namespaces, customResourceSelectors),
138+
syncer: component.NewSyncer(kubectl.CLI, &namespaces, logger.GetFormatter()),
139+
manifestsNamespaces: &manifestsNamespaces,
140+
hookRunner: hooks.NewDeployRunner(kubectl.CLI, d.LifecycleHooks, &namespaces, logger.GetFormatter(), hooks.NewDeployEnvOpts(labeller.GetRunID(), kubectl.KubeContext, namespaces), &manifestsNamespaces),
141+
workingDir: cfg.GetWorkingDir(),
142+
globalConfig: cfg.GlobalConfig(),
143+
defaultRepo: cfg.DefaultRepo(),
144+
multiLevelRepo: cfg.MultiLevelRepo(),
145+
kubectl: kubectl,
146+
insecureRegistries: cfg.GetInsecureRegistries(),
147+
labeller: labeller,
144148
// hydratedManifests refers to the DIR in the `skaffold apply DIR`. Used in both v1 and v2.
145149
hydratedManifests: cfg.HydratedManifests(),
146150
transformableAllowlist: transformableAllowlist,
@@ -270,6 +274,7 @@ func (k *Deployer) Deploy(ctx context.Context, out io.Writer, builds []graph.Art
270274
k.statusMonitor.RegisterDeployManifests(manifests)
271275
endTrace()
272276
k.trackNamespaces(namespaces)
277+
*k.manifestsNamespaces = namespaces
273278
return nil
274279
}
275280

‎pkg/skaffold/hooks/deploy.go

+22-15
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ import (
2020
"context"
2121
"fmt"
2222
"io"
23-
"strings"
2423
"sync"
2524

2625
corev1 "k8s.io/api/core/v1"
2726

27+
deployutil "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/deploy/util"
2828
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubectl"
2929
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/kubernetes/logger"
3030
"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/output"
@@ -37,8 +37,8 @@ var (
3737
NewCloudRunDeployRunner = newCloudRunDeployRunner
3838
)
3939

40-
func newDeployRunner(cli *kubectl.CLI, d latest.DeployHooks, namespaces *[]string, formatter logger.Formatter, opts DeployEnvOpts) Runner {
41-
return deployRunner{d, cli, namespaces, formatter, opts, new(sync.Map)}
40+
func newDeployRunner(cli *kubectl.CLI, d latest.DeployHooks, namespaces *[]string, formatter logger.Formatter, opts DeployEnvOpts, manifestsNamespaces *[]string) Runner {
41+
return deployRunner{d, cli, namespaces, manifestsNamespaces, formatter, opts, new(sync.Map)}
4242
}
4343

4444
func newCloudRunDeployRunner(d latest.CloudRunDeployHooks, opts DeployEnvOpts) Runner {
@@ -69,38 +69,45 @@ func NewDeployEnvOpts(runID string, kubeContext string, namespaces []string) Dep
6969
return DeployEnvOpts{
7070
RunID: runID,
7171
KubeContext: kubeContext,
72-
Namespaces: strings.Join(namespaces, ","),
72+
Namespaces: namespaces,
7373
}
7474
}
7575

7676
type deployRunner struct {
7777
latest.DeployHooks
78-
cli *kubectl.CLI
79-
namespaces *[]string
80-
formatter logger.Formatter
81-
opts DeployEnvOpts
82-
visitedContainers *sync.Map // maintain a list of previous iteration containers, so that they can be skipped
78+
cli *kubectl.CLI
79+
namespaces *[]string
80+
manifestsNamespaces *[]string
81+
formatter logger.Formatter
82+
opts DeployEnvOpts
83+
visitedContainers *sync.Map // maintain a list of previous iteration containers, so that they can be skipped
8384
}
8485

8586
func (r deployRunner) RunPreHooks(ctx context.Context, out io.Writer) error {
86-
return r.run(ctx, out, r.PreHooks, phases.PreDeploy)
87+
return r.run(ctx, out, r.PreHooks, phases.PreDeploy, nil)
8788
}
8889

8990
func (r deployRunner) RunPostHooks(ctx context.Context, out io.Writer) error {
90-
return r.run(ctx, out, r.PostHooks, phases.PostDeploy)
91+
return r.run(ctx, out, r.PostHooks, phases.PostDeploy, r.manifestsNamespaces)
9192
}
9293

93-
func (r deployRunner) getEnv() []string {
94+
func (r deployRunner) getEnv(manifestsNs *[]string) []string {
95+
mergedOpts := r.opts
96+
97+
if manifestsNs != nil {
98+
mergedOpts.Namespaces = deployutil.ConsolidateNamespaces(r.opts.Namespaces, *manifestsNs)
99+
}
100+
94101
common := getEnv(staticEnvOpts)
95-
deploy := getEnv(r.opts)
102+
deploy := getEnv(mergedOpts)
96103
return append(common, deploy...)
97104
}
98105

99-
func (r deployRunner) run(ctx context.Context, out io.Writer, hooks []latest.DeployHookItem, phase phase) error {
106+
func (r deployRunner) run(ctx context.Context, out io.Writer, hooks []latest.DeployHookItem, phase phase, manifestsNs *[]string) error {
100107
if len(hooks) > 0 {
101108
output.Default.Fprintln(out, fmt.Sprintf("Starting %s hooks...", phase))
102109
}
103-
env := r.getEnv()
110+
env := r.getEnv(manifestsNs)
104111
for _, h := range hooks {
105112
if h.HostHook != nil {
106113
hook := hostHook{*h.HostHook, env}

‎pkg/skaffold/hooks/deploy_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestDeployHooks(t *testing.T) {
9595
namespaces := []string{"np1", "np2"}
9696
opts := NewDeployEnvOpts("run_id", testKubeContext, namespaces)
9797
formatter := func(corev1.Pod, corev1.ContainerStatus, func() bool) log.Formatter { return mockLogFormatter{} }
98-
runner := NewDeployRunner(&kubectl.CLI{KubeContext: testKubeContext}, deployer.LifecycleHooks, &namespaces, formatter, opts)
98+
runner := NewDeployRunner(&kubectl.CLI{KubeContext: testKubeContext}, deployer.LifecycleHooks, &namespaces, formatter, opts, nil)
9999

100100
t.Override(&util.DefaultExecCommand,
101101
testutil.CmdRunWithOutput("kubectl --context context1 exec pod1 --namespace np1 -c container1 -- foo pre-hook", preContainerHookOut).

‎pkg/skaffold/hooks/env.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,17 @@ type RenderEnvOpts struct {
5959
Namespaces string
6060
}
6161

62+
type Namespaces []string
63+
64+
func (ns Namespaces) String() string {
65+
return strings.Join(ns, ",")
66+
}
67+
6268
// DeployEnvOpts contains the environment variables to be set in a deploy type lifecycle hook executor.
6369
type DeployEnvOpts struct {
6470
RunID string
6571
KubeContext string
66-
Namespaces string
72+
Namespaces Namespaces
6773
}
6874

6975
type Config interface {

‎pkg/skaffold/hooks/env_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func TestGetEnv(t *testing.T) {
135135
input: DeployEnvOpts{
136136
RunID: "1234",
137137
KubeContext: "minikube",
138-
Namespaces: "np1,np2,np3",
138+
Namespaces: Namespaces{"np1", "np2", "np3"},
139139
},
140140
expected: []string{
141141
"SKAFFOLD_RUN_ID=1234",

0 commit comments

Comments
 (0)