fix: GRPCRoute health check ignores stale observedGeneration conditions (#28086)

Project: argoproj/argo-cd | Language: Go (Lua health checks) | Concepts: consistency-models, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/argoproj/argo-cd/pull/28087

The problem

The GRPCRoute health check logic was not filtering parent status conditions by their observedGeneration field. This meant that after a spec change, conditions from previous generations (which could be Degraded or Healthy) were still considered, even if the controller hadn't yet reconciled the new spec. This could cause Argo CD to incorrectly report the route as Healthy or Degraded based on outdated information.

Why it's non-trivial

A naive fix might just check the latest condition or aggregate all conditions, ignoring the fact that only those matching the current metadata.generation are valid. Without filtering by generation, the health check could misreport the resource status, especially during rollouts or rapid spec changes, where stale conditions linger until the controller updates them.

The approach

1. Introduce a helper to filter parents by observedGeneration:

+-- isParentGenerationObserved checks if a parent's conditions match the current resource generation
+-- For GRPCRoute, observedGeneration is stored in each condition within a parent
+function isParentGenerationObserved(obj, parent)
+  if obj.metadata.generation == nil then
+    -- If no generation is set, accept all conditions
+    return true
+  end
+
+  if parent.conditions == nil or #parent.conditions == 0 then
+    return false
+  end
+
+  -- Check if all conditions have observedGeneration matching current generation
+  for _, condition in ipairs(parent.conditions) do
+    if condition.observedGeneration ~= nil then
+      if condition.observedGeneration ~= obj.metadata.generation then
+        return false
+      end
+    end
+  end
+
+  return true
+end

This function returns true only if all conditions for a parent match the current resource generation. This ensures only up-to-date status is considered.

2. Apply the filter when checking parent conditions:

+        -- Skip this parent if it's not from the current generation
+        if not isParentGenerationObserved(obj, parent) then
+          goto continue
+        end

Parents with stale (outdated) conditions are skipped, so their status won't affect the health calculation.

3. Only report Healthy if a current-generation parent is found:

-          hs.status = "Healthy"
-          hs.message = "GRPCRoute is healthy"
-          return hs
+          -- Only mark as healthy if we found a parent from the current generation
+          if isParentGenerationObserved(obj, parent) then
+            hs.status = "Healthy"
+            hs.message = "GRPCRoute is healthy"
+            return hs
+          end

This prevents reporting Healthy if only stale parents are present.

4. Add targeted tests for multiple generations and all-stale cases:

+- healthStatus:
+    status: Healthy
+    message: GRPCRoute is healthy
+  inputPath: testdata/healthy_multiple_generations.yaml
+- healthStatus:
+    status: Progressing
+    message: Waiting for GRPCRoute status
+  inputPath: testdata/progressing_all_stale.yaml

These test cases ensure the logic is correct when both current and stale generations are present, and when only stale generations exist.

From the review thread

ppapapetrou76: can we add a UT for the case that all parents have stale conditions which means the status is "Progressing" ?

obliadp: Good call, added now

crenshaw-dev: I'm puzzled by the idea of skipping anything with a stale generation... What happens if we have 1 that is stale and 1 that is healthy? We'd just return Healthy? Wouldn't it make more sense to return Progressing?
Just trying to understand, you know the resource better than I do, so will trust your judgment. :-)

obliadp: The intent is to treat a stale parent as "no current information" rather than "failed" or "mid-rollout."
The only thing we never want stale conditions to do is drive a Degraded (that was the bug).
For Healthy vs Progressing, the logic is consistent: if no parent has current-gen conditions we fall through to Progressing ("waiting for status"), and if at least one parent confirms current-gen health we report Healthy.
An actual active parent still at gen 2 while the route is gen 3 just means that controller hasn't reconciled yet. it'll report on its own and flip us to Degraded if the new spec actually breaks it.

Takeaways

• Always filter status conditions by observedGeneration or equivalent version fields to avoid acting on stale data in distributed systems.
• Test edge cases where both current and stale status information coexist, as this is common during rollouts or reconciliation delays.
• Explicitly handle "no current information" as a distinct state (e.g., Progressing), rather than letting outdated failures or successes affect the resource's reported health.