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

Remove labels from builders and deployers #4485

Closed
wants to merge 1 commit into from
Closed
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
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)
}
32 changes: 4 additions & 28 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func TestKubectlRender(t *testing.T) {
tests := []struct {
description string
builds []build.Artifact
labels []deploy.Labeller
input string
expectedOut string
}{
Expand All @@ -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:
Expand All @@ -68,8 +66,6 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
spec:
Expand All @@ -90,7 +86,6 @@ spec:
Tag: "gcr.io/project/image2:tag2",
},
},
labels: []deploy.Labeller{},
input: `apiVersion: v1
kind: Pod
metadata:
Expand All @@ -105,8 +100,6 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
spec:
Expand Down Expand Up @@ -150,8 +143,6 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
spec:
Expand All @@ -162,8 +153,6 @@ spec:
apiVersion: v1
kind: Pod
metadata:
labels:
skaffold.dev/deployer: kubectl
name: my-pod-456
namespace: default
spec:
Expand Down Expand Up @@ -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())
Expand All @@ -211,7 +200,6 @@ func TestHelmRender(t *testing.T) {
tests := []struct {
description string
builds []build.Artifact
labels []deploy.Labeller
helmReleases []latest.HelmRelease
expectedOut string
}{
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
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
8 changes: 0 additions & 8 deletions pkg/skaffold/build/tag/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 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,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
Expand Down
7 changes: 0 additions & 7 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,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{
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 (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)
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 (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 {
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 @@ -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.
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