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

Add tolerations for Stackdriver Logging and Metadata Agents. #69737

Merged
merged 1 commit into from
Oct 15, 2018
Merged

Add tolerations for Stackdriver Logging and Metadata Agents. #69737

merged 1 commit into from
Oct 15, 2018

Conversation

qingling128
Copy link
Contributor

What this PR does / why we need it:
Followup of #68920. This makes sure taints are respected.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

Add tolerations for Stackdriver Logging and Metadata Agents.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/gcp and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 12, 2018
@qingling128
Copy link
Contributor Author

/assign @MaciekPytel

@qingling128
Copy link
Contributor Author

/assign @x13n

@MaciekPytel
Copy link
Contributor

/ok-to-test
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 12, 2018
@@ -58,6 +58,11 @@ spec:
restartPolicy: Always
schedulerName: default-scheduler
terminationGracePeriodSeconds: 30
tolerations:

Choose a reason for hiding this comment

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

Discussion topic: do we need similar tolerations in the cluster-level agent spec? I suspect the only reason we would is if all nodes are tainted. @x13n @MaciekPytel @kawych @piosz @bmoyles0117

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of that conversation happened in #68920 (comment).

Choose a reason for hiding this comment

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

Ah, thanks for the pointer. I see the priority class now. Given that we'll only cherry-pick this into 1.12 and 1.11, this should work. If we ever need a 1.10 cherry-pick, we'd have to add stronger tolerations (e.g., CriticalAddonsOnly).

Copy link
Member

Choose a reason for hiding this comment

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

CriticalAddonsOnly isn't really stronger - the ones added here are wildcard ones, so they include it already.

Choose a reason for hiding this comment

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

I meant for the cluster-level agent, which currently doesn't have any tolerations.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, right. I think if someone is tainting all their nodes, something may break anyway, but yes, this could at least mitigate the problem in 1.10.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a lot of things won't schedule if you taint all your nodes (DNS, metrics-server, ...). I don't think cluster-level agent should have stronger tolerations than other cluster-level system components.

@x13n
Copy link
Member

x13n commented Oct 15, 2018

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qingling128, x13n

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 Oct 15, 2018
@x13n
Copy link
Member

x13n commented Oct 15, 2018

/kind bug

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Oct 15, 2018
@k8s-ci-robot k8s-ci-robot merged commit 8928e70 into kubernetes:master Oct 15, 2018
@qingling128
Copy link
Contributor Author

Created cherry-pick PRs into 1.11 and 1.12 branches.

k8s-ci-robot added a commit that referenced this pull request Oct 18, 2018
…69737-upstream-release-1.12

Automated cherry pick of #69737: Add tolerations for Stackdriver Logging and Metadata Agents.
k8s-ci-robot added a commit that referenced this pull request Oct 19, 2018
…69737-upstream-release-1.11

Automated cherry pick of #69737: Add tolerations for Stackdriver Logging and Metadata Agents.
k8s-ci-robot added a commit that referenced this pull request Nov 27, 2018
…69737-upstream-release-1.10

Automated cherry pick of #69737: Add tolerations for Stackdriver Logging and Metadata Agents.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/bug Categorizes issue or PR as related to a bug. 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/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. 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

5 participants