Move deployers into separate packages (#4812)

* Move deployers into separate packages

* review comments and fixing linters

* rename a few functions

cmd/skaffold/app/cmd/debug.go

@@ -23,7 +23,7 @@ import (
 	"github.com/spf13/cobra"
 
 	debugging "github.com/GoogleContainerTools/skaffold/pkg/skaffold/debug"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 )
 
 // for tests
@@ -44,7 +44,7 @@ func NewCmdDebug() *cobra.Command {
 
 func runDebug(ctx context.Context, out io.Writer) error {
 	opts.PortForward.ForwardPods = true
-	deploy.AddManifestTransform(debugging.ApplyDebuggingTransforms)
+	manifest.AddTransform(debugging.ApplyDebuggingTransforms)
 
 	return doDev(ctx, out)
 }

cmd/skaffold/app/cmd/filter.go

@@ -29,8 +29,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	debugging "github.com/GoogleContainerTools/skaffold/pkg/skaffold/debug"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
@@ -65,7 +64,7 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA
 		if err != nil {
 			return err
 		}
-		manifestList := kubectl.ManifestList([][]byte{bytes})
+		manifestList := manifest.ManifestList([][]byte{bytes})
 		if debuggingFilters {
 			// TODO(bdealwis): refactor this code
 			debugHelpersRegistry, err := config.GetDebugHelpersRegistry(opts.GlobalConfig)
@@ -77,7 +76,7 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA
 				return err
 			}
 
-			manifestList, err = debugging.ApplyDebuggingTransforms(manifestList, buildArtifacts, deploy.Registries{
+			manifestList, err = debugging.ApplyDebuggingTransforms(manifestList, buildArtifacts, manifest.Registries{
 				DebugHelpersRegistry: debugHelpersRegistry,
 				InsecureRegistries:   insecureRegistries,
 			})

integration/render_test.go

@@ -31,7 +31,8 @@ import (
 	"github.com/GoogleContainerTools/skaffold/integration/skaffold"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
@@ -76,7 +77,7 @@ spec:
 		t.NewTempDir().
 			Write("deployment.yaml", test.input).
 			Chdir()
-		deployer, err := deploy.NewKubectlDeployer(&runcontext.RunContext{
+		deployer, err := kubectl.NewDeployer(&runcontext.RunContext{
 			WorkingDir: ".",
 			Cfg: latest.Pipeline{
 				Deploy: latest.DeployConfig{
@@ -231,7 +232,7 @@ spec:
 				Write("deployment.yaml", test.input).
 				Chdir()
 
-			deployer, err := deploy.NewKubectlDeployer(&runcontext.RunContext{
+			deployer, err := kubectl.NewDeployer(&runcontext.RunContext{
 				WorkingDir: ".",
 				Cfg: latest.Pipeline{
 					Deploy: latest.DeployConfig{
@@ -419,7 +420,7 @@ spec:
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			deployer := deploy.NewHelmDeployer(&runcontext.RunContext{
+			deployer := helm.NewDeployer(&runcontext.RunContext{
 				Cfg: latest.Pipeline{
 					Deploy: latest.DeployConfig{
 						DeployType: latest.DeployType{

pkg/skaffold/debug/debug.go

@@ -29,9 +29,8 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 )
 
@@ -50,7 +49,7 @@ var (
 )
 
 // ApplyDebuggingTransforms applies language-platform-specific transforms to a list of manifests.
-func ApplyDebuggingTransforms(l kubectl.ManifestList, builds []build.Artifact, registries deploy.Registries) (kubectl.ManifestList, error) {
+func ApplyDebuggingTransforms(l manifest.ManifestList, builds []build.Artifact, registries manifest.Registries) (manifest.ManifestList, error) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
@@ -63,8 +62,8 @@ func ApplyDebuggingTransforms(l kubectl.ManifestList, builds []build.Artifact, r
 	return applyDebuggingTransforms(l, retriever, registries.DebugHelpersRegistry)
 }
 
-func applyDebuggingTransforms(l kubectl.ManifestList, retriever configurationRetriever, debugHelpersRegistry string) (kubectl.ManifestList, error) {
-	var updated kubectl.ManifestList
+func applyDebuggingTransforms(l manifest.ManifestList, retriever configurationRetriever, debugHelpersRegistry string) (manifest.ManifestList, error) {
+	var updated manifest.ManifestList
 	for _, manifest := range l {
 		obj, _, err := decodeFromYaml(manifest, nil, nil)
 		if err != nil {

pkg/skaffold/debug/debug_test.go

@@ -24,7 +24,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
@@ -509,7 +509,7 @@ spec:
 				return imageConfiguration{}, nil
 			}
 
-			result, err := applyDebuggingTransforms(kubectl.ManifestList{[]byte(test.in)}, retriever, "HELPERS")
+			result, err := applyDebuggingTransforms(manifest.ManifestList{[]byte(test.in)}, retriever, "HELPERS")
 
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.out, result.String())
 		})

pkg/skaffold/deploy/deploy_mux.go

@@ -20,67 +20,42 @@ import (
 	"bytes"
 	"context"
 	"io"
-	"sort"
 	"strings"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/util"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 )
 
 // DeployerMux forwards all method calls to the deployers it contains.
 // When encountering an error, it aborts and returns the error. Otherwise,
 // it collects the results and returns it in bulk.
 type DeployerMux []Deployer
 
-type unit struct{}
-
-// stringSet helps to de-duplicate a set of strings.
-type stringSet map[string]unit
-
-func newStringSet() stringSet {
-	return make(map[string]unit)
-}
-
-// insert adds strings to the set.
-func (s stringSet) insert(strings ...string) {
-	for _, item := range strings {
-		s[item] = unit{}
-	}
-}
-
-// toList returns the sorted list of inserted strings.
-func (s stringSet) toList() []string {
-	var res []string
-	for item := range s {
-		res = append(res, item)
-	}
-	sort.Strings(res)
-	return res
-}
-
 func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []build.Artifact) ([]string, error) {
-	seenNamespaces := newStringSet()
+	seenNamespaces := util.NewStringSet()
 
 	for _, deployer := range m {
 		namespaces, err := deployer.Deploy(ctx, w, as)
 		if err != nil {
 			return nil, err
 		}
-		seenNamespaces.insert(namespaces...)
+		seenNamespaces.Insert(namespaces...)
 	}
 
-	return seenNamespaces.toList(), nil
+	return seenNamespaces.ToList(), nil
 }
 
 func (m DeployerMux) Dependencies() ([]string, error) {
-	deps := newStringSet()
+	deps := util.NewStringSet()
 	for _, deployer := range m {
 		result, err := deployer.Dependencies()
 		if err != nil {
 			return nil, err
 		}
-		deps.insert(result...)
+		deps.Insert(result...)
 	}
-	return deps.toList(), nil
+	return deps.ToList(), nil
 }
 
 func (m DeployerMux) Cleanup(ctx context.Context, w io.Writer) error {
@@ -103,5 +78,5 @@ func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []build.Artifac
 	}
 
 	allResources := strings.Join(resources, "\n---\n")
-	return outputRenderedManifests(allResources, filepath, w)
+	return manifest.Write(allResources, filepath, w)
 }

pkg/skaffold/deploy/helm.go → pkg/skaffold/deploy/helm/helm.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package deploy
+package helm
 
 import (
 	"bufio"
@@ -35,15 +35,19 @@ import (
 	"time"
 
 	"github.com/blang/semver"
-	"github.com/cenkalti/backoff/v4"
+	backoff "github.com/cenkalti/backoff/v4"
 	"github.com/mitchellh/go-homedir"
 	"github.com/sirupsen/logrus"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/label"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/types"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/manifest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/walk"
@@ -69,8 +73,8 @@ var (
 	osExecutable = os.Executable
 )
 
-// HelmDeployer deploys workflows using the helm CLI
-type HelmDeployer struct {
+// Deployer deploys workflows using the helm CLI
+type Deployer struct {
 	*latest.HelmDeploy
 
 	kubeContext string
@@ -89,9 +93,9 @@ type HelmDeployer struct {
 	bV semver.Version
 }
 
-// NewHelmDeployer returns a configured HelmDeployer
-func NewHelmDeployer(cfg Config, labels map[string]string) *HelmDeployer {
-	return &HelmDeployer{
+// NewDeployer returns a configured Deployer
+func NewDeployer(cfg kubectl.Config, labels map[string]string) *Deployer {
+	return &Deployer{
 		HelmDeploy:  cfg.Pipeline().Deploy.HelmDeploy,
 		kubeContext: cfg.GetKubeContext(),
 		kubeConfig:  cfg.GetKubeConfig(),
@@ -103,15 +107,15 @@ func NewHelmDeployer(cfg Config, labels map[string]string) *HelmDeployer {
 }
 
 // Deploy deploys the build results to the Kubernetes cluster
-func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]string, error) {
+func (h *Deployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]string, error) {
 	hv, err := h.binVer(ctx)
 	if err != nil {
 		return nil, fmt.Errorf(versionErrorString, err)
 	}
 
 	logrus.Infof("Deploying with helm v%s ...", hv)
 
-	var dRes []Artifact
+	var dRes []types.Artifact
 	nsMap := map[string]struct{}{}
 	valuesSet := map[string]bool{}
 
@@ -149,7 +153,7 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build
 		}
 	}
 
-	if err := labelDeployResults(h.labels, dRes); err != nil {
+	if err := label.Apply(h.labels, dRes); err != nil {
 		return nil, fmt.Errorf("adding labels: %w", err)
 	}
 
@@ -163,7 +167,7 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build
 }
 
 // Dependencies returns a list of files that the deployer depends on.
-func (h *HelmDeployer) Dependencies() ([]string, error) {
+func (h *Deployer) Dependencies() ([]string, error) {
 	var deps []string
 
 	for _, release := range h.Releases {
@@ -223,7 +227,7 @@ func (h *HelmDeployer) Dependencies() ([]string, error) {
 }
 
 // Cleanup deletes what was deployed by calling Deploy.
-func (h *HelmDeployer) Cleanup(ctx context.Context, out io.Writer) error {
+func (h *Deployer) Cleanup(ctx context.Context, out io.Writer) error {
 	hv, err := h.binVer(ctx)
 	if err != nil {
 		return fmt.Errorf(versionErrorString, err)
@@ -259,7 +263,7 @@ func (h *HelmDeployer) Cleanup(ctx context.Context, out io.Writer) error {
 }
 
 // Render generates the Kubernetes manifests and writes them out
-func (h *HelmDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, offline bool, filepath string) error {
+func (h *Deployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, offline bool, filepath string) error {
 	hv, err := h.binVer(ctx)
 	if err != nil {
 		return fmt.Errorf(versionErrorString, err)
@@ -321,11 +325,11 @@ func (h *HelmDeployer) Render(ctx context.Context, out io.Writer, builds []build
 		renderedManifests.Write(outBuffer.Bytes())
 	}
 
-	return outputRenderedManifests(renderedManifests.String(), filepath, out)
+	return manifest.Write(renderedManifests.String(), filepath, out)
 }
 
 // exec executes the helm command, writing combined stdout/stderr to the provided writer
-func (h *HelmDeployer) exec(ctx context.Context, out io.Writer, useSecrets bool, env []string, args ...string) error {
+func (h *Deployer) exec(ctx context.Context, out io.Writer, useSecrets bool, env []string, args ...string) error {
 	if args[0] != "version" {
 		args = append([]string{"--kube-context", h.kubeContext}, args...)
 		args = append(args, h.Flags.Global...)
@@ -350,7 +354,7 @@ func (h *HelmDeployer) exec(ctx context.Context, out io.Writer, useSecrets bool,
 }
 
 // deployRelease deploys a single release
-func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r latest.HelmRelease, builds []build.Artifact, valuesSet map[string]bool, helmVersion semver.Version) ([]Artifact, error) {
+func (h *Deployer) deployRelease(ctx context.Context, out io.Writer, r latest.HelmRelease, builds []build.Artifact, valuesSet map[string]bool, helmVersion semver.Version) ([]types.Artifact, error) {
 	releaseName, err := util.ExpandEnvTemplate(r.Name, nil)
 	if err != nil {
 		return nil, fmt.Errorf("cannot parse the release name template: %w", err)
@@ -413,10 +417,10 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates
 	} else {
 		if r.UpgradeOnChange != nil && !*r.UpgradeOnChange {
 			logrus.Infof("Release %s already installed...", releaseName)
-			return []Artifact{}, nil
+			return []types.Artifact{}, nil
 		} else if r.UpgradeOnChange == nil && r.Remote {
 			logrus.Infof("Release %s not upgraded as it is remote...", releaseName)
-			return []Artifact{}, nil
+			return []types.Artifact{}, nil
 		}
 	}
 
@@ -474,7 +478,7 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates
 }
 
 // getRelease confirms that a release is visible to helm
-func (h *HelmDeployer) getRelease(ctx context.Context, helmVersion semver.Version, releaseName string, namespace string) (bytes.Buffer, error) {
+func (h *Deployer) getRelease(ctx context.Context, helmVersion semver.Version, releaseName string, namespace string) (bytes.Buffer, error) {
 	// Retry, because under Helm 2, at least, a release may not be immediately visible
 	opts := backoff.NewExponentialBackOff()
 	opts.MaxElapsedTime = 4 * time.Second
@@ -495,7 +499,7 @@ func (h *HelmDeployer) getRelease(ctx context.Context, helmVersion semver.Versio
 }
 
 // binVer returns the version of the helm binary found in PATH. May be cached.
-func (h *HelmDeployer) binVer(ctx context.Context) (semver.Version, error) {
+func (h *Deployer) binVer(ctx context.Context) (semver.Version, error) {
 	// Return the cached version value if non-zero
 	if h.bV.Major != 0 || h.bV.Minor != 0 {
 		return h.bV, nil
@@ -732,7 +736,7 @@ func envVarForImage(imageName string, digest string) map[string]string {
 }
 
 // packageChart packages the chart and returns the path to the resulting chart archive
-func (h *HelmDeployer) packageChart(ctx context.Context, r latest.HelmRelease) (string, error) {
+func (h *Deployer) packageChart(ctx context.Context, r latest.HelmRelease) (string, error) {
 	// Allow a test to sneak a predictable path in
 	tmpDir := h.pkgTmpDir
 
@@ -778,7 +782,7 @@ func (h *HelmDeployer) packageChart(ctx context.Context, r latest.HelmRelease) (
 	return output[idx:], nil
 }
 
-func (h *HelmDeployer) generateSkaffoldDebugFilter(buildsFile string) []string {
+func (h *Deployer) generateSkaffoldDebugFilter(buildsFile string) []string {
 	args := []string{"filter", "--debugging", "--kube-context", h.kubeContext}
 	if len(buildsFile) > 0 {
 		args = append(args, "--build-artifacts", buildsFile)