From 806f9fbfe937e943f6738cf6e01fc2c8a522c526 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Fri, 10 Jul 2020 11:08:57 -0700 Subject: [PATCH] Remove all labels except run-id and managed-by --- integration/helm_test.go | 2 - integration/render_test.go | 32 ++------------- pkg/skaffold/build/build.go | 2 - pkg/skaffold/build/cluster/types.go | 8 ---- pkg/skaffold/build/cluster/types_test.go | 4 -- pkg/skaffold/build/gcb/types.go | 8 ---- pkg/skaffold/build/local/types.go | 15 ------- pkg/skaffold/build/tag/custom.go | 8 ---- pkg/skaffold/build/tag/date_time.go | 8 ---- pkg/skaffold/build/tag/env_template.go | 7 ---- pkg/skaffold/build/tag/git_commit.go | 8 ---- pkg/skaffold/build/tag/sha256.go | 8 ---- pkg/skaffold/build/tag/tag.go | 3 -- pkg/skaffold/deploy/deploy.go | 6 +-- pkg/skaffold/deploy/deploy_mux.go | 16 ++------ pkg/skaffold/deploy/deploy_mux_test.go | 48 +++------------------- pkg/skaffold/deploy/helm.go | 32 +++++++-------- pkg/skaffold/deploy/helm_test.go | 15 ++++--- pkg/skaffold/deploy/kubectl.go | 29 ++++++------- pkg/skaffold/deploy/kubectl/labels.go | 2 +- pkg/skaffold/deploy/kubectl/labels_test.go | 2 +- pkg/skaffold/deploy/kubectl_test.go | 40 ++++++------------ pkg/skaffold/deploy/kustomize.go | 30 ++++++-------- pkg/skaffold/deploy/kustomize_test.go | 35 ++++------------ pkg/skaffold/deploy/labeller.go | 38 ++++------------- pkg/skaffold/deploy/labels.go | 16 -------- pkg/skaffold/deploy/status_check_test.go | 3 +- pkg/skaffold/runner/deploy.go | 2 +- pkg/skaffold/runner/new.go | 14 +++---- pkg/skaffold/runner/notification.go | 4 +- pkg/skaffold/runner/render.go | 2 +- pkg/skaffold/runner/runner.go | 2 +- pkg/skaffold/runner/runner_test.go | 4 +- pkg/skaffold/runner/timings.go | 9 +--- pkg/skaffold/runner/timings_test.go | 4 +- pkg/skaffold/sync/sync_test.go | 4 +- 36 files changed, 114 insertions(+), 356 deletions(-) diff --git a/integration/helm_test.go b/integration/helm_test.go index d95bab12060..78b646fc41e 100644 --- a/integration/helm_test.go +++ b/integration/helm_test.go @@ -33,10 +33,8 @@ func TestHelmDeploy(t *testing.T) { env := []string{fmt.Sprintf("TEST_NS=%s", ns.Name)} skaffold.Deploy("--images", "gcr.io/k8s-skaffold/skaffold-helm").InDir("testdata/helm").InNs(ns.Name).WithEnv(env).RunOrFail(t) - // check if labels are set correctly for deployment dep := client.GetDeployment("skaffold-helm-" + ns.Name) testutil.CheckDeepEqual(t, dep.Name, dep.ObjectMeta.Labels["release"]) - testutil.CheckDeepEqual(t, "helm", dep.ObjectMeta.Labels["skaffold.dev/deployer"]) skaffold.Delete().InDir("testdata/helm").InNs(ns.Name).WithEnv(env).RunOrFail(t) } diff --git a/integration/render_test.go b/integration/render_test.go index 773081848af..ca02c056a32 100644 --- a/integration/render_test.go +++ b/integration/render_test.go @@ -43,7 +43,6 @@ func TestKubectlRender(t *testing.T) { tests := []struct { description string builds []build.Artifact - labels []deploy.Labeller input string expectedOut string }{ @@ -55,7 +54,6 @@ func TestKubectlRender(t *testing.T) { Tag: "gcr.io/k8s-skaffold/skaffold:test", }, }, - labels: []deploy.Labeller{}, input: `apiVersion: v1 kind: Pod metadata: @@ -68,8 +66,6 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: my-pod-123 namespace: default spec: @@ -90,7 +86,6 @@ spec: Tag: "gcr.io/project/image2:tag2", }, }, - labels: []deploy.Labeller{}, input: `apiVersion: v1 kind: Pod metadata: @@ -105,8 +100,6 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: my-pod-123 namespace: default spec: @@ -150,8 +143,6 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: my-pod-123 namespace: default spec: @@ -162,8 +153,6 @@ spec: apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: my-pod-456 namespace: default spec: @@ -193,9 +182,9 @@ spec: Opts: config.SkaffoldOptions{ AddSkaffoldLabels: true, }, - }) + }, nil) var b bytes.Buffer - err := deployer.Render(context.Background(), &b, test.builds, test.labels, false, "") + err := deployer.Render(context.Background(), &b, test.builds, false, "") t.CheckNoError(err) t.CheckDeepEqual(test.expectedOut, b.String()) @@ -211,7 +200,6 @@ func TestHelmRender(t *testing.T) { tests := []struct { description string builds []build.Artifact - labels []deploy.Labeller helmReleases []latest.HelmRelease expectedOut string }{ @@ -223,7 +211,6 @@ func TestHelmRender(t *testing.T) { Tag: "gke-loadbalancer:test", }, }, - labels: []deploy.Labeller{}, helmReleases: []latest.HelmRelease{{ Name: "gke_loadbalancer", ChartPath: "testdata/gke_loadbalancer/loadbalancer-helm", @@ -282,7 +269,6 @@ spec: Tag: "gcr.io/k8s-skaffold/skaffold-helm:sha256-nonsenslettersandnumbers", }, }, - labels: []deploy.Labeller{}, helmReleases: []latest.HelmRelease{{ Name: "skaffold-helm", ChartPath: "testdata/helm/skaffold-helm", @@ -381,9 +367,9 @@ spec: }, }, }, - }) + }, nil) var b bytes.Buffer - err := deployer.Render(context.Background(), &b, test.builds, test.labels, true, "") + err := deployer.Render(context.Background(), &b, test.builds, true, "") t.CheckNoError(err) t.CheckDeepEqual(test.expectedOut, b.String()) @@ -530,12 +516,7 @@ kind: Pod metadata: labels: app.kubernetes.io/managed-by: SOMEDYNAMICVALUE - skaffold.dev/builder: local - skaffold.dev/cleanup: "true" - skaffold.dev/deployer: kubectl - skaffold.dev/docker-api-version: SOMEDYNAMICVALUE skaffold.dev/run-id: SOMEDYNAMICVALUE - skaffold.dev/tag-policy: git-commit name: my-pod-123 spec: containers: @@ -638,12 +619,7 @@ kind: Pod metadata: labels: app.kubernetes.io/managed-by: SOMEDYNAMICVALUE - skaffold.dev/builder: local - skaffold.dev/cleanup: "true" - skaffold.dev/deployer: kustomize - skaffold.dev/docker-api-version: SOMEDYNAMICVALUE skaffold.dev/run-id: SOMEDYNAMICVALUE - skaffold.dev/tag-policy: git-commit this-is-from: kustomization.yaml name: my-pod-123 spec: diff --git a/pkg/skaffold/build/build.go b/pkg/skaffold/build/build.go index 75d00909a10..94420e02d0f 100644 --- a/pkg/skaffold/build/build.go +++ b/pkg/skaffold/build/build.go @@ -35,8 +35,6 @@ type Artifact struct { // This could include pushing to a authorized repository or loading the nodes with the image. // If artifacts is supplied, the builder should only rebuild those artifacts. type Builder interface { - Labels() map[string]string - Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]Artifact, error) // Prune removes images built with Skaffold diff --git a/pkg/skaffold/build/cluster/types.go b/pkg/skaffold/build/cluster/types.go index 8a4a4a4f319..32d5d48c673 100644 --- a/pkg/skaffold/build/cluster/types.go +++ b/pkg/skaffold/build/cluster/types.go @@ -22,7 +22,6 @@ import ( "io" "time" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" @@ -54,13 +53,6 @@ func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) { }, nil } -// Labels are labels specific to cluster builder. -func (b *Builder) Labels() map[string]string { - return map[string]string{ - constants.Labels.Builder: "cluster", - } -} - func (b *Builder) Prune(ctx context.Context, out io.Writer) error { return nil } diff --git a/pkg/skaffold/build/cluster/types_test.go b/pkg/skaffold/build/cluster/types_test.go index 3372317f5f9..14f2f85e6b8 100644 --- a/pkg/skaffold/build/cluster/types_test.go +++ b/pkg/skaffold/build/cluster/types_test.go @@ -105,10 +105,6 @@ func TestNewBuilder(t *testing.T) { } } -func TestLabels(t *testing.T) { - testutil.CheckDeepEqual(t, map[string]string{"skaffold.dev/builder": "cluster"}, (&Builder{}).Labels()) -} - func TestPruneIsNoop(t *testing.T) { pruneError := (&Builder{}).Prune(context.TODO(), nil) testutil.CheckDeepEqual(t, nil, pruneError) diff --git a/pkg/skaffold/build/gcb/types.go b/pkg/skaffold/build/gcb/types.go index 7da63fe4a40..39e26bf0046 100644 --- a/pkg/skaffold/build/gcb/types.go +++ b/pkg/skaffold/build/gcb/types.go @@ -23,7 +23,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" ) @@ -91,13 +90,6 @@ func NewBuilder(runCtx *runcontext.RunContext) *Builder { } } -// Labels are labels specific to Google Cloud Build. -func (b *Builder) Labels() map[string]string { - return map[string]string{ - constants.Labels.Builder: "google-cloud-build", - } -} - func (b *Builder) Prune(ctx context.Context, out io.Writer) error { return nil // noop } diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index 4b32ad55c6d..4458e2bd1ba 100644 --- a/pkg/skaffold/build/local/types.go +++ b/pkg/skaffold/build/local/types.go @@ -24,7 +24,6 @@ import ( "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" @@ -93,20 +92,6 @@ func (b *Builder) PushImages() bool { return b.pushImages } -// Labels are labels specific to local builder. -func (b *Builder) Labels() map[string]string { - labels := map[string]string{ - constants.Labels.Builder: "local", - } - - v, err := b.localDocker.ServerVersion(context.Background()) - if err == nil { - labels[constants.Labels.DockerAPIVersion] = fmt.Sprintf("%v", v.APIVersion) - } - - return labels -} - // Prune uses the docker API client to remove all images built with Skaffold func (b *Builder) Prune(ctx context.Context, out io.Writer) error { return b.localDocker.Prune(ctx, out, b.builtImages, b.pruneChildren) diff --git a/pkg/skaffold/build/tag/custom.go b/pkg/skaffold/build/tag/custom.go index dcf8e515859..96e260d9ce7 100644 --- a/pkg/skaffold/build/tag/custom.go +++ b/pkg/skaffold/build/tag/custom.go @@ -19,20 +19,12 @@ package tag import ( "errors" "fmt" - - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" ) type CustomTag struct { Tag string } -func (c *CustomTag) Labels() map[string]string { - return map[string]string{ - constants.Labels.TagPolicy: "custom", - } -} - // GenerateFullyQualifiedImageName tags an image with the custom tag func (c *CustomTag) GenerateFullyQualifiedImageName(workingDir, imageName string) (string, error) { tag := c.Tag diff --git a/pkg/skaffold/build/tag/date_time.go b/pkg/skaffold/build/tag/date_time.go index 064b33ccfc1..5cd7e80d880 100644 --- a/pkg/skaffold/build/tag/date_time.go +++ b/pkg/skaffold/build/tag/date_time.go @@ -21,8 +21,6 @@ import ( "time" "4d63.com/tz" - - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" ) const tagTime = "2006-01-02_15-04-05.999_MST" @@ -44,12 +42,6 @@ func NewDateTimeTagger(format, timezone string) Tagger { } } -func (tagger *dateTimeTagger) Labels() map[string]string { - return map[string]string{ - constants.Labels.TagPolicy: "dateTimeTagger", - } -} - // GenerateFullyQualifiedImageName tags an image with the supplied image name and the current timestamp func (tagger *dateTimeTagger) GenerateFullyQualifiedImageName(workingDir, imageName string) (string, error) { format := tagTime diff --git a/pkg/skaffold/build/tag/env_template.go b/pkg/skaffold/build/tag/env_template.go index 083bbbc9619..e9a53432bd3 100644 --- a/pkg/skaffold/build/tag/env_template.go +++ b/pkg/skaffold/build/tag/env_template.go @@ -22,7 +22,6 @@ import ( "strings" "text/template" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" ) @@ -43,12 +42,6 @@ func NewEnvTemplateTagger(t string) (Tagger, error) { }, nil } -func (t *envTemplateTagger) Labels() map[string]string { - return map[string]string{ - constants.Labels.TagPolicy: "envTemplateTagger", - } -} - // GenerateFullyQualifiedImageName tags an image with the custom tag func (t *envTemplateTagger) GenerateFullyQualifiedImageName(workingDir, imageName string) (string, error) { tag, err := util.ExecuteEnvTemplate(t.Template.Option("missingkey=error"), map[string]string{ diff --git a/pkg/skaffold/build/tag/git_commit.go b/pkg/skaffold/build/tag/git_commit.go index d4ddf0407ed..5c3849b2ed5 100644 --- a/pkg/skaffold/build/tag/git_commit.go +++ b/pkg/skaffold/build/tag/git_commit.go @@ -26,7 +26,6 @@ import ( "github.com/sirupsen/logrus" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" ) @@ -58,13 +57,6 @@ func NewGitCommit(prefix, variant string) (*GitCommit, error) { }, nil } -// Labels are labels specific to the git tagger. -func (c *GitCommit) Labels() map[string]string { - return map[string]string{ - constants.Labels.TagPolicy: "git-commit", - } -} - // GenerateFullyQualifiedImageName tags an image with the supplied image name and the git commit. func (c *GitCommit) GenerateFullyQualifiedImageName(workingDir string, imageName string) (string, error) { ref, err := c.runGitFn(workingDir) diff --git a/pkg/skaffold/build/tag/sha256.go b/pkg/skaffold/build/tag/sha256.go index c0b4e4f4115..9db3d9347b5 100644 --- a/pkg/skaffold/build/tag/sha256.go +++ b/pkg/skaffold/build/tag/sha256.go @@ -17,20 +17,12 @@ limitations under the License. package tag import ( - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" ) // ChecksumTagger tags an image by the sha256 of the image tarball type ChecksumTagger struct{} -// Labels are labels specific to the sha256 tagger. -func (c *ChecksumTagger) Labels() map[string]string { - return map[string]string{ - constants.Labels.TagPolicy: "sha256", - } -} - func (c *ChecksumTagger) GenerateFullyQualifiedImageName(workingDir, imageName string) (string, error) { parsed, err := docker.ParseReference(imageName) if err != nil { diff --git a/pkg/skaffold/build/tag/tag.go b/pkg/skaffold/build/tag/tag.go index 879898a0772..4736ef8268c 100644 --- a/pkg/skaffold/build/tag/tag.go +++ b/pkg/skaffold/build/tag/tag.go @@ -21,9 +21,6 @@ type ImageTags map[string]string // Tagger is an interface for tag strategies to be implemented against type Tagger interface { - // Labels produces labels to indicate the used tagger in deployed pods. - Labels() map[string]string - // GenerateFullyQualifiedImageName resolves the fully qualified image name for an artifact. // The workingDir is the root directory of the artifact with respect to the Skaffold root, // and imageName is the base name of the image. diff --git a/pkg/skaffold/deploy/deploy.go b/pkg/skaffold/deploy/deploy.go index 7f59b7c1333..ebb461c6e0f 100644 --- a/pkg/skaffold/deploy/deploy.go +++ b/pkg/skaffold/deploy/deploy.go @@ -26,11 +26,9 @@ import ( // Deployer is the Deploy API of skaffold and responsible for deploying // the build results to a Kubernetes cluster type Deployer interface { - Labels() map[string]string - // Deploy should ensure that the build results are deployed to the Kubernetes // cluster. - Deploy(context.Context, io.Writer, []build.Artifact, []Labeller) *Result + Deploy(context.Context, io.Writer, []build.Artifact) *Result // Dependencies returns a list of files that the deployer depends on. // In dev mode, a redeploy will be triggered @@ -41,7 +39,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, []Labeller, bool, string) error + Render(context.Context, io.Writer, []build.Artifact, bool, string) error } type Result struct { diff --git a/pkg/skaffold/deploy/deploy_mux.go b/pkg/skaffold/deploy/deploy_mux.go index 83a15414bf7..dff9291f27d 100644 --- a/pkg/skaffold/deploy/deploy_mux.go +++ b/pkg/skaffold/deploy/deploy_mux.go @@ -57,18 +57,10 @@ func (s stringSet) toList() []string { return res } -func (m DeployerMux) Labels() map[string]string { - labels := make(map[string]string) - for _, deployer := range m { - copyMap(labels, deployer.Labels()) - } - return labels -} - -func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []build.Artifact, ls []Labeller) *Result { +func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []build.Artifact) *Result { seenNamespaces := newStringSet() for _, deployer := range m { - result := deployer.Deploy(ctx, w, as, ls) + result := deployer.Deploy(ctx, w, as) if result.err != nil { return result } @@ -98,11 +90,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, ls []Labeller, offline bool, filepath string) error { +func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []build.Artifact, offline bool, filepath string) error { resources, buf := []string{}, &bytes.Buffer{} for _, deployer := range m { buf.Reset() - if err := deployer.Render(ctx, buf, as, ls, offline, "" /* never write to files */); err != nil { + if err := deployer.Render(ctx, buf, as, offline, "" /* never write to files */); err != nil { return err } resources = append(resources, buf.String()) diff --git a/pkg/skaffold/deploy/deploy_mux_test.go b/pkg/skaffold/deploy/deploy_mux_test.go index d0297b0b5b6..ed58dfa1d9b 100644 --- a/pkg/skaffold/deploy/deploy_mux_test.go +++ b/pkg/skaffold/deploy/deploy_mux_test.go @@ -42,10 +42,6 @@ type MockDeployer struct { renderErr error } -func (m *MockDeployer) Labels() map[string]string { - return m.labels -} - func (m *MockDeployer) Dependencies() ([]string, error) { return m.dependencies, m.dependenciesErr } @@ -79,14 +75,14 @@ func (m *MockDeployer) WithRenderErr(err error) *MockDeployer { return m } -func (m *MockDeployer) Deploy(context.Context, io.Writer, []build.Artifact, []Labeller) *Result { +func (m *MockDeployer) Deploy(context.Context, io.Writer, []build.Artifact) *Result { return &Result{ namespaces: m.deployNamespaces, err: m.deployErr, } } -func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []build.Artifact, _ []Labeller, _ bool, _ string) error { +func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []build.Artifact, _ bool, _ string) error { w.Write([]byte(m.renderResult)) return m.renderErr } @@ -106,40 +102,6 @@ func (m *MockDeployer) WithRenderResult(renderResult string) *MockDeployer { return m } -func TestDeployerMux_Labels(t *testing.T) { - tests := []struct { - name string - labels1 map[string]string - labels2 map[string]string - expected map[string]string - }{ - { - name: "disjoint labels", - labels1: map[string]string{"key-a": "val-a"}, - labels2: map[string]string{"key-b": "val-b"}, - expected: map[string]string{"key-a": "val-a", "key-b": "val-b"}, - }, - { - name: "last wins for overlapping labels", - labels1: map[string]string{"key": "first"}, - labels2: map[string]string{"key": "second"}, - expected: map[string]string{"key": "second"}, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - deployerMux := DeployerMux([]Deployer{ - NewMockDeployer().WithLabel(test.labels1), - NewMockDeployer().WithLabel(test.labels2), - }) - - actual := deployerMux.Labels() - testutil.CheckDeepEqual(t, test.expected, actual) - }) - } -} - func TestDeployerMux_Deploy(t *testing.T) { tests := []struct { name string @@ -187,7 +149,7 @@ func TestDeployerMux_Deploy(t *testing.T) { NewMockDeployer().WithDeployNamespaces(test.namespaces2).WithDeployErr(test.err2), }) - result := deployerMux.Deploy(context.Background(), nil, nil, nil) + result := deployerMux.Deploy(context.Background(), nil, nil) testutil.CheckErrorAndDeepEqual(t, test.shouldErr, result.err, test.expectedNs, result.namespaces) }) } @@ -284,7 +246,7 @@ func TestDeployerMux_Render(t *testing.T) { }) buf := &bytes.Buffer{} - err := deployerMux.Render(context.Background(), buf, nil, nil, true, "") + err := deployerMux.Render(context.Background(), buf, nil, true, "") testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedRender, buf.String()) }) } @@ -300,7 +262,7 @@ func TestDeployerMux_Render(t *testing.T) { NewMockDeployer().WithRenderResult(test.render2).WithRenderErr(test.err2), }) - err := deployerMux.Render(context.Background(), nil, nil, nil, true, tmpDir.Path("render")) + err := deployerMux.Render(context.Background(), nil, nil, true, tmpDir.Path("render")) testutil.CheckError(t, false, err) file, _ := os.Open(tmpDir.Path("render")) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index 57b362ee3e3..eb76c084221 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -65,21 +65,24 @@ var ( type HelmDeployer struct { *latest.HelmDeploy - kubeContext string - kubeConfig string - namespace string - forceDeploy bool - addSkaffoldLabels bool + kubeContext string + kubeConfig string + namespace string // packaging temporary directory, used for predictable test output pkgTmpDir string + labels map[string]string + + forceDeploy bool + addSkaffoldLabels bool + // bV is the helm binary version bV semver.Version } // NewHelmDeployer returns a configured HelmDeployer -func NewHelmDeployer(runCtx *runcontext.RunContext) *HelmDeployer { +func NewHelmDeployer(runCtx *runcontext.RunContext, labels map[string]string) *HelmDeployer { return &HelmDeployer{ HelmDeploy: runCtx.Cfg.Deploy.HelmDeploy, kubeContext: runCtx.KubeContext, @@ -87,18 +90,12 @@ func NewHelmDeployer(runCtx *runcontext.RunContext) *HelmDeployer { namespace: runCtx.Opts.Namespace, forceDeploy: runCtx.Opts.Force, addSkaffoldLabels: runCtx.Opts.AddSkaffoldLabels, - } -} - -// Labels returns the Kubernetes labels used by this deployer -func (h *HelmDeployer) Labels() map[string]string { - return map[string]string{ - constants.Labels.Deployer: "helm", + labels: labels, } } // Deploy deploys the build results to the Kubernetes cluster -func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result { +func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) *Result { event.DeployInProgress() hv, err := h.binVer(ctx) @@ -143,8 +140,9 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build event.DeployComplete() - labels := merge(h.addSkaffoldLabels, h, labellers...) - labelDeployResults(labels, dRes) + if h.addSkaffoldLabels { + labelDeployResults(h.labels, dRes) + } // Collect namespaces in a string namespaces := make([]string, 0, len(nsMap)) @@ -249,7 +247,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, labellers []Labeller, offline bool, filepath string) error { +func (h *HelmDeployer) 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) diff --git a/pkg/skaffold/deploy/helm_test.go b/pkg/skaffold/deploy/helm_test.go index ebf6c3ddefb..a9aa4687960 100644 --- a/pkg/skaffold/deploy/helm_test.go +++ b/pkg/skaffold/deploy/helm_test.go @@ -769,9 +769,9 @@ func TestHelmDeploy(t *testing.T) { event.InitializeState(test.runContext.Cfg, "test", true, true, true) - deployer := NewHelmDeployer(test.runContext) + deployer := NewHelmDeployer(test.runContext, nil) deployer.pkgTmpDir = tmpDir - result := deployer.Deploy(context.Background(), ioutil.Discard, test.builds, nil) + result := deployer.Deploy(context.Background(), ioutil.Discard, test.builds) t.CheckError(test.shouldErr, result.GetError()) t.CheckDeepEqual(test.expectedWarnings, fakeWarner.Warnings) @@ -838,7 +838,7 @@ func TestHelmCleanup(t *testing.T) { event.InitializeState(test.runContext.Cfg, "test", true, true, true) - deployer := NewHelmDeployer(test.runContext) + deployer := NewHelmDeployer(test.runContext, nil) err := deployer.Cleanup(context.Background(), ioutil.Discard) t.CheckError(test.shouldErr, err) t.CheckDeepEqual(test.expectedWarnings, fakeWarner.Warnings) @@ -946,9 +946,8 @@ func TestHelmDependencies(t *testing.T) { SetValues: map[string]string{"some.key": "somevalue"}, SkipBuildDependencies: test.skipBuildDependencies, Remote: test.remote, - }, - }, - }, false)) + }}, + }, false), nil) deps, err := deployer.Dependencies() @@ -1120,10 +1119,10 @@ func TestHelmRender(t *testing.T) { file = t.NewTempDir().Path(test.outputFile) } - deployer := NewHelmDeployer(test.runContext) + deployer := NewHelmDeployer(test.runContext, nil) t.Override(&util.DefaultExecCommand, test.commands) - err := deployer.Render(context.Background(), ioutil.Discard, test.builds, nil, true, file) + err := deployer.Render(context.Background(), ioutil.Discard, test.builds, true, file) t.CheckError(test.shouldErr, err) if file != "" { diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index 34aa10cfed0..98835844452 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/pkg/skaffold/deploy/kubectl.go @@ -32,7 +32,6 @@ import ( "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" deploy "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" @@ -52,13 +51,14 @@ type KubectlDeployer struct { defaultRepo *string kubectl deploy.CLI insecureRegistries map[string]bool + labels map[string]string addSkaffoldLabels bool skipRender bool } // NewKubectlDeployer returns a new KubectlDeployer for a DeployConfig filled // with the needed configuration for `kubectl apply` -func NewKubectlDeployer(runCtx *runcontext.RunContext) *KubectlDeployer { +func NewKubectlDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KubectlDeployer { return &KubectlDeployer{ KubectlDeploy: runCtx.Cfg.Deploy.KubectlDeploy, workingDir: runCtx.WorkingDir, @@ -72,18 +72,13 @@ func NewKubectlDeployer(runCtx *runcontext.RunContext) *KubectlDeployer { insecureRegistries: runCtx.InsecureRegistries, addSkaffoldLabels: runCtx.Opts.AddSkaffoldLabels, skipRender: runCtx.Opts.SkipRender, - } -} - -func (k *KubectlDeployer) Labels() map[string]string { - return map[string]string{ - constants.Labels.Deployer: "kubectl", + labels: labels, } } // Deploy templates the provided manifests with a simple `find and replace` and // runs `kubectl apply` on those manifests -func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result { +func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) *Result { event.DeployInProgress() var manifests deploy.ManifestList @@ -91,7 +86,7 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu if k.skipRender { manifests, err = k.readManifests(ctx, false) } else { - manifests, err = k.renderManifests(ctx, out, builds, labellers, false) + manifests, err = k.renderManifests(ctx, out, builds, false) } if err != nil { event.DeployFailed(err) @@ -228,8 +223,8 @@ 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, labellers []Labeller, offline bool, filepath string) error { - manifests, err := k.renderManifests(ctx, out, builds, labellers, offline) +func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, offline bool, filepath string) error { + manifests, err := k.renderManifests(ctx, out, builds, offline) if err != nil { return err } @@ -237,7 +232,7 @@ func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []bu return outputRenderedManifests(manifests.String(), filepath, out) } -func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller, offline bool) (deploy.ManifestList, error) { +func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact, offline bool) (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) @@ -298,9 +293,11 @@ func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, bu } } - manifests, err = manifests.SetLabels(merge(k.addSkaffoldLabels, k, labellers...)) - if err != nil { - return nil, fmt.Errorf("setting labels in manifests: %w", err) + if k.addSkaffoldLabels { + manifests, err = manifests.SetLabels(k.labels) + if err != nil { + return nil, fmt.Errorf("setting labels in manifests: %w", err) + } } return manifests, nil diff --git a/pkg/skaffold/deploy/kubectl/labels.go b/pkg/skaffold/deploy/kubectl/labels.go index 8ef369780ec..0fc80182ce1 100644 --- a/pkg/skaffold/deploy/kubectl/labels.go +++ b/pkg/skaffold/deploy/kubectl/labels.go @@ -72,7 +72,7 @@ func (r *labelsSetter) Visit(o map[string]interface{}, k string, v interface{}) } for k, v := range r.labels { - // Don't replace existing label. + // Don't overwrite existing labels if _, present := labels[k]; !present { labels[k] = v } diff --git a/pkg/skaffold/deploy/kubectl/labels_test.go b/pkg/skaffold/deploy/kubectl/labels_test.go index b62b116652c..499b64c1c1f 100644 --- a/pkg/skaffold/deploy/kubectl/labels_test.go +++ b/pkg/skaffold/deploy/kubectl/labels_test.go @@ -141,7 +141,7 @@ metadata: testutil.CheckErrorAndDeepEqual(t, false, err, manifests.String(), resultManifest.String()) } -func TestSetNoLabelInCRDSchema(t *testing.T) { +func TestSetLabelInCRDSchema(t *testing.T) { manifests := ManifestList{[]byte(`apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go index 84c920a23b5..5b1f9116597 100644 --- a/pkg/skaffold/deploy/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl_test.go @@ -205,9 +205,9 @@ func TestKubectlDeploy(t *testing.T) { Namespace: testNamespace, Force: test.forceDeploy, }, - }) + }, nil) - err := k.Deploy(context.Background(), ioutil.Discard, test.builds, nil).GetError() + err := k.Deploy(context.Background(), ioutil.Discard, test.builds).GetError() t.CheckError(test.shouldErr, err) }) @@ -288,7 +288,7 @@ func TestKubectlCleanup(t *testing.T) { Opts: config.SkaffoldOptions{ Namespace: testNamespace, }, - }) + }, nil) err := k.Cleanup(context.Background(), ioutil.Discard) t.CheckError(test.shouldErr, err) @@ -343,7 +343,7 @@ func TestKubectlDeployerRemoteCleanup(t *testing.T) { Opts: config.SkaffoldOptions{ Namespace: testNamespace, }, - }) + }, nil) err := k.Cleanup(context.Background(), ioutil.Discard) t.CheckNoError(err) @@ -363,8 +363,6 @@ func TestKubectlRedeploy(t *testing.T) { AndRunInput("kubectl --context kubecontext --namespace testNamespace apply -f -", `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: leeroy-app spec: containers: @@ -374,8 +372,6 @@ spec: apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: leeroy-web spec: containers: @@ -385,8 +381,6 @@ spec: AndRunInput("kubectl --context kubecontext --namespace testNamespace apply -f -", `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl name: leeroy-app spec: containers: @@ -412,27 +406,27 @@ spec: Namespace: testNamespace, AddSkaffoldLabels: true, }, - }) + }, nil) // Deploy one manifest err := deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{ {ImageName: "leeroy-web", Tag: "leeroy-web:v1"}, {ImageName: "leeroy-app", Tag: "leeroy-app:v1"}, - }, nil).GetError() + }).GetError() t.CheckNoError(err) // Deploy one manifest since only one image is updated err = deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{ {ImageName: "leeroy-web", Tag: "leeroy-web:v1"}, {ImageName: "leeroy-app", Tag: "leeroy-app:v2"}, - }, nil).GetError() + }).GetError() t.CheckNoError(err) // Deploy zero manifest since no image is updated err = deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{ {ImageName: "leeroy-web", Tag: "leeroy-web:v1"}, {ImageName: "leeroy-app", Tag: "leeroy-app:v2"}, - }, nil).GetError() + }).GetError() t.CheckNoError(err) }) } @@ -497,7 +491,7 @@ func TestDependencies(t *testing.T) { }, }, }, - }) + }, nil) dependencies, err := k.Dependencies() t.CheckNoError(err) @@ -510,7 +504,6 @@ func TestKubectlRender(t *testing.T) { tests := []struct { description string builds []build.Artifact - labels []Labeller input string expected string }{ @@ -522,7 +515,6 @@ func TestKubectlRender(t *testing.T) { Tag: "gcr.io/k8s-skaffold/skaffold:test", }, }, - labels: []Labeller{}, input: `apiVersion: v1 kind: Pod metadata: @@ -535,8 +527,6 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -570,8 +560,6 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -598,8 +586,6 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -636,9 +622,9 @@ spec: AddSkaffoldLabels: true, DefaultRepo: defaultRepo, }, - }) + }, nil) var b bytes.Buffer - err := deployer.Render(context.Background(), &b, test.builds, test.labels, true, "") + err := deployer.Render(context.Background(), &b, test.builds, true, "") t.CheckNoError(err) t.CheckDeepEqual(test.expected, b.String()) }) @@ -688,9 +674,9 @@ func TestGCSManifests(t *testing.T) { Namespace: testNamespace, SkipRender: test.skipRender, }, - }) + }, nil) - err := k.Deploy(context.Background(), ioutil.Discard, nil, nil).GetError() + err := k.Deploy(context.Background(), ioutil.Discard, nil).GetError() t.CheckError(test.shouldErr, err) }) diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 6f5d03ea30e..2898848cf97 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -32,7 +32,6 @@ import ( "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" deploy "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" @@ -97,12 +96,13 @@ type KustomizeDeployer struct { kubectl deploy.CLI insecureRegistries map[string]bool + labels map[string]string BuildArgs []string globalConfig string addSkaffoldLabels bool } -func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { +func NewKustomizeDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KustomizeDeployer { return &KustomizeDeployer{ KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, kubectl: deploy.CLI{ @@ -114,21 +114,15 @@ func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { BuildArgs: runCtx.Cfg.Deploy.KustomizeDeploy.BuildArgs, globalConfig: runCtx.Opts.GlobalConfig, addSkaffoldLabels: runCtx.Opts.AddSkaffoldLabels, - } -} - -// Labels returns the labels specific to kustomize. -func (k *KustomizeDeployer) Labels() map[string]string { - return map[string]string{ - constants.Labels.Deployer: "kustomize", + labels: labels, } } // Deploy runs `kubectl apply` on the manifest generated by kustomize. -func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result { +func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) *Result { event.DeployInProgress() - manifests, err := k.renderManifests(ctx, out, builds, labellers) + manifests, err := k.renderManifests(ctx, out, builds) if err != nil { event.DeployFailed(err) return NewDeployErrorResult(err) @@ -154,7 +148,7 @@ func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds [] return NewDeploySuccessResult(namespaces) } -func (k *KustomizeDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) (deploy.ManifestList, error) { +func (k *KustomizeDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact) (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) @@ -186,9 +180,11 @@ func (k *KustomizeDeployer) renderManifests(ctx context.Context, out io.Writer, } } - manifests, err = manifests.SetLabels(merge(k.addSkaffoldLabels, k, labellers...)) - if err != nil { - return nil, fmt.Errorf("setting labels in manifests: %w", err) + if k.addSkaffoldLabels { + manifests, err = manifests.SetLabels(k.labels) + if err != nil { + return nil, fmt.Errorf("setting labels in manifests: %w", err) + } } return manifests, nil @@ -221,8 +217,8 @@ func (k *KustomizeDeployer) Dependencies() ([]string, error) { return deps.toList(), nil } -func (k *KustomizeDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller, offline bool, filepath string) error { - manifests, err := k.renderManifests(ctx, out, builds, labellers) +func (k *KustomizeDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, offline bool, filepath string) error { + manifests, err := k.renderManifests(ctx, out, builds) if err != nil { return err } diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 5c51eb31281..444b9147319 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -108,8 +108,8 @@ func TestKustomizeDeploy(t *testing.T) { Namespace: testNamespace, Force: test.forceDeploy, }, - }) - err := k.Deploy(context.Background(), ioutil.Discard, test.builds, nil).GetError() + }, nil) + err := k.Deploy(context.Background(), ioutil.Discard, test.builds).GetError() t.CheckError(test.shouldErr, err) }) @@ -184,7 +184,7 @@ func TestKustomizeCleanup(t *testing.T) { Opts: config.SkaffoldOptions{ Namespace: testNamespace, }, - }) + }, nil) err := k.Cleanup(context.Background(), ioutil.Discard) t.CheckError(test.shouldErr, err) @@ -396,7 +396,7 @@ func TestDependenciesForKustomization(t *testing.T) { }, }, KubeContext: testKubeContext, - }) + }, nil) deps, err := k.Dependencies() t.CheckErrorAndDeepEqual(test.shouldErr, err, tmpDir.Paths(test.expected...), deps) @@ -469,13 +469,6 @@ func TestKustomizeBuildCommandArgs(t *testing.T) { } } -type testLabels struct { - labels map[string]string -} - -func (t *testLabels) Labels() map[string]string { - return t.labels -} func TestKustomizeRender(t *testing.T) { type kustomizationCall struct { folder string @@ -484,7 +477,7 @@ func TestKustomizeRender(t *testing.T) { tests := []struct { description string builds []build.Artifact - labels []Labeller + labels map[string]string kustomizations []kustomizationCall expected string shouldErr bool @@ -520,8 +513,6 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kustomize namespace: default spec: containers: @@ -543,12 +534,7 @@ spec: Tag: "gcr.io/project/image2:tag2", }, }, - labels: []Labeller{ - &testLabels{ - labels: map[string]string{ - "user/label": "test", - }}, - }, + labels: map[string]string{"user/label": "test"}, kustomizations: []kustomizationCall{ { folder: ".", @@ -569,7 +555,6 @@ spec: kind: Pod metadata: labels: - skaffold.dev/deployer: kustomize user/label: test namespace: default spec: @@ -621,8 +606,6 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kustomize namespace: default spec: containers: @@ -632,8 +615,6 @@ spec: apiVersion: v1 kind: Pod metadata: - labels: - skaffold.dev/deployer: kustomize namespace: default spec: containers: @@ -671,9 +652,9 @@ spec: Namespace: testNamespace, AddSkaffoldLabels: true, }, - }) + }, test.labels) var b bytes.Buffer - err := k.Render(context.Background(), &b, test.builds, test.labels, true, "") + err := k.Render(context.Background(), &b, test.builds, true, "") t.CheckError(test.shouldErr, err) t.CheckDeepEqual(test.expected, b.String()) }) diff --git a/pkg/skaffold/deploy/labeller.go b/pkg/skaffold/deploy/labeller.go index 9c71bf2dfbf..688601f0ba8 100644 --- a/pkg/skaffold/deploy/labeller.go +++ b/pkg/skaffold/deploy/labeller.go @@ -21,59 +21,35 @@ import ( "strings" "github.com/google/uuid" - - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/version" ) const ( K8sManagedByLabelKey = "app.kubernetes.io/managed-by" RunIDLabel = "skaffold.dev/run-id" - unknownVersion = "unknown" - empty = "" ) var runID = uuid.New().String() // DefaultLabeller adds K8s style managed-by label and a run-specific UUID label type DefaultLabeller struct { - opts config.SkaffoldOptions - version string - runID string + customLabels []string + runID string } -func NewLabeller(opts config.SkaffoldOptions) *DefaultLabeller { - verStr := version.Get().Version - if verStr == empty { - verStr = unknownVersion - } +func NewLabeller(customLabels []string) *DefaultLabeller { return &DefaultLabeller{ - opts: opts, - version: verStr, - runID: runID, + customLabels: customLabels, + runID: runID, } } func (d *DefaultLabeller) Labels() map[string]string { labels := map[string]string{ - K8sManagedByLabelKey: fmt.Sprintf("skaffold-%s", d.version), + K8sManagedByLabelKey: "skaffold", RunIDLabel: d.runID, } - if d.opts.Cleanup { - labels["skaffold.dev/cleanup"] = "true" - } - if d.opts.Tail { - labels["skaffold.dev/tail"] = "true" - } - if d.opts.Namespace != "" { - labels["skaffold.dev/namespace"] = d.opts.Namespace - } - for i, profile := range d.opts.Profiles { - key := fmt.Sprintf("skaffold.dev/profile.%d", i) - labels[key] = profile - } - for _, cl := range d.opts.CustomLabels { + for _, cl := range d.customLabels { l := strings.SplitN(cl, "=", 2) if len(l) == 1 { labels[l[0]] = "" diff --git a/pkg/skaffold/deploy/labels.go b/pkg/skaffold/deploy/labels.go index d2cd3b9a945..eb7f22f5b6c 100644 --- a/pkg/skaffold/deploy/labels.go +++ b/pkg/skaffold/deploy/labels.go @@ -43,25 +43,9 @@ type Artifact struct { // Labeller can give key/value labels to set on deployed resources. type Labeller interface { - // Labels keys must be prefixed with "skaffold.dev/" Labels() map[string]string } -// merge merges the labels from multiple sources. -func merge(addSkaffoldLabels bool, deployer Labeller, sources ...Labeller) map[string]string { - if !addSkaffoldLabels { - return map[string]string{} - } - - merged := deployer.Labels() - - for _, src := range sources { - copyMap(merged, src.Labels()) - } - - return merged -} - // retry 3 times to give the object time to propagate to the API server const ( tries = 3 diff --git a/pkg/skaffold/deploy/status_check_test.go b/pkg/skaffold/deploy/status_check_test.go index e97aa1a7e6f..7778a3bcd72 100644 --- a/pkg/skaffold/deploy/status_check_test.go +++ b/pkg/skaffold/deploy/status_check_test.go @@ -31,7 +31,6 @@ import ( utilpointer "k8s.io/utils/pointer" "github.com/GoogleContainerTools/skaffold/pkg/diag" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/resource" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" @@ -40,7 +39,7 @@ import ( ) func TestGetDeployments(t *testing.T) { - labeller := NewLabeller(config.SkaffoldOptions{}) + labeller := NewLabeller(nil) tests := []struct { description string deps []*appsv1.Deployment diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go index 20d8ec2d6b0..fb5a88a36fe 100644 --- a/pkg/skaffold/runner/deploy.go +++ b/pkg/skaffold/runner/deploy.go @@ -64,7 +64,7 @@ See https://skaffold.dev/docs/pipeline-stages/taggers/#how-tagging-works`) } } - deployResult := r.deployer.Deploy(ctx, out, artifacts, r.labellers) + deployResult := r.deployer.Deploy(ctx, out, artifacts) r.hasDeployed = true if err := deployResult.GetError(); err != nil { return err diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index d7a1c6721a4..1fffece8357 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -78,10 +78,9 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { return nil, fmt.Errorf("initializing cache: %w", err) } - defaultLabeller := deploy.NewLabeller(runCtx.Opts) // runCtx.Opts is last to let users override/remove any label - // deployer labels are added during deployment - labellers := []deploy.Labeller{builder, tagger, defaultLabeller} + defaultLabeller := deploy.NewLabeller(runCtx.Opts.CustomLabels) + labeller := defaultLabeller builder, tester, deployer = WithTimings(builder, tester, deployer, runCtx.Opts.CacheArtifacts) if runCtx.Opts.Notification { @@ -112,7 +111,7 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { intentChan: intentChan, }, kubectlCLI: kubectlCLI, - labellers: labellers, + labeller: labeller, defaultLabeller: defaultLabeller, podSelector: kubernetes.NewImageList(), cache: artifactCache, @@ -192,18 +191,19 @@ func getSyncer(runCtx *runcontext.RunContext) sync.Syncer { } func getDeployer(runCtx *runcontext.RunContext) deploy.Deployer { + labels := deploy.NewLabeller(runCtx.Opts.CustomLabels).Labels() var deployers deploy.DeployerMux if runCtx.Cfg.Deploy.HelmDeploy != nil { - deployers = append(deployers, deploy.NewHelmDeployer(runCtx)) + deployers = append(deployers, deploy.NewHelmDeployer(runCtx, labels)) } if runCtx.Cfg.Deploy.KubectlDeploy != nil { - deployers = append(deployers, deploy.NewKubectlDeployer(runCtx)) + deployers = append(deployers, deploy.NewKubectlDeployer(runCtx, labels)) } if runCtx.Cfg.Deploy.KustomizeDeploy != nil { - deployers = append(deployers, deploy.NewKustomizeDeployer(runCtx)) + deployers = append(deployers, deploy.NewKustomizeDeployer(runCtx, labels)) } // avoid muxing overhead when only a single deployer is configured diff --git a/pkg/skaffold/runner/notification.go b/pkg/skaffold/runner/notification.go index aac4e3584da..051e911a70e 100644 --- a/pkg/skaffold/runner/notification.go +++ b/pkg/skaffold/runner/notification.go @@ -41,8 +41,8 @@ type withNotification struct { deploy.Deployer } -func (w withNotification) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []deploy.Labeller) *deploy.Result { - dr := w.Deployer.Deploy(ctx, out, builds, labellers) +func (w withNotification) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) *deploy.Result { + dr := w.Deployer.Deploy(ctx, out, builds) if err := dr.GetError(); err != nil { fmt.Fprint(out, terminalBell) } diff --git a/pkg/skaffold/runner/render.go b/pkg/skaffold/runner/render.go index b58b4713c01..294bcb6aeea 100644 --- a/pkg/skaffold/runner/render.go +++ b/pkg/skaffold/runner/render.go @@ -40,5 +40,5 @@ func (r *SkaffoldRunner) Render(ctx context.Context, out io.Writer, builds []bui if r.runCtx.Opts.DigestSource == noneDigestSource { color.Default.Fprintln(out, "--digest-source set to 'none', tags listed in Kubernetes manifests will be used for render") } - return r.deployer.Render(ctx, out, builds, r.labellers, offline, filepath) + return r.deployer.Render(ctx, out, builds, offline, filepath) } diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 6e677241c2a..490c1cb8613 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -66,7 +66,7 @@ type SkaffoldRunner struct { cache cache.Cache changeSet changeSet runCtx *runcontext.RunContext - labellers []deploy.Labeller + labeller deploy.Labeller defaultLabeller *deploy.DefaultLabeller builds []build.Artifact diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index a77ec54d3fd..c5e2065860b 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -150,7 +150,7 @@ func (t *TestBench) Test(_ context.Context, _ io.Writer, artifacts []build.Artif return nil } -func (t *TestBench) Deploy(_ context.Context, _ io.Writer, artifacts []build.Artifact, _ []deploy.Labeller) *deploy.Result { +func (t *TestBench) Deploy(_ context.Context, _ io.Writer, artifacts []build.Artifact) *deploy.Result { if len(t.deployErrors) > 0 { err := t.deployErrors[0] t.deployErrors = t.deployErrors[1:] @@ -163,7 +163,7 @@ func (t *TestBench) Deploy(_ context.Context, _ io.Writer, artifacts []build.Art return deploy.NewDeploySuccessResult(t.namespaces) } -func (t *TestBench) Render(context.Context, io.Writer, []build.Artifact, []deploy.Labeller, bool, string) error { +func (t *TestBench) Render(context.Context, io.Writer, []build.Artifact, bool, string) error { return nil } diff --git a/pkg/skaffold/runner/timings.go b/pkg/skaffold/runner/timings.go index 40574806f84..1883167c288 100644 --- a/pkg/skaffold/runner/timings.go +++ b/pkg/skaffold/runner/timings.go @@ -22,7 +22,6 @@ import ( "time" "github.com/sirupsen/logrus" - "k8s.io/apimachinery/pkg/labels" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" @@ -51,10 +50,6 @@ type withTimings struct { cacheArtifacts bool } -func (w withTimings) Labels() map[string]string { - return labels.Merge(w.Builder.Labels(), w.Deployer.Labels()) -} - func (w withTimings) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) { if len(artifacts) == 0 && w.cacheArtifacts { return nil, nil @@ -82,11 +77,11 @@ func (w withTimings) Test(ctx context.Context, out io.Writer, builds []build.Art return nil } -func (w withTimings) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []deploy.Labeller) *deploy.Result { +func (w withTimings) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact) *deploy.Result { start := time.Now() color.Default.Fprintln(out, "Starting deploy...") - dr := w.Deployer.Deploy(ctx, out, builds, labellers) + dr := w.Deployer.Deploy(ctx, out, builds) if err := dr.GetError(); err != nil { return dr } diff --git a/pkg/skaffold/runner/timings_test.go b/pkg/skaffold/runner/timings_test.go index d5578a424d2..490f07513e8 100644 --- a/pkg/skaffold/runner/timings_test.go +++ b/pkg/skaffold/runner/timings_test.go @@ -70,7 +70,7 @@ type mockDeployer struct { err bool } -func (m *mockDeployer) Deploy(context.Context, io.Writer, []build.Artifact, []deploy.Labeller) *deploy.Result { +func (m *mockDeployer) Deploy(context.Context, io.Writer, []build.Artifact) *deploy.Result { if m.err { return deploy.NewDeployErrorResult(errors.New("Unable to deploy")) } @@ -219,7 +219,7 @@ func TestTimingsDeploy(t *testing.T) { _, _, deployer := WithTimings(nil, nil, d, false) var out bytes.Buffer - res := deployer.Deploy(context.Background(), &out, nil, nil) + res := deployer.Deploy(context.Background(), &out, nil) t.CheckError(test.shouldErr, res.GetError()) t.CheckMatches(test.shouldOutput, out.String()) diff --git a/pkg/skaffold/sync/sync_test.go b/pkg/skaffold/sync/sync_test.go index 449804a334a..2e41b5a650f 100644 --- a/pkg/skaffold/sync/sync_test.go +++ b/pkg/skaffold/sync/sync_test.go @@ -862,7 +862,7 @@ var pod = &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "podname", Labels: map[string]string{ - "app.kubernetes.io/managed-by": "skaffold-dirty", + "app.kubernetes.io/managed-by": "skaffold", }, }, Status: v1.PodStatus{ @@ -882,7 +882,7 @@ var nonRunningPod = &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "podname", Labels: map[string]string{ - "app.kubernetes.io/managed-by": "skaffold-dirty", + "app.kubernetes.io/managed-by": "skaffold", }, }, Status: v1.PodStatus{