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

Remove more docker references in kubelet #7586

Merged
merged 2 commits into from Apr 30, 2015

Conversation

yujuhong
Copy link
Contributor

This change also renames TrimRuntimePrefixFromImage to TrimRuntimePrefix to
better reflect that the usage is not limited to images (e.g. ID).

This change also renames TrimRuntimePrefixFromImage to TrimRuntimePrefix to
better reflect that the usage is not limited to images (e.g. ID).
if err != nil {
return nil, err
}
if len(dockerContainers) == 0 {
return nil, ErrNoKubeletContainers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unsure why we need to report ErrNoKubeletContainers, as opposed to ErrContainerNotFound. Do we really care about the difference? If that's the case, I would do more plumbing and change the behavior back to reporting ErrNoKubeletContainers

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, ErrContainerNotFound sounds good. This was added since there was a request for concrete errors in server.go. The handling of both errors there is the same actually. If you're up for it we can probably remove ErrNoKubeletContainers. Feel free to do here or in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@yujuhong
Copy link
Contributor Author

/cc @vmarmol, @yifan-gu

@vmarmol vmarmol self-assigned this Apr 30, 2015
@vmarmol
Copy link
Contributor

vmarmol commented Apr 30, 2015

LGTM

We no longer use it.
@yujuhong
Copy link
Contributor Author

Add a new commit to remove ErrNoKubeletContainers.

@vmarmol
Copy link
Contributor

vmarmol commented Apr 30, 2015

LGTM, thanks @yujuhong!

@vmarmol
Copy link
Contributor

vmarmol commented Apr 30, 2015

Shippable is green, merging.

vmarmol added a commit that referenced this pull request Apr 30, 2015
Remove more docker references in kubelet
@vmarmol vmarmol merged commit a94aeb2 into kubernetes:master Apr 30, 2015
@yujuhong yujuhong deleted the container_info branch May 1, 2015 21: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.

None yet

3 participants