Merge pull request #505 from posit-dev/feature/ordered-push

bschwedler · web-flow · commit a63ffd5342ed · 2026-05-12T16:22:50.000Z
Push image versions in deterministic order to fix Docker Hub display
diff --git a/.github/workflows/bakery-build-native.yml b/.github/workflows/bakery-build-native.yml
@@ -99,7 +99,6 @@ jobs:
       contents: read
     outputs:
       platform-matrix: ${{ steps.images-by-platform.outputs.platform_matrix }}
-      versions-matrix: ${{ steps.images-by-version.outputs.versions_matrix }}
 
     steps:
       - name: Checkout
@@ -125,21 +124,6 @@ jobs:
           [[ -n "$DEV_STREAM" ]] && ARGS+=(--dev-stream "$DEV_STREAM")
           result=$(bakery ci matrix "${ARGS[@]}")
           echo "platform_matrix=$(echo "$result" | jq --compact-output .)" >> "$GITHUB_OUTPUT"
-      - name: Images by Version
-        id: images-by-version
-        env:
-          DEV_VERSIONS: ${{ inputs.dev-versions }}
-          MATRIX_VERSIONS: ${{ inputs.matrix-versions }}
-          IMAGE_VERSION: ${{ inputs.image-version }}
-          DEV_STREAM: ${{ inputs.dev-stream }}
-          CONTEXT: ${{ inputs.context }}
-        run: |
-          IMAGE_VERSION="${IMAGE_VERSION#v}"
-          ARGS=(--quiet --dev-versions "$DEV_VERSIONS" --matrix-versions "$MATRIX_VERSIONS" --exclude platform --context "$CONTEXT")
-          [[ -n "$IMAGE_VERSION" ]] && ARGS+=(--image-version "$IMAGE_VERSION")
-          [[ -n "$DEV_STREAM" ]] && ARGS+=(--dev-stream "$DEV_STREAM")
-          result=$(bakery ci matrix "${ARGS[@]}")
-          echo "versions_matrix=$(echo "$result" | jq --compact-output .)" >> "$GITHUB_OUTPUT"
 
   build-test:
     name: "Build/Test ${{ matrix.img.image }}:${{ matrix.img.version }} (${{ matrix.img.platform }})"
@@ -279,17 +263,11 @@ jobs:
           overwrite: 'true'
 
   merge:
-    name: "Merge/Push ${{ matrix.img.image }}:${{ matrix.img.version }}"
+    name: "Merge/Push"
     permissions:
       contents: read
       packages: write
-    needs:
-      - matrix
-      - build-test
-    strategy:
-      fail-fast: false
-      matrix:
-        img: ${{ fromJson(needs.matrix.outputs.versions-matrix) }}
+    needs: [build-test]
     runs-on: ${{ inputs.merge-builder }}
 
     steps:
@@ -357,7 +335,7 @@ jobs:
       - name: Download Metadata
         uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c  # v8.0.1
         with:
-          pattern: "${{ matrix.img.image }}-${{ matrix.img.version }}-*-metadata"
+          pattern: "*-metadata"
           merge-multiple: true
 
       - name: List files
diff --git a/posit-bakery/posit_bakery/image/image_target.py b/posit-bakery/posit_bakery/image/image_target.py
@@ -5,14 +5,15 @@
 from datetime import datetime
 from enum import Enum
 from pathlib import Path
-from typing import Annotated
+from typing import Annotated, TYPE_CHECKING
 
 import python_on_whales
 from pydantic import BaseModel, computed_field, ConfigDict, Field, model_validator
 
 from posit_bakery.config.image import ImageVersion, ImageVariant, ImageVersionOS
 from posit_bakery.config.image.build_os import DEFAULT_PLATFORMS
 from posit_bakery.config.image.build_secret import BuildSecret
+from posit_bakery.config.image.parsed_version import version_sort_key
 from posit_bakery.config.registry import Registry, BaseRegistry
 from posit_bakery.config.repository import Repository
 from posit_bakery.config.tag import TagPattern, TagPatternFilter
@@ -21,6 +22,10 @@
 from posit_bakery.image.image_metadata import MetadataFile, BuildMetadata
 from posit_bakery.settings import SETTINGS
 
+# Local under TYPE_CHECKING to avoid a circular import
+if TYPE_CHECKING:
+    from posit_bakery.config.image.parsed_version import ParsedVersion
+
 log = logging.getLogger(__name__)
 
 
@@ -314,6 +319,31 @@ def is_primary_variant(self) -> bool:
             return True
         return self.image_variant.primary
 
+    @property
+    def push_sort_key(self) -> tuple[str, bool, "ParsedVersion", int, str, str, str]:
+        """Deterministic ordering for push to ordered-display registries (e.g. Docker Hub).
+
+        Tuple semantics, ascending sort:
+          1. image_name        — group all targets of one image together.
+          2. is_latest         — False before True; latest target pushed LAST in its group.
+          3. version           — ParsedVersion (semver §11) or MIN for matrix/unparseable.
+          4. primary_score     — 0..2; (primary OS + primary variant) target pushes LAST
+                                 within a version, so its simplest tag is most-recent.
+          5. version.name      — stable lex tiebreaker (load-bearing for matrix rows that
+                                 collapse to MIN under (3)).
+          6. variant.name      — stable tiebreaker.
+          7. os.name           — stable tiebreaker.
+        """
+        return (
+            self.image_name,
+            self.is_latest,
+            version_sort_key(self.image_version),
+            int(self.is_primary_os) + int(self.is_primary_variant),
+            self.image_version.name,
+            self.image_variant.name if self.image_variant else "",
+            self.image_os.name if self.image_os else "",
+        )
+
     @computed_field
     @property
     def containerfile(self) -> Path:
diff --git a/posit-bakery/posit_bakery/plugins/builtin/oras/__init__.py b/posit-bakery/posit_bakery/plugins/builtin/oras/__init__.py
@@ -105,6 +105,9 @@ def execute(
         **kwargs,
     ) -> list[ToolCallResult]:
         """Execute ORAS merge workflow against the given image targets."""
+        # Sort so latest pushes last; Docker Hub displays tags by push-time order.
+        targets = sorted(targets, key=lambda t: t.push_sort_key)
+        log.info("ORAS merge order: %s", ", ".join(str(t) for t in targets))
         results = []
 
         for target in targets:
diff --git a/posit-bakery/test/image/test_image_target.py b/posit-bakery/test/image/test_image_target.py
@@ -6,6 +6,7 @@
 import python_on_whales
 
 from posit_bakery.config.dependencies import PythonDependencyVersions, RDependencyVersions
+from posit_bakery.config.image.parsed_version import ParsedVersion
 from posit_bakery.config.tag import default_tag_patterns, TagPatternFilter
 from posit_bakery.const import OCI_LABEL_PREFIX, POSIT_LABEL_PREFIX
 from posit_bakery.image.image_metadata import BuildMetadata
@@ -1037,3 +1038,113 @@ def test_get_merge_sources_single_platform(self, basic_standard_image_target):
         sources = basic_standard_image_target.get_merge_sources()
 
         assert sources == ["image@sha256:digest"]
+
+
+class TestPushSortKey:
+    """Tests for ImageTarget.push_sort_key — see ordered-push design spec."""
+
+    @staticmethod
+    def _make(
+        *,
+        image_name="connect",
+        is_latest=False,
+        version_name="2026.03.0",
+        is_primary_os=True,
+        is_primary_variant=True,
+        variant_name="Standard",
+        os_name="Ubuntu 24.04",
+        is_matrix=False,
+    ):
+        """Build a MagicMock(spec=ImageTarget) with the inputs push_sort_key reads."""
+        target = MagicMock(spec=ImageTarget)
+        target.image_name = image_name
+        target.is_latest = is_latest
+        target.is_primary_os = is_primary_os
+        target.is_primary_variant = is_primary_variant
+
+        image_version = MagicMock()
+        image_version.name = version_name
+        image_version.parsed_version = None if is_matrix else ParsedVersion.parse(version_name)
+        target.image_version = image_version
+
+        variant = MagicMock()
+        variant.name = variant_name
+        target.image_variant = variant
+
+        os_obj = MagicMock()
+        os_obj.name = os_name
+        target.image_os = os_obj
+
+        # Bind the real property — push_sort_key reads only these attributes,
+        # so we can borrow ImageTarget's actual implementation.
+        target.push_sort_key = ImageTarget.push_sort_key.fget(target)
+        return target
+
+    def test_latest_sorts_last(self):
+        """Within one image+version, the is_latest=True target sorts after is_latest=False."""
+        non_latest = self._make(is_latest=False, version_name="2026.04.0")
+        latest = self._make(is_latest=True, version_name="2026.04.0")
+        assert sorted([latest, non_latest], key=lambda t: t.push_sort_key) == [non_latest, latest]
+
+    def test_within_version_primary_sorts_last(self):
+        """Within one version, primary_score 0 < 1 < 2 — most-primary pushed last."""
+        non_primary = self._make(
+            is_primary_os=False, is_primary_variant=False, os_name="Ubuntu 22.04", variant_name="Minimal"
+        )
+        partial_primary = self._make(
+            is_primary_os=True, is_primary_variant=False, os_name="Ubuntu 24.04", variant_name="Minimal"
+        )
+        full_primary = self._make(
+            is_primary_os=True, is_primary_variant=True, os_name="Ubuntu 24.04", variant_name="Standard"
+        )
+        ordered = sorted(
+            [full_primary, non_primary, partial_primary],
+            key=lambda t: t.push_sort_key,
+        )
+        assert ordered == [non_primary, partial_primary, full_primary]
+
+    def test_older_version_sorts_first(self):
+        """Two non-latest stable targets — older (2026.03.0) sorts before newer (2026.04.0)."""
+        old = self._make(version_name="2026.03.0")
+        new = self._make(version_name="2026.04.0")
+        assert sorted([new, old], key=lambda t: t.push_sort_key) == [old, new]
+
+    def test_dev_prerelease_orders_correctly(self):
+        """Locks in ParsedVersion semver §11: daily < dev < release at same release tuple."""
+        daily = self._make(version_name="2026.04.0-daily+92")
+        dev = self._make(version_name="2026.04.0-dev+485-gdb8245deea")
+        release = self._make(version_name="2026.04.0")
+        assert sorted([release, dev, daily], key=lambda t: t.push_sort_key) == [daily, dev, release]
+
+    def test_matrix_non_latest_rows_use_name_tiebreaker(self):
+        """Non-latest matrix rows collapse to MIN on version key; deterministic via version.name lex."""
+        a = self._make(is_matrix=True, version_name="R4.3-python3.11")
+        b = self._make(is_matrix=True, version_name="R4.3-python3.12")
+        c = self._make(is_matrix=True, version_name="R4.4-python3.11")
+        ordered = sorted([c, a, b], key=lambda t: t.push_sort_key)
+        assert [t.image_version.name for t in ordered] == [
+            "R4.3-python3.11",
+            "R4.3-python3.12",
+            "R4.4-python3.11",
+        ]
+
+    def test_matrix_latest_row_sorts_last(self):
+        """The is_latest=True matrix row sorts after all non-latest matrix rows."""
+        non_latest = self._make(is_matrix=True, version_name="R4.3-python3.11", is_latest=False)
+        latest = self._make(is_matrix=True, version_name="R4.4-python3.12", is_latest=True)
+        assert sorted([latest, non_latest], key=lambda t: t.push_sort_key) == [non_latest, latest]
+
+    def test_multi_image_grouped(self):
+        """Two image_names in one list — output fully grouped, no interleaving."""
+        connect_old = self._make(image_name="connect", version_name="2026.03.0")
+        connect_new = self._make(image_name="connect", version_name="2026.04.0", is_latest=True)
+        content_a = self._make(image_name="connect-content", is_matrix=True, version_name="R4.3-python3.11")
+        content_b = self._make(
+            image_name="connect-content", is_matrix=True, version_name="R4.4-python3.12", is_latest=True
+        )
+        ordered = sorted(
+            [content_b, connect_old, content_a, connect_new],
+            key=lambda t: t.push_sort_key,
+        )
+        names = [t.image_name for t in ordered]
+        assert names == ["connect", "connect", "connect-content", "connect-content"]
diff --git a/posit-bakery/test/plugins/builtin/oras/test_oras_plugin.py b/posit-bakery/test/plugins/builtin/oras/test_oras_plugin.py
@@ -1,5 +1,6 @@
 """Tests for the OrasPlugin."""
 
+import logging
 import subprocess
 from pathlib import Path
 from unittest.mock import MagicMock, patch
@@ -36,6 +37,7 @@ def mock_target_with_sources():
         "ghcr.io/posit-dev/test/tmp@sha256:arm64digest",
     ]
     mock_target.labels = {"org.opencontainers.image.title": "Test Image"}
+    mock_target.push_sort_key = (0,)
 
     mock_tag = MagicMock()
     mock_tag.destination = "ghcr.io/posit-dev/test-image"
@@ -53,6 +55,7 @@ def mock_target_without_sources():
     mock_target.image_name = "no-sources"
     mock_target.uid = "no-sources-1-0-0"
     mock_target.get_merge_sources.return_value = []
+    mock_target.push_sort_key = (0,)
     return mock_target
 
 
@@ -153,6 +156,62 @@ def test_execute_mixed_targets(self, plugin, mock_target_with_sources, mock_targ
         assert len(results) == 1
         assert results[0].target is mock_target_with_sources
 
+    def test_execute_processes_targets_in_push_sort_key_order(self, plugin, caplog):
+        """Targets are processed in ascending push_sort_key order, regardless of input order."""
+
+        def make_target(name, sort_key):
+            t = MagicMock(spec=ImageTarget)
+            t.image_name = name
+            t.uid = f"{name}-uid"
+            t.context = MagicMock(spec=ImageTargetContext)
+            t.context.base_path = Path("/project")
+            t.settings = MagicMock(spec=ImageTargetSettings)
+            t.settings.temp_registry = "ghcr.io/posit-dev"
+            t.get_merge_sources.return_value = [f"ghcr.io/posit-dev/{name}/tmp@sha256:digest"]
+            t.labels = {}
+            mock_tag = MagicMock()
+            mock_tag.destination = f"ghcr.io/posit-dev/{name}"
+            mock_tag.suffix = "1.0.0"
+            mock_tag.__str__ = lambda self: f"ghcr.io/posit-dev/{name}:1.0.0"
+            t.tags = StringableList([mock_tag])
+            # Override push_sort_key to a controlled tuple so the test is independent of
+            # ImageVersion / ImageVariant internals.
+            t.push_sort_key = sort_key
+            t.__str__ = lambda self: name
+            return t
+
+        # Input order is intentionally scrambled.
+        targets = [
+            make_target("c-second", (1,)),
+            make_target("a-first", (0,)),
+            make_target("d-last", (3,)),
+            make_target("b-third", (2,)),
+        ]
+        expected_order = ["a-first", "c-second", "b-third", "d-last"]
+
+        call_order = []
+
+        def fake_run(self_workflow, dry_run=False):
+            call_order.append(self_workflow.image_target.image_name)
+            return OrasMergeWorkflowResult(success=True, destinations=[])
+
+        with (
+            patch("posit_bakery.plugins.builtin.oras.oras.find_oras_bin", return_value="oras"),
+            patch(
+                "posit_bakery.plugins.builtin.oras.OrasMergeWorkflow.run",
+                autospec=True,
+                side_effect=fake_run,
+            ),
+            caplog.at_level(logging.INFO, logger="posit_bakery.plugins.builtin.oras"),
+        ):
+            plugin.execute(Path("/project"), targets)
+
+        assert call_order == expected_order, f"got {call_order}, want {expected_order}"
+        order_log_lines = [r for r in caplog.records if "ORAS merge order:" in r.getMessage()]
+        assert len(order_log_lines) == 1, "expected exactly one ORAS merge order log line"
+        msg = order_log_lines[0].getMessage()
+        assert msg.endswith("a-first, c-second, b-third, d-last"), msg
+
 
 class TestOrasPluginCLI:
     def test_register_cli_adds_oras_command(self, plugin):
