feat(fleet): add config_change scenario + generic fleet service-config mount#35
Merged
Merged
Conversation
…g mount Add a `config_change` fleet automation scenario that verifies a live operational config change, delivered out-of-band by the control plane, is applied AND reconciled on a running subject with no restart: deliver_config provides config A (target -> dead endpoint, delivery blocked); the scenario confirms delivery is blocked, pushes the changed config from configs/update.vmf (target -> working receiver) over the fleet link, and asserts delivery starts. Reuses the existing deliver_config field and the configs/update.* file convention, so no new case field is introduced. Make the fleet service-config mount subject-agnostic: add a generic Subject.FleetServiceConfigPath field. When a case sets fleet.deliver_config and the subject declares this path, the harness mounts the case config as the service/bootstrap config there and runs the subject without a config-path override, so the delivered operational config governs the pipeline. This replaces a hardcoded subject name and in-container path in the renderer; the path now lives only on the subject definition alongside every other vendor's paths. This also fixes fleet directors never bringing up their data pipeline: with a single -config-path file serving as both service and system config, the delivered operational config was never reloaded, so listeners never started. Affected all fleet cases relying on the data path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds fleet config-update support across validation, compose wiring, receiver correctness checks, and runner scenarios. It introduces a subject fleet service config path, enables config-change validation, and adds fleet runs that verify delivery and verifier behavior before and after updates. ChangesFleet config update flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/config/case.go`:
- Around line 1230-1233: The scenario validator in the switch over
tc.Fleet.Scenario currently allows config_change without requiring a prior
delivered config, which lets tests validate an initial config instead of a live
A→B transition. Update the validation logic in the case handling for
config_change so it rejects empty tc.Fleet.DeliverConfig (while keeping other
scenarios unchanged), ensuring runFleetAutomationCorrectness only accepts
config_change when a prior config was already delivered.
In `@internal/runner/runner.go`:
- Around line 3956-3960: The correctness check in the runner currently compares
against an absolute received-line total, which can let pre-existing traffic
satisfy Phase 2 without proving any new delivery after the config push. Update
the logic around the receiver validation in runner.go to use rmB.LinesReceived
as the baseline and require the post-change count to exceed that baseline by the
expected amount, while keeping the existing minRecv/leak handling in the flow.
Make the fix in the code path that evaluates the receiver totals so the success
condition depends on additional lines received after the change, not the
absolute total.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2a44fb0b-9a93-42a6-ac2d-2e94467402b0
📒 Files selected for processing (4)
internal/config/case.gointernal/config/subject.gointernal/orchestrator/docker.gointernal/runner/runner.go
Deploying pipebench with
|
| Latest commit: |
e2e833d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e32c1494.pipebench.pages.dev |
| Branch Preview URL: | https://feat-fleet-config-change-sce.pipebench.pages.dev |
yusufozturk
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
config_changefleet automation scenario and makes the fleet service-config mount subject-agnostic (no vendor-specific literals in the renderer). Also fixes fleet subjects never bringing up their data pipeline.What's new
config_changescenario (internal/runner/runner.go): verifies a live operational config change delivered out-of-band by the control plane is applied and reconciled on a running subject with no restart:deliver_configdelivers config A — target points at a dead endpoint, so delivery is blocked.configs/update.vmf(target → working receiver) over the fleet link.receiver >= correctness.min_received).Reuses the existing
deliver_configfield and theconfigs/update.*file convention (mirrorsconfig_updatereadingconfigs/update.yml) — no new case field.Generic
Subject.FleetServiceConfigPath(internal/config/subject.go): declares the in-container path where a fleet/managed subject reads its service/bootstrap config (operational config arrives out-of-band from the control plane). When a case setsfleet.deliver_configand the subject declares this path, the harness mounts the case config there and runs the subject without a config-path override.Why
Previously the renderer hardcoded a subject name and an in-container path to achieve this. That path now lives only on the subject definition, alongside every other vendor's paths — keeping the renderer subject-agnostic and the public repo free of one vendor's internal layout.
Bug fix included
Fleet subjects never brought up their data pipeline: with a single
-config-pathfile serving as both service and system config, the delivered operational config was never reloaded, so listeners never started. This silently affected all fleet cases that rely on the data path (e.g.live_data).Testing
go build ./...clean.update.vmf(received well above threshold). The companion bench case isfleet_vmf_config_change_correctness(separate repo).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
vmetricsubject via a dedicated service config path.Bug Fixes
config_changewith required prior configuration context./metricsto includeRequiredSubstringresults when configured.