Enable multiple deployers in `skaffold.yaml` by corneliusweig · Pull Request #3392 · GoogleContainerTools/skaffold

corneliusweig · 2019-12-16T00:34:36Z

Should merge before : n/a

Should merge after : #3391, ~~#3390~~

Description

Some users find it limiting that they can only configure a single deployer in Skaffold (#2875, #3292). This PR enables using multiple deployers.

User facing changes

Users get the possibility to configure multiple deployers.

⚠️ When merging profiles into the base pipeline, Skaffold used to wipe other deployers. This is no longer the case, because Skaffold now handles more than one deployer.

Before

skaffold.yaml allows to configure exactly one deployer (kubectl, kustomize, helm).

After

Users can configure all three deployers at once. Iow, the oneOf=deployer constraint in skaffold.yaml is dropped.

Next PRs.

n/a

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes unit tests
Mentions any output changes.
Adds documentation as needed: user docs, YAML reference, CLI reference.
Adds integration tests if needed.

Reviewer Notes

The code flow looks good.
Unit test added.
User facing changes look good.

Release Notes

- The deployer section in `skaffold.yaml` now accepts multiple deployers in a single pipeline.
- When applying profiles, deployers in the base profile no longer get wiped when merging in the deployer from the profile.

corneliusweig · 2019-12-16T00:39:57Z

I went with the approach to simply drop the oneOf constraint. Another option would have been to add a list of deployers in the deploy section, like so:

deploy:
  kubeContext: something
  deployers:
  - kustomize: {}
  - kubectl: {}
  - helm: {}
  - ...

However, the nested deploy.deployers looks very weird and it is not clear if a list of deployers would even be desirable.

codecov · 2019-12-16T00:42:04Z

Codecov Report

Merging #3392 into master will increase coverage by 0.55%.
The diff coverage is n/a.

Impacted Files	Coverage Δ
pkg/skaffold/build/cluster/kaniko.go	`0% <0%> (-55.13%)`	⬇️
pkg/skaffold/deploy/kustomize.go	`73.75% <0%> (-2.86%)`	⬇️
pkg/skaffold/generate_pipeline/profile.go	`32.87% <0%> (-2.65%)`	⬇️
pkg/skaffold/build/jib/jib.go	`72.79% <0%> (-0.7%)`	⬇️
pkg/skaffold/deploy/kubectl/visitor.go	`93.54% <0%> (-0.21%)`	⬇️
pkg/skaffold/build/jib/maven.go	`100% <0%> (ø)`	⬆️
pkg/skaffold/build/build.go	`0% <0%> (ø)`	⬆️
pkg/skaffold/docker/syncmap.go	`67.16% <0%> (ø)`	⬆️
pkg/skaffold/build/jib/gradle.go	`100% <0%> (ø)`	⬆️
pkg/skaffold/build/gcb/buildpacks.go	`100% <0%> (ø)`	⬆️
... and 27 more

This allows re-use by other deployer renderers. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

This Deployer implemenation forwards commands to all of the deployers in his list. It will be useful to allow multiple deployers in skaffold.yaml. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Some use-cases require a hybrid mode where users need to use different deployers at the same time. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

So far, deployers were marked with `oneOf`, so that only one of them could be active at a time. Since this no longer holds, profile activation now merges the deployers from profiles and base. This change adapts the tests accordingly. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

This is necessary because the new logic merges profile deployer into the main pipeline instead of replacing existing deployers. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

corneliusweig · 2020-01-04T13:30:09Z

I've finally added the missing integration test, so it's no longer WIP :)

balopat · 2020-01-15T19:06:58Z

docs/content/en/docs/pipeline-stages/deployers.md

 controls how Skaffold builds artifacts. To use a specific tool for deploying
 artifacts, add the value representing the tool and options for using the tool
-to the `deploy` section.
+to the `deploy` section. You may even use different tools at the same time.


Suggested change

to the `deploy` section. You may even use different tools at the same time.

to the `deploy` section. You may even use a combination of each available deployment approach at the same time.

I'll not apply this suggestion right now, to not interfere with the kokoro run. But please go ahead and apply this if you think it's ready for merging!

I just pushed it - will merge as soon as kokoro is done again :)

balopat · 2020-01-15T20:38:16Z

I just rebased it - hope that's fine!

corneliusweig · 2020-01-15T20:52:28Z

I just rebased it - hope that's fine!

Yes of course!

corneliusweig requested review from balopat, dgageot, nkubala and tejal29 as code owners December 16, 2019 00:34

pull-request-size bot added the size/XL label Dec 16, 2019

googlebot added the cla: yes label Dec 16, 2019

This was referenced Dec 16, 2019

Add forwarding deployer muxer to enable multiple deployers #3391

Merged

Apply resource labels in the deployer #3390

Merged

corneliusweig changed the title ~~Enable multiple deployers in skaffold.yaml~~ WIP Enable multiple deployers in skaffold.yaml Dec 16, 2019

balopat added the !! wip !! label Dec 18, 2019

corneliusweig force-pushed the w/deployer-mux/config branch from 512a92f to d746cfc Compare December 20, 2019 23:14

kvokka mentioned this pull request Dec 24, 2019

Skaffold generates strange warnings when running with many profiles #3277

Closed

corneliusweig added 8 commits January 4, 2020 13:41

Extract writing of rendered manifests

0b920e6

This allows re-use by other deployer renderers. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Add delegating Deployer muxer

5a06098

This Deployer implemenation forwards commands to all of the deployers in his list. It will be useful to allow multiple deployers in skaffold.yaml. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Review comments: prefer not to depend on external utils

be44da2

Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Enable hybrid deployment mode with multiple deployers

bdf6d61

Some use-cases require a hybrid mode where users need to use different deployers at the same time. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Ensure only kustomize is deployed in debug test

8dbb20f

This is necessary because the new logic merges profile deployer into the main pipeline instead of replacing existing deployers. Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

Add integration test for multiple deployers

f02d953

Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

[doc] Mention multiple deployer capability

ec3eaf0

Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>

corneliusweig force-pushed the w/deployer-mux/config branch from d746cfc to ec3eaf0 Compare January 4, 2020 12:56

corneliusweig changed the title ~~WIP Enable multiple deployers in skaffold.yaml~~ Enable multiple deployers in skaffold.yaml Jan 4, 2020

balopat self-assigned this Jan 15, 2020

balopat removed the !! wip !! label Jan 15, 2020

balopat reviewed Jan 15, 2020

View reviewed changes

balopat added the needs-rebase label Jan 15, 2020

Merge branch 'master' into w/deployer-mux/config

8eb5083

balopat added kokoro:run runs the kokoro jobs on a PR and removed needs-rebase labels Jan 15, 2020

kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jan 15, 2020

update docs

b6a88bf

balopat added the pr/lgtm label Jan 15, 2020

balopat approved these changes Jan 15, 2020

View reviewed changes

balopat added the kokoro:run runs the kokoro jobs on a PR label Jan 16, 2020

kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jan 16, 2020

dgageot merged commit fd09f86 into GoogleContainerTools:master Jan 16, 2020

corneliusweig deleted the w/deployer-mux/config branch January 16, 2020 07:52

nkubala mentioned this pull request Feb 28, 2020

Use helm deploy next to kubectl #528

Closed

1Const1 mentioned this pull request Jul 11, 2024

skaffold 2 with multiple deployers kubectl and helm in one time config values in skaffold.yaml #9473

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable multiple deployers in `skaffold.yaml`#3392

Enable multiple deployers in `skaffold.yaml`#3392
dgageot merged 10 commits intoGoogleContainerTools:masterfrom
corneliusweig:w/deployer-mux/config

corneliusweig commented Dec 16, 2019 •

edited

Loading

Uh oh!

corneliusweig commented Dec 16, 2019

Uh oh!

codecov bot commented Dec 16, 2019 •

edited

Loading

Uh oh!

corneliusweig commented Jan 4, 2020

Uh oh!

balopat Jan 15, 2020

Uh oh!

corneliusweig Jan 15, 2020

Uh oh!

balopat Jan 15, 2020

Uh oh!

balopat commented Jan 15, 2020

Uh oh!

corneliusweig commented Jan 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	to the `deploy` section. You may even use different tools at the same time.
	to the `deploy` section. You may even use a combination of each available deployment approach at the same time.

Conversation

corneliusweig commented Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corneliusweig commented Dec 16, 2019

Uh oh!

codecov bot commented Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

corneliusweig commented Jan 4, 2020

Uh oh!

balopat Jan 15, 2020

Choose a reason for hiding this comment

Uh oh!

corneliusweig Jan 15, 2020

Choose a reason for hiding this comment

Uh oh!

balopat Jan 15, 2020

Choose a reason for hiding this comment

Uh oh!

balopat commented Jan 15, 2020

Uh oh!

corneliusweig commented Jan 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

corneliusweig commented Dec 16, 2019 •

edited

Loading

codecov bot commented Dec 16, 2019 •

edited

Loading