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

Add standalone npd on GCI. #40206

Merged
merged 1 commit into from
Feb 18, 2017
Merged

Add standalone npd on GCI. #40206

merged 1 commit into from
Feb 18, 2017

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Jan 20, 2017 •
edited
Loading

This PR added standalone NPD in GCE GCI cluster. I already verified the PR, and it should work.

/cc @dchen1107 @fabioy @andyxning @kubernetes/sig-node-misc

Release note:

Node Problem Detector is beta now. New features added: journald support, standalone mode and arbitrary system log monitoring.

@Random-Liu Random-Liu added area/reliability sig/node Categorizes an issue or PR as relevant to SIG Node. labels Jan 20, 2017
@Random-Liu Random-Liu added this to the v1.6 milestone Jan 20, 2017
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 20, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@Random-Liu Random-Liu added release-note Denotes a PR that will be considered when it comes time to generate release notes. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jan 20, 2017
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 20, 2017
@Random-Liu Random-Liu mentioned this pull request Jan 24, 2017
Closed
10 tasks
@k8s-github-robot k8s-github-robot assigned deads2k and jbeda and unassigned dchen1107 and deads2k Jan 30, 2017
@deads2k deads2k removed their assignment Jan 31, 2017
@zmerlynn zmerlynn removed their assignment Feb 8, 2017
liggitt
liggitt reviewed Feb 10, 2017
View reviewed changes
plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go Outdated
@@ -298,6 +298,8 @@ func ClusterRoleBindings() []rbac.ClusterRoleBinding {
rbac.NewClusterBinding("system:node").Groups(user.NodesGroup).BindingOrDie(),
rbac.NewClusterBinding("system:node-proxier").Users(user.KubeProxy).BindingOrDie(),
rbac.NewClusterBinding("system:kube-controller-manager").Users(user.KubeControllerManager).BindingOrDie(),
// FIXME: Define me in yaml.
rbac.NewClusterBinding("system:node-problem-detector").Users(user.NodeProblemDetector).BindingOrDie(),
Copy link
Member

Choose a reason for hiding this comment

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

maybe something more like https://github.com/kubernetes/kubernetes/pull/40760/files#diff-6e3b476b9225d1213dc6ad13e453fc16R1138 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. :)

@Random-Liu Random-Liu changed the title [Do Not Merge] Add standalone npd on GCI. Add standalone npd on GCI. Feb 10, 2017
@Random-Liu
Copy link
Member Author

Random-Liu commented Feb 10, 2017 •
edited
Loading

@dchen1107 This PR is ready for review. But note that before merging this PR, we need to merge all pending PRs in npd, and release the v0.3.0-alpha.1 version of NPD.

/cc @liggitt Please take a look at the RBAC role binding. I put it in the addon directory now.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 17, 2017
@Random-Liu Random-Liu force-pushed the add-standalone-npd branch 2 times, most recently from c427a18 to 0255b8b Compare February 17, 2017 23:15
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 17, 2017
@dchen1107 dchen1107 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 Feb 18, 2017
@dchen1107
Copy link
Member

dchen1107 commented Feb 18, 2017 •
edited
Loading

Please update your pr description above:

However, there are still dependencies need to be resolved before merging this PR:
Figure out how to do role binding for standalone NPD. (Ref #40021)

I believe the issue is resolved in this pr.

Get NPD journald support PR merged, build a release tar ball, and update the tar version and path in this PR.

I believe those dependent prs are merged.

@dchen1107
Copy link
Member

/lgtm
and
/approve

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

The following people have approved this PR: Random-Liu, dchen1107

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

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2017
@Random-Liu
Copy link
Member Author

Apply LGTM based on #40206 (comment).

@Random-Liu Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 18, 2017
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit ff12e56 into kubernetes:master Feb 18, 2017
@Random-Liu Random-Liu deleted the add-standalone-npd branch February 18, 2017 10:45
@liggitt liggitt mentioned this pull request Feb 19, 2017
Merged
This was referenced Feb 19, 2017
Closed
Merged
@Random-Liu Random-Liu mentioned this pull request Mar 22, 2017
Merged
k8s-github-robot pushed a commit that referenced this pull request Mar 23, 2017
Merge pull request #43537 from Random-Liu/disable-master-taint-by-def…
c415325 
…ault

Automatic merge from submit-queue

Add an env KUBE_ENABLE_MASTER_NOSCHEDULE_TAINT and disable it by default

This PR changed master `NoSchedule` taint to opt-in.

As is discussed with @bgrant0607 @janetkuo, `NoSchedule` master taint breaks existing user workload, we should not enable it by default.

Previously, NPD required the taint because it can only support one OS distro with a specific configuration. If master and node are using different OS distros, NPD will not work either on master or node. However, we've already fixed this in #40206, so for NPD it's fine to disable the taint.

This should work, but I'll still try it in my cluster to confirm.

@kubernetes/sig-scheduling-misc @dchen1107 @mikedanese
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

Assignees

@dchen1107 dchen1107

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/reliability 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 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.6
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@Random-Liu @k8s-reviewable @dchen1107 @k8s-github-robot @liggitt @jbeda @zmerlynn @deads2k @k8s-ci-robot