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

Standardize image defaulting #355

Merged
merged 3 commits into from
Aug 16, 2021
Merged

Conversation

clamoriniere
Copy link
Collaborator

What does this PR do?

Update defaulting images logic and have a dedicated package for defaulting.

The idea here is to update the Agent and Cluster-Agent image version with
the lastest stable version each time we release the Operator.

Motivation

Keep up-to-date the agent and cluster-agent when deploying a new Operator version.

Additional Notes

N/A

Describe your test plan

Deploy the example examples/datadogagent/datadog-agent-with-credential-secret.yaml

  • Check that the Agent image tag is 7.30.0
  • Check that the Cluster-Agent image tag is 1.14.0

@clamoriniere clamoriniere requested review from a team as code owners August 11, 2021 16:59
@clamoriniere clamoriniere added this to the v0.7 milestone Aug 11, 2021
Copy link
Contributor

@apigirl apigirl left a comment

Choose a reason for hiding this comment

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

no docs team review needed on this one

pkg/defaulting/images_test.go Outdated Show resolved Hide resolved
pkg/defaulting/images.go Outdated Show resolved Hide resolved
pkg/defaulting/images.go Show resolved Hide resolved
pkg/defaulting/images.go Outdated Show resolved Hide resolved
api/v1alpha1/datadogagent_default.go Show resolved Hide resolved
@@ -620,7 +621,7 @@ func TestDefaultDatadogAgentSpecAgent(t *testing.T) {
UseExtendedDaemonset: NewBoolPointer(false),
Image: &ImageConfig{
Name: "gcr.io/datadog/agent:6.26.0",
Tag: defaultAgentImageTag,
Tag: defaulting.AgentLatestVersion,
Copy link
Member

Choose a reason for hiding this comment

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

The name in the line above doesn't correspond to "latest".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I keep Latest in the name, because it correspond to the "latest released" version.
Do you think we should find another name?

Copy link
Member

Choose a reason for hiding this comment

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

What I meant in this case is that the Name field in the line above already includes the version (6.26.0) and it does not correspond to defaulting.AgentLatestVersion. I think that :6.26.0 shouldn't be there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I now understand this is a weird behaviour of the defaulting, that doesn't check if the Name contains the tag. I will try to fix it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated the defaulting logic to only set the "default tag" if the tag is not already present in the image name.
you can review the change in this commit

@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2021

Codecov Report

Merging #355 (46d0b8b) into main (18dac27) will increase coverage by 25.96%.
The diff coverage is 90.90%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #355       +/-   ##
===========================================
+ Coverage   38.76%   64.72%   +25.96%     
===========================================
  Files          64       63        -1     
  Lines       11426     6898     -4528     
===========================================
+ Hits         4429     4465       +36     
+ Misses       6678     2114     -4564     
  Partials      319      319               
Flag Coverage Δ
unittests 64.72% <90.90%> (+25.96%) ⬆️

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

Impacted Files Coverage Δ
api/v1alpha1/datadogagent_types.go 100.00% <ø> (ø)
cmd/kubectl-datadog/agent/upgrade/upgrade.go 10.71% <0.00%> (ø)
...md/kubectl-datadog/clusteragent/upgrade/upgrade.go 10.71% <0.00%> (ø)
api/v1alpha1/datadogagent_default.go 81.20% <83.33%> (ø)
controllers/datadogagent/clusteragent.go 72.59% <100.00%> (ø)
controllers/datadogagent/clusterchecksrunner.go 79.34% <100.00%> (ø)
controllers/datadogagent/utils.go 84.27% <100.00%> (-0.03%) ⬇️
pkg/defaulting/images.go 100.00% <100.00%> (ø)
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 18dac27...46d0b8b. Read the comment docs.

@clamoriniere clamoriniere merged commit 7cb27ba into main Aug 16, 2021
@clamoriniere clamoriniere deleted the clamoriniere/defaulting-images branch August 16, 2021 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants