Improve defaulting when spec.Agent|ClusterAgent empty #351
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
* Improve agent defaulting by enabling it if `spec.Agent` is empty. * Improve ClusterAgent defaulting by enabling it if `spec.ClusterAgent` is empty. * Fix credentials verification function.
clamoriniere
force-pushed
the
clamorinere/defaulting-fix
branch
from
dd88076
to
8c9b7ce
Compare
Codecov Report
@@ Coverage Diff @@
## main #351 +/- ##
==========================================
- Coverage 64.36% 64.36% -0.01%
==========================================
Files 62 62
Lines 6879 6907 +28
==========================================
+ Hits 4428 4446 +18
- Misses 2135 2144 +9
- Partials 316 317 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
clamoriniere
changed the title
| @@ -902,7 +909,10 @@ func DefaultDatadogFeaturePrometheusScrape(ft *DatadogFeatures) *PrometheusScrap | |||
| func DefaultDatadogFeatureNetworkMonitoring(ft *DatadogFeatures) *NetworkMonitoringConfig { | |||
| if ft.NetworkMonitoring == nil { | |||
| ft.NetworkMonitoring = &NetworkMonitoringConfig{Enabled: NewBoolPointer(false)} | |||
| return ft.NetworkMonitoring | |||
|
|
|||
| if !BoolValue(ft.NetworkMonitoring.Enabled) { | |||
There was a problem hiding this comment.
This seems like it'll always be the case, why adding it?
There was a problem hiding this comment.
if never we change the default Enabled value.
I tried to use the same pattern on every Default... fonction
| @@ -920,15 +930,23 @@ func DefaultDatadogFeatureNetworkMonitoring(ft *DatadogFeatures) *NetworkMonitor | |||
| func DefaultDatadogAgentSpecClusterAgent(clusterAgent *DatadogAgentSpecClusterAgentSpec) *DatadogAgentSpecClusterAgentSpec { | |||
| if IsEqualStruct(*clusterAgent, DatadogAgentSpecClusterAgentSpec{}) { | |||
| clusterAgent.Enabled = NewBoolPointer(true) | |||
| return clusterAgent | |||
|
|
|||
| if !BoolValue(clusterAgent.Enabled) { | |||
There was a problem hiding this comment.
This will never be the case I believe
There was a problem hiding this comment.
Agree, it was to follow the same pattern everywhere.
| @@ -999,7 +1017,10 @@ func DefaultDatadogAgentSpecClusterAgentConfig(dca *DatadogAgentSpecClusterAgent | |||
| func DefaultExternalMetrics(conf *ClusterAgentConfig) *ExternalMetricsConfig { | |||
| if conf.ExternalMetrics == nil { | |||
| conf.ExternalMetrics = &ExternalMetricsConfig{Enabled: NewBoolPointer(false)} | |||
| return conf.ExternalMetrics | |||
|
|
|||
| if !BoolValue(conf.ExternalMetrics.Enabled) { | |||
| @@ -1019,7 +1040,10 @@ func DefaultExternalMetrics(conf *ClusterAgentConfig) *ExternalMetricsConfig { | |||
| func DefaultAdmissionController(conf *ClusterAgentConfig) *AdmissionControllerConfig { | |||
| if conf.AdmissionController == nil { | |||
| conf.AdmissionController = &AdmissionControllerConfig{Enabled: NewBoolPointer(defaultAdmissionControllerEnabled)} | |||
| return conf.AdmissionController | |||
|
|
|||
| if !BoolValue(conf.AdmissionController.Enabled) { | |||
| @@ -1075,7 +1099,10 @@ func DefaultDatadogClusterAgentImage(dca *DatadogAgentSpecClusterAgentSpec, name | |||
| func DefaultDatadogAgentSpecClusterChecksRunner(clusterChecksRunner *DatadogAgentSpecClusterChecksRunnerSpec) *DatadogAgentSpecClusterChecksRunnerSpec { | |||
| if IsEqualStruct(clusterChecksRunner, DatadogAgentSpecClusterChecksRunnerSpec{}) { | |||
| clusterChecksRunner.Enabled = NewBoolPointer(false) | |||
| return clusterChecksRunner | |||
|
|
|||
| if !BoolValue(clusterChecksRunner.Enabled) { | |||
| @@ -763,7 +766,10 @@ func getClusterAgentMetricsProviderPort(config datadoghqv1alpha1.ClusterAgentCon | |||
| } | |||
|
|
|||
| func getAdmissionControllerServiceName(dda *datadoghqv1alpha1.DatadogAgent) string { | |||
| if isClusterAgentEnabled(dda.Spec.ClusterAgent) && dda.Spec.ClusterAgent.Config.AdmissionController != nil && dda.Spec.ClusterAgent.Config.AdmissionController.ServiceName != nil { | |||
| if isClusterAgentEnabled(dda.Spec.ClusterAgent) && | |||
| dda.Spec.ClusterAgent.Config != nil && | |||
There was a problem hiding this comment.
why is the config necessary for the admission controller?
There was a problem hiding this comment.
because the function takes value in dda.Spec.ClusterAgent.Config.AdmissionController.
due to the bug in the DefaultClusterAgent() when if IsEqualStruct(*clusterAgent, DatadogAgentSpecClusterAgentSpec{}) was true, the dda.Spec.ClusterAgent.Config was nil.
I think it is ok to keep this change, because it is always better to check point value before using it.
What does this PR do?
spec.Agentis empty.spec.ClusterAgentis empty.
Motivation
DatadogAgent should valide even if only "spec.Credentials" and
spec.Featuresareprovided.
Additional Notes
N/A
Describe your test plan
Deploy the example:
examples/datadogagent/datadog-agent-with-credential-secret.yamlExpected results: