feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep) - #526

Closed
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition
Closed

feat(api): spec.paused + metav1.Condition with listType=map (CRD v1alpha2-prep)#526
bussyjd wants to merge 1 commit into
feat/controller-gen-codegenfrom
feat/crd-paused-spec-and-metav1-condition

Conversation

@bussyjd

Copy link
Copy Markdown
Contributor

Why

The architecture review surfaced two CRD-design problems that bundle naturally because both rewrite types.go and 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:

  • OpenAPI validation (anyone can write obol.org/paused: yes and silently get unpaused)
  • RBAC scoping (annotation is part of metadata, gated by patch verb on parent)
  • SSA conflict detection (annotations are a single map field)
  • Status observability (no Paused condition; observers parse other-condition reasons)

2. Custom Condition type misses ObservedGeneration + SSA-safe merging

api-conventions: every condition MUST carry observedGeneration so clients can tell stale-view from current-view. CRD YAML needs x-kubernetes-list-type: map + x-kubernetes-list-map-keys: [type] for SSA to dedupe-by-type instead of replacing the whole array.

Before

 pause/resume:
metadata.annotations["obol.org/paused"] = "true" <- annotation
status synthesized: PaymentGateReady=False, reason=Paused <- side effect
conditions:
custom Condition{Type, Status, Reason, Message, LastTransitionTime}
CRD yaml: type: array (no listType)
<- SSA replaces the whole array; second reconciler stomps
no observedGeneration field
<- clients can't tell if condition reflects current spec
Ready:
setCondition(\"Ready\", ...) called from several reconcile paths
<- drift between Ready and the predicate conditions

After

 pause/resume:
spec.paused: bool <- typed, validated
status.conditions[Paused]: {Status, Reason: PausedBySpec | PausedByAnnotation, ObservedGeneration}
legacy annotation still read for one release <- deprecation log on use
conditions:
metav1.Condition (upstream type, has ObservedGeneration)
CRD yaml: x-kubernetes-list-type: map <- SSA-safe merging
x-kubernetes-list-map-keys: [\"type\"]
+patchStrategy=merge +patchMergeKey=type
Ready:
pure rollup: ModelReady AND UpstreamHealthy AND PaymentGateReady
AND RoutePublished AND Registered
computed once at end of Reconcile
no independent Ready setCondition calls

What changed

  • internal/monetizeapi/types.go:
    • []Condition -> []metav1.Condition in 3 status structs
    • +listType=map +listMapKey=type +patchStrategy=merge +patchMergeKey=type markers
    • spec.paused: bool added to ServiceOfferSpec (with kubebuilder:default=false)
    • IsPaused() reads spec first, annotation as fallback (1-release deprecation)
    • IsPausedByAnnotation() exposed so controller can emit deprecation log + tag Paused condition reason
    • Paused printer column added (priority=1)
  • internal/serviceoffercontroller/controller.go:
    • Sets typed Paused condition each reconcile (PausedBySpec / PausedByAnnotation / NotPaused)
    • Once-per-offer deprecation log when reading paused state from annotation
    • Ready computed as final rollup via rollupReady() (no independent Ready setCondition paths)
  • internal/serviceoffercontroller/render.go + purchase.go + agent.go:
    • setCondition / setPurchaseCondition / setAgentCondition delegate to apimachinery/pkg/api/meta.SetStatusCondition (dedupe-by-type, lastTransitionTime, ObservedGeneration handled centrally)
    • isConditionTrue defers to apimeta.IsStatusConditionTrue
  • 5 *-crd.yaml regenerated via just 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 array
    • paused: type: boolean default: false on spec
    • observedGeneration accepted on every condition entry
    • Printer column for Paused
  • New paused_condition_test.go covers: 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 override
  • Existing embed_crd_test.go::TestServiceOfferCRD_PrinterColumns updated to expect the new Paused column

Frontend follow-up (out of scope)

obol-stack-front-end writes obol.org/paused directly via sell.ts:103,163. A separate follow-up PR will flip frontend writes to spec.paused. The 1-release annotation read keeps the FE working until that lands.

Test plan

  • just generate produces zero diff when re-run (idempotent)
  • go build ./... clean
  • go vet clean on changed packages
  • go test ./internal/monetizeapi/... ./internal/serviceoffercontroller/... ./internal/x402/... ./internal/embed/... ./cmd/obol/... green
  • Tests: spec.paused respected, annotation respected (backwards compat), Paused condition appears, Ready is the rollup, ObservedGeneration populated
  • Manual: kubectl patch serviceoffer foo -p '{\"spec\":{\"paused\":true}}' --type=merge -> controller observes, sets Paused condition, tears down route

Stacks 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.

…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.
@bussyjd

Copy link
Copy Markdown
ContributorAuthor

Obsoleted by #535 which removes pause entirely. Bundle PR #536 includes the drain replacement.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bussyjd