feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
Closed
bussyjd wants to merge 1 commit into
Closed
Conversation
…D v1alpha2-prep)
Bundled CRD-design fixes from the architecture review. Both rewrite
types.go and every *-crd.yaml — must not split (guaranteed merge
conflicts otherwise).
1. obol.org/paused annotation → spec.paused: bool + Paused condition
api-conventions: annotations are non-identifying metadata, NOT
control input. Compare Deployment.spec.paused, CronJob.spec.suspend.
Adds spec.paused (typed, validated by OpenAPI), keeps the annotation
read for one release (deprecation log on use), adds a Paused
condition to status as the observed-state mirror.
2. Custom Condition → metav1.Condition
Adds ObservedGeneration (api-conventions requires it on every
condition: it tells clients whether the condition refers to a
recent spec or a stale view).
Adds +listType=map + +listMapKey=type markers — without these,
SSA on conditions arrays creates duplicate entries when two
reconcilers touch different condition types. CRD YAMLs gain
x-kubernetes-list-type: map.
3. Ready is now a pure rollup
Computed once at end of Reconcile from
(ModelReady ∧ UpstreamHealthy ∧ PaymentGateReady ∧ RoutePublished
∧ Registered). Removes independent Ready setCondition calls that
were drifting from the predicate conditions.
This is the LAST of 14 PRs in the post-architecture-review roadmap
(items 11+12 bundled per the review's "do not split" recommendation).
Backwards compat:
- obol.org/paused annotation still honored (one release window).
- PausedByAnnotation reason on the Paused condition makes the
legacy code path visible to operators.
- Will drop annotation reading in v0.11.0.
Frontend pause/resume (currently writes the annotation) needs a
follow-up PR on obol-stack-front-end to flip to spec.paused — out of
scope here.
6 tasks
Collaborator
Author
5 tasks
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
The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite
types.goand every*-crd.yaml:1. Pause-as-annotation violates K8s convention
api-conventions: annotations are "non-identifying metadata", NOT control input. Upstream uses typed spec fields:
Deployment.spec.paused,CronJob.spec.suspend. The annotation approach loses:obol.org/paused: yesand silently get unpaused)2. Custom Condition type misses ObservedGeneration + SSA-safe merging
api-conventions: every condition MUST carry
observedGenerationso clients can tell stale-view from current-view. CRD YAML needsx-kubernetes-list-type: map+x-kubernetes-list-map-keys: [type]for SSA to dedupe-by-type instead of replacing the whole array.Before
After
What changed
internal/monetizeapi/types.go:[]Condition->[]metav1.Conditionin 3 status structs+listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=typemarkersspec.paused: booladded to ServiceOfferSpec (withkubebuilder:default=false)IsPaused()reads spec first, annotation as fallback (1-release deprecation)IsPausedByAnnotation()exposed so controller can emit deprecation log + tag Paused condition reasonPausedprinter column added (priority=1)internal/serviceoffercontroller/controller.go:PausedBySpec/PausedByAnnotation/NotPaused)rollupReady()(no independent Ready setCondition paths)internal/serviceoffercontroller/render.go+purchase.go+agent.go:setCondition/setPurchaseCondition/setAgentConditiondelegate toapimachinery/pkg/api/meta.SetStatusCondition(dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)isConditionTruedefers toapimeta.IsStatusConditionTrue*-crd.yamlregenerated viajust generate(controller-gen v0.16.5 from PR feat(monetizeapi): controller-gen as canonical CRD schema source #525):x-kubernetes-list-type: map+x-kubernetes-list-map-keys: [type]on every conditions arraypaused: type: boolean default: falseonspecobservedGenerationaccepted on every condition entrypaused_condition_test.gocovers: spec.paused path, annotation back-compat path, Paused condition presence + reason + ObservedGeneration, Ready rollup correctness (True only when all five predicates True), explicit-generation argument overrideembed_crd_test.go::TestServiceOfferCRD_PrinterColumnsupdated to expect the new Paused columnFrontend follow-up (out of scope)
obol-stack-front-endwritesobol.org/pauseddirectly viasell.ts:103,163. A separate follow-up PR will flip frontend writes tospec.paused. The 1-release annotation read keeps the FE working until that lands.Test plan
just generateproduces zero diff when re-run (idempotent)go build ./...cleango vetclean on changed packagesgo test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/...greenkubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge-> controller observes, sets Paused condition, tears down routeStacks on
PR #525 (controller-gen). Rebase onto main after PR #525 merges.
Final PR in roadmap
This is the 14th and final PR in the post-7-agent-architecture-review roadmap. The bundled items (11+12 in the original numbering) were merged into one PR per the review's "do not split" recommendation.