numaproj / numaplane

Control Plane for Numaproj
Apache License 2.0
4 stars 3 forks source link

Incorporate ObservedGeneration into Rollouts' Statuses #88

Closed juliev0 closed 3 days ago

juliev0 commented 1 week ago

Summary

When we return our health status to ArgoCD, we want to make sure that what we return isn't outdated. This article describes the risk. If a spec has changed but hasn't yet been reconciled, the Status may look like everything is in a good "Running" state even though it hasn't been reconciled.

For each of our Rollout CRDs, we should include an ObservedGeneration field in our Status.

We should also make sure that if we're still in the process of updating our CR, our Phase should be "Pending" and not "Running" yet. (Maybe also change the name "Running" to "Deployed".)

This way when we write our ArgoCD health check script, we can making it conditional that Rollout.metadata.Generation==Rollout.status.observedGeneration && Rollout.status.phase=="Deployed" && Rollout.status.conditions[ChildResourcesHealthy]==true (the first 2 conditions verify that we've incorporated the latest spec and also it's done being incorporated)

Consider that in order to truly decide we can be in "Deployed" state, ultimately we need to have confidence that our children have also been reconciled. Numaflow now has an issue that its Pipeline and ISBService need to include ObservedGeneration as well. So, if our child resource's generation matches its observedGeneration, we can set our own phase to "Deployed".

Note that for Pipelines, since we first pause (drain) the pipeline prior to updating it and running, we will be in "Pending" state that whole time.

Proposal linked here


Message from the maintainers:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

juliev0 commented 1 week ago

Added this document for CR Status change recommendation

xdevxy commented 3 days ago

Based on our discussion, summarized the latest proposal and linked in the description.