datadog-operator icon indicating copy to clipboard operation
datadog-operator copied to clipboard

add csi config

Open adel121 opened this issue 7 months ago • 1 comments

What does this PR do?

A brief description of the change being made with this pull request.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

Write there any instructions and details you may have to test your PR.

Checklist

  • [ ] PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • [ ] PR has a milestone or the qa/skip-qa label

adel121 avatar Jun 16 '25 14:06 adel121

Codecov Report

:x: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 38.68%. Comparing base (8f25eca) to head (5b4fb6f).

Files with missing lines Patch % Lines
pkg/testutils/builder.go 0.00% 3 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1981   +/-   ##
=======================================
  Coverage   38.67%   38.68%           
=======================================
  Files         251      251           
  Lines       25235    25244    +9     
=======================================
+ Hits         9759     9765    +6     
- Misses      14915    14918    +3     
  Partials      561      561           
Flag Coverage Δ
unittests 38.68% <66.66%> (+<0.01%) :arrow_up:

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

Files with missing lines Coverage Δ
internal/controller/datadogagent/global/global.go 54.58% <100.00%> (+1.16%) :arrow_up:
pkg/testutils/builder.go 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

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

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar Jun 16 '25 15:06 codecov-commenter

Hi Adel, Thanks for this PR. I added some edits. In addition to those changes, it seems that the descriptions have both Enabled and enables. Which tense is this meant to use? Let me know if you have any questions.

Hello @iadjivon , thank you for the quick review.

What I meant to say is that the property "Enabled" enables the usage of csi driver.

If you think it is better to just say "Enables usage of CSI driver ...", we can change it.

adel121 avatar Jul 16 '25 09:07 adel121

Hi Adel, Thanks for this PR. I added some edits. In addition to those changes, it seems that the descriptions have both Enabled and enables. Which tense is this meant to use? Let me know if you have any questions.

Hello @iadjivon , thank you for the quick review.

What I meant to say is that the property "Enabled" enables the usage of csi driver.

If you think it is better to just say "Enables usage of CSI driver ...", we can change it.

Ah, I see! I think in this case using Enables works well and avoids any confusion. For example:

"description": "Enables usage of CSI driver in Datadog Agent.\nRequires installation of Datadog CSI Driver https://github.com/DataDog/helm-charts/tree/main/charts/datadog-csi-driver\nDefault: false",

Also, note that the D and A are capitalized when writing the Datadog Agent.
We should be all set after these changes! Thank you Adel!

iadjivon avatar Jul 17 '25 17:07 iadjivon

Hi Adel, Thanks for this PR. I added some edits. In addition to those changes, it seems that the descriptions have both Enabled and enables. Which tense is this meant to use? Let me know if you have any questions.

Hello @iadjivon , thank you for the quick review. What I meant to say is that the property "Enabled" enables the usage of csi driver. If you think it is better to just say "Enables usage of CSI driver ...", we can change it.

Ah, I see! I think in this case using Enables works well and avoids any confusion. For example:

"description": "Enables usage of CSI driver in Datadog Agent.\nRequires installation of Datadog CSI Driver https://github.com/DataDog/helm-charts/tree/main/charts/datadog-csi-driver\nDefault: false",

Also, note that the D and A are capitalized when writing the Datadog Agent. We should be all set after these changes! Thank you Adel!

I updated the code. Thanks!

As a side note, I noticed that a huge number of property comments in the codebase start by "Enabled enables". So it might be something worth updating for consistency. I didn't update them in this PR because they are irrelevant to it.

adel121 avatar Jul 18 '25 10:07 adel121