Skip to content

bundle: route config sync edits to the block that defines them - #6117

Closed
ilyakuz-db wants to merge 1 commit into
mainfrom
configsync/block-provenance
Closed

bundle: route config sync edits to the block that defines them#6117
ilyakuz-db wants to merge 1 commit into
mainfrom
configsync/block-provenance

Conversation

@ilyakuz-db

@ilyakuz-db ilyakuz-db commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Stack

PR What it does
1 this PR Route each change to the one block that defines it
2 #6135 Removing an element defined in several blocks deletes it from each
3 #6134 A key change is written as a key rewrite in every defining block

Each PR is based on the one above it and carries the acceptance tests for its own behaviour.

Problem

A resource's list field — a job's tasks or job_clusters, a pipeline's clusters — can be defined in more than one physical YAML region: a top-level resources.<type>.<name>.<list> block and a targets.<target>.resources…<list> override, either of which may live in its own included file. Loading merges those regions into one list, and keyed lists are also sorted by key, but on disk they stay separate regions that have to be edited independently.

A remote change is expressed against the merged view, so resolveSelectors found the element in the merged list and yamlFileIndex converted the merged index to a physical one by counting elements that share Location().File and have a smaller Line. That has no notion of block: a top-level block and a target override in the same file are indistinguishable. FieldCandidates compounded it by guessing — it emitted [path, "targets.<t>."+path], and patch.go tried the top-level pointer first and fell back only on error, so a top-level block that happened to have an element at that index silently accepted the patch.

config-remote-sync runs unattended: nobody reads its output or exit code, and it re-runs periodically. So a silently wrong write is the worst outcome — worse than leaving a change for the next run.

Changes

New bundle/configsync/blockindex.go recovers the physical blocks and builds a dyn.Location -> block mapping. resolveSelectors now records the sequence elements a change path traverses, and each merged index is rewritten into the index inside the chosen block. yamlFileIndex is deleted.

Selecting a target folds its overrides into the resources tree and drops the targets subtree, so the two regions are no longer distinguishable in the loaded configuration. They are recovered by parsing the contributing files again — the file list comes from the locations already on the merged tree, and each file is parsed directly with config.LoadFromBytes. Parsing the files rather than reloading through the mutator pipeline matters: the pipeline resolves includes, reports through logdiag, and executes the bundle's preinit script, none of which belongs to reading back a source location. preinit runs once per sync, as it did before this PR.

How a destination gets picked:

  • element defined in exactly one block → that block;
  • element assembled from several blocks → the leaf field decides, since merging records each field's location separately, so max_retries and timeout_seconds on the same task can go to different blocks;
  • leaf field itself defined in several blocks → the block owning Locations()[0], which is the definition that won the merge and therefore the value that was deployed. Writing any other copy would leave the effective value unchanged;
  • brand-new element or field → the block declaring the resource, keeping it out of a target-specific scope the user did not ask for;
  • nothing identifies a single block → left unapplied rather than written to a guess.

Index bookkeeping (indicesToReplaceMap, indexOperations) is now scoped per block, so a removal in one block no longer shifts indices in another.

Scope

This PR routes every change to one destination, which is the whole story for a field edit. Two cases need more than one destination and are the follow-ups in the stack above. Until #6135 lands, removing an element defined in several blocks is left unapplied: safe, but it never converges.

Tests

Four new directories under acceptance/bundle/config-remote-sync/split/, sharing one test.toml. Unlike the existing directories here they omit Cloud = true, so they run against the in-process test server under both deployment engines and therefore execute in normal CI.

Each fails on the parent commit and passes here:

  • keyed_edit — a task defined only in the target block, and a top-level task whose merged position differs from its position in the block. Before: the first appends a keyless - timeout_seconds: 111 to the top-level block, and the next sync reports tasks[task_key='']: remove for the element it just created.
  • keyed_twoblock — a task defined in both blocks: one field per block, one field set in both, plus a brand-new field. Before: the both-blocks field was written to the top-level copy while the target's value stayed in effect, so the edit silently did nothing.
  • multifile — one target's override spread across two included files. Before: the edit went to the top-level databricks.yml.
  • positional — pipeline clusters, which diff by position rather than by key. Before: a field edit on the target-block cluster failed with parent path ... does not exist.

All 21 pre-existing directories are byte-for-byte unchanged, including job_multiple_tasks (edit/add/remove/rename on a single-block job — the evidence that the common path is unaffected) and multiple_files (renames with depends_on references across included files).

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

19 files changed
Suggested: @pietern
Also eligible: @denik, @andrewnester, @anton-107, @janniklasrose, @lennartkats-db, @shreyas-goenka

/bundle/ - needs approval

Files: bundle/configsync/blockindex.go, bundle/configsync/resolve.go, bundle/configsync/resolve_test.go
Suggested: @pietern
Also eligible: @denik, @andrewnester, @anton-107, @janniklasrose, @lennartkats-db, @shreyas-goenka

Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db) can approve all areas.
See OWNERS for ownership rules.

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 3aaaf93

Run: 30768875321

Env 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 4 4 313 1071 4:17
💚​ aws windows 4 4 315 1069 6:20
💚​ azure linux 4 4 313 1070 4:27
💚​ azure windows 4 4 315 1068 4:00
💚​ gcp linux 1 5 313 1071 4:05
💚​ gcp windows 1 5 315 1069 5:57
8 interesting tests: 4 RECOVERED, 4 SKIP
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S
💚​ TestFetchRepositoryInfoAPI_FromRepo/root 💚​R 💚​R 💚​R 💚​R
💚​ TestFetchRepositoryInfoAPI_FromRepo/subdir 💚​R 💚​R 💚​R 💚​R
Top 3 slowest tests (at least 2 minutes):
duration env testname
5:16 aws windows TestAccept
4:52 gcp windows TestAccept
2:58 azure windows TestAccept

@ilyakuz-db
ilyakuz-db force-pushed the configsync/acc-split-list-baseline branch from c6a5c96 to 711e1c7 Compare July 31, 2026 13:33
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from 12b54e7 to ac0c041 Compare July 31, 2026 13:33
@ilyakuz-db
ilyakuz-db force-pushed the configsync/acc-split-list-baseline branch from 711e1c7 to 00635ac Compare July 31, 2026 13:41
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from ac0c041 to 388627d Compare July 31, 2026 13:41
@ilyakuz-db
ilyakuz-db force-pushed the configsync/acc-split-list-baseline branch from 00635ac to 0d5de02 Compare July 31, 2026 17:55
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from 388627d to 0dec516 Compare July 31, 2026 17:55
@ilyakuz-db
ilyakuz-db force-pushed the configsync/acc-split-list-baseline branch from 0d5de02 to 3854cff Compare July 31, 2026 19:16
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch 2 times, most recently from f957e06 to 7075667 Compare August 2, 2026 12:21
@ilyakuz-db
ilyakuz-db force-pushed the configsync/acc-split-list-baseline branch from 3854cff to 17c2439 Compare August 2, 2026 14:01
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from 7075667 to 8454025 Compare August 2, 2026 14:01
@ilyakuz-db
ilyakuz-db force-pushed the configsync/acc-split-list-baseline branch from 17c2439 to 269d3f2 Compare August 2, 2026 14:46
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch 2 times, most recently from f6b4d04 to 7387cfe Compare August 2, 2026 18:53
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from 7387cfe to f8e1487 Compare August 2, 2026 19:58
@ilyakuz-db
ilyakuz-db changed the base branch from configsync/acc-split-list-baseline to main August 2, 2026 19:59
@ilyakuz-db
ilyakuz-db force-pushed the configsync/block-provenance branch from f8e1487 to 3aaaf93 Compare August 2, 2026 21:52
@ilyakuz-db

Copy link
Copy Markdown
Contributor Author

Consolidated into #6138 — the stack is merged into a single PR with the same final tree.

@ilyakuz-db ilyakuz-db closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants