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 unused (?) HealthCheck from KubeletClient #12913

Merged
merged 1 commit into from
Aug 19, 2015

Conversation

gmarek
Copy link
Contributor

@gmarek gmarek commented Aug 19, 2015

@gmarek gmarek added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Aug 19, 2015
@k8s-bot
Copy link

k8s-bot commented Aug 19, 2015

GCE e2e build/test passed for commit 33c894a.

@@ -52,6 +52,7 @@ func proxyContext(version string) {
f := NewFramework("proxy")
prefix := "/api/" + version

// Port here has to be kept in sync with default kubelet port.
Copy link
Member

Choose a reason for hiding this comment

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

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 don't understand what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

Kind of a nit/suggestion. What I mean is the next line could be:

It("should proxy logs on node with explicit kubelet port", func() {
     nodeProxyTest(f, version, fmt.Sprintf(":%v/logs/", ports.KubeletPort))
})

Then no need for the comment and we only have one thing to keep in sync which is the pkg/master/ports constants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be no longer true and each Kubelet will be able to have different port.

@yujuhong
Copy link
Contributor

LGTM. Thanks for the cleanup!

@yujuhong yujuhong added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2015
saad-ali added a commit that referenced this pull request Aug 19, 2015
Remove unused (?) HealthCheck from KubeletClient
@saad-ali saad-ali merged commit 25dfc99 into kubernetes:master Aug 19, 2015
@gmarek gmarek deleted the remove_healthcheck branch March 17, 2016 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants