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

Fix cluster-agent defaulting to enable it by default #349

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

clamoriniere
Copy link
Collaborator

What does this PR do?

Fix the default of the cluster-agent to all be defaulted to enabled:true.

Motivation

In a specific when the section "spec.clusterAgent" is not provided
in the DatadogAgent instance, the cluster-agent is not defaulted to
spec.clusterAgent.enabled:true.

Additional Notes

N/A

Describe your test plan

Deploy the example: datadog-agent-with-credential-secret.yaml

The cluster-agent should be deployed. Defaulting value can be also
verified by checking the value of dataddog.status.defaultOverride.clusterAgent.enabled

@clamoriniere clamoriniere added bug Something isn't working component/controller labels Aug 5, 2021
@clamoriniere clamoriniere added this to the v0.7 milestone Aug 5, 2021
@clamoriniere clamoriniere requested a review from a team as a code owner August 5, 2021 10:36
@clamoriniere clamoriniere force-pushed the clamoriniere/fix-cluster-agent-defaulting branch from dda352f to 1d42c39 Compare August 5, 2021 10:37
@@ -29,7 +29,7 @@ spec:
- --enable-leader-election
- --pprof
image: controller:latest
imagePullPolicy: Always
imagePullPolicy: IfNotPresent
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ease local testing

In a specific when the section "spec.clusterAgent" is not provided
in the DatadogAgent instance, the cluster-agent is not defaulted to
`spec.clusterAgent.enabled:true`.

This change fix it, and update the tests according to this new behaviour.
@clamoriniere clamoriniere force-pushed the clamoriniere/fix-cluster-agent-defaulting branch from 1d42c39 to 4da115f Compare August 5, 2021 10:39
@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2021

Codecov Report

Merging #349 (4da115f) into main (1e43add) will increase coverage by 25.86%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #349       +/-   ##
===========================================
+ Coverage   38.11%   63.98%   +25.86%     
===========================================
  Files          63       61        -2     
  Lines       11237     6694     -4543     
===========================================
  Hits         4283     4283               
+ Misses       6647     2104     -4543     
  Partials      307      307               
Flag Coverage Δ
unittests 63.98% <66.66%> (+25.86%) ⬆️

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

Impacted Files Coverage Δ
api/v1alpha1/datadogagent_default.go 83.24% <50.00%> (ø)
controllers/datadogagent/utils.go 84.11% <100.00%> (ø)
api/v1alpha1/zz_generated.openapi.go
api/v1alpha1/zz_generated.deepcopy.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e43add...4da115f. Read the comment docs.

@@ -123,6 +123,7 @@ var _ = Describe("DatadogAgent Controller", func() {
It("It should create DaemonSet", func() {
options := &testutils.NewDatadogAgentOptions{
UseEDS: false,
ClusterAgentDisabled: true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to not modify the current test suite since it focus on validating Daemonset creation

@@ -17,7 +17,7 @@ import (
type NewDatadogAgentOptions struct {
ExtraLabels map[string]string
ExtraAnnotations map[string]string
ClusterAgentEnabled bool
ClusterAgentDisabled bool
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

convert Enabled to Disabled, because default is now "enabled".

@clamoriniere clamoriniere merged commit 11ad61d into main Aug 5, 2021
@clamoriniere clamoriniere deleted the clamoriniere/fix-cluster-agent-defaulting branch August 5, 2021 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component/controller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants