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

Fix: Skaffold reloads unchanged, existing image again into the cluster #4983

Merged
merged 1 commit into from Nov 12, 2020

Conversation

anshlykov
Copy link
Contributor

@anshlykov anshlykov commented Oct 31, 2020

Related: #4955

Description
the command kubectl get nodes -ojsonpath = '{@.items[*].status.images[*].names[*]}' returns a list with normalized image refs. The returned image may differ from artifact.Tag. This PR adds image normalization before checking already loaded images.

@anshlykov anshlykov requested a review from a team as a code owner October 31, 2020 20:27
@google-cla google-cla bot added the cla: yes label Oct 31, 2020
@codecov
Copy link

codecov bot commented Oct 31, 2020

Codecov Report

Merging #4983 into master will increase coverage by 0.01%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4983      +/-   ##
==========================================
+ Coverage   72.27%   72.28%   +0.01%     
==========================================
  Files         361      361              
  Lines       12640    12670      +30     
==========================================
+ Hits         9135     9159      +24     
- Misses       2827     2830       +3     
- Partials      678      681       +3     
Impacted Files Coverage Δ
pkg/skaffold/runner/load_images.go 94.73% <50.00%> (-5.27%) ⬇️
pkg/skaffold/build/buildpacks/types.go 100.00% <0.00%> (ø)
pkg/skaffold/docker/parse.go 85.47% <0.00%> (+0.16%) ⬆️
pkg/skaffold/build/buildpacks/lifecycle.go 81.72% <0.00%> (+0.56%) ⬆️

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 cd2a38f...18496ce. Read the comment docs.

@tejal29 tejal29 added the kokoro:run runs the kokoro jobs on a PR label Nov 2, 2020
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Nov 2, 2020
Copy link
Member

@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.

This change looks good to me and makes sense. We shd ship this change

However, i am not sure how it fixes #4955.

From the issue logs looks like we were checking a tag "eu.gcr.io/tools-org-veroo/party-service:d46578239dd325395e15e8ab16a2c499616a15e662ed196c18549e8d2bea38b6" which exist in the kubectl get nodes -o jsonpath='{@.items[*].status.images[*].names[*]}

am i missing something obvious?

@anshlykov
Copy link
Contributor Author

From the issue logs looks like we were checking a tag "eu.gcr.io/tools-org-veroo/party-service:d46578239dd325395e15e8ab16a2c499616a15e662ed196c18549e8d2bea38b6" which exist in the kubectl get nodes -o jsonpath='{@.items[].status.images[].names[*]}

@tejal29 You're right. My bad I missed it in the description. It looks like I faced with another issue.

@anshlykov
Copy link
Contributor Author

But also I was unable to fully reproduce the bug described here, perhaps due to a lack of information.

@tejal29 tejal29 merged commit 416804a into GoogleContainerTools:master Nov 12, 2020
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.

None yet

3 participants