Skip to content

Commit

Permalink
Remove labels from builders and deployers (#4499)
Browse files Browse the repository at this point in the history
  • Loading branch information
nkubala authored Jul 23, 2020
1 parent 83b491a commit 418d2cb
Show file tree
Hide file tree
Showing 36 changed files with 116 additions and 361 deletions.
2 changes: 0 additions & 2 deletions integration/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
36 changes: 6 additions & 30 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ spec:
},
},
},
})
}, nil)
var b bytes.Buffer
err := deployer.Render(context.Background(), &b, test.builds, test.labels, false, test.renderPath)
err := deployer.Render(context.Background(), &b, test.builds, false, test.renderPath)

t.CheckNoError(err)
dat, err := ioutil.ReadFile(test.renderPath)
Expand All @@ -109,7 +109,6 @@ func TestKubectlRender(t *testing.T) {
tests := []struct {
description string
builds []build.Artifact
labels []deploy.Labeller
input string
expectedOut string
}{
Expand All @@ -121,7 +120,6 @@ func TestKubectlRender(t *testing.T) {
Tag: "gcr.io/k8s-skaffold/skaffold:test",
},
},
labels: []deploy.Labeller{},
input: `apiVersion: v1
kind: Pod
metadata:
Expand All @@ -134,8 +132,6 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
spec:
Expand All @@ -156,7 +152,6 @@ spec:
Tag: "gcr.io/project/image2:tag2",
},
},
labels: []deploy.Labeller{},
input: `apiVersion: v1
kind: Pod
metadata:
Expand All @@ -171,8 +166,6 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
spec:
Expand Down Expand Up @@ -216,8 +209,6 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
spec:
Expand All @@ -228,8 +219,6 @@ spec:
apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-456
namespace: default
spec:
Expand Down Expand Up @@ -259,9 +248,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())
Expand All @@ -277,7 +266,6 @@ func TestHelmRender(t *testing.T) {
tests := []struct {
description string
builds []build.Artifact
labels []deploy.Labeller
helmReleases []latest.HelmRelease
expectedOut string
}{
Expand All @@ -289,7 +277,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",
Expand Down Expand Up @@ -348,7 +335,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",
Expand Down Expand Up @@ -447,9 +433,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())
Expand Down Expand Up @@ -596,12 +582,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:
Expand Down Expand Up @@ -704,12 +685,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:
Expand Down
2 changes: 0 additions & 2 deletions pkg/skaffold/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions pkg/skaffold/build/cluster/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
4 changes: 0 additions & 4 deletions pkg/skaffold/build/cluster/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 0 additions & 8 deletions pkg/skaffold/build/gcb/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
15 changes: 0 additions & 15 deletions pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 0 additions & 9 deletions pkg/skaffold/build/tag/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,12 @@ package tag

import (
"errors"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
)

type CustomTag struct {
Tag string
}

// Labels are labels specific to the custom tagger.
func (t *CustomTag) Labels() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "custom",
}
}

// GenerateTag generates a tag using the custom tag.
func (t *CustomTag) GenerateTag(_, _ string) (string, error) {
tag := t.Tag
Expand Down
9 changes: 0 additions & 9 deletions pkg/skaffold/build/tag/date_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -44,13 +42,6 @@ func NewDateTimeTagger(format, timezone string) Tagger {
}
}

// Labels are labels specific to the dateTime tagger.
func (t *dateTimeTagger) Labels() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "dateTimeTagger",
}
}

// GenerateTag generates a tag using the current timestamp.
func (t *dateTimeTagger) GenerateTag(_, _ string) (string, error) {
format := tagTime
Expand Down
8 changes: 0 additions & 8 deletions pkg/skaffold/build/tag/env_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"strings"
"text/template"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
)

Expand All @@ -43,13 +42,6 @@ func NewEnvTemplateTagger(t string) (Tagger, error) {
}, nil
}

// Labels are labels specific to the envTemplate tagger.
func (t *envTemplateTagger) Labels() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "envTemplateTagger",
}
}

// GenerateTag generates a tag from a template referencing environment variables.
func (t *envTemplateTagger) GenerateTag(_, imageName string) (string, error) {
tag, err := util.ExecuteEnvTemplate(t.Template.Option("missingkey=error"), map[string]string{
Expand Down
8 changes: 0 additions & 8 deletions pkg/skaffold/build/tag/git_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
)

Expand Down Expand Up @@ -58,13 +57,6 @@ func NewGitCommit(prefix, variant string) (*GitCommit, error) {
}, nil
}

// Labels are labels specific to the git tagger.
func (t *GitCommit) Labels() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "git-commit",
}
}

// GenerateTag generates a tag from the git commit.
func (t *GitCommit) GenerateTag(workingDir, _ string) (string, error) {
ref, err := t.runGitFn(workingDir)
Expand Down
8 changes: 0 additions & 8 deletions pkg/skaffold/build/tag/sha256.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (t *ChecksumTagger) Labels() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "sha256",
}
}

// GenerateTag returns either the current tag or `latest`.
func (t *ChecksumTagger) GenerateTag(_, imageName string) (string, error) {
parsed, err := docker.ParseReference(imageName)
Expand Down
3 changes: 0 additions & 3 deletions pkg/skaffold/build/tag/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,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

// GenerateTag generates a tag for an artifact.
GenerateTag(workingDir, imageName string) (string, error)
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/skaffold/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Loading

0 comments on commit 418d2cb

Please sign in to comment.