Skip to content

Commit 7b18b11

Browse files
nkubalankubala
authored
Track deployment namespaces in Deployer (#6170)
* Track deployment namespaces in Deployer * fix test name and unnecessary variable * Remove Namespaces and associated methods from RunContext:
1 parent b3e0201 commit 7b18b11

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+374
-628
lines changed

‎pkg/skaffold/access/access.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@ import (
2525
// that accesses and exposes deployed resources from Skaffold.
2626
type Accessor interface {
2727
// Start starts the resource accessor.
28-
Start(context.Context, io.Writer, []string) error
28+
Start(context.Context, io.Writer) error
2929

3030
// Stop stops the resource accessor.
3131
Stop()
3232
}
3333

3434
type NoopAccessor struct{}
3535

36-
func (n *NoopAccessor) Start(context.Context, io.Writer, []string) error { return nil }
36+
func (n *NoopAccessor) Start(context.Context, io.Writer) error { return nil }
3737

3838
func (n *NoopAccessor) Stop() {}

‎pkg/skaffold/access/access_mux.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ import (
2323

2424
type AccessorMux []Accessor
2525

26-
func (a AccessorMux) Start(ctx context.Context, out io.Writer, namespaces []string) error {
26+
func (a AccessorMux) Start(ctx context.Context, out io.Writer) error {
2727
for _, accessor := range a {
28-
if err := accessor.Start(ctx, out, namespaces); err != nil {
28+
if err := accessor.Start(ctx, out); err != nil {
2929
return err
3030
}
3131
}

‎pkg/skaffold/debug/debug.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type Config interface {
3030
// that attaches to and helps debug deployed resources from Skaffold.
3131
type Debugger interface {
3232
// Start starts the debugger.
33-
Start(context.Context, []string) error
33+
Start(context.Context) error
3434

3535
// Stop stops the debugger.
3636
Stop()
@@ -41,7 +41,7 @@ type Debugger interface {
4141

4242
type NoopDebugger struct{}
4343

44-
func (n *NoopDebugger) Start(context.Context, []string) error { return nil }
44+
func (n *NoopDebugger) Start(context.Context) error { return nil }
4545

4646
func (n *NoopDebugger) Stop() {}
4747

‎pkg/skaffold/debug/debugger_mux.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import "context"
2020

2121
type DebuggerMux []Debugger
2222

23-
func (d DebuggerMux) Start(ctx context.Context, namespaces []string) error {
23+
func (d DebuggerMux) Start(ctx context.Context) error {
2424
for _, debugger := range d {
25-
if err := debugger.Start(ctx, namespaces); err != nil {
25+
if err := debugger.Start(ctx); err != nil {
2626
return err
2727
}
2828
}

‎pkg/skaffold/deploy/component/kubernetes/accessor_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestGetAccessor(t *testing.T) {
6767
if test.enabled {
6868
opts.Append("1") // default enabled mode
6969
}
70-
a := NewAccessor(mockAccessConfig{opts: opts}, test.description, nil, nil, label.NewLabeller(false, nil, ""))
70+
a := NewAccessor(mockAccessConfig{opts: opts}, test.description, nil, nil, label.NewLabeller(false, nil, ""), nil)
7171
fmt.Fprintf(os.Stdout, "retrieved accessor: %+v\n", a)
7272
t.CheckDeepEqual(test.isNoop, reflect.Indirect(reflect.ValueOf(a)).Type() == reflect.TypeOf(access.NoopAccessor{}))
7373
})

‎pkg/skaffold/deploy/component/kubernetes/component.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ var (
5252
k8sMonitor map[string]status.Monitor
5353
)
5454

55-
func newAccessor(cfg portforward.Config, kubeContext string, cli *kubectl.CLI, podSelector kubernetes.PodSelector, labeller label.Config) access.Accessor {
55+
func newAccessor(cfg portforward.Config, kubeContext string, cli *kubectl.CLI, podSelector kubernetes.PodSelector, labeller label.Config, namespaces *[]string) access.Accessor {
5656
accessLock.Lock()
5757
defer accessLock.Unlock()
5858
if k8sAccessor == nil {
@@ -62,7 +62,7 @@ func newAccessor(cfg portforward.Config, kubeContext string, cli *kubectl.CLI, p
6262
if !cfg.PortForwardOptions().Enabled() {
6363
k8sAccessor[kubeContext] = &access.NoopAccessor{}
6464
}
65-
m := portforward.NewForwarderManager(cli, podSelector, labeller.RunIDSelector(), cfg.Mode(), cfg.PortForwardOptions(), cfg.PortForwardResources())
65+
m := portforward.NewForwarderManager(cli, podSelector, labeller.RunIDSelector(), cfg.Mode(), namespaces, cfg.PortForwardOptions(), cfg.PortForwardResources())
6666
if m == nil {
6767
k8sAccessor[kubeContext] = &access.NoopAccessor{}
6868
} else {
@@ -73,12 +73,12 @@ func newAccessor(cfg portforward.Config, kubeContext string, cli *kubectl.CLI, p
7373
return k8sAccessor[kubeContext]
7474
}
7575

76-
func newDebugger(mode config.RunMode, podSelector kubernetes.PodSelector) debug.Debugger {
76+
func newDebugger(mode config.RunMode, podSelector kubernetes.PodSelector, namespaces *[]string) debug.Debugger {
7777
if mode != config.RunModes.Debug {
7878
return &debug.NoopDebugger{}
7979
}
8080

81-
return debugging.NewContainerManager(podSelector)
81+
return debugging.NewContainerManager(podSelector, namespaces)
8282
}
8383

8484
func newImageLoader(cfg k8sloader.Config, cli *kubectl.CLI) loader.ImageLoader {
@@ -88,11 +88,11 @@ func newImageLoader(cfg k8sloader.Config, cli *kubectl.CLI) loader.ImageLoader {
8888
return &loader.NoopImageLoader{}
8989
}
9090

91-
func newLogger(config k8slogger.Config, cli *kubectl.CLI, podSelector kubernetes.PodSelector) log.Logger {
92-
return k8slogger.NewLogAggregator(cli, podSelector, config)
91+
func newLogger(config k8slogger.Config, cli *kubectl.CLI, podSelector kubernetes.PodSelector, namespaces *[]string) log.Logger {
92+
return k8slogger.NewLogAggregator(cli, podSelector, namespaces, config)
9393
}
9494

95-
func newMonitor(cfg k8sstatus.Config, kubeContext string, labeller *label.DefaultLabeller) status.Monitor {
95+
func newMonitor(cfg k8sstatus.Config, kubeContext string, labeller *label.DefaultLabeller, namespaces *[]string) status.Monitor {
9696
monitorLock.Lock()
9797
defer monitorLock.Unlock()
9898
if k8sMonitor == nil {
@@ -103,12 +103,12 @@ func newMonitor(cfg k8sstatus.Config, kubeContext string, labeller *label.Defaul
103103
if enabled != nil && !*enabled { // assume disabled only if explicitly set to false
104104
k8sMonitor[kubeContext] = &status.NoopMonitor{}
105105
} else {
106-
k8sMonitor[kubeContext] = k8sstatus.NewStatusMonitor(cfg, labeller)
106+
k8sMonitor[kubeContext] = k8sstatus.NewStatusMonitor(cfg, labeller, namespaces)
107107
}
108108
}
109109
return k8sMonitor[kubeContext]
110110
}
111111

112-
func newSyncer(config sync.Config, cli *kubectl.CLI) sync.Syncer {
113-
return sync.NewPodSyncer(cli, config)
112+
func newSyncer(cli *kubectl.CLI, namespaces *[]string) sync.Syncer {
113+
return sync.NewPodSyncer(cli, namespaces)
114114
}

‎pkg/skaffold/deploy/component/kubernetes/debugger_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func TestGetDebugger(t *testing.T) {
4848

4949
for _, test := range tests {
5050
testutil.Run(t, test.description, func(t *testutil.T) {
51-
d := NewDebugger(test.runMode, nil)
51+
d := NewDebugger(test.runMode, nil, nil)
5252
t.CheckDeepEqual(test.isNoop, reflect.Indirect(reflect.ValueOf(d)).Type() == reflect.TypeOf(debug.NoopDebugger{}))
5353
})
5454
}

‎pkg/skaffold/deploy/component/kubernetes/monitor_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestGetMonitor(t *testing.T) {
6363

6464
for _, test := range tests {
6565
testutil.Run(t, test.description, func(t *testutil.T) {
66-
m := NewMonitor(mockStatusConfig{statusCheck: test.statusCheck}, test.description, label.NewLabeller(false, nil, ""))
66+
m := NewMonitor(mockStatusConfig{statusCheck: test.statusCheck}, test.description, label.NewLabeller(false, nil, ""), nil)
6767
t.CheckDeepEqual(test.isNoop, reflect.Indirect(reflect.ValueOf(m)).Type() == reflect.TypeOf(status.NoopMonitor{}))
6868
})
6969
}

‎pkg/skaffold/deploy/deploy.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ import (
3232
// the build results to a Kubernetes cluster
3333
type Deployer interface {
3434
// Deploy should ensure that the build results are deployed to the Kubernetes
35-
// cluster. Returns the list of impacted namespaces.
36-
Deploy(context.Context, io.Writer, []graph.Artifact) ([]string, error)
35+
// cluster.
36+
Deploy(context.Context, io.Writer, []graph.Artifact) error
3737

3838
// Dependencies returns a list of files that the deployer depends on.
3939
// In dev mode, a redeploy will be triggered

‎pkg/skaffold/deploy/deploy_mux.go

+6-10
Original file line numberDiff line numberDiff line change
@@ -99,33 +99,29 @@ func (m DeployerMux) RegisterLocalImages(images []graph.Artifact) {
9999
}
100100
}
101101

102-
func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []graph.Artifact) ([]string, error) {
103-
seenNamespaces := util.NewStringSet()
104-
102+
func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []graph.Artifact) error {
105103
for i, deployer := range m.deployers {
106104
eventV2.DeployInProgress(i)
107105
w = output.WithEventContext(w, constants.Deploy, strconv.Itoa(i), "skaffold")
108106
ctx, endTrace := instrumentation.StartTrace(ctx, "Deploy")
109107

110-
namespaces, err := deployer.Deploy(ctx, w, as)
111-
if err != nil {
108+
if err := deployer.Deploy(ctx, w, as); err != nil {
112109
eventV2.DeployFailed(i, err)
113110
endTrace(instrumentation.TraceEndError(err))
114-
return nil, err
111+
return err
115112
}
116-
seenNamespaces.Insert(namespaces...)
117113
if m.iterativeStatusCheck {
118-
if err = deployer.GetStatusMonitor().Check(ctx, w); err != nil {
114+
if err := deployer.GetStatusMonitor().Check(ctx, w); err != nil {
119115
eventV2.DeployFailed(i, err)
120116
endTrace(instrumentation.TraceEndError(err))
121-
return nil, err
117+
return err
122118
}
123119
}
124120
eventV2.DeploySucceeded(i)
125121
endTrace()
126122
}
127123

128-
return seenNamespaces.ToList(), nil
124+
return nil
129125
}
130126

131127
func (m DeployerMux) Dependencies() ([]string, error) {

‎pkg/skaffold/deploy/deploy_mux_test.go

+19-43
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,13 @@ import (
3939
func NewMockDeployer() *MockDeployer { return &MockDeployer{labels: make(map[string]string)} }
4040

4141
type MockDeployer struct {
42-
labels map[string]string
43-
deployNamespaces []string
44-
deployErr error
45-
dependencies []string
46-
dependenciesErr error
47-
cleanupErr error
48-
renderResult string
49-
renderErr error
42+
labels map[string]string
43+
deployErr error
44+
dependencies []string
45+
dependenciesErr error
46+
cleanupErr error
47+
renderResult string
48+
renderErr error
5049
}
5150

5251
func (m *MockDeployer) GetAccessor() access.Accessor {
@@ -106,20 +105,15 @@ func (m *MockDeployer) WithRenderErr(err error) *MockDeployer {
106105
return m
107106
}
108107

109-
func (m *MockDeployer) Deploy(context.Context, io.Writer, []graph.Artifact) ([]string, error) {
110-
return m.deployNamespaces, m.deployErr
108+
func (m *MockDeployer) Deploy(context.Context, io.Writer, []graph.Artifact) error {
109+
return m.deployErr
111110
}
112111

113112
func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []graph.Artifact, _ bool, _ string) error {
114113
w.Write([]byte(m.renderResult))
115114
return m.renderErr
116115
}
117116

118-
func (m *MockDeployer) WithDeployNamespaces(namespaces []string) *MockDeployer {
119-
m.deployNamespaces = namespaces
120-
return m
121-
}
122-
123117
func (m *MockDeployer) WithDependencies(dependencies []string) *MockDeployer {
124118
m.dependencies = dependencies
125119
return m
@@ -141,32 +135,14 @@ func TestDeployerMux_Deploy(t *testing.T) {
141135
shouldErr bool
142136
}{
143137
{
144-
name: "disjoint namespaces are combined",
145-
namespaces1: []string{"ns-a"},
146-
namespaces2: []string{"ns-b"},
147-
expectedNs: []string{"ns-a", "ns-b"},
148-
},
149-
{
150-
name: "repeated namespaces are not duplicated",
151-
namespaces1: []string{"ns-a", "ns-c"},
152-
namespaces2: []string{"ns-b", "ns-c"},
153-
expectedNs: []string{"ns-a", "ns-b", "ns-c"},
154-
},
155-
{
156-
name: "short-circuits when first call fails",
157-
namespaces1: []string{"ns-a"},
158-
err1: fmt.Errorf("failed in first"),
159-
namespaces2: []string{"ns-b"},
160-
expectedNs: nil,
161-
shouldErr: true,
138+
name: "short-circuits when first call fails",
139+
err1: fmt.Errorf("failed in first"),
140+
shouldErr: true,
162141
},
163142
{
164-
name: "when second call fails",
165-
namespaces1: []string{"ns-a"},
166-
namespaces2: []string{"ns-b"},
167-
err2: fmt.Errorf("failed in second"),
168-
expectedNs: nil,
169-
shouldErr: true,
143+
name: "when second call fails",
144+
err2: fmt.Errorf("failed in second"),
145+
shouldErr: true,
170146
},
171147
}
172148

@@ -181,13 +157,13 @@ func TestDeployerMux_Deploy(t *testing.T) {
181157
}}})
182158

183159
deployerMux := NewDeployerMux([]Deployer{
184-
NewMockDeployer().WithDeployNamespaces(test.namespaces1).WithDeployErr(test.err1),
185-
NewMockDeployer().WithDeployNamespaces(test.namespaces2).WithDeployErr(test.err2),
160+
NewMockDeployer().WithDeployErr(test.err1),
161+
NewMockDeployer().WithDeployErr(test.err2),
186162
}, false)
187163

188-
namespaces, err := deployerMux.Deploy(context.Background(), nil, nil)
164+
err := deployerMux.Deploy(context.Background(), nil, nil)
189165

190-
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedNs, namespaces)
166+
testutil.CheckError(t, test.shouldErr, err)
191167
})
192168
}
193169
}

0 commit comments

Comments
 (0)