Add logic for creating default deployer in runner #5541
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5541 +/- ##
==========================================
- Coverage 71.39% 71.21% -0.18%
==========================================
Files 400 400
Lines 14839 14934 +95
==========================================
+ Hits 10594 10635 +41
- Misses 3473 3511 +38
- Partials 772 788 +16
Continue to review full report at Codecov.
|
nkubala
force-pushed
the
default-deployer
branch
from
62910b0 to
abd548e
Compare
| @@ -233,6 +236,107 @@ func getSyncer(cfg sync.Config) sync.Syncer { | |||
| return sync.NewSyncer(cfg) | |||
| } | |||
|
|
|||
| /* | |||
| The "default deployer" is used in `skaffold apply`, which uses a `kubectl` deployer to actuate resources | |||
There was a problem hiding this comment.
can we move this to pkg/skaffold/apply/deployer.go or something similar?
There was a problem hiding this comment.
yeah it probably should....i didn't do it here because it's a bigger refactor and i didn't want to mix organization patterns. let me go take a stab at the refactor though, new PR pending
There was a problem hiding this comment.
my comment was only scoped to the new function added. Are you talking about refactoring your other PR? This function doesn't seem to be used anywhere else.
There was a problem hiding this comment.
ahh sorry i misread your comment, i thought you were suggesting putting it in pkg/skaffold/deploy. i actually think that's where it should be, so the runner would make calls like deploy.GetDeployer and build.GetBuilder instead of having private getDeployer and getBuilder functions which do pre-processing and then instantiate.
IMO this pattern makes more sense, but is a bigger refactor, so rather than set an example with this PR (but have mismatched organization patterns) i decided to just put it alongside the pre-existing getDeployer function. i can move it to an apply package so it's more clear what it's used for though.
separately - this PR just adds this logic for ease of review, it's only actually used in the other PR which adds the apply command.
There was a problem hiding this comment.
make sense. I thought it's putting too many things in new.go.
Co-authored-by: Gaurav <[email protected]>
This change adds logic for creating a default deployer, to create arbitrary resources provided as hydrated manifests to
skaffold apply. This will create akubectldeployer, regardless of deploy configuration from the provided skaffold.yaml (except for a select set of configuration passed to thekubectldeployer itself).As per the comment in code:
Part 1 of #4856