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.

Sign up for GitHub

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

Jump to bottom

Update NPD config for GCI #65342

Merged
merged 1 commit into from
Jun 26, 2018
Merged

Update NPD config for GCI #65342

merged 1 commit into from
Jun 26, 2018

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Jun 21, 2018 •
edited
Loading

What this PR does / why we need it:
Use kubernetes/node-problem-detector#180 on GCI

Special notes for your reviewer:
This is currently pending an NPD release.

Release note:

On COS, NPD creates a node condition for frequent occurrences of unregister_netdevice

/assign @Random-Liu
/sig node
/kind feature
/priority important-soon

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. kind/feature Categorizes issue or PR as related to a new feature. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 21, 2018
@k8s-ci-robot k8s-ci-robot requested review from gmarek and vishh June 21, 2018 22:58
@dashpole
Copy link
Contributor Author

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 21, 2018
Random-Liu
Random-Liu reviewed Jun 21, 2018
View reviewed changes
cluster/gce/gci/configure-helper.sh
echo "Using node problem detector binary at ${npd_bin}"
local flags="${NPD_TEST_LOG_LEVEL:-"--v=2"} ${NPD_TEST_ARGS:-}"
flags+=" --logtostderr"
flags+=" --system-log-monitors=${km_config},${dm_config}"
flags+=" --custom-plugin-monitors=${custom_km_config}"
Copy link
Member

Choose a reason for hiding this comment

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

Let's still add the EXTRA ARG for extensibility.

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

@dashpole dashpole force-pushed the npd_args branch 3 times, most recently from a05a69f to f3f9e0a Compare June 25, 2018 16:41
@dashpole
Copy link
Contributor Author

/retest

@dashpole
Copy link
Contributor Author

/restest

@dashpole
Copy link
Contributor Author

/retest

@Random-Liu
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2018
@dashpole
Copy link
Contributor Author

/assign @dchen1107
for approval

@dashpole
Copy link
Contributor Author

/unassign @dchen1107
/assign @vishh
since dawn is OOO

@k8s-ci-robot k8s-ci-robot assigned vishh and unassigned dchen1107 Jun 26, 2018
vishh
vishh reviewed Jun 26, 2018
View reviewed changes
cluster/gce/gci/configure-helper.sh
@@ -1181,13 +1181,18 @@ function start-node-problem-detector {
local -r km_config="${KUBE_HOME}/node-problem-detector/config/kernel-monitor.json"
# TODO(random-liu): Handle this for alternative container runtime.
local -r dm_config="${KUBE_HOME}/node-problem-detector/config/docker-monitor.json"
local -r custom_km_config="${KUBE_HOME}/node-problem-detector/config/kernel-monitor-counter.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this file included in the release? Doesn't it need change to Bazel rules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is part of the NPD release tarball.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it is added by the change in the NPD version.

@vishh
Copy link
Contributor

vishh commented Jun 26, 2018 via email

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, Random-Liu, vishh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2018
@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 65342, 65460). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 2dbb9c8 into kubernetes:master Jun 26, 2018
This was referenced Jun 26, 2018
Merged
Merged
@dashpole dashpole deleted the npd_args branch June 26, 2018 22:59
@dashpole dashpole mentioned this pull request Jun 26, 2018
Merged
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jun 27, 2018
k8s-github-robot pushed a commit that referenced this pull request Jun 29, 2018
Merge pull request #65504 from dashpole/automated-cherry-pick-of-#653…
a66be7c 
…42-upstream-release-1.10

Automatic merge from submit-queue.

Automated cherry pick of #65342: update NPD version to v0.5.0 for gci

Cherry pick of #65342 on release-1.10.

#65342: update NPD version to v0.5.0 for gci
k8s-github-robot pushed a commit that referenced this pull request Jun 30, 2018
Merge pull request #65505 from dashpole/automated-cherry-pick-of-#653…
ae042b7 
…42-upstream-release-1.9

Automatic merge from submit-queue.

Automated cherry pick of #65342: update NPD version to v0.5.0 for gci

Cherry pick of #65342 on release-1.9.

#65342: update NPD version to v0.5.0 for gci
k8s-github-robot pushed a commit that referenced this pull request Jul 3, 2018
Merge pull request #65503 from dashpole/automated-cherry-pick-of-#653…
44c813d 
…42-upstream-release-1.11

Automatic merge from submit-queue.

Automated cherry pick of #65342: update NPD version to v0.5.0 for gci

Cherry pick of #65342 on release-1.11.

#65342: update NPD version to v0.5.0 for gci
@geekgonecrazy geekgonecrazy mentioned this pull request Jul 11, 2018
Closed
@geekgonecrazy
Copy link

@dashpole any idea when this change actually makes it into gke? I see 1.10.5-gke.2 just released but its still using node-problem-detector 0.4.1

It'd be nice to at the very least get this identifying nodes with kernel issues like #64743 and resolve them.

@dashpole
Copy link
Contributor Author

@geekgonecrazy 1.9.10, 1.10.6, 1.11.1, 1.12.0

@geekgonecrazy
Copy link

@dashpole perfect! Thanks for your response!

@wangzhen127 wangzhen127 mentioned this pull request Nov 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vishh vishh vishh left review comments

@Random-Liu Random-Liu Random-Liu left review comments

@gmarek gmarek Awaiting requested review from gmarek

Assignees

@Random-Liu Random-Liu

@vishh vishh

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@dashpole @Random-Liu @vishh @k8s-ci-robot @k8s-github-robot @geekgonecrazy @dchen1107