[Orch] Add CRD RC handler #1340

JLineaweaver · 2024-08-02T19:56:39Z

What does this PR do?

Adds remote config support for custom resources in the operator. This new code will merge the user defined configuration with the configuration stored in remote config. Operator users with remote config enabled should no longer require a custom operator configuration to send custom 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?

Cluster Agent: v7.42

Describe your test plan

RC is only available on datad0g.com. Update operator to use staging api key and app key. Update DD_SITE to datad0g.com.
Make sure remoteConfig is enabled on the Operator deployment -remoteConfigEnabled=true in the args of the container
Make sure operator has list and watch permission on * / *
Update DatadogAgent to include

    orchestratorExplorer:
      enabled: true
    remoteConfiguration:
      enabled: true

Turn on/off custom resources indexing in UI
You should be able to see this section in status of DatadogAgent

  remoteConfigConfiguration:
    features:
      orchestratorExplorer:
        customResources:
        - datadoghq.com/v2alpha1/datadogagents
        - mutations.gatekeeper.sh/v1/assign
        - kueue.x-k8s.io/v1beta1/admissionchecks
        - test.com/v1/josh
        ........

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

github-actions

This pull request does not contain a valid label. Please add one of the following labels: bug, enhancement, refactoring, documentation, tooling, dependencies

codecov-commenter · 2024-08-02T20:40:21Z

Codecov Report

Attention: Patch coverage is 9.81595% with 147 lines in your changes missing coverage. Please review.

Project coverage is 48.65%. Comparing base (d5201ff) to head (8a16d90).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/remoteconfig/orchestrator_k8s_crd.go	0.00%	92 Missing ⚠️
pkg/remoteconfig/updater.go	0.00%	48 Missing ⚠️
...tadogagent/feature/orchestratorexplorer/feature.go	69.56%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1340      +/-   ##
==========================================
- Coverage   48.88%   48.65%   -0.23%     
==========================================
  Files         223      224       +1     
  Lines       19728    19850     +122     
==========================================
+ Hits         9644     9658      +14     
- Misses       9577     9685     +108     
  Partials      507      507

Flag	Coverage Δ
unittests	`48.65% <9.81%> (-0.23%)`	⬇️

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

Files with missing lines	Coverage Δ
...tadogagent/feature/orchestratorexplorer/feature.go	`77.48% <69.56%> (-1.75%)`	⬇️
pkg/remoteconfig/updater.go	`0.00% <0.00%> (ø)`
pkg/remoteconfig/orchestrator_k8s_crd.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 d5201ff...8a16d90. Read the comment docs.

pkg/remoteconfig/orchestrator_k8s_crd.go

timothyalexandersoftware · 2024-08-15T13:15:14Z

pkg/remoteconfig/updater.go

+	CoreAgent     *CoreAgentFeaturesConfig      `json:"config,omitempty"`
+	SystemProbe   *SystemProbeFeaturesConfig    `json:"system_probe,omitempty"`
+	SecurityAgent *SecurityAgentFeaturesConfig  `json:"security_agent,omitempty"`
+	CRDs          *CustomResourceDefinitionURLs `json:"crds,omitempty"`


Take this with a grain of salt since I'm not too familiar with the operator, but this looks to me like structurally, configs are broken out by agent binary. Should the CRD configs be wrapped inside a ClusterAgentFeatureConfig type?

Yeah that's a great call. I think this is a good idea for future work

JLineaweaver · 2024-09-17T15:53:13Z

pkg/remoteconfig/updater.go

@@ -137,7 +146,7 @@ func (r *RemoteConfigUpdater) Start(apiKey string, site string, clusterName stri
 		"",
 		r.serviceConf.baseRawURL,
 		r.serviceConf.hostname,
-		[]string{fmt.Sprintf("cluster_name:%s", r.serviceConf.clusterName)},
+		r.getTags,


This is a required breaking change

…1473)

kangyili · 2024-10-18T15:25:28Z

/merge

dd-devflow · 2024-10-18T15:25:39Z

🚂 MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

kangyili · 2024-10-18T15:32:29Z

/remove

dd-devflow · 2024-10-18T15:32:35Z

🚂 Devflow: /remove

dd-devflow · 2024-10-18T15:32:37Z

⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

If you need support, contact us on Slack #devflow!

celenechang · 2024-10-18T18:40:21Z

go.sum

@@ -65,6 +65,8 @@ github.com/DataDog/datadog-agent/pkg/proto v0.55.0-rc.10 h1:ERkVmUoDPttyVKSCJM1f
 github.com/DataDog/datadog-agent/pkg/proto v0.55.0-rc.10/go.mod h1:gHkSUTn6H6UEZQHY3XWBIGNjfI3Tdi0IxlrxIFBWDwU=
 github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.55.0-rc.10 h1:nwJ2JWfjCmf6tpJD1RYHh4JV5HO2Njg6smxOGi8MyOE=
 github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.55.0-rc.10/go.mod h1:3yFk56PJ57yS1GqI9HAsS4PSlAeGCC9RQA7jxKzYj6g=
+github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.59.0-rc.1 h1:CIXKFvUsp5zgE+egvx+QrRTw8r54FMPZVu+z25UNqWs=


are there any changes between rc1 and rc4?

There is only go mod change. The latest 2 commits: https://github.com/DataDog/datadog-agent/commits/pkg/remoteconfig/state/v0.59.0-rc.4/pkg/remoteconfig/state

internal/controller/datadogagent/feature/orchestratorexplorer/feature.go

internal/controller/datadogagent/feature/orchestratorexplorer/feature_test.go

pkg/remoteconfig/updater.go

celenechang · 2024-10-18T19:04:20Z

pkg/remoteconfig/orchestrator_k8s_crd.go

+	}
+
+	// Merge configs
+	var finalConfig DatadogAgentRemoteConfig


i wonder if it would be clearer to return ClusterAgentFeaturesConfig instead of the entire DatadogAgentRemoteConfig. i got nervous for a moment that we were going to overwrite other parts of the status

I think it follows the default pattern, which always returns the entire configuration.

datadog-operator/pkg/remoteconfig/updater.go

Line 317 in 03611c4

func (r *RemoteConfigUpdater) parseReceivedUpdates(updates map[string]state.RawConfig, applyStatus func(string, state.ApplyStatus)) (DatadogAgentRemoteConfig, error) {

Ultimately, we need to adhere to the function getAndUpdateDatadogAgentWithRetry, which takes the complete configuration and an update function as its input.

datadog-operator/pkg/remoteconfig/updater.go

Line 253 in 03611c4

func (r *RemoteConfigUpdater) getAndUpdateDatadogAgentWithRetry(ctx context.Context, cfg DatadogAgentRemoteConfig, f func(v2alpha1.DatadogAgent, DatadogAgentRemoteConfig) error) error {

internal/controller/datadogagent/feature/orchestratorexplorer/feature_test.go

levan-m · 2024-10-18T21:45:54Z

pkg/remoteconfig/updater.go

@@ -63,6 +65,7 @@ type DatadogAgentRemoteConfig struct {
 	ID            string                       `json:"id,omitempty"`
 	Name          string                       `json:"name,omitempty"`
 	CoreAgent     *CoreAgentFeaturesConfig     `json:"config,omitempty"`
+	ClusterAgent  *ClusterAgentFeaturesConfig  `json:"cluster_agent,omitempty"`


This structure is unmarshalled from the RawConfig here and

datadog-operator/pkg/remoteconfig/updater.go

Lines 330 to 331 in 03611c4

var configData DatadogAgentRemoteConfig

if err := json.Unmarshal(c.Config, &configData); err != nil {

And this config won't contain ClusterAgentFeatureConfig so curious why is this added here?

This struct has several updaters, and we have the CRD updater running after the agent updater, where we populate the ClusterAgent field.

datadog-operator/pkg/remoteconfig/updater.go

Line 183 in 03611c4

rcClient.Subscribe(string(state.ProductOrchestratorK8sCRDs), r.crdConfigUpdateCallback)

datadog-operator/pkg/remoteconfig/orchestrator_k8s_crd.go

Lines 74 to 81 in 03611c4

if finalConfig.ClusterAgent == nil {

finalConfig.ClusterAgent = &ClusterAgentFeaturesConfig{}

}

if finalConfig.ClusterAgent.CRDs == nil {

finalConfig.ClusterAgent.CRDs = &CustomResourceDefinitionURLs{}

}

finalConfig.ClusterAgent.CRDs.Crds = &crds

Sorry if my question wasn't clear. I'm confused why the ClusterAgentFeaturesConfig is added here.

This structure

datadog-operator/pkg/remoteconfig/updater.go

Lines 64 to 71 in 03611c4

type DatadogAgentRemoteConfig struct {

ID string `json:"id,omitempty"`

Name string `json:"name,omitempty"`

CoreAgent *CoreAgentFeaturesConfig `json:"config,omitempty"`

ClusterAgent *ClusterAgentFeaturesConfig `json:"cluster_agent,omitempty"`

SystemProbe *SystemProbeFeaturesConfig `json:"system_probe,omitempty"`

SecurityAgent *SecurityAgentFeaturesConfig `json:"security_agent,omitempty"`

}

is for RC product ProductAgentConfig supporting CoreAgent, SystemProbe and SecurityAgent features. It's subscribed here

datadog-operator/pkg/remoteconfig/updater.go

Line 181 in 03611c4

rcClient.Subscribe(string(state.ProductAgentConfig), r.agentConfigUpdateCallback)

While

datadog-operator/pkg/remoteconfig/updater.go

Line 183 in 03611c4

rcClient.Subscribe(string(state.ProductOrchestratorK8sCRDs), r.crdConfigUpdateCallback)

subscribes a different ProductOrchestratorK8sCRDs. So I'm curious why it's put under DatadogAgentRemoteConfig structure instead of having it's own structure?

levan-m · 2024-10-18T21:49:24Z

pkg/remoteconfig/orchestrator_k8s_crd.go

+	if finalConfig.ClusterAgent.CRDs == nil {
+		finalConfig.ClusterAgent.CRDs = &CustomResourceDefinitionURLs{}
+	}
+	finalConfig.ClusterAgent.CRDs.Crds = &crds


Current implementation for security products unmarshals configuration_order and uses to merge multiple configs:

datadog-operator/pkg/remoteconfig/updater.go

Lines 323 to 324 in 03611c4

if c.Metadata.ID == "configuration_order" {

if err := json.Unmarshal(c.Config, &order); err != nil {

do we need similar logic for CRDs too?

In the agentConfigUpdateCallback, we have several components that need to be updated and merged in a specific order.

datadog-operator/pkg/remoteconfig/updater.go

Lines 181 to 183 in 03611c4

rcClient.Subscribe(string(state.ProductAgentConfig), r.agentConfigUpdateCallback)

rcClient.Subscribe(string(state.ProductOrchestratorK8sCRDs), r.crdConfigUpdateCallback)

But for CRD, we only need to update its specific section, so maintaining an order isn't necessary in this case.

* [Orch] Add CRD RC handler * Remove print statement * Refactor to separate file * Move last function * Use CRD specific status * Add cluster agent config to RC * Correctly set product * Fix product and add logging * Add fixes for crd nil pointers * Revert accidental commit * Update dependencies and add tag getter function * Go.mod change * Reset go.mod * Update remoteconfig/state * Fix updater package and work sum * Clean up logs and force restart of DCA on CR changes * Add a lock around get and update of DDA * Improve comments and change test to use orchexp for annotation * Change to not do annotations every single time * Fill in orchestrator explorer for tests * Go mod update * Modify retry logic so it doesn't it for the entire update * Fix go.mod * Check in config crd stuff * Remove hard coded product and update go.mod * Revert go.mod back and fix errors * Go.sum update * overwrite cr by incoming data instead of appending to the old data (#1473) * feedback * feedback * rename to OrchestratorK8sCRDRemoteConfig --------- Co-authored-by: Kangyi LI <kangyi.li@datadoghq.com> Co-authored-by: levan-m <116471169+levan-m@users.noreply.github.com>

* [Orch] Add CRD RC handler * Remove print statement * Refactor to separate file * Move last function * Use CRD specific status * Add cluster agent config to RC * Correctly set product * Fix product and add logging * Add fixes for crd nil pointers * Revert accidental commit * Update dependencies and add tag getter function * Go.mod change * Reset go.mod * Update remoteconfig/state * Fix updater package and work sum * Clean up logs and force restart of DCA on CR changes * Add a lock around get and update of DDA * Improve comments and change test to use orchexp for annotation * Change to not do annotations every single time * Fill in orchestrator explorer for tests * Go mod update * Modify retry logic so it doesn't it for the entire update * Fix go.mod * Check in config crd stuff * Remove hard coded product and update go.mod * Revert go.mod back and fix errors * Go.sum update * overwrite cr by incoming data instead of appending to the old data (#1473) * feedback * feedback * rename to OrchestratorK8sCRDRemoteConfig --------- Co-authored-by: Joshua Lineaweaver <JLineaweaver@gmail.com> Co-authored-by: Kangyi LI <kangyi.li@datadoghq.com> Co-authored-by: Fanny Jiang <fanny.jiang@datadoghq.com>

[Orch] Add CRD RC handler

025921e

github-actions bot requested changes Aug 2, 2024

View reviewed changes

JLineaweaver added the enhancement New feature or request label Aug 2, 2024

github-actions bot approved these changes Aug 2, 2024

View reviewed changes

Remove print statement

076f859

Merge branch 'main' into jlineaweaver/cap-1652

8375683

JLineaweaver added this to the v1.9.0 milestone Aug 9, 2024

JLineaweaver added 3 commits August 13, 2024 10:05

Refactor to separate file

1476ba6

Move last function

ddc5784

Use CRD specific status

102ae6d

timothyalexandersoftware reviewed Aug 15, 2024

View reviewed changes

pkg/remoteconfig/orchestrator_k8s_crd.go Show resolved Hide resolved

timothyalexandersoftware reviewed Aug 15, 2024

View reviewed changes

Add cluster agent config to RC

51d2cf9

levan-m modified the milestones: v1.9.0, v1.10.0 Aug 26, 2024

JLineaweaver added 6 commits September 12, 2024 13:22

Correctly set product

4e3052f

Fix product and add logging

b0d74d1

Add fixes for crd nil pointers

f996a47

Revert accidental commit

1ebee5b

Merge branch 'main' into jlineaweaver/cap-1652

c001b38

Update dependencies and add tag getter function

2e14647

JLineaweaver commented Sep 17, 2024

View reviewed changes

Merge branch 'main' into jlineaweaver/cap-1652

994024f

JLineaweaver force-pushed the jlineaweaver/cap-1652 branch 2 times, most recently from 72892db to 994024f Compare September 20, 2024 13:52

JLineaweaver and others added 4 commits September 24, 2024 10:45

Merge branch 'main' into jlineaweaver/cap-1652

e3e4c4d

Go.mod change

6a9cbda

Merge branch 'main' into jlineaweaver/cap-1652

4cd558a

Reset go.mod

2973537

levan-m modified the milestones: v1.11.0, v1.10.0 Oct 16, 2024

overwrite cr by incoming data instead of appending to the old data (#…

03611c4

…1473)

dd-devflow bot added the mergequeue-status: waiting label Oct 18, 2024

dd-devflow bot removed the mergequeue-status: waiting label Oct 18, 2024

dd-devflow bot added the mergequeue-status: removed label Oct 18, 2024

celenechang reviewed Oct 18, 2024

View reviewed changes

internal/controller/datadogagent/feature/orchestratorexplorer/feature.go Outdated Show resolved Hide resolved

celenechang reviewed Oct 18, 2024

View reviewed changes

internal/controller/datadogagent/feature/orchestratorexplorer/feature_test.go Outdated Show resolved Hide resolved

celenechang reviewed Oct 18, 2024

View reviewed changes

pkg/remoteconfig/updater.go Outdated Show resolved Hide resolved

celenechang reviewed Oct 18, 2024

View reviewed changes

internal/controller/datadogagent/feature/orchestratorexplorer/feature_test.go Outdated Show resolved Hide resolved

levan-m reviewed Oct 18, 2024

View reviewed changes

kangyili and others added 4 commits October 21, 2024 09:25

feedback

c3fcffe

feedback

a0b81b9

rename to OrchestratorK8sCRDRemoteConfig

c7e4cc1

Merge branch 'main' into jlineaweaver/cap-1652

59f6780

levan-m approved these changes Oct 22, 2024

View reviewed changes

Merge branch 'main' into jlineaweaver/cap-1652

8a16d90

levan-m merged commit ae5b9e1 into main Oct 22, 2024
19 checks passed

levan-m deleted the jlineaweaver/cap-1652 branch October 22, 2024 15:23

levan-m mentioned this pull request Oct 22, 2024

[Orch] Add CRD RC handler (#1340) #1479

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Orch] Add CRD RC handler #1340

[Orch] Add CRD RC handler #1340

JLineaweaver commented Aug 2, 2024 •

edited by tbavelier

Loading

github-actions bot left a comment

codecov-commenter commented Aug 2, 2024 •

edited

Loading

timothyalexandersoftware Aug 15, 2024

JLineaweaver Aug 15, 2024

JLineaweaver Aug 15, 2024

JLineaweaver Sep 17, 2024

kangyili commented Oct 18, 2024

dd-devflow bot commented Oct 18, 2024

kangyili commented Oct 18, 2024

dd-devflow bot commented Oct 18, 2024

dd-devflow bot commented Oct 18, 2024

celenechang Oct 18, 2024

kangyili Oct 21, 2024 •

edited

Loading

celenechang Oct 18, 2024 •

edited

Loading

kangyili Oct 21, 2024

levan-m Oct 18, 2024

kangyili Oct 21, 2024

levan-m Oct 21, 2024

levan-m Oct 18, 2024

kangyili Oct 21, 2024

	var configData DatadogAgentRemoteConfig
	if err := json.Unmarshal(c.Config, &configData); err != nil {

	if finalConfig.ClusterAgent == nil {
	finalConfig.ClusterAgent = &ClusterAgentFeaturesConfig{}
	}
	if finalConfig.ClusterAgent.CRDs == nil {
	finalConfig.ClusterAgent.CRDs = &CustomResourceDefinitionURLs{}
	}
	finalConfig.ClusterAgent.CRDs.Crds = &crds

	type DatadogAgentRemoteConfig struct {
	ID string `json:"id,omitempty"`
	Name string `json:"name,omitempty"`
	CoreAgent *CoreAgentFeaturesConfig `json:"config,omitempty"`
	ClusterAgent *ClusterAgentFeaturesConfig `json:"cluster_agent,omitempty"`
	SystemProbe *SystemProbeFeaturesConfig `json:"system_probe,omitempty"`
	SecurityAgent *SecurityAgentFeaturesConfig `json:"security_agent,omitempty"`
	}

	if c.Metadata.ID == "configuration_order" {
	if err := json.Unmarshal(c.Config, &order); err != nil {

	rcClient.Subscribe(string(state.ProductAgentConfig), r.agentConfigUpdateCallback)

	rcClient.Subscribe(string(state.ProductOrchestratorK8sCRDs), r.crdConfigUpdateCallback)

[Orch] Add CRD RC handler #1340

[Orch] Add CRD RC handler #1340

Conversation

JLineaweaver commented Aug 2, 2024 • edited by tbavelier Loading

What does this PR do?

Motivation

Additional Notes

Minimum Agent Versions

Describe your test plan

Checklist

github-actions bot left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kangyili commented Oct 18, 2024

dd-devflow bot commented Oct 18, 2024

kangyili commented Oct 18, 2024

dd-devflow bot commented Oct 18, 2024

dd-devflow bot commented Oct 18, 2024

Choose a reason for hiding this comment

kangyili Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

celenechang Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLineaweaver commented Aug 2, 2024 •

edited by tbavelier

Loading

codecov-commenter commented Aug 2, 2024 •

edited

Loading

kangyili Oct 21, 2024 •

edited

Loading

celenechang Oct 18, 2024 •

edited

Loading