Skip to content

Add Labels to Metadata #6782

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

Merged
merged 1 commit into from
Nov 8, 2021
Merged

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Oct 30, 2021

fixes: #7425
Description

  • Add labels to metadata so that skaffold files can have labels identifying
    the config.

  • Using labels is useful for being able to match skaffold files.

  • Concretely, we want to use labels to identify the different environments
    corresponding to a skaffold file (e.g. dev/prod etc...). These labels
    our used by our CD tooling to identify the skaffold files which need to
    be built.

User facing changes (remove if N/A)

After this change users can include labels in their skaffold.yaml files; e.g.

kind: Config
metadata:
  name: some config
  labels:
     dev: "true"

@jlewi jlewi requested a review from a team as a code owner October 30, 2021 22:43
@jlewi jlewi requested a review from aaron-prindle October 30, 2021 22:43
@google-cla google-cla bot added the cla: yes label Oct 30, 2021
@codecov
Copy link

codecov bot commented Oct 30, 2021

Codecov Report

Merging #6782 (b59786c) into main (290280e) will decrease coverage by 1.09%.
The diff coverage is 65.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6782      +/-   ##
==========================================
- Coverage   70.48%   69.39%   -1.10%     
==========================================
  Files         515      540      +25     
  Lines       23150    24596    +1446     
==========================================
+ Hits        16317    17068     +751     
- Misses       5776     6396     +620     
- Partials     1057     1132      +75     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/flags.go 89.00% <0.00%> (-1.82%) ⬇️
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <ø> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/cmd/lint.go 52.94% <52.94%> (ø)
cmd/skaffold/app/cmd/cmd.go 70.49% <75.00%> (-0.57%) ⬇️
cmd/skaffold/app/cmd/delete.go 69.23% <80.00%> (+13.67%) ⬆️
cmd/skaffold/app/cmd/debug.go 100.00% <100.00%> (ø)
... and 134 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc06016...b59786c. Read the comment docs.

@jlewi
Copy link
Contributor Author

jlewi commented Oct 31, 2021

Looks like the linter is failing because I updated v2beta24

Invalid changes:
----------------
diff --git a/pkg/skaffold/schema/v2beta24/config.go b/pkg/skaffold/schema/v2beta24/config.go
time="2021-10-30T22:51:36Z" level=fatal msg="structural changes detected"
index dcb198ef0..1eea3ba3a 100755
--- a/pkg/skaffold/schema/v2beta24/config.go
+++ b/pkg/skaffold/schema/v2beta24/config.go
@@ -58,6 +58,9 @@ type SkaffoldConfig struct {
 type Metadata struct {
 	// Name is an identifier for the project.
 	Name string `yaml:"name,omitempty"`
+
+	// Labels is a map of labels identifying the project.
+	Labels map[string]string `yaml:"labels,omitempty"`
 }
 
 // Pipeline describes a Skaffold pipeline.
exit status 1
FAILED hack/check-schema-changes.sh

I would expect this to be a backwards compatible change since its purely additive.

What would be the correct way to introduce this change? Should I create a new version?

@aaron-prindle
Copy link
Contributor

@jlewi thanks for the submission here. Yes, creating a new schema version would be the correct thing to do in this case

@briandealwis
Copy link
Member

As a rule, we don't make changes to previously published schema versions.

@jlewi
Copy link
Contributor Author

jlewi commented Nov 2, 2021

Apologies; I should have checked the development guide first.
https://github.com/GoogleContainerTools/skaffold/blob/main/DEVELOPMENT.md#making-a-config-change

Might take me a couple days to follow up on this. Feel free to mark this PR as draft or wip.

* Add labels to metadata so that skaffold files can have labels identifying
  the config.

* Using labels is useful for being able to match skaffold files.
* Concretely, we want to use labels to identify the different environments
  corresponding to a skaffold file (e.g. dev/prod etc...). These labels
  our used by our CD tooling to identify the skaffold files which need to
  be built.
@jlewi
Copy link
Contributor Author

jlewi commented Nov 6, 2021

This updated. PTAL.

@aaron-prindle aaron-prindle added the kokoro:force-run forces a kokoro re-run on a PR label Nov 8, 2021
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Nov 8, 2021
Copy link
Contributor

@aaron-prindle aaron-prindle left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks adding this

@aaron-prindle aaron-prindle merged commit b1081e6 into GoogleContainerTools:main Nov 8, 2021
@jlewi jlewi mentioned this pull request May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add labels to Metadata
5 participants