Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move deployers into separate packages #4812

Merged
merged 3 commits into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/skaffold/app/cmd/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.AddManifestTransform(debugging.ApplyDebuggingTransforms)

return doDev(ctx, out)
}
7 changes: 3 additions & 4 deletions cmd/skaffold/app/cmd/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand All @@ -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,
})
Expand Down
9 changes: 5 additions & 4 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
9 changes: 4 additions & 5 deletions pkg/skaffold/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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()

Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/debug/debug_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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())
})
Expand Down
43 changes: 9 additions & 34 deletions pkg/skaffold/deploy/deploy_mux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.OutputRenderedManifests(allResources, filepath, w)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OutputRenderedManifests to probably OutputRender

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about manifest.Write()? the fact that the manifests are rendered seems like an implementation detail to me

}
48 changes: 26 additions & 22 deletions pkg/skaffold/deploy/helm.go → pkg/skaffold/deploy/helm/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package deploy
package helm

import (
"bufio"
Expand All @@ -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"
Expand All @@ -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
Expand All @@ -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(),
Expand All @@ -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{}

Expand Down Expand Up @@ -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.LabelDeployResults(h.labels, dRes); err != nil {
return nil, fmt.Errorf("adding labels: %w", err)
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.OutputRenderedManifests(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...)
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
Loading