Move Debugger from Runner into Deployer

[nkubala]

Related: #5813, #5936

Description

This change moves the Debugger object from the Runner into the Deployer, since debugging-behavior is implicitly tied to the underlying Deployer implementation. See #5809 for a more detailed description of why we need to do this.

This change adds one method to the Deployer interface:

type Deployer interface {
  ...

  GetDebugger() debug.Debugger
}

The GetDebugger() method is used by the Runner to retrieve theDebugger implementation and actually orchestrate that behavior in the dev loop.

[nkubala]

@gsquared94 re: https://github.com/GoogleContainerTools/skaffold/pull/5936/files#r647219723

I wonder if GetResourcePreviewer().Stop(), GetDebugger().Stop(), etc. can be called from the individual deployers directly instead of in dev.go or the runners. Then we wouldn't need the DebuggerMux, ResourcePreviewerMux, etc. Also since these elements are already decomposed in this PR to be independent for each constituent Deployer, it seems counter intuitive to again accumulate them in ..Mux and calling them together.

I'm not sure we can do this. the Deployer is now responsible for its associated Debugger implementation, but the Runner is still responsible for orchestrating the Deployers at a macro level, and that includes all of their associated components as well. it's sort of the same reason we can't have a Deployer call Deploy() on itself - this obviously seems silly, but the point is that the Deployer can only know how to deploy, not when.

[codecov]

Codecov Report

Merging #6021 (5511f4c) into master (5c2e24c) will decrease coverage by 0.03%.
The diff coverage is 50.40%.

@@            Coverage Diff             @@
##           master    #6021      +/-   ##
==========================================
- Coverage   70.77%   70.73%   -0.04%     
==========================================
  Files         462      464       +2     
  Lines       17897    17924      +27     
==========================================
+ Hits        12666    12678      +12     
- Misses       4299     4314      +15     
  Partials      932      932              

Impacted Files	Coverage Δ
pkg/skaffold/debug/debugger_mux.go	0.00% <0.00%> (ø)
pkg/skaffold/deploy/deploy_mux.go	70.73% <0.00%> (-5.59%)	⬇️
pkg/skaffold/debug/apply_transforms.go	32.83% <32.83%> (ø)
pkg/skaffold/deploy/kpt/kpt.go	81.54% <50.00%> (-0.22%)	⬇️
pkg/skaffold/deploy/kubectl/kubectl.go	68.87% <50.00%> (-0.20%)	⬇️
pkg/skaffold/deploy/kustomize/kustomize.go	75.34% <50.00%> (-0.36%)	⬇️
pkg/skaffold/debug/cnb.go	91.25% <66.66%> (ø)
pkg/skaffold/deploy/helm/deploy.go	78.92% <66.66%> (-0.25%)	⬇️
pkg/skaffold/runner/v1/dev.go	72.85% <66.66%> (-0.13%)	⬇️
pkg/skaffold/debug/provider.go	72.72% <72.72%> (ø)
... and 13 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c2e24c...5511f4c. Read the comment docs.

[gsquared94]

if image == artifact.ImageName || image == artifact.Tag 

are we intentionally matching against both? If we're expecting to always match by only artifactName or only tag maybe we could be specific, or else call it out in the function name/description?

[nkubala]

I didn't write this, just moved it :)

I assume it's intentional, maybe this can happen either before or after we compute the tag? @briandealwis would know more. I think we probably shouldn't touch this in this PR though

[gsquared94]

is Name() being called somewhere?

[gsquared94]

lgtm!