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

Allow the EDS to deploy pods on unschedulable nodes #43

Merged
merged 2 commits into from
Oct 2, 2020

Conversation

L3n41c
Copy link
Member

@L3n41c L3n41c commented Oct 1, 2020

What does this PR do?

Allow the ExtendedDaemonSet controller to deploy pods on unschedulable nodes.

Motivation

Have the same behaviour as with the regular Kubernetes DaemonSet.
https://kubernetes.io/docs/concepts/workloads/controllers/daemonset/#taints-and-tolerations

Additional Notes

Anything else we should know when reviewing?

Describe your test plan

Cordon a node and check that the EDS controller still deploy a pod on it.

@L3n41c L3n41c added this to the v0.3 milestone Oct 1, 2020
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: bug, enhancement, documentation

@L3n41c L3n41c added bug Something isn't working component/controller labels Oct 1, 2020
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2020

Codecov Report

Merging #43 into master will increase coverage by 0.66%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
+ Coverage   36.80%   37.46%   +0.66%     
==========================================
  Files          29       29              
  Lines        1508     1500       -8     
==========================================
+ Hits          555      562       +7     
+ Misses        879      865      -14     
+ Partials       74       73       -1     
Flag Coverage Δ
#unittests 37.46% <33.33%> (+0.66%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
controllers/extendeddaemonset/controller.go 68.93% <0.00%> (ø)
...xtendeddaemonsetreplicaset/scheduler/predicates.go 21.34% <ø> (+9.10%) ⬆️
pkg/controller/utils/pod/create.go 25.53% <0.00%> (-0.56%) ⬇️
controllers/extendeddaemonsetreplicaset/filters.go 70.37% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7eb5d61...f1b4b02. Read the comment docs.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

@L3n41c L3n41c marked this pull request as ready for review October 1, 2020 12:09
@L3n41c L3n41c requested a review from a team as a code owner October 1, 2020 12:09
@vboulineau
Copy link
Contributor

We should be able to test this in E2E tests by building Nodes with custom taints.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

@L3n41c L3n41c merged commit ae25ec7 into master Oct 2, 2020
@L3n41c L3n41c deleted the lenaic/unschedulable_nodes branch October 2, 2020 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component/controller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants