-
Notifications
You must be signed in to change notification settings - Fork 1k
add(datadog-csi-driver): tolerations support #2134
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
Conversation
adel121
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution.
Looks good.
Some things to keep in mind:
- We need to bump the csi chart version in datadog chart dependencies.
- Tolerations will be applied if driver is installed using the datadog-csi-driver directly. If it is installed indirectly via datadog-agent chart, tolerations should be set in
values.yamlas follows to pass it down to the child chart:
datadog-csi-driver:
tolerations:
- key: "k"
operator: "Exists"
effect: "NoSchedule"
Co-authored-by: Adel Haj Hassan <[email protected]>
@adel121 I have just updated the dependencies as well as the |
|
Thanks for taking care of this of updating the datadog-agent, and apologies for any confusion caused in my previous comment. By But unfortunately this will fail the CI because version I suggest not updating datadog-agent chart in this PR. Let's focus on CSI for now and we can bump the csi dependency version and add the examples (Which look great 🙇 ). Feel free to open a separate PR for datadog-agent chart if you prefer, and I can approve it once CSI chart is published after merging this current PR. |
adel121
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment, thanks!
This reverts commit 315166e.
Thanks for the review, got it - as the test will fail since the helm chart will not be there! 🥇 |
|
Hello The CI was having issues for the external contribution branches. We cherry-picked your commits and created a branch internally. It is already merged. Closing this one. Thank you for contributing |
What this PR does / why we need it:
The CSI driver's Daemonset should be able to be provisioned in tainted nodes - otherwise we will get errors like the following if we try to access it through a workload in a tainted node:
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
<chartName>/minor-version,<chartName>/patch-version, or<chartName>/no-version-bump)datadogordatadog-operatorchart or value changes, update the test baselines (run:make update-test-baselines)GitHub CI takes care of the below, but are still required:
.github/helm-docs.sh)CHANGELOG.mdhas been updatedREADME.md