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 kubemark hollow-npd. #41703

Merged
merged 1 commit into from
Feb 19, 2017
Merged

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Feb 19, 2017

In NPD v0.3.0-alpha.1, node problem detector will error out if the specified log file doesn't exist.

Previously, kubemark uses a non-exist log file /log/faillog to make npd idle, as hollow-npd. However, it won't work with new npd.

This PR changed the log path to /dev/null, so that npd won't be able to read anything, and /dev/null will definitely exist in the container.

I started kubemark cluster with this change myself, and it works properly now.

@foxish @shyamjvs @wojtek-t

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 19, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

The following people have approved this PR: Random-Liu

Needs approval from an approver in each of these OWNERS Files:

We suggest the following people:
cc @wojtek-t
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Feb 19, 2017
@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 Feb 19, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@foxish
Copy link
Contributor

foxish commented Feb 19, 2017

I'm not familiar with this part of the code. Can one of @gmarek, @shyamjvs @wojtek-t take a look?

@foxish
Copy link
Contributor

foxish commented Feb 19, 2017

@k8s-bot kops aws e2e test this

@foxish foxish added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 19, 2017
@foxish
Copy link
Contributor

foxish commented Feb 19, 2017

I don't think we can get owner approvals tonight. @Random-Liu explained the change to me, and it seems reasonable and appears to fix the problem. I'll merge on green to unblock the submit queue.

@Random-Liu
Copy link
Member Author

@foxish Thanks for taking care of this!

@foxish foxish merged commit 8c8dca0 into kubernetes:master Feb 19, 2017
@krzyzacy
Copy link
Member

Wow thanks! I was also staring at it and has no clue at all 😝

@foxish
Copy link
Contributor

foxish commented Feb 19, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants