Skip to content

[FA] Read name from UPDATER_TASK#2952

Open
coignetp wants to merge 3 commits intomainfrom
paul/fa-fixup-tasks
Open

[FA] Read name from UPDATER_TASK#2952
coignetp wants to merge 3 commits intomainfrom
paul/fa-fixup-tasks

Conversation

@coignetp
Copy link
Copy Markdown
Contributor

Re-do of #2913.

What does this PR do?

Read the target DatadogAgent namespace/name from the UPDATER_TASK params directly (via a new parseTaskNSN helper) for start, stop, and promote operations, instead of storing it in a experimentTarget field on the daemon struct.

Motivation

Previously, stop and promote operations relied on d.experimentTarget being set by a prior start call. This stateful approach was fragile — if the daemon restarted between start and stop/promote, the target was lost. Carrying the name in every task payload makes each operation self-contained and idempotent.

Additional Notes

  • Removes the experimentTarget types.NamespacedName field from the daemon struct.
  • NamespacedName is now a field of experimentParams (carried in the task itself).
  • validateOperation tests for empty name/namespace are replaced by parseTaskNSN tests.

Minimum Agent Versions

No minimum version requirements.

Describe your test plan

Unit tests in pkg/fleet/daemon_test.go and pkg/fleet/remote_config_test.go are updated to pass NamespacedName inside experimentParams. New TestParseTaskNSN_* tests cover the valid, missing-name, and missing-namespace cases.

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
  • All commits are signed (see: signing commits)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 80.95238% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.35%. Comparing base (953ade2) to head (7b91959).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
pkg/fleet/daemon.go 77.14% 4 Missing and 4 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2952      +/-   ##
==========================================
+ Coverage   40.91%   41.35%   +0.44%     
==========================================
  Files         324      327       +3     
  Lines       28743    29310     +567     
==========================================
+ Hits        11760    12121     +361     
- Misses      16129    16321     +192     
- Partials      854      868      +14     
Flag Coverage Δ
unittests 41.35% <80.95%> (+0.44%) ⬆️

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

Files with missing lines Coverage Δ
pkg/fleet/experiment.go 84.61% <100.00%> (+0.94%) ⬆️
pkg/fleet/remote_config.go 100.00% <ø> (ø)
pkg/fleet/daemon.go 71.62% <77.14%> (+6.17%) ⬆️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@datadog-datadog-prod-us1-2
Copy link
Copy Markdown

datadog-datadog-prod-us1-2 Bot commented Apr 28, 2026

Code Coverage

🎯 Code Coverage (details)
Patch Coverage: 80.00%
Overall Coverage: 41.43% (+0.40%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 7b91959 | Docs | Datadog PR Page | Give us feedback!

@coignetp coignetp marked this pull request as ready for review April 28, 2026 14:00
@coignetp coignetp requested a review from a team April 28, 2026 14:00
@coignetp coignetp requested a review from a team as a code owner April 28, 2026 14:00
@coignetp coignetp added this to the v1.27.0 milestone Apr 28, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f30fbac802

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread pkg/fleet/daemon.go Outdated
Comment on lines +357 to +358
stable, _ := d.getPackageConfigVersions(req.Package)
d.setPackageConfigVersions(req.Package, stable, "")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve promoted version on no-op promote

Update the no-op promote path to carry forward the promoted config into StableConfigVersion before clearing ExperimentConfigVersion. In the recover-after-restart case (status already Promoted, installer state still stable=S, experiment=E), this code currently writes stable=S, experiment="", which drops the promoted version E even though it is now live. That leaves backend/operator state inconsistent and can cause later expected_state checks and task planning to use the wrong baseline.

Useful? React with 👍 / 👎.

…-back phase

When `promoteDatadogAgentExperiment` hits the no-op branch, it previously
always cleared the experiment config version. If the phase was already
`Promoted` (e.g. after a daemon crash between status update and
`setPackageConfigVersions`), the experiment version should move to stable
instead so the backend's expected_state baseline stays correct.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants