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

service discovery: Add operator support #1449

Merged

Conversation

guyarb
Copy link
Contributor

@guyarb guyarb commented Oct 6, 2024

What does this PR do?

Adds service-discovery check to the operator.

Motivation

Allow to control service-discovery from the operator.

Additional Notes

Minimum Agent Versions

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

  • Agent: v7.58.0
  • Cluster Agent: N/A

Describe your test plan

  • Create kind cluster
  • Follow instructions installations instructions https://github.com/DataDog/datadog-operator/blob/main/docs/how-to-contribute.md#testing-the-operator-for-development
  • Use the following configuration
    apiVersion: datadoghq.com/v2alpha1
    kind: DatadogAgent
    metadata:
      name: datadog
    spec:
      global:
        env:
          - name: DD_HOSTNAME
            value: "example-kind"
        clusterName: my-example-cluster
        credentials:
          apiSecret:
            secretName: datadog-secret
            keyName: api-key
          appSecret:
            secretName: datadog-secret
            keyName: app-key
      features:
        serviceDiscovery:
          enabled: true
      override:
        nodeAgent:
          image:
            tag: 7.59.0-linux-jmx
  • Use api key to ddstaging
  • Wait 5m and check you have entries here

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

@guyarb guyarb requested review from a team as code owners October 6, 2024 13:52
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

@guyarb guyarb added the enhancement New feature or request label Oct 6, 2024
@guyarb guyarb added this to the v1.10.0 milestone Oct 6, 2024
@guyarb guyarb force-pushed the guy.arbitman/USMON-1227-service-discovery-to-agent-operator branch from d583968 to cc34cfd Compare October 6, 2024 14:08
@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2024

Codecov Report

Attention: Patch coverage is 73.33333% with 20 lines in your changes missing coverage. Please review.

Project coverage is 48.95%. Comparing base (009f82d) to head (9757d42).

Files with missing lines Patch % Lines
...r/datadogagent/feature/servicediscovery/feature.go 83.33% 9 Missing and 1 partial ⚠️
internal/controller/testutils/agent.go 0.00% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1449      +/-   ##
==========================================
+ Coverage   48.86%   48.95%   +0.09%     
==========================================
  Files         225      226       +1     
  Lines       19959    20034      +75     
==========================================
+ Hits         9753     9808      +55     
- Misses       9695     9714      +19     
- Partials      511      512       +1     
Flag Coverage Δ
unittests 48.95% <73.33%> (+0.09%) ⬆️

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

Files with missing lines Coverage Δ
api/datadoghq/v2alpha1/datadogagent_default.go 89.88% <100.00%> (+0.14%) ⬆️
api/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
internal/controller/datadogagent/controller.go 55.17% <ø> (ø)
...r/datadogagent/feature/servicediscovery/feature.go 83.33% <83.33%> (ø)
internal/controller/testutils/agent.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 009f82d...9757d42. Read the comment docs.

Copy link
Contributor

@hestonhoffman hestonhoffman left a comment

Choose a reason for hiding this comment

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

Approved with a tiny edit

docs/configuration.v2alpha1.md Outdated Show resolved Hide resolved
@levan-m levan-m modified the milestones: v1.10.0, v1.11.0 Oct 10, 2024
Copy link
Contributor

@levan-m levan-m left a comment

Choose a reason for hiding this comment

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

I updated PR from main and CI is failing, could you please fix it?
Test case didn't work for me, the query doesn't give me any results, is this expected?

Otherwise, change looks good. Confirmed trace-agent container and DD_SERVICE_DISCOVERY are added correctly.

@@ -486,6 +488,14 @@ type USMFeatureConfig struct {
Enabled *bool `json:"enabled,omitempty"`
}

// ServiceDiscoveryFeatureConfig configures the service discovery check feature.
type ServiceDiscoveryFeatureConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a top level feature instead of being part of USM feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Service discovery is not a USM feature, it is a new product

@guyarb
Copy link
Contributor Author

guyarb commented Nov 10, 2024

Test case didn't work for me, the query doesn't give me any results, is this expected?

Which hostname did you use?
Did you verify the api key belongs with ddstaging?

Otherwise, change looks good. Confirmed trace-agent container and DD_SERVICE_DISCOVERY are added correctly.

The change does not affect trace-agent, but system-probe and agent

@levan-m
Copy link
Contributor

levan-m commented Nov 11, 2024

I'm not sure what happened but today I can see data populated in the table. I tried both example-levan and example-kind.
Regarding system-probe, sorry for confusion I misread the container name added after enabling the feature.

@levan-m levan-m merged commit 9ad6b9a into main Nov 11, 2024
20 checks passed
@levan-m levan-m deleted the guy.arbitman/USMON-1227-service-discovery-to-agent-operator branch November 11, 2024 19:21
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