Repository navigation
feat: Make Ready the summary of every other condition - #74
Merged
Merged
Conversation
Ready was True as soon as a MAIN was elected, so it stayed True through a roll, a scale-down, a resize or a rejected license. kstatus, and with it Flux health checks, decides on Ready alone for a custom resource, so a GitOps step waiting on a MemgraphCluster passed while the operator was still working. Ready is now True only when the cluster serves (a MAIN behind a reachable coordinator leader) and LicenseApplied, Converged and Updated are all True for the current generation. Otherwise it carries the reason of the first thing that is not, in that order: not serving (NoMainElected, WorkloadsNotReady, CoordinatorUnreachable, NoCoordinatorLeader, ApplyFailed), then the license, membership, and the roll, its message naming the condition the reason came from. A True decided for an older generation makes it Unknown/Reconciling. The new True reason is Reconciled; MainElected is gone from the API. Whether the cluster serves is now a pass-local verdict, recorded where Ready used to be set and folded into Ready by summarizeReady at the end of Reconcile, so it never reaches the API. A pass that ends before it observes the cluster keeps a not-serving Ready. status.main still says a MAIN is serving throughout. The envtest suite now creates the default license Secret every cluster reads, as an install following the README has, since without it LicenseApplied is pending and Ready can never be True. The quickstart waits on Ready.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ReadywasTrueas soon as a MAIN was elected, so it stayedTruethrough a roll, a scale-down, a resize or a rejected license. For a custom resource, kstatus (sigs.k8s.io/cli-utils, which Flux health checks use) checksobservedGeneration, thenReconciling/Stalled, thenReady, and treats everything else as Current. We set neither of the first two, so a GitOps step waiting on aMemgraphClusterpassed while the operator was still working. Strimzi's and CloudNativePG'sReadymean "reconciled and healthy". This change makes ours mean that too, keeping four condition types.What
Readyis now the summary:ReadyTrue/ReconciledLicenseApplied,ConvergedandUpdatedare allTruefor the current generationFalse/ the serving reasonNoMainElected,WorkloadsNotReady,CoordinatorUnreachable,NoCoordinatorLeader,ApplyFailed(same reasons as before)False/ borrowed reasonLicenseApplied→Converged→Updatedthat isn'tTruesupplies the reason, with a message such asConverged: Retiring data instance instance_2Unknown/ReconcilingTrueonly for an older generation, or hasn't been decided yetReadyused to be set (servingOrNot,notServingCondition) under an internalServingtype.summarizeReadyfolds it intoReadyat the end ofReconcileand removes it, so it never reaches the API. A pass that ends before observing the cluster (a deferred shrink, a StatefulSet being recreated) keeps a not-servingReadyas it was.status.mainstill names the serving MAIN throughout, so "serving but not done" stays visible without a fifth condition.ReasonReconciledadded;ReasonMainElectedremoved (it only described the serving verdict now). The CRD schema is unchanged, so no chart bump.READYcolumn, and the quickstart now waits onReady),docs/external-access.md,docs/flags.md,docs/monitoring.md, and a CLAUDE.md conventions bullet.Behavior change: anyone treating
Ready=Trueas "a MAIN is up" should readstatus.maininstead. It's acceptable onv1alpha1, but it's a change.Testing
readyCondition: all-True, not serving beats everything, license → membership → roll precedence, a staleTruegivesUnknown, and the no-observation fallback both ways.ReadystaysTruewhile X" now assert a MAIN still serves andReady=Falsewith X's reason (expectServingButNotReady). The bootstrap spec expectsReady=True/Reconciled.LicenseAppliedis pending andReadycould never beTrue. License specs use Secrets of their own.make test,make lint,make chart-verify. The e2e suite only reads pod readiness, so it's unaffected.