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.

Sign up for GitHub

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

Add labels to render #3558

Merged
merged 5 commits into from
Jan 30, 2020
Merged

Add labels to render #3558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f2f3d89
Add labeller to render
arminbuerkle Jan 22, 2020
e6ec845
Add labeller to integration test
arminbuerkle Jan 22, 2020
82ca5ed
Add kustomize render labeller unit test
arminbuerkle Jan 22, 2020
9af954b
Add kubectl render labeller unit test
arminbuerkle Jan 22, 2020
8add2ed
rebase from master and add one more test
tejal29 Jan 27, 2020
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
2 changes: 1 addition & 1 deletion cmd/skaffold/app/cmd/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ var FlagRegistry = []Flag{
Value: &opts.CustomLabels,
DefValue: []string{},
FlagAddMethod: "StringSliceVar",
DefinedOn: []string{"dev", "run", "debug", "deploy"},
DefinedOn: []string{"dev", "run", "debug", "deploy", "render"},
},
{
Name: "toot",
Expand Down
2 changes: 2 additions & 0 deletions docs/content/en/docs/references/cli/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,7 @@ The following options can be passed to any command:
Options:
-d, --default-repo='': Default repository value (overrides global config)
-f, --filename='skaffold.yaml': Filename or URL to the pipeline file
-l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels
--loud=false: Show the build logs and output
-n, --namespace='': Run deployments in the specified namespace
--output='': file to write rendered manifests to
Expand All @@ -670,6 +671,7 @@ Env vars:

* `SKAFFOLD_DEFAULT_REPO` (same as `--default-repo`)
* `SKAFFOLD_FILENAME` (same as `--filename`)
* `SKAFFOLD_LABEL` (same as `--label`)
* `SKAFFOLD_LOUD` (same as `--loud`)
* `SKAFFOLD_NAMESPACE` (same as `--namespace`)
* `SKAFFOLD_OUTPUT` (same as `--output`)
Expand Down
13 changes: 12 additions & 1 deletion integration/render_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestKubectlRender(t *testing.T) {
tests := []struct {
description string
builds []build.Artifact
labels []deploy.Labeller
input string
expectedOut string
}{
Expand All @@ -47,6 +48,7 @@ func TestKubectlRender(t *testing.T) {
Tag: "gcr.io/k8s-skaffold/skaffold:test",
},
},
labels: []deploy.Labeller{},
input: `apiVersion: v1
kind: Pod
spec:
Expand All @@ -57,6 +59,8 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
namespace: default
spec:
containers:
Expand All @@ -76,6 +80,7 @@ spec:
Tag: "gcr.io/project/image2:tag2",
},
},
labels: []deploy.Labeller{},
input: `apiVersion: v1
kind: Pod
spec:
Expand All @@ -88,6 +93,8 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
namespace: default
spec:
containers:
Expand Down Expand Up @@ -126,6 +133,8 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
namespace: default
spec:
containers:
Expand All @@ -135,6 +144,8 @@ spec:
apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
namespace: default
spec:
containers:
Expand Down Expand Up @@ -162,7 +173,7 @@ spec:
},
})
var b bytes.Buffer
err := deployer.Render(context.Background(), &b, test.builds, "")
err := deployer.Render(context.Background(), &b, test.builds, test.labels, "")

t.CheckNoError(err)
t.CheckDeepEqual(test.expectedOut, b.String())
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type Deployer interface {

// Render generates the Kubernetes manifests replacing the build results and
// writes them to the given file path
Render(context.Context, io.Writer, []build.Artifact, string) error
Render(context.Context, io.Writer, []build.Artifact, []Labeller, string) error
}

type Result struct {
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/deploy/deploy_mux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ func (m DeployerMux) Cleanup(ctx context.Context, w io.Writer) error {
return nil
}

func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []build.Artifact, filepath string) error {
func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []build.Artifact, ls []Labeller, filepath string) error {
resources, buf := []string{}, &bytes.Buffer{}
for _, deployer := range m {
buf.Reset()
if err := deployer.Render(ctx, buf, as, "" /* never write to files */); err != nil {
if err := deployer.Render(ctx, buf, as, ls, "" /* never write to files */); err != nil {
return err
}
resources = append(resources, buf.String())
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/deploy/deploy_mux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (m *MockDeployer) Deploy(context.Context, io.Writer, []build.Artifact, []La
err: m.deployErr,
}
}
func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []build.Artifact, _ string) error {
func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []build.Artifact, _ []Labeller, _ string) error {
w.Write([]byte(m.renderResult))
return m.renderErr
}
Expand Down Expand Up @@ -251,7 +251,7 @@ func TestDeployerMux_Render(t *testing.T) {
})

buf := &bytes.Buffer{}
err := deployerMux.Render(context.Background(), buf, nil, "")
err := deployerMux.Render(context.Background(), buf, nil, nil, "")
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedRender, buf.String())
})
}
Expand All @@ -268,7 +268,7 @@ func TestDeployerMux_Render(t *testing.T) {
NewMockDeployer().WithRenderResult(test.render2).WithRenderErr(test.err2),
})

err := deployerMux.Render(context.Background(), nil, nil, tempDir.Path("render"))
err := deployerMux.Render(context.Background(), nil, nil, nil, tempDir.Path("render"))
testutil.CheckError(t, false, err)

file, _ := os.Open(tempDir.Path("render"))
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/helm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ func generateGetFilesArgs(m map[string]string, valuesSet map[string]bool) []stri
return args
}

func (h *HelmDeployer) Render(context.Context, io.Writer, []build.Artifact, string) error {
func (h *HelmDeployer) Render(context.Context, io.Writer, []build.Artifact, []Labeller, string) error {
return errors.New("not yet implemented")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/helm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ func TestHelmRender(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
deployer := NewHelmDeployer(&runcontext.RunContext{})
actual := deployer.Render(context.Background(), ioutil.Discard, []build.Artifact{}, "tmp/dir")
actual := deployer.Render(context.Background(), ioutil.Discard, []build.Artifact{}, nil, "tmp/dir")
t.CheckError(test.shouldErr, actual)
})
}
Expand Down
19 changes: 9 additions & 10 deletions pkg/skaffold/deploy/kubectl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (k *KubectlDeployer) Labels() map[string]string {
// runs `kubectl apply` on those manifests
func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result {
event.DeployInProgress()
manifests, err := k.renderManifests(ctx, out, builds)
manifests, err := k.renderManifests(ctx, out, builds, labellers)

if err != nil {
event.DeployFailed(err)
Expand All @@ -84,12 +84,6 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu
return NewDeploySuccessResult(nil)
}

manifests, err = manifests.SetLabels(merge(k, labellers...))
if err != nil {
event.DeployFailed(err)
return NewDeployErrorResult(errors.Wrap(err, "setting labels in manifests"))
}

namespaces, err := manifests.CollectNamespaces()
if err != nil {
event.DeployInfoEvent(errors.Wrap(err, "could not fetch deployed resource namespace. "+
Expand Down Expand Up @@ -216,16 +210,16 @@ func (k *KubectlDeployer) readRemoteManifest(ctx context.Context, name string) (
return manifest.Bytes(), nil
}

func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, filepath string) error {
manifests, err := k.renderManifests(ctx, out, builds)
func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller, filepath string) error {
manifests, err := k.renderManifests(ctx, out, builds, labellers)
if err != nil {
return err
}

return dumpToFileOrWriter(manifests.String(), filepath, out)
}

func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact) (deploy.ManifestList, error) {
func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) (deploy.ManifestList, error) {
if err := k.kubectl.CheckVersion(ctx); err != nil {
color.Default.Fprintln(out, "kubectl client version:", k.kubectl.Version(ctx))
color.Default.Fprintln(out, err)
Expand Down Expand Up @@ -268,5 +262,10 @@ func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, bu
}
}

manifests, err = manifests.SetLabels(merge(k, labellers...))
if err != nil {
return nil, errors.Wrap(err, "setting labels in manifests")
}

return manifests, nil
}
57 changes: 46 additions & 11 deletions pkg/skaffold/deploy/kubectl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,9 @@ func TestKubectlRender(t *testing.T) {
tests := []struct {
description string
builds []build.Artifact
labels []Labeller
input string
expected string
}{
{
description: "normal render",
Expand All @@ -491,12 +493,26 @@ func TestKubectlRender(t *testing.T) {
Tag: "gcr.io/k8s-skaffold/skaffold:test",
},
},
labels: []Labeller{},
input: `apiVersion: v1
kind: Pod
metadata:
Copy link
Contributor

Choose a reason for hiding this comment

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

was there a reason for adding this in input?

  metadata
     namespace: default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! If metadata doesn't exist, the default labeller doesn't execute, therefore not adding any labels.

Without it skaffold.dev/deployer: kubectl won't be added.

namespace: default
spec:
containers:
- image: gcr.io/k8s-skaffold/skaffold
name: skaffold
`,
expected: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
namespace: default
spec:
containers:
- image: gcr.io/k8s-skaffold/skaffold:test
name: skaffold
`,
},
{
Expand All @@ -512,38 +528,57 @@ spec:
},
},
input: `apiVersion: v1
kind: Pod
spec:
containers:
- image: gcr.io/project/image1
name: image1
- image: gcr.io/project/image2
name: image2
`,
kind: Pod
metadata:
namespace: default
spec:
containers:
- image: gcr.io/project/image1
name: image1
- image: gcr.io/project/image2
name: image2
`,
expected: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
namespace: default
spec:
containers:
- image: gcr.io/project/image1:tag1
name: image1
- image: gcr.io/project/image2:tag2
name: image2
`,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
tmpDir := t.NewTempDir().
Write("deployment.yaml", test.input)

t.Override(&util.DefaultExecCommand, testutil.
CmdRunOut("kubectl version --client -ojson", kubectlVersion).
AndRunOut("kubectl --context kubecontext create --dry-run -oyaml -f deployment.yaml", test.input))
AndRunOut("kubectl --context kubecontext create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), test.input))

deployer := NewKubectlDeployer(&runcontext.RunContext{
WorkingDir: ".",
Cfg: latest.Pipeline{
Deploy: latest.DeployConfig{
DeployType: latest.DeployType{
KubectlDeploy: &latest.KubectlDeploy{
Manifests: []string{"deployment.yaml"},
Manifests: []string{tmpDir.Path("deployment.yaml")},
},
},
},
},
KubeContext: testKubeContext,
})
var b bytes.Buffer
err := deployer.Render(context.Background(), &b, test.builds, "")
err := deployer.Render(context.Background(), &b, test.builds, test.labels, "")
t.CheckNoError(err)
t.CheckDeepEqual(test.expected, b.String())
})
}
}
4 changes: 2 additions & 2 deletions pkg/skaffold/deploy/kustomize.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,8 @@ func (k *KustomizeDeployer) Dependencies() ([]string, error) {
return deps.toList(), nil
}

func (k *KustomizeDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, filepath string) error {
manifests, err := k.renderManifests(ctx, out, builds, nil)
func (k *KustomizeDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller, filepath string) error {
manifests, err := k.renderManifests(ctx, out, builds, labellers)
if err != nil {
return err
}
Expand Down
Loading