wip

Author: jopemachine

src/ai/backend/manager/data/deployment/types.py

@@ -164,21 +164,15 @@ class DeploymentSubStatus(enum.StrEnum):
 class DeploymentSubStep(DeploymentSubStatus):
     """Sub-steps for the DEPLOYING lifecycle phase.
 
-    Active states:
-    - PROVISIONING: New revision routes are being provisioned; waiting for readiness.
-    - PROGRESSING: Actively replacing old routes with new routes.
-    - ROLLING_BACK: Actively rolling back failed new routes to previous revision.
-
-    Terminal markers (no handler execution, trigger transition only):
-    - COMPLETED: All strategy conditions satisfied; ready for revision swap.
-    - ROLLED_BACK: Rollback finished; ready for cleanup and transition to READY.
+    - PROVISIONING: New revision routes are being provisioned and old routes
+      are being drained.  The main handler for rolling updates.
+    - ROLLING_BACK: Clearing deploying_revision and transitioning to READY.
+    - COMPLETED: All strategy conditions satisfied; triggers revision swap.
     """
 
     PROVISIONING = "provisioning"
-    PROGRESSING = "progressing"
     ROLLING_BACK = "rolling_back"
     COMPLETED = "completed"
-    ROLLED_BACK = "rolled_back"
 
 
 @dataclass(frozen=True)

src/ai/backend/manager/sokovan/deployment/handlers/deploying.py

@@ -307,10 +307,6 @@ def target_statuses(cls) -> list[DeploymentLifecycleStatus]:
                 lifecycle=EndpointLifecycle.DEPLOYING,
                 sub_status=DeploymentSubStep.PROVISIONING,
             ),
-            DeploymentLifecycleStatus(
-                lifecycle=EndpointLifecycle.DEPLOYING,
-                sub_status=DeploymentSubStep.PROGRESSING,
-            ),
         ]
 
     @classmethod

src/ai/backend/manager/sokovan/deployment/strategy/rolling_update.py

@@ -70,9 +70,10 @@ def evaluate_cycle(
 
         FSM flow:
             1. Classify routes into old / new by revision_id.
-            2. If any new route is PROVISIONING -> PROVISIONING (wait).
-            3. If no old routes remain and new_healthy >= desired -> COMPLETED.
-            4. Compute allowed surge/unavailable, decide create/terminate -> PROGRESSING.
+            2. If any new route is PROVISIONING → PROVISIONING (wait).
+            3. If no old routes remain and new_healthy >= desired → COMPLETED.
+            4. Compute allowed surge/unavailable, decide create/terminate
+               → PROVISIONING (with route mutations).
 
         Rollback is not decided by the FSM — the coordinator's timeout
         sweep handles it by transitioning to ROLLING_BACK when the
@@ -165,7 +166,7 @@ def _compute_progressing(
         classified: _ClassifiedRoutes,
         desired: int,
     ) -> StrategyCycleResult:
-        """Compute surge/unavailable budget and return PROGRESSING with route mutations."""
+        """Compute surge/unavailable budget and return PROVISIONING with route mutations."""
         max_surge = self._spec.max_surge  # extra routes allowed above desired
         max_unavailable = self._spec.max_unavailable  # routes allowed to be down
 
@@ -190,7 +191,7 @@ def _compute_progressing(
                 route_changes.drain_route_ids.append(route.route_id)
 
         log.debug(
-            "deployment {}: PROGRESSING create={}, terminate={}, "
+            "deployment {}: PROVISIONING create={}, terminate={}, "
             "old_active={}, new_healthy={}, new_unhealthy={}, new_prov={}",
             deployment.id,
             to_create,
@@ -202,7 +203,7 @@ def _compute_progressing(
         )
 
         return StrategyCycleResult(
-            sub_step=DeploymentSubStep.PROGRESSING,
+            sub_step=DeploymentSubStep.PROVISIONING,
             route_changes=route_changes,
         )
 

src/ai/backend/manager/sokovan/deployment/strategy/types.py

@@ -31,8 +31,7 @@ class RouteChanges:
 class StrategyCycleResult:
     """Result of evaluating a single deployment's strategy cycle.
 
-    ``sub_step`` indicates the next state: PROVISIONING, PROGRESSING,
-    or COMPLETED.
+    ``sub_step`` indicates the next state: PROVISIONING or COMPLETED.
     """
 
     sub_step: DeploymentSubStep

tests/unit/manager/sokovan/deployment/strategy/test_applier.py

@@ -70,7 +70,7 @@ def summary_with_rollout() -> StrategyEvaluationSummary:
 @pytest.fixture
 def summary_with_drain() -> StrategyEvaluationSummary:
     return _build_summary(
-        {uuid4(): DeploymentSubStep.PROGRESSING},
+        {uuid4(): DeploymentSubStep.PROVISIONING},
         route_changes=RouteChanges(drain_route_ids=[uuid4()]),
     )
 
@@ -90,7 +90,7 @@ def completed_summary() -> tuple[StrategyEvaluationSummary, set[UUID]]:
 @pytest.fixture
 def rolled_back_summary() -> tuple[StrategyEvaluationSummary, set[UUID]]:
     ep_id = uuid4()
-    summary = _build_summary({ep_id: DeploymentSubStep.ROLLED_BACK})
+    summary = _build_summary({ep_id: DeploymentSubStep.PROVISIONING})
     return summary, {ep_id}
 
 
@@ -103,7 +103,7 @@ def mixed_summary() -> tuple[StrategyEvaluationSummary, UUID, UUID, UUID]:
         {
             provisioning_id: DeploymentSubStep.PROVISIONING,
             completed_id: DeploymentSubStep.COMPLETED,
-            rolled_back_id: DeploymentSubStep.ROLLED_BACK,
+            rolled_back_id: DeploymentSubStep.PROVISIONING,
         },
         route_changes=RouteChanges(
             rollout_specs=[MagicMock()],

tests/unit/manager/sokovan/deployment/strategy/test_rolling_update.py

@@ -1,10 +1,14 @@
 """Tests for the rolling update FSM evaluation (BEP-1049).
 
 Tests cover:
-- FSM state transitions: PROVISIONING, PROGRESSING, ROLLED_BACK, COMPLETED
+- FSM state transitions: PROVISIONING and COMPLETED
 - max_surge / max_unavailable budget calculations
 - Multi-cycle progression and termination priority
 - Edge cases and boundary conditions
+
+Note: Rollback is not decided by the FSM — the coordinator's timeout
+sweep handles it.  The FSM only returns PROVISIONING (with or without
+route mutations) or COMPLETED.
 """
 
 from __future__ import annotations
@@ -136,13 +140,13 @@ class TestBasicFSMStates:
     """Test fundamental FSM transitions."""
 
     def test_no_routes_creates_new(self) -> None:
-        """First cycle with 0 routes → PROGRESSING, creates desired count."""
+        """First cycle with 0 routes → PROVISIONING with route creation."""
         deployment = make_deployment(desired=1)
         spec = RollingUpdateSpec(max_surge=1, max_unavailable=0)
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, [])
 
-        assert result.sub_step == DeploymentSubStep.PROGRESSING
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
         assert len(result.route_changes.rollout_specs) == 1
         assert len(result.route_changes.drain_route_ids) == 0
 
@@ -181,8 +185,11 @@ def test_completed_when_all_new_healthy_and_no_old(self) -> None:
             pytest.param(RouteStatus.TERMINATED, id="terminated"),
         ],
     )
-    def test_rollback_when_all_new_in_terminal_state(self, failed_status: RouteStatus) -> None:
-        """All new routes in terminal state (with old still present) → ROLLED_BACK."""
+    def test_all_new_failed_retries_creation(self, failed_status: RouteStatus) -> None:
+        """All new routes failed → FSM retries by creating new routes.
+
+        Rollback is handled by the coordinator's timeout sweep, not the FSM.
+        """
         deployment = make_deployment(desired=1)
         spec = RollingUpdateSpec(max_surge=1, max_unavailable=0)
         routes = [
@@ -192,7 +199,7 @@ def test_rollback_when_all_new_in_terminal_state(self, failed_status: RouteStatu
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.ROLLED_BACK
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
 
 
 # ===========================================================================
@@ -388,7 +395,7 @@ def test_not_completed_when_old_still_exists(self) -> None:
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.PROGRESSING
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
         assert len(result.route_changes.drain_route_ids) == 1
 
 
@@ -400,11 +407,8 @@ def test_not_completed_when_old_still_exists(self) -> None:
 class TestRouteStatusClassification:
     """Test how different route statuses affect classification."""
 
-    # FIXME: Code classifies DEGRADED as new_provisioning (same as PROVISIONING),
-    # so this test should expect DeploymentSubStep.PROVISIONING, not ROLLED_BACK.
-    # Verify the intended behavior and update accordingly.
-    def test_degraded_new_triggers_rollback(self) -> None:
-        """DEGRADED new routes — see FIXME above for classification concern."""
+    def test_degraded_new_waits_provisioning(self) -> None:
+        """DEGRADED new routes are treated as PROVISIONING (still warming up)."""
         deployment = make_deployment(desired=1)
         spec = RollingUpdateSpec(max_surge=1, max_unavailable=0)
         routes = [
@@ -413,10 +417,10 @@ def test_degraded_new_triggers_rollback(self) -> None:
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.ROLLED_BACK
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
 
-    def test_unhealthy_new_triggers_rollback(self) -> None:
-        """All new UNHEALTHY (none healthy, none provisioning) → ROLLED_BACK."""
+    def test_unhealthy_new_retries(self) -> None:
+        """All new UNHEALTHY → PROVISIONING (retries, timeout handles rollback)."""
         deployment = make_deployment(desired=1)
         spec = RollingUpdateSpec(max_surge=1, max_unavailable=0)
         routes = [
@@ -425,7 +429,7 @@ def test_unhealthy_new_triggers_rollback(self) -> None:
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.ROLLED_BACK
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
 
     @pytest.mark.parametrize(
         "inactive_status",
@@ -459,7 +463,7 @@ def test_partial_new_failure_continues_progress(self) -> None:
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.PROGRESSING
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
 
     def test_old_provisioning_counted_as_active(self) -> None:
         """Old routes in PROVISIONING are counted as old_active."""
@@ -472,7 +476,7 @@ def test_old_provisioning_counted_as_active(self) -> None:
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.PROGRESSING
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
 
 
 # ===========================================================================
@@ -548,7 +552,7 @@ def test_more_new_healthy_than_desired_still_completes(self) -> None:
         assert result.sub_step == DeploymentSubStep.COMPLETED
 
     def test_only_failed_new_no_old_rolls_back(self) -> None:
-        """Only failed new routes, no old → ROLLED_BACK."""
+        """Only failed new routes, no old → PROVISIONING (retries creation)."""
         deployment = make_deployment(desired=2)
         spec = RollingUpdateSpec(max_surge=1, max_unavailable=0)
         routes = [
@@ -558,7 +562,7 @@ def test_only_failed_new_no_old_rolls_back(self) -> None:
 
         result = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
-        assert result.sub_step == DeploymentSubStep.ROLLED_BACK
+        assert result.sub_step == DeploymentSubStep.PROVISIONING
 
     def test_all_old_inactive_no_new_creates_desired(self) -> None:
         """All old routes are terminated, no new → create desired."""
@@ -579,7 +583,7 @@ def test_deploying_rev_none_rejected(self) -> None:
         spec = RollingUpdateSpec(max_surge=1, max_unavailable=0)
         routes = [make_route(revision_id=OLD_REV, status=RouteStatus.HEALTHY)]
 
-        with pytest.raises(ValueError, match="deploying_revision_id must not be None"):
+        with pytest.raises(Exception):  # InvalidEndpointState
             RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes)
 
     def test_route_without_revision_classified_as_old(self) -> None:
@@ -676,7 +680,7 @@ def test_step_by_step_rolling_update(self) -> None:
         r4 = RollingUpdateStrategy(spec).evaluate_cycle(deployment, routes_c4)
         assert len(r4.route_changes.rollout_specs) == 0
         assert len(r4.route_changes.drain_route_ids) == 1
-        assert r4.sub_step == DeploymentSubStep.PROGRESSING
+        assert r4.sub_step == DeploymentSubStep.PROVISIONING
 
         # Cycle 5: 0 old, 5 new healthy → completed
         routes_c5 = [make_route(revision_id=NEW_REV, status=RouteStatus.HEALTHY) for _ in range(5)]