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

Support for CRD-specific watch namespaces configuration #1235

Merged
merged 15 commits into from
Jul 12, 2024

Conversation

levan-m
Copy link
Contributor

@levan-m levan-m commented Jun 14, 2024

What does this PR do?

CECO-1021

For Reviewer: first commit is actual change; second commit is mostly refactor.

controller-runtime 0.16 allows cache configuration on resource level. This change adds CRD-specific watch namespace env vars and sets them on corresponding CRDs or related resources.

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

Test using DatadogMonitors CRD, use following manifests to create monitors in two different namespaces.

  • Namespace monitor1

    apiVersion: datadoghq.com/v1alpha1
    kind: DatadogMonitor
    metadata:
      name: datadog-monitor-test1
      namespace: monitor1
    spec:
      query: "avg(last_10m):avg:system.disk.in_use{*} by {host} > 0.5"
      type: "metric alert"
      name: "Test monitor made from DatadogMonitor monitor1"
      message: "We are running out of disk space!"
      tags:
        - "test:datadog"
  • Namespace monitor2

    apiVersion: datadoghq.com/v1alpha1
    kind: DatadogMonitor
    metadata:
      name: datadog-monitor-test2
      namespace: monitor2
    spec:
      query: "avg(last_10m):avg:system.disk.in_use{*} by {host} > 0.5"
      type: "metric alert"
      name: "Test monitor made from DatadogMonitor monitor2"
      message: "We are running out of disk space!"
      tags:
        - "test:datadog"

Operator doesn't watch namespace1 or namespace2.

  1. Run operator with datadogMonitorEnabled set to true (with API, APP key set), running in datadog namespace.
  2. Set default watch namespaces env variable to current namespace
        - name: WATCH_NAMESPACE
          valueFrom:
            fieldRef:
              fieldPath: metadata.namespace

Operator shouldn't reconcile the monitors from other namespaces (nothing logged or created).

Add monitor1 namespace

  1. In above setup add following env variable
      - name: DD_MONITOR_WATCH_NAMESPACE
        value: "monitor1"

Observe monitor from monitor1 namespace getting reconciled - check logs and datadog app to confirm it's created.

  1. Add monitor2 to above env var

Observer monitor in monitor2 reconciled as well.

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

@levan-m levan-m requested review from a team as code owners June 14, 2024 14:23
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, refactoring, documentation, tooling, dependencies

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, refactoring, documentation, tooling, dependencies

@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 63.63636% with 40 lines in your changes missing coverage. Please review.

Project coverage is 55.03%. Comparing base (4bc766f) to head (10a4a65).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1235      +/-   ##
==========================================
+ Coverage   54.87%   55.03%   +0.16%     
==========================================
  Files         243      243              
  Lines       28120    28165      +45     
==========================================
+ Hits        15430    15502      +72     
+ Misses      11815    11785      -30     
- Partials      875      878       +3     
Flag Coverage Δ
unittests 55.03% <63.63%> (+0.16%) ⬆️

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

Files Coverage Δ
main.go 0.00% <0.00%> (ø)
pkg/config/config.go 67.92% <67.96%> (+67.92%) ⬆️

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 4bc766f...10a4a65. Read the comment docs.

@levan-m levan-m added this to the v1.8.0 milestone Jun 14, 2024
Copy link
Contributor

@brett0000FF brett0000FF left a comment

Choose a reason for hiding this comment

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

I know you're still drafting this, but I left a couple very minor suggestions in the meantime. Please let Docs know if add more content and need another look!

docs/configuration.v1alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v1alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v1alpha1.md Outdated Show resolved Hide resolved
docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
@levan-m levan-m force-pushed the levan-m/watch-namespace-byObject branch from 2b402f3 to 18dc6cf Compare June 27, 2024 01:34
@levan-m levan-m changed the title [draft] watch namespace by object Support for CRD-specific watch namespaces configuration Jun 28, 2024
main.go Outdated Show resolved Hide resolved
Co-authored-by: Celene <celene@datadoghq.com>
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
levan-m and others added 5 commits July 8, 2024 14:40
Co-authored-by: Celene <celene@datadoghq.com>
Co-authored-by: Celene <celene@datadoghq.com>
Co-authored-by: Celene <celene@datadoghq.com>
Co-authored-by: Celene <celene@datadoghq.com>
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
@khewonc khewonc added enhancement New feature or request and removed do-not-merge labels Jul 11, 2024
levan-m and others added 3 commits July 11, 2024 11:44
Co-authored-by: Celene <celene@datadoghq.com>
Co-authored-by: khewonc <39867936+khewonc@users.noreply.github.com>
@levan-m levan-m merged commit 7ece56c into main Jul 12, 2024
19 checks passed
@levan-m levan-m deleted the levan-m/watch-namespace-byObject branch July 12, 2024 15:49
mftoure pushed a commit that referenced this pull request Oct 3, 2024
* CRD-specific watch namespaces

* Support for CRD-specific watch namespaces configuration

* Update main.go

Co-authored-by: Celene <celene@datadoghq.com>

* Update pkg/config/config.go

Co-authored-by: Celene <celene@datadoghq.com>

* Update pkg/config/config.go

Co-authored-by: Celene <celene@datadoghq.com>

* Update pkg/config/config.go

Co-authored-by: Celene <celene@datadoghq.com>

* Update pkg/config/config.go

Co-authored-by: Celene <celene@datadoghq.com>

* Watch nodes when introspection is enabled; Refactor

* Set DefaultNamespace to agent namespaces

* Update pkg/config/config.go

Co-authored-by: Celene <celene@datadoghq.com>

* Update pkg/config/config.go

Co-authored-by: khewonc <39867936+khewonc@users.noreply.github.com>

---------

Co-authored-by: Celene <celene@datadoghq.com>
Co-authored-by: khewonc <39867936+khewonc@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants