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 PullImage and add corresponding node e2e test #24126

Merged
merged 1 commit into from
Apr 18, 2016

Conversation

Random-Liu
Copy link
Member

Fixes #24101. This is a bug introduced by #23506, since ref #23563.

The root cause of #24101 is described here.

This PR

  1. Fixes Pod in RunContainerError Status  #24101 by decoding the messages returned during pulling image, and return error if any of the messages contains error.
  2. Add the node e2e test to detect this kind of failure.
  3. Get present check out of ConformanceImage.Remove() and ConformanceImage.Pull(). Because sometimes we may expect error to occur in PullImage() and RemoveImage(), but even that doesn't happen, the Present() check will still return error and let the test pass.

@yujuhong @freehan @liangchenye

Also /cc @resouer, because he is doing the image related functions refactoring.

@Random-Liu Random-Liu added kind/bug Categorizes issue or PR as related to a bug. area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. labels Apr 12, 2016
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Apr 12, 2016
@Random-Liu Random-Liu added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Apr 12, 2016
@Random-Liu Random-Liu assigned yujuhong and unassigned spxtr Apr 12, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 12, 2016

GCE e2e build/test passed for commit 1fb331df5dad99bd12cde747f749a25ec65dbe7f.

@k8s-bot
Copy link

k8s-bot commented Apr 12, 2016

GCE e2e build/test passed for commit 8450b039a97b6f4f6eef4341e790df2090c4a61d.

}
})
})
Context("when testing image that does not exist", func() {
var invalidImage ConformanceImage
var invalidImageTag string
It("it should not pull successfully [Conformance]", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "it":
It("should not pull

Copy link
Member Author

Choose a reason for hiding this comment

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

ACK

@yujuhong
Copy link
Contributor

Looks good overall with some nits.

@Random-Liu
Copy link
Member Author

@yujuhong Addressed comments. Thanks for reviewing! :)

@yujuhong yujuhong added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 12, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 12, 2016

GCE e2e build/test passed for commit 2b9f6be.

@k8s-bot
Copy link

k8s-bot commented Apr 14, 2016

GCE e2e build/test passed for commit 2b9f6be.

@k8s-bot
Copy link

k8s-bot commented Apr 16, 2016

GCE e2e build/test passed for commit 2b9f6be.

@k8s-bot
Copy link

k8s-bot commented Apr 18, 2016

GCE e2e build/test passed for commit 2b9f6be.

@k8s-bot
Copy link

k8s-bot commented Apr 18, 2016

GCE e2e build/test passed for commit 2b9f6be.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Apr 18, 2016

GCE e2e build/test passed for commit 2b9f6be.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit d37e6ad into kubernetes:master Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubelet kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants