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

Change SkaffoldOption Labeller to not include a comma in the label value #1169

Merged

Conversation

GeertJohan
Copy link
Contributor

When running skaffold to deploy to my cluster, I get tens of errors like this:

time="2018-10-16T10:03:34Z" level=warning msg="error adding label to runtime object: patching resource nlx-test-haarlem/txlog-ui: Ingress.extensions \"txlog-ui\" is invalid: metadata.labels: Invalid value: \"release,deploy-test\": a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue',  or 'my_value',  or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')"

Comma's are not allowed in label values. Skaffold uses them when multiple profiles are used.

This simple fix changes the separator from comma to a double underscore.

@dgageot
Copy link
Contributor

dgageot commented Oct 16, 2018

@GeertJohan +1 for the fix! You have to fix the TestLabels test though

@codecov-io
Copy link

Codecov Report

Merging #1169 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1169   +/-   ##
=======================================
  Coverage   42.18%   42.18%           
=======================================
  Files          89       89           
  Lines        3999     3999           
=======================================
  Hits         1687     1687           
  Misses       2143     2143           
  Partials      169      169
Impacted Files Coverage Δ
pkg/skaffold/config/options.go 88.23% <100%> (ø) ⬆️

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 aad90b0...3388987. Read the comment docs.

@r2d4
Copy link
Contributor

r2d4 commented Oct 16, 2018

Thank you 👍

@r2d4 r2d4 merged commit 4f78ea2 into GoogleContainerTools:master Oct 16, 2018
@GeertJohan GeertJohan deleted the bugfix/comma-in-label-value branch October 16, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants