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 to handle partial image overrides input correctly #652

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

levan-m
Copy link
Contributor

@levan-m levan-m commented Nov 29, 2022

What does this PR do?

Proposes a fix for an issue report in this Slack message - summarizing below.

When override.nodeAgent.image isn't fully provided the image URI in agent pod/deployment is corrupt.
For

  override:
    nodeAgent:
      image:
        tag: 7.41.0-rc.5

Generated image URI is

    image: gcr.io/datadoghq/:7.41.0-rc.5

Same happens with DCA and CCR as well.

Motivation

Fix will allow users to provide only partial overrides and use previuos defaults.

Additional Notes

n/a

Describe your test plan

  • Added unit test for missing scenarios.
  • Tested locally with default operator setup and combinations of overrides, making sure final image URI is as expected.

@levan-m levan-m requested review from a team as code owners November 29, 2022 22:32
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

@codecov-commenter
Copy link

Codecov Report

Merging #652 (2362442) into main (194a480) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #652      +/-   ##
==========================================
+ Coverage   58.55%   58.57%   +0.02%     
==========================================
  Files         144      144              
  Lines       17345    17355      +10     
==========================================
+ Hits        10156    10166      +10     
  Misses       6581     6581              
  Partials      608      608              
Flag Coverage Δ
unittests 58.57% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
...ntrollers/datadogagent/override/podtemplatespec.go 79.23% <100.00%> (+1.73%) ⬆️

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 194a480...2362442. Read the comment docs.

@@ -443,3 +514,21 @@ func TestPodTemplateSpec(t *testing.T) {
})
}
}

func fakePodTemplateManagers(image string, t *testing.T) *fake.PodTemplateManagers {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe you could rename that method to reflect that it's dedicated to give a podTemplateManager with images overidden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed

@levan-m levan-m added the bug Something isn't working label Nov 30, 2022
@levan-m levan-m merged commit 6a497a2 into main Dec 2, 2022
@levan-m levan-m deleted the levan-m/fix_image_tag_overrides branch December 2, 2022 16:26
levan-m added a commit that referenced this pull request Dec 14, 2022
levan-m added a commit that referenced this pull request Dec 16, 2022
CharlyF pushed a commit that referenced this pull request Jan 4, 2023
mftoure pushed a commit that referenced this pull request Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants