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

Add md5 hash config annotation to helm check #1111

Merged
merged 2 commits into from Mar 8, 2024

Conversation

fanny-jiang
Copy link
Contributor

@fanny-jiang fanny-jiang commented Mar 6, 2024

What does this PR do?

Add md5 hash annotation to helm check configMap and cluster agent pod template spec for helm check config changes.

Motivation

QA #1060

Helm check config changes were not being taken into account by the cluster agent/CCR and the cluster agent and CCR pods were not restarting after config changes.

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?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

QA steps for #1060 +

  • datadog-helm-check-config configmap should have the annotation checksum/helm_check-custom-config: <md5_hash>
  • Change any helmCheck config other than helmCheck.enabled: e.g
    • helmCheck.collectEvents: true to helmCheck.collectEvents: false or
    • helmCheck.valuesAsTags: {"image.tag": "image_tag", "image.repository": "image_repo"} to helmCheck.valuesAsTags: {"image.tag": "image_tag"}
  • Uninstall and reinstall any helm chart
  • CCR and/or DCA pods should restart
  • Check for expected behavior in the Datadog app

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

@fanny-jiang fanny-jiang added the bug Something isn't working label Mar 6, 2024
@fanny-jiang fanny-jiang added this to the v1.5.0 milestone Mar 6, 2024
@fanny-jiang fanny-jiang requested review from a team as code owners March 6, 2024 21:26
}

f.configAnnotationValue = hash
f.configAnnotationKey = fmt.Sprintf("checksum/%s-config", feature.HelmCheckIDType)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we add a new const for the helm check config annotation key? There's the MD5ChecksumAnnotationKey = "checksum/%s-custom-config" const, but it's more for custom configs

Copy link
Contributor

Choose a reason for hiding this comment

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

That const is used for CC feature already and don't see problem using it here

f.customConfigAnnotationValue = hash
f.customConfigAnnotationKey = object.GetChecksumAnnotationKey(feature.ClusterChecksIDType)

@fanny-jiang fanny-jiang requested a review from levan-m March 8, 2024 20:28
@fanny-jiang fanny-jiang merged commit e0fa00d into main Mar 8, 2024
19 checks passed
@fanny-jiang fanny-jiang deleted the fanny/CECO-996/fix-helmcheck branch March 8, 2024 23:25
fanny-jiang added a commit that referenced this pull request Mar 11, 2024
* Add md5 hash for config changes

* Apply review suggestions

(cherry picked from commit e0fa00d)
fanny-jiang added a commit that referenced this pull request Mar 11, 2024
* Add md5 hash for config changes

* Apply review suggestions

(cherry picked from commit e0fa00d)
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.

None yet

2 participants