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

[CECO-1763] Controller for generic custom resource #1534

Merged
merged 30 commits into from
Jan 8, 2025

Conversation

tbavelier
Copy link
Member

@tbavelier tbavelier commented Nov 21, 2024

What does this PR do?

Introduces a generic controller relying on Datadog Go client to handle CRUD operations on multiple resources. Initial support includes notebooks and synthetics (api/browser tests).
Examples:

apiVersion: datadoghq.com/v1alpha1
kind: DatadogGenericResource
metadata:
  name: api-test
spec:
    type: synthetics_api_test
    jsonSpec: |-
        {
           [...]
        }
apiVersion: datadoghq.com/v1alpha1
kind: DatadogGenericResource
metadata:
  name: notebook
spec:
    type: notebook
    jsonSpec: |-
        {
           [...]
        }

Additional Notes

Next steps:

  • Document fully-fledged examples
  • Document onboarding strategy (e.g. export current resource from the API, create the associated YAML, deploy them)
  • Document internally how to add new resources
  • UI button to export to manifest
  • Add option to start the controller in the Helm chart

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

  1. Include datadogGenericResourceEnabled=true in args for the manager container to enable the controller, as well as DD_API_KEY and DD_APP_KEY in the envs
  2. Deploy examples
  3. Update their jsonSpec
  4. Ensure the changes are reflected in the UI
  5. Delete the resources
  6. Ensure they are removed from the UI

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

@tbavelier tbavelier added this to the v1.12.0 milestone Nov 21, 2024
@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 39.47368% with 299 lines in your changes missing coverage. Please review.

Project coverage is 48.75%. Comparing base (db00883) to head (df68861).

Files with missing lines Patch % Lines
...al/controller/datadoggenericresource/controller.go 38.97% 74 Missing and 9 partials ⚠️
...nternal/controller/datadoggenericresource/utils.go 57.05% 67 Missing ⚠️
...al/controller/datadoggenericresource/synthetics.go 0.00% 48 Missing ⚠️
...nal/controller/datadoggenericresource/notebooks.go 17.02% 39 Missing ⚠️
...al/controller/datadoggenericresource_controller.go 0.00% 15 Missing ⚠️
pkg/datadogclient/client.go 0.00% 14 Missing ⚠️
cmd/main.go 0.00% 12 Missing ⚠️
internal/controller/setup.go 31.25% 11 Missing ⚠️
...nal/controller/datadoggenericresource/finalizer.go 74.35% 7 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1534      +/-   ##
==========================================
- Coverage   48.94%   48.75%   -0.20%     
==========================================
  Files         227      235       +8     
  Lines       20636    21120     +484     
==========================================
+ Hits        10101    10296     +195     
- Misses      10010    10287     +277     
- Partials      525      537      +12     
Flag Coverage Δ
unittests 48.75% <39.47%> (-0.20%) ⬇️

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

Files with missing lines Coverage Δ
...datadoghq/v1alpha1/datadoggenericresource_types.go 100.00% <100.00%> (ø)
...oghq/v1alpha1/datadoggenericresource_validation.go 100.00% <100.00%> (ø)
...nal/controller/datadoggenericresource/finalizer.go 74.35% <74.35%> (ø)
internal/controller/setup.go 49.33% <31.25%> (-2.16%) ⬇️
cmd/main.go 0.00% <0.00%> (ø)
pkg/datadogclient/client.go 0.00% <0.00%> (ø)
...al/controller/datadoggenericresource_controller.go 0.00% <0.00%> (ø)
...nal/controller/datadoggenericresource/notebooks.go 17.02% <17.02%> (ø)
...al/controller/datadoggenericresource/synthetics.go 0.00% <0.00%> (ø)
...nternal/controller/datadoggenericresource/utils.go 57.05% <57.05%> (ø)
... and 1 more

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 db00883...df68861. Read the comment docs.

@tbavelier tbavelier added enhancement New feature or request and removed do-not-merge labels Nov 29, 2024
@tbavelier tbavelier marked this pull request as ready for review November 29, 2024 15:49
@tbavelier tbavelier requested a review from a team as a code owner November 29, 2024 15:49
api/datadoghq/v1alpha1/datadoggenericcr_types.go Outdated Show resolved Hide resolved
api/datadoghq/v1alpha1/datadoggenericcr_types.go Outdated Show resolved Hide resolved
type operation string

const (
// mockSubresource is used to mock the subresource in tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way to avoid this in the prod code?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could mock an HTTP client for the CRUD Go api, so we send requests there instead of the real API/a non existing client but that would be more convoluted/a general improvement for other controllers to have this mock server/client for API requests

internal/controller/datadoggenericcr_controller.go Outdated Show resolved Hide resolved
@levan-m
Copy link
Contributor

levan-m commented Jan 4, 2025

Left some suggestions/nits. Overall looks good but handling of multiple resource APIs (in utils.go) can be refactored to more maintainable code.

@levan-m levan-m merged commit 24de754 into main Jan 8, 2025
19 checks passed
@levan-m levan-m deleted the tbavelier/generic_crd_controller branch January 8, 2025 15:13
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.

3 participants