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

[kubectl] apply labels by patching yaml #1489

Merged

Conversation

@dgageot
Copy link
Member

dgageot commented Jan 18, 2019

This is a much simpler way of labelling resources when using kubectl.

Signed-off-by: David Gageot david@gageot.net

@dgageot dgageot requested review from balopat, nkubala and r2d4 as code owners Jan 18, 2019
@dgageot dgageot changed the title [kubectl] apply labels by patching [kubectl] apply labels by patching yaml Jan 18, 2019
@dgageot dgageot force-pushed the dgageot:simpler-kubectl-labelling branch from 9bd2788 to 3d94770 Jan 19, 2019
@nkubala

This comment has been minimized.

Copy link
Member

nkubala commented Jan 19, 2019

I'm 85% sure there's a reason we didn't do it this way in the first place, but I can't remember. @r2d4 @dlorenc am I crazy

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 19, 2019

Codecov Report

Merging #1489 into master will increase coverage by 0.18%.
The diff coverage is 63.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1489      +/-   ##
==========================================
+ Coverage   46.51%   46.69%   +0.18%     
==========================================
  Files         118      119       +1     
  Lines        5044     5060      +16     
==========================================
+ Hits         2346     2363      +17     
+ Misses       2464     2463       -1     
  Partials      234      234
Impacted Files Coverage Δ
pkg/skaffold/deploy/labels.go 15.85% <ø> (+6.09%) ⬆️
pkg/skaffold/deploy/kubectl/cli.go 0% <0%> (ø) ⬆️
pkg/skaffold/deploy/kustomize.go 36% <0%> (+1.38%) ⬆️
pkg/skaffold/deploy/kubectl.go 75.86% <66.66%> (-1.75%) ⬇️
pkg/skaffold/deploy/kubectl/labels.go 78.57% <78.57%> (ø)
pkg/skaffold/deploy/util.go 68% <0%> (-8%) ⬇️

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 6437d83...dd8e017. Read the comment docs.

@dgageot

This comment has been minimized.

Copy link
Member Author

dgageot commented Jan 19, 2019

@nkubala the problem is that now, there's two implementations. One for kubctl/kustomize, one for helm. Maybe you wanted to have a single implementation.

@dgageot dgageot force-pushed the dgageot:simpler-kubectl-labelling branch 3 times, most recently from 10d119b to 75a8e44 Jan 23, 2019
@googlebot googlebot added the cla: yes label Jan 24, 2019
@dgageot dgageot force-pushed the dgageot:simpler-kubectl-labelling branch from 75a8e44 to 05b812d Jan 25, 2019
@nkubala

This comment has been minimized.

Copy link
Member

nkubala commented Jan 25, 2019

@dgageot I don't think that was it, but I can't think of a good reason now so forget about it.

Kokoro is failing but it looks like it might have been a network issue, restarting the build

@dgageot dgageot force-pushed the dgageot:simpler-kubectl-labelling branch 7 times, most recently from 7adf3c0 to 9447adc Jan 25, 2019
@dgageot dgageot requested a review from priyawadhwa as a code owner Jan 30, 2019
@dgageot dgageot force-pushed the dgageot:simpler-kubectl-labelling branch 4 times, most recently from 86e57d3 to de1a6f5 Jan 31, 2019
Signed-off-by: David Gageot <david@gageot.net>
@dgageot dgageot force-pushed the dgageot:simpler-kubectl-labelling branch from de1a6f5 to dd8e017 Feb 4, 2019
@nkubala
nkubala approved these changes Feb 4, 2019
Copy link
Member

nkubala left a comment

:shipit:

@balopat balopat merged commit c9d3235 into GoogleContainerTools:master Feb 5, 2019
4 checks passed
4 checks passed
cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
kokoro CI build successful.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.