Skip to content

Commit

Permalink
Use annotations instead of labels on created k8s resources
Browse files Browse the repository at this point in the history
  • Loading branch information
nkubala committed Jul 13, 2020
1 parent 6af3198 commit 5ecd3b9
Show file tree
Hide file tree
Showing 24 changed files with 221 additions and 74 deletions.
8 changes: 4 additions & 4 deletions integration/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
annotations:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
Expand Down Expand Up @@ -105,7 +105,7 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
annotations:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
Expand Down Expand Up @@ -150,7 +150,7 @@ spec:
expectedOut: `apiVersion: v1
kind: Pod
metadata:
labels:
annotations:
skaffold.dev/deployer: kubectl
name: my-pod-123
namespace: default
Expand All @@ -162,7 +162,7 @@ spec:
apiVersion: v1
kind: Pod
metadata:
labels:
annotations:
skaffold.dev/deployer: kubectl
name: my-pod-456
namespace: default
Expand Down
2 changes: 2 additions & 0 deletions pkg/skaffold/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type Artifact struct {
type Builder interface {
Labels() map[string]string

Annotations() 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
4 changes: 4 additions & 0 deletions pkg/skaffold/build/cluster/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) {

// Labels are labels specific to cluster builder.
func (b *Builder) Labels() map[string]string {
return map[string]string{}
}

func (b *Builder) Annotations() map[string]string {
return map[string]string{
constants.Labels.Builder: "cluster",
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/build/cluster/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ func TestNewBuilder(t *testing.T) {
}

func TestLabels(t *testing.T) {
testutil.CheckDeepEqual(t, map[string]string{"skaffold.dev/builder": "cluster"}, (&Builder{}).Labels())
testutil.CheckDeepEqual(t, map[string]string{}, (&Builder{}).Labels())
testutil.CheckDeepEqual(t, map[string]string{"skaffold.dev/builder": "cluster"}, (&Builder{}).Annotations())
}

func TestPruneIsNoop(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions pkg/skaffold/build/gcb/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ func NewBuilder(runCtx *runcontext.RunContext) *Builder {

// Labels are labels specific to Google Cloud Build.
func (b *Builder) Labels() map[string]string {
return nil
}

func (b *Builder) Annotations() map[string]string {
return map[string]string{
constants.Labels.Builder: "google-cloud-build",
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ func (b *Builder) PushImages() bool {

// Labels are labels specific to local builder.
func (b *Builder) Labels() map[string]string {
return map[string]string{}
}

func (b *Builder) Annotations() map[string]string {
labels := map[string]string{
constants.Labels.Builder: "local",
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/skaffold/build/tag/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ type CustomTag struct {
}

func (c *CustomTag) Labels() map[string]string {
return map[string]string{}
}

func (c *CustomTag) Annotations() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "custom",
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/build/tag/date_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ func NewDateTimeTagger(format, timezone string) Tagger {
}
}

func (tagger *dateTimeTagger) Labels() map[string]string {
func (t *dateTimeTagger) Labels() map[string]string {
return map[string]string{}
}

func (t *dateTimeTagger) Annotations() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "dateTimeTagger",
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/skaffold/build/tag/env_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ func NewEnvTemplateTagger(t string) (Tagger, error) {
}

func (t *envTemplateTagger) Labels() map[string]string {
return map[string]string{}
}

func (t *envTemplateTagger) Annotations() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "envTemplateTagger",
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/skaffold/build/tag/git_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ func NewGitCommit(prefix, variant string) (*GitCommit, error) {

// Labels are labels specific to the git tagger.
func (c *GitCommit) Labels() map[string]string {
return map[string]string{}
}

// Labels are labels specific to the git tagger.
func (c *GitCommit) Annotations() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "git-commit",
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/skaffold/build/tag/sha256.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ type ChecksumTagger struct{}

// Labels are labels specific to the sha256 tagger.
func (c *ChecksumTagger) Labels() map[string]string {
return map[string]string{}
}

// Labels are labels specific to the sha256 tagger.
func (c *ChecksumTagger) Annotations() map[string]string {
return map[string]string{
constants.Labels.TagPolicy: "sha256",
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/skaffold/build/tag/tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ type Tagger interface {
// Labels produces labels to indicate the used tagger in deployed pods.
Labels() map[string]string

Annotations() 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
8 changes: 6 additions & 2 deletions pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ func NewHelmDeployer(runCtx *runcontext.RunContext) *HelmDeployer {

// Labels returns the Kubernetes labels used by this deployer
func (h *HelmDeployer) Labels() map[string]string {
return map[string]string{}
}

func (h *HelmDeployer) Annotations() map[string]string {
return map[string]string{
constants.Labels.Deployer: "helm",
}
Expand Down Expand Up @@ -143,8 +147,8 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build

event.DeployComplete()

labels := merge(h.addSkaffoldLabels, h, labellers...)
labelDeployResults(labels, dRes)
labels, annotations := merge(h.addSkaffoldLabels, h, labellers...)
labelAndAnnotateDeployResults(labels, annotations, dRes)

// Collect namespaces in a string
namespaces := make([]string, 0, len(nsMap))
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ func NewKubectlDeployer(runCtx *runcontext.RunContext) *KubectlDeployer {
}

func (k *KubectlDeployer) Labels() map[string]string {
return map[string]string{}
}

func (k *KubectlDeployer) Annotations() map[string]string {
return map[string]string{
constants.Labels.Deployer: "kubectl",
}
Expand Down Expand Up @@ -298,7 +302,7 @@ func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, bu
}
}

manifests, err = manifests.SetLabels(merge(k.addSkaffoldLabels, k, labellers...))
manifests, err = manifests.SetLabelsAndAnnotations(merge(k.addSkaffoldLabels, k, labellers...))
if err != nil {
return nil, fmt.Errorf("setting labels in manifests: %w", err)
}
Expand Down
47 changes: 28 additions & 19 deletions pkg/skaffold/deploy/kubectl/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (
)

// SetLabels add labels to a list of Kubernetes manifests.
func (l *ManifestList) SetLabels(labels map[string]string) (ManifestList, error) {
replacer := newLabelsSetter(labels)
func (l *ManifestList) SetLabelsAndAnnotations(labels, annotations map[string]string) (ManifestList, error) {
replacer := newLabelsAndAnnotationsSetter(labels, annotations)

updated, err := l.Visit(replacer)
if err != nil {
Expand All @@ -36,45 +36,54 @@ func (l *ManifestList) SetLabels(labels map[string]string) (ManifestList, error)
return updated, nil
}

type labelsSetter struct {
labels map[string]string
type labelsAndAnnotationsSetter struct {
labels map[string]string
annotations map[string]string
}

func newLabelsSetter(labels map[string]string) *labelsSetter {
return &labelsSetter{
labels: labels,
func newLabelsAndAnnotationsSetter(labels, annotations map[string]string) *labelsAndAnnotationsSetter {
return &labelsAndAnnotationsSetter{
labels: labels,
annotations: annotations,
}
}

func (r *labelsSetter) Visit(o map[string]interface{}, k string, v interface{}) bool {
func (r *labelsAndAnnotationsSetter) Visit(o map[string]interface{}, k string, v interface{}) bool {
if k != "metadata" {
return true
}

if len(r.labels) == 0 {
return false
}

metadata, ok := v.(map[string]interface{})
if !ok {
return true
}

l, present := metadata["labels"]
if visitAndReplace("labels", r.labels, &metadata) {
return true
}
return visitAndReplace("annotations", r.annotations, &metadata)
}

func visitAndReplace(fieldName string, values map[string]string, metadata *map[string]interface{}) bool {
if len(values) == 0 {
return false
}

field, present := (*metadata)[fieldName]
if !present {
metadata["labels"] = r.labels
(*metadata)[fieldName] = values
return false
}

labels, ok := l.(map[string]interface{})
existingValues, ok := field.(map[string]interface{})
if !ok {
return true
}

for k, v := range r.labels {
// Don't replace existing label.
if _, present := labels[k]; !present {
labels[k] = v
for k, v := range values {
// Don't overwrite existing values
if _, present := existingValues[k]; !present {
existingValues[k] = v
}
}

Expand Down
Loading

0 comments on commit 5ecd3b9

Please sign in to comment.