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

Deployers should only rely on their specific config #739

Merged
merged 1 commit into from
Jun 24, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 8 additions & 7 deletions pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,19 @@ import (
)

type HelmDeployer struct {
*v1alpha2.DeployConfig
*v1alpha2.HelmDeploy

kubeContext string
namespace string
}

// NewHelmDeployer returns a new HelmDeployer for a DeployConfig filled
// with the needed configuration for `helm`
func NewHelmDeployer(cfg *v1alpha2.DeployConfig, kubeContext string, namespace string) *HelmDeployer {
func NewHelmDeployer(cfg *v1alpha2.HelmDeploy, kubeContext string, namespace string) *HelmDeployer {
return &HelmDeployer{
DeployConfig: cfg,
kubeContext: kubeContext,
namespace: namespace,
HelmDeploy: cfg,
kubeContext: kubeContext,
namespace: namespace,
}
}

Expand All @@ -62,7 +63,7 @@ func (h *HelmDeployer) Labels() map[string]string {

func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) ([]Artifact, error) {
deployResults := []Artifact{}
for _, r := range h.HelmDeploy.Releases {
for _, r := range h.Releases {
results, err := h.deployRelease(out, r, builds)
if err != nil {
releaseName, _ := evaluateReleaseName(r.Name)
Expand All @@ -80,7 +81,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 {
for _, r := range h.HelmDeploy.Releases {
for _, r := range h.Releases {
if err := h.deleteRelease(out, r); err != nil {
releaseName, _ := evaluateReleaseName(r.Name)
return errors.Wrapf(err, "deploying %s", releaseName)
Expand Down
77 changes: 32 additions & 45 deletions pkg/skaffold/deploy/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@ import (
"os/exec"
"testing"

"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
"github.com/sirupsen/logrus"
)

var testBuilds = []build.Artifact{
Expand All @@ -48,59 +47,47 @@ var testBuildsFoo = []build.Artifact{
},
}

var testDeployConfig = &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
HelmDeploy: &v1alpha2.HelmDeploy{
Releases: []v1alpha2.HelmRelease{
{
Name: "skaffold-helm",
ChartPath: "examples/test",
Values: map[string]string{
"image.tag": "skaffold-helm",
},
Overrides: map[string]interface{}{
"foo": "bar",
},
SetValues: map[string]string{
"some.key": "somevalue",
},
},
var testDeployConfig = &v1alpha2.HelmDeploy{
Releases: []v1alpha2.HelmRelease{
{
Name: "skaffold-helm",
ChartPath: "examples/test",
Values: map[string]string{
"image.tag": "skaffold-helm",
},
Overrides: map[string]interface{}{
"foo": "bar",
},
SetValues: map[string]string{
"some.key": "somevalue",
},
},
},
}

var testDeployConfigParameterUnmatched = &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
HelmDeploy: &v1alpha2.HelmDeploy{
Releases: []v1alpha2.HelmRelease{
{
Name: "skaffold-helm",
ChartPath: "examples/test",
Values: map[string]string{
"image.tag": "skaffold-helm-unmatched",
},
},
var testDeployConfigParameterUnmatched = &v1alpha2.HelmDeploy{
Releases: []v1alpha2.HelmRelease{
{
Name: "skaffold-helm",
ChartPath: "examples/test",
Values: map[string]string{
"image.tag": "skaffold-helm-unmatched",
},
},
},
}

var testDeployFooWithPackaged = &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
HelmDeploy: &v1alpha2.HelmDeploy{
Releases: []v1alpha2.HelmRelease{
{
Name: "foo",
ChartPath: "testdata/foo",
Values: map[string]string{
"image.tag": "foo",
},
Packaged: &v1alpha2.HelmPackaged{
Version: "0.1.2",
AppVersion: "1.2.3",
},
},
var testDeployFooWithPackaged = &v1alpha2.HelmDeploy{
Releases: []v1alpha2.HelmRelease{
{
Name: "foo",
ChartPath: "testdata/foo",
Values: map[string]string{
"image.tag": "foo",
},
Packaged: &v1alpha2.HelmPackaged{
Version: "0.1.2",
AppVersion: "1.2.3",
},
},
},
Expand Down
14 changes: 7 additions & 7 deletions pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ import (

// KubectlDeployer deploys workflows using kubectl CLI.
type KubectlDeployer struct {
*v1alpha2.DeployConfig
*v1alpha2.KubectlDeploy

workingDir string
kubeContext string
}

// NewKubectlDeployer returns a new KubectlDeployer for a DeployConfig filled
// with the needed configuration for `kubectl apply`
func NewKubectlDeployer(workingDir string, cfg *v1alpha2.DeployConfig, kubeContext string) *KubectlDeployer {
func NewKubectlDeployer(workingDir string, cfg *v1alpha2.KubectlDeploy, kubeContext string) *KubectlDeployer {
return &KubectlDeployer{
DeployConfig: cfg,
workingDir: workingDir,
kubeContext: kubeContext,
KubectlDeploy: cfg,
workingDir: workingDir,
kubeContext: kubeContext,
}
}

Expand Down Expand Up @@ -141,7 +141,7 @@ func parseManifestsForDeploys(manifests manifestList) ([]Artifact, error) {

// readManifests reads the manifests to deploy/delete.
func (k *KubectlDeployer) readManifests() (manifestList, error) {
files, err := k.manifestFiles(k.KubectlDeploy.Manifests)
files, err := k.manifestFiles(k.Manifests)
if err != nil {
return nil, errors.Wrap(err, "expanding user manifest list")
}
Expand All @@ -159,7 +159,7 @@ func (k *KubectlDeployer) readManifests() (manifestList, error) {
}
}

for _, m := range k.KubectlDeploy.RemoteManifests {
for _, m := range k.RemoteManifests {
manifest, err := k.readRemoteManifest(m)
if err != nil {
return nil, errors.Wrap(err, "get remote manifests")
Expand Down
52 changes: 14 additions & 38 deletions pkg/skaffold/deploy/kubectl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,20 +59,16 @@ spec:
func TestKubectlDeploy(t *testing.T) {
var tests = []struct {
description string
cfg *v1alpha2.DeployConfig
cfg *v1alpha2.KubectlDeploy
builds []build.Artifact
command util.Command
shouldErr bool
}{
{
description: "parameter mismatch",
shouldErr: true,
cfg: &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
KubectlDeploy: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
},
cfg: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
builds: []build.Artifact{
{
Expand All @@ -84,12 +80,8 @@ func TestKubectlDeploy(t *testing.T) {
{
description: "missing manifest file",
shouldErr: true,
cfg: &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
KubectlDeploy: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
},
cfg: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
builds: []build.Artifact{
{
Expand All @@ -100,12 +92,8 @@ func TestKubectlDeploy(t *testing.T) {
},
{
description: "deploy success",
cfg: &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
KubectlDeploy: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
},
cfg: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext apply -f -", nil),
builds: []build.Artifact{
Expand All @@ -118,12 +106,8 @@ func TestKubectlDeploy(t *testing.T) {
{
description: "deploy command error",
shouldErr: true,
cfg: &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
KubectlDeploy: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
},
cfg: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext apply -f -", fmt.Errorf("")),
builds: []build.Artifact{
Expand Down Expand Up @@ -159,29 +143,21 @@ func TestKubectlDeploy(t *testing.T) {
func TestKubectlCleanup(t *testing.T) {
var tests = []struct {
description string
cfg *v1alpha2.DeployConfig
cfg *v1alpha2.KubectlDeploy
command util.Command
shouldErr bool
}{
{
description: "cleanup success",
cfg: &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
KubectlDeploy: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
},
cfg: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext delete -f -", nil),
},
{
description: "cleanup error",
cfg: &v1alpha2.DeployConfig{
DeployType: v1alpha2.DeployType{
KubectlDeploy: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
},
cfg: &v1alpha2.KubectlDeploy{
Manifests: []string{"test/deployment.yaml"},
},
command: testutil.NewFakeCmd("kubectl --context kubecontext delete -f -", errors.New("BUG")),
shouldErr: true,
Expand Down
9 changes: 5 additions & 4 deletions pkg/skaffold/deploy/kustomize.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ import (
)

type KustomizeDeployer struct {
*v1alpha2.DeployConfig
*v1alpha2.KustomizeDeploy

kubeContext string
}

func NewKustomizeDeployer(cfg *v1alpha2.DeployConfig, kubeContext string) *KustomizeDeployer {
func NewKustomizeDeployer(cfg *v1alpha2.KustomizeDeploy, kubeContext string) *KustomizeDeployer {
return &KustomizeDeployer{
DeployConfig: cfg,
kubeContext: kubeContext,
KustomizeDeploy: cfg,
kubeContext: kubeContext,
}
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ func getDeployer(cfg *v1alpha2.DeployConfig, kubeContext string, namespace strin
if err != nil {
return nil, errors.Wrap(err, "finding current directory")
}
return deploy.NewKubectlDeployer(cwd, cfg, kubeContext), nil
return deploy.NewKubectlDeployer(cwd, cfg.KubectlDeploy, kubeContext), nil

case cfg.HelmDeploy != nil:
return deploy.NewHelmDeployer(cfg, kubeContext, namespace), nil
return deploy.NewHelmDeployer(cfg.HelmDeploy, kubeContext, namespace), nil

case cfg.KustomizeDeploy != nil:
return deploy.NewKustomizeDeployer(cfg, kubeContext), nil
return deploy.NewKustomizeDeployer(cfg.KustomizeDeploy, kubeContext), nil

default:
return nil, fmt.Errorf("Unknown deployer for config %+v", cfg)
Expand Down