Hold an engine Deployment unready until its rollout finishes

docs/content/models/model-deployment.md

@@ -261,8 +261,8 @@ whether the model answers requests.
 | `ReplicasReady` | Every replica in `spec.replicas` is at full capacity. |
 | `Ready` | `ReplicasReady` is True and the referenced `ModelCache` exists. |
 
-A deployment with `ReplicasServing=True` and `Ready=False` is degraded. It
-serves from the replicas and copies that are up. `kubectl get` shows
+A deployment with `ReplicasServing=True` and `Ready=False` is degraded, or is
+rolling out an update. It serves from the replicas and copies that are up. `kubectl get` shows
 `ReplicasServing` in the `SERVING` column, and a replica's `ModelServing` in
 its own.
 
@@ -274,9 +274,10 @@ waiting on:
 - `ModelServing`: at least one copy of every engine is ready, and the endpoint
   picker and routes in front of them are in place. The deployment routes to a
   replica only while it's serving.
-- `ModelReady`: every copy is ready. Its message counts the ready copies, and
-  its reason is `Degraded` while the replica serves below full capacity. The
-  replica's `Ready` mirrors it.
+- `ModelReady`: every copy is ready and runs the current spec. Its message
+  counts the ready copies. Its reason is `Degraded` while the replica serves
+  below full capacity, and `RollingOut` while an update replaces copies that
+  still serve. The replica's `Ready` mirrors it.
 
 To see which replica a deployment is waiting on:
 

functions/compose-model-replica/function/backends/base.py

@@ -491,20 +491,34 @@ def workload_keys(replica: v1alpha1.ModelReplica) -> list[str]:
 # CEL readiness query matching a Deployment's or LeaderWorkerSet's Available=True
 # condition. Both publish this condition but neither publishes a Ready
 # condition, so provider-kubernetes' DeriveFromObject policy (which only checks a
-# Ready condition) can never mark them ready. The has() guard keeps the query
-# false (not erroring) before the workload first writes status.conditions.
+# Ready condition) can never mark them ready. The has() guards keep the query
+# false (not erroring) before the workload first writes its status.
 AVAILABLE_CEL = (
-    'has(object.status.conditions) && object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
+    "has(object.status) && has(object.status.conditions) && "
+    'object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
 )
 
-# CEL readiness query for an engine's Deployment or LeaderWorkerSet with every
-# copy ready. Available alone isn't enough: a Deployment stays Available with up
-# to its maxUnavailable (25% by default) of copies down. So the query also
-# requires a ready pod, or ready group, per copy.
+# CEL readiness query for an engine's LeaderWorkerSet with every copy ready.
+# lws sets Available only once every group is ready and runs the current
+# template, and the query also requires a ready group per copy.
 ALL_COPIES_READY_CEL = (
     AVAILABLE_CEL + " && has(object.status.readyReplicas) && object.status.readyReplicas >= object.spec.replicas"
 )
 
+# CEL readiness query for an engine's Deployment with every copy ready and
+# running the current spec. Available alone isn't enough: a Deployment stays
+# Available with up to its maxUnavailable (25% by default) of copies down. A
+# ready pod per copy isn't enough either, because readyReplicas also counts
+# the previous ReplicaSet's pods while a rollout replaces them. So the query
+# also requires the Deployment controller to have processed the current spec,
+# every copy to be updated, and no pod from an older ReplicaSet to remain.
+DEPLOYMENT_ALL_COPIES_READY_CEL = (
+    ALL_COPIES_READY_CEL + " && has(object.status.observedGeneration)"
+    " && object.status.observedGeneration == object.metadata.generation"
+    " && has(object.status.updatedReplicas) && object.status.updatedReplicas >= object.spec.replicas"
+    " && has(object.status.replicas) && object.status.replicas == object.status.updatedReplicas"
+)
+
 # Grove publishes no Ready or Available condition on a PodCliqueSet, so
 # readiness comes from its replica counters: a replica counts as available once
 # every clique and scaling group is at or above minAvailable. observedGeneration

functions/compose-model-replica/function/backends/native.py

@@ -119,7 +119,7 @@ def build(
 
         composed = {
             base.workload_key(engine): base.wrap_object(
-                provider_config, deployment, cel_query=base.ALL_COPIES_READY_CEL, watch=True
+                provider_config, deployment, cel_query=base.DEPLOYMENT_ALL_COPIES_READY_CEL, watch=True
             ),
         }
         # Gated like the container's resources above, and for the same reason.

functions/compose-model-replica/function/fn.py

@@ -267,8 +267,8 @@ def ready(key: str) -> bool:
         if serving and sum(ready_copies.values()) < sum(copies.values()):
             waiting = CONDITION_REASON_DEGRADED
         elif serving:
-            # Every copy counts as ready but a workload doesn't yet, as during
-            # a LeaderWorkerSet's rolling update.
+            # Each copy counts as ready but a workload doesn't yet, as while a
+            # rolling update replaces copies that still serve.
             waiting = CONDITION_REASON_ROLLING_OUT
         elif accepted and copy_each:
             waiting = CONDITION_REASON_WAITING_FOR_ROUTING

functions/compose-model-replica/tests/test_backends.py

@@ -273,7 +273,8 @@ def _epp(
             "readiness": {
                 "policy": "DeriveFromCelQuery",
                 "celQuery": (
-                    'has(object.status.conditions) && object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
+                    "has(object.status) && has(object.status.conditions) && "
+                    'object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
                     ' && !object.status.conditions.exists(c, c.type == "Progressing" && c.status == "False")'
                 ),
             },
@@ -457,8 +458,13 @@ def _deployment(
             "readiness": {
                 "policy": "DeriveFromCelQuery",
                 "celQuery": (
-                    'has(object.status.conditions) && object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
+                    "has(object.status) && has(object.status.conditions) && "
+                    'object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
                     " && has(object.status.readyReplicas) && object.status.readyReplicas >= object.spec.replicas"
+                    " && has(object.status.observedGeneration)"
+                    " && object.status.observedGeneration == object.metadata.generation"
+                    " && has(object.status.updatedReplicas) && object.status.updatedReplicas >= object.spec.replicas"
+                    " && has(object.status.replicas) && object.status.replicas == object.status.updatedReplicas"
                 ),
             },
             "watch": True,
@@ -516,7 +522,8 @@ def _leader_worker_set(
             "readiness": {
                 "policy": "DeriveFromCelQuery",
                 "celQuery": (
-                    'has(object.status.conditions) && object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
+                    "has(object.status) && has(object.status.conditions) && "
+                    'object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
                     " && has(object.status.readyReplicas) && object.status.readyReplicas >= object.spec.replicas"
                 ),
             },

functions/compose-model-replica/tests/test_fn.py

@@ -237,8 +237,13 @@ def _deployment(*, ready: fnv1.Ready, name: str, claim_template_name: str, label
                     "readiness": {
                         "policy": "DeriveFromCelQuery",
                         "celQuery": (
-                            'has(object.status.conditions) && object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
+                            "has(object.status) && has(object.status.conditions) && "
+                            'object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
                             " && has(object.status.readyReplicas) && object.status.readyReplicas >= object.spec.replicas"
+                            " && has(object.status.observedGeneration)"
+                            " && object.status.observedGeneration == object.metadata.generation"
+                            " && has(object.status.updatedReplicas) && object.status.updatedReplicas >= object.spec.replicas"
+                            " && has(object.status.replicas) && object.status.replicas == object.status.updatedReplicas"
                         ),
                     },
                     "watch": True,
@@ -457,7 +462,8 @@ def _epp(*, ready: fnv1.Ready, config_checksum: str) -> fnv1.Resource:
                     "readiness": {
                         "policy": "DeriveFromCelQuery",
                         "celQuery": (
-                            'has(object.status.conditions) && object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
+                            "has(object.status) && has(object.status.conditions) && "
+                            'object.status.conditions.exists(c, c.type == "Available" && c.status == "True")'
                             ' && !object.status.conditions.exists(c, c.type == "Progressing" && c.status == "False")'
                         ),
                     },