Skip to content

Commit 8429cfa

Browse files
fix: prevent fresh+reconsolidation item name collision in CEFS consolidation (#2059)
Co-authored-by: mattgodbolt-molty <mattgodbolt-molty@users.noreply.github.com>
1 parent b16b732 commit 8429cfa

3 files changed

Lines changed: 73 additions & 2 deletions

File tree

bin/lib/cefs/consolidation.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,11 +683,25 @@ def prepare_consolidation_items(
683683
"""
684684
items_for_consolidation = []
685685
subdir_mapping = {}
686+
seen_subdir_names: dict[str, str] = {} # subdir_name -> item.name, for duplicate detection
686687

687688
for item in group:
688689
# Use the installable name as subdirectory name (sanitized for filesystem)
689690
subdir_name = sanitize_path_for_filename(Path(item.name))
690691

692+
# Guard against duplicate subdir names within the same group. This can happen when a
693+
# fresh item and a reconsolidation item have the same installable name — both produce
694+
# the same sanitized subdir_name and parallel workers would race to write to the same
695+
# extraction directory (OSError: Directory not empty).
696+
if subdir_name in seen_subdir_names:
697+
raise ValueError(
698+
f"Duplicate subdir name '{subdir_name}' in consolidation group: "
699+
f"'{item.name}' (from_reconsolidation={item.from_reconsolidation}) conflicts with "
700+
f"'{seen_subdir_names[subdir_name]}'. "
701+
f"Fresh items and reconsolidation items with the same name must not appear in the same group."
702+
)
703+
seen_subdir_names[subdir_name] = item.name
704+
691705
# For reconsolidation items, we already have the extraction path
692706
if item.from_reconsolidation:
693707
extraction_path = item.extraction_path

bin/lib/cli/cefs.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,8 +596,18 @@ def consolidate(
596596

597597
if recon_candidates:
598598
_LOGGER.info("Found %d items from consolidated images for reconsolidation", len(recon_candidates))
599-
# Reconsolidation candidates are already in the right format
600-
cefs_items.extend(recon_candidates)
599+
# Filter out reconsolidation candidates whose name already appears as a fresh item.
600+
# If both a fresh item and a reconsolidation item have the same name they would
601+
# produce the same sanitized subdir_name and race when extracted in parallel.
602+
fresh_names = {item.name for item in cefs_items}
603+
filtered_recon = [c for c in recon_candidates if c.name not in fresh_names]
604+
skipped = len(recon_candidates) - len(filtered_recon)
605+
if skipped:
606+
_LOGGER.info(
607+
"Skipping %d reconsolidation candidate(s) whose name already appears as a fresh item",
608+
skipped,
609+
)
610+
cefs_items.extend(filtered_recon)
601611

602612
if not cefs_items:
603613
_LOGGER.warning("No CEFS items found matching filter: %s", " ".join(filter_) if filter_ else "all")

bin/test/cefs/consolidation_test.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -862,3 +862,50 @@ def test_gather_reconsolidation_candidates(tmp_path):
862862
assert len(candidates_filtered) == 2
863863
filtered_names = {c.name for c in candidates_filtered}
864864
assert filtered_names == {"tools/small 1.0.0", "tools/small 2.0.0"}
865+
866+
867+
def test_duplicate_subdir_names_in_consolidation_group_are_rejected(tmp_path):
868+
"""Regression test: fresh item + reconsolidation item with the same name must not appear
869+
in the same consolidation group.
870+
871+
Production failure (2026-04-10, run #24232582644):
872+
OSError: [Errno 39] Directory not empty:
873+
.tmp_extract_2d3e5ffc/compilers_swift_static-sdk_sdk-0-1-0_6.2.4
874+
-> compilers_swift_static-sdk_sdk-0-1-0_6.2.4
875+
876+
Both 'compilers/swift/static-sdk/sdk-0-1-0 6.2.4' (fresh install) and a
877+
reconsolidation candidate from an existing consolidated image produced the
878+
same sanitized subdir_name. Parallel workers raced to write the same
879+
directory, and the second found it non-empty.
880+
"""
881+
mount_point = Path("/cefs")
882+
883+
fresh_path = tmp_path / "swift-static-sdk-6.2.4"
884+
cefs_target = mount_point / "7e" / "7e37bea4f759832456d69f12_swift-6.2.4-static-sdk"
885+
fresh_path.symlink_to(cefs_target)
886+
887+
recon_path = tmp_path / "swift-static-sdk-6.2.4-recon"
888+
889+
group = [
890+
ConsolidationCandidate(
891+
name="compilers/swift/static-sdk/sdk-0-1-0 6.2.4",
892+
nfs_path=fresh_path,
893+
squashfs_path=Path("/efs/cefs-images/7e/7e37bea4f759832456d69f12_swift-6.2.4-static-sdk.sqfs"),
894+
size=238_000_000,
895+
from_reconsolidation=False,
896+
),
897+
ConsolidationCandidate(
898+
name="compilers/swift/static-sdk/sdk-0-1-0 6.2.4", # same name as above
899+
nfs_path=recon_path,
900+
squashfs_path=Path("/efs/cefs-images/26/26ccf7a2fd1f9d4667de2307_consolidated.sqfs"),
901+
size=50_000_000,
902+
extraction_path=Path("compilers_swift_static-sdk_sdk-0-1-0_6.2.4"),
903+
from_reconsolidation=True,
904+
),
905+
]
906+
907+
# Both items sanitize to the same subdir_name: 'compilers_swift_static-sdk_sdk-0-1-0_6.2.4'.
908+
# prepare_consolidation_items must detect this and raise ValueError — if it silently
909+
# returned both, parallel workers would race to extract to the same directory.
910+
with pytest.raises(ValueError, match="Duplicate subdir name"):
911+
prepare_consolidation_items(group, mount_point)

0 commit comments

Comments
 (0)