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

Support for replacing image names in env vars #995

Closed

Conversation

ajbouh
Copy link
Contributor

@ajbouh ajbouh commented Sep 19, 2018

Specifically env vars that end with _IMAGE

Specifically env vars that end with _IMAGE
@codecov-io
Copy link

codecov-io commented Sep 19, 2018

Codecov Report

Merging #995 into master will increase coverage by 0.17%.
The diff coverage is 65.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #995      +/-   ##
==========================================
+ Coverage   40.61%   40.79%   +0.17%     
==========================================
  Files          68       68              
  Lines        2969     3010      +41     
==========================================
+ Hits         1206     1228      +22     
- Misses       1639     1657      +18     
- Partials      124      125       +1
Impacted Files Coverage Δ
pkg/skaffold/deploy/kubectl/images.go 75% <65.45%> (-25%) ⬇️

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 f728fd9...6be8c5a. Read the comment docs.

@mattfysh
Copy link

This will be very handy for kicking off jobs needing the skaffold-tagged image names. For now, a workaround is to annotate the pod that needs the image name, using key "image". It will match and get picked up by the replacer

@dgageot dgageot added the kokoro:run runs the kokoro jobs on a PR label Jan 28, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Jan 28, 2019
@dgageot dgageot added kokoro:run runs the kokoro jobs on a PR needs-rebase labels Jan 28, 2019
Copy link
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Can we please add some tests for it?

@ajbouh
Copy link
Contributor Author

ajbouh commented Mar 13, 2019

I don't know how bad the rebase will be so I'm reluctant to invest further in this PR. Also unclear what the appetite for merging this is. Thoughts on these points?

@tejal29
Copy link
Contributor

tejal29 commented Mar 20, 2019

I think we are thinking of Adding Env Based tagger as part of #922 for skaffold build images.
See #922 (comment) for more details.

I agree, lets close this PR for now and revisit if this is needed once #922 is resolved.

@tejal29 tejal29 closed this Mar 20, 2019
@ajbouh
Copy link
Contributor Author

ajbouh commented Mar 21, 2019

What is env based tagger? That sounds like a way to use environment variables to decide on the docker image tag?

If so, that's unrelated to what this PR does

@tejal29
Copy link
Contributor

tejal29 commented Mar 21, 2019

Yes it unrelated to the PR. Since you mentioned you don't want to invest further at this time, I suggesting revisiting this once build changes are in.
Feel free to re-open this if you want to get this in before that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes kokoro:run runs the kokoro jobs on a PR needs-rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants