diff --git a/docs/content/models/model-deployment.md b/docs/content/models/model-deployment.md index e851d998f..0b7543934 100644 --- a/docs/content/models/model-deployment.md +++ b/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: diff --git a/functions/compose-model-replica/function/backends/base.py b/functions/compose-model-replica/function/backends/base.py index b08f67519..354db6da8 100644 --- a/functions/compose-model-replica/function/backends/base.py +++ b/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 diff --git a/functions/compose-model-replica/function/backends/native.py b/functions/compose-model-replica/function/backends/native.py index 5c8a3133a..3481a3654 100644 --- a/functions/compose-model-replica/function/backends/native.py +++ b/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. diff --git a/functions/compose-model-replica/function/fn.py b/functions/compose-model-replica/function/fn.py index 56be2da1d..c429ecd92 100644 --- a/functions/compose-model-replica/function/fn.py +++ b/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 diff --git a/functions/compose-model-replica/tests/test_backends.py b/functions/compose-model-replica/tests/test_backends.py index 3bc81c5e3..d4a0c0b7c 100644 --- a/functions/compose-model-replica/tests/test_backends.py +++ b/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" ), }, diff --git a/functions/compose-model-replica/tests/test_fn.py b/functions/compose-model-replica/tests/test_fn.py index 599432ec4..4ee370073 100644 --- a/functions/compose-model-replica/tests/test_fn.py +++ b/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")' ), },