From fdb3001798a737aa2cfccaf60d3eeeca22baf818 Mon Sep 17 00:00:00 2001 From: bussyjd Date: Sat, 23 May 2026 23:34:39 +0400 Subject: [PATCH] feat(api): spec.paused field + metav1.Condition with listType=map (CRD v1alpha2-prep) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- cmd/obol/sell.go | 7 +- cmd/obol/sell_test.go | 19 +- internal/embed/embed_crd_test.go | 2 +- .../base/templates/agent-crd.yaml | 41 +++- .../base/templates/purchaserequest-crd.yaml | 41 +++- .../base/templates/serviceoffer-crd.yaml | 60 +++++- internal/monetizeapi/types.go | 76 +++++--- internal/monetizeapi/zz_generated.deepcopy.go | 23 +-- internal/serviceoffercontroller/agent.go | 40 ++-- internal/serviceoffercontroller/agent_test.go | 4 +- internal/serviceoffercontroller/controller.go | 88 +++++++-- .../serviceoffercontroller/controller_test.go | 2 +- .../identity_render_test.go | 2 +- .../paused_condition_test.go | 178 ++++++++++++++++++ internal/serviceoffercontroller/purchase.go | 58 +++--- .../purchase_lifecycle_test.go | 14 +- .../purchase_pure_test.go | 9 +- internal/serviceoffercontroller/render.go | 58 +++--- .../serviceoffercontroller/render_test.go | 44 ++--- internal/x402/network_norm_test.go | 2 +- internal/x402/serviceoffer_source_test.go | 8 +- 21 files changed, 568 insertions(+), 208 deletions(-) create mode 100644 internal/serviceoffercontroller/paused_condition_test.go diff --git a/cmd/obol/sell.go b/cmd/obol/sell.go index 379fb2d9..8cff1fc5 100644 --- a/cmd/obol/sell.go +++ b/cmd/obol/sell.go @@ -40,6 +40,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/urfave/cli/v3" "gopkg.in/yaml.v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func sellCommand(cfg *config.Config) *cli.Command { @@ -1236,7 +1237,7 @@ func explorerTxURL(network, txHash string) string { // or "Disabled", non-failure paths // ⚠ Status=False — failure / blocked // ⏳ Status=Unknown / empty — pending -func formatConditionLine(cond monetizeapi.Condition) string { +func formatConditionLine(cond metav1.Condition) string { icon := conditionIcon(cond) parts := []string{cond.Type} if cond.Reason != "" { @@ -1252,7 +1253,7 @@ func formatConditionLine(cond monetizeapi.Condition) string { // conditionIcon picks a glyph based on the condition's status + reason. The // glyphs are plain unicode (no lipgloss) so the function is safe to call from // pure unit tests; coloring is applied at print time via the ui package. -func conditionIcon(cond monetizeapi.Condition) string { +func conditionIcon(cond metav1.Condition) string { switch cond.Status { case "True": switch cond.Reason { @@ -2105,7 +2106,7 @@ func explorerBaseURL(canonicalName string) string { } // isConditionTrue reports whether the named condition exists with Status=True. -func isConditionTrue(conds []monetizeapi.Condition, name string) bool { +func isConditionTrue(conds []metav1.Condition, name string) bool { for _, c := range conds { if c.Type == name { return c.Status == "True" diff --git a/cmd/obol/sell_test.go b/cmd/obol/sell_test.go index 9f3991ea..5b68812b 100644 --- a/cmd/obol/sell_test.go +++ b/cmd/obol/sell_test.go @@ -16,6 +16,7 @@ import ( x402verifier "github.com/ObolNetwork/obol-stack/internal/x402" "github.com/urfave/cli/v3" "gopkg.in/yaml.v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // ───────────────────────────────────────────────────────────────────────────── @@ -315,7 +316,7 @@ func TestServiceOfferStatusLines(t *testing.T) { Endpoint: "/services/demo", AgentID: "5008", RegistrationTxHash: "0xabc", - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "Registered", Status: "True", Reason: "Registered", Message: "Published registration document and recorded agent 5008"}, }, }, @@ -455,15 +456,15 @@ func TestExplorerTxURL(t *testing.T) { func TestConditionIcon(t *testing.T) { tests := []struct { name string - cond monetizeapi.Condition + cond metav1.Condition want string }{ - {"true succeeded", monetizeapi.Condition{Status: "True", Reason: "Reconciled"}, "✓"}, - {"true skipped", monetizeapi.Condition{Status: "True", Reason: "Skipped"}, "ℹ"}, - {"true disabled", monetizeapi.Condition{Status: "True", Reason: "Disabled"}, "ℹ"}, - {"false failed", monetizeapi.Condition{Status: "False", Reason: "Unhealthy"}, "⚠"}, - {"unknown pending", monetizeapi.Condition{Status: "Unknown"}, "⏳"}, - {"empty pending", monetizeapi.Condition{}, "⏳"}, + {"true succeeded", metav1.Condition{Status: "True", Reason: "Reconciled"}, "✓"}, + {"true skipped", metav1.Condition{Status: "True", Reason: "Skipped"}, "ℹ"}, + {"true disabled", metav1.Condition{Status: "True", Reason: "Disabled"}, "ℹ"}, + {"false failed", metav1.Condition{Status: "False", Reason: "Unhealthy"}, "⚠"}, + {"unknown pending", metav1.Condition{Status: "Unknown"}, "⏳"}, + {"empty pending", metav1.Condition{}, "⏳"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -504,7 +505,7 @@ func TestAgentRegistryNFTURL(t *testing.T) { } func TestIsConditionTrue(t *testing.T) { - conds := []monetizeapi.Condition{ + conds := []metav1.Condition{ {Type: "Ready", Status: "True"}, {Type: "Registered", Status: "False"}, {Type: "Pending", Status: "Unknown"}, diff --git a/internal/embed/embed_crd_test.go b/internal/embed/embed_crd_test.go index 2fca04f6..81cc9068 100644 --- a/internal/embed/embed_crd_test.go +++ b/internal/embed/embed_crd_test.go @@ -194,7 +194,7 @@ func TestServiceOfferCRD_PrinterColumns(t *testing.T) { t.Fatal("additionalPrinterColumns missing or wrong type") } - expected := []string{"Type", "Model", "Price", "Network", "Ready", "Age"} + expected := []string{"Type", "Model", "Price", "Network", "Ready", "Paused", "Age"} if len(cols) != len(expected) { t.Fatalf("got %d printer columns, want %d", len(cols), len(expected)) } diff --git a/internal/embed/infrastructure/base/templates/agent-crd.yaml b/internal/embed/infrastructure/base/templates/agent-crd.yaml index 8338c0d6..9ac6302f 100644 --- a/internal/embed/infrastructure/base/templates/agent-crd.yaml +++ b/internal/embed/infrastructure/base/templates/agent-crd.yaml @@ -106,32 +106,63 @@ spec: properties: conditions: items: + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: - description: Last time the condition transitioned. + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. format: date-time type: string message: - description: Human-readable message with details. + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer reason: - description: Machine-readable reason for the condition. + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ type: string status: - description: Status of the condition. + description: status of the condition, one of True, False, Unknown. enum: - "True" - "False" - Unknown type: string type: - description: Condition type. + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string required: + - lastTransitionTime + - message + - reason - status - type type: object type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map endpoint: description: |- Cluster-internal URL for the agent runtime (e.g. diff --git a/internal/embed/infrastructure/base/templates/purchaserequest-crd.yaml b/internal/embed/infrastructure/base/templates/purchaserequest-crd.yaml index af32f441..12571065 100644 --- a/internal/embed/infrastructure/base/templates/purchaserequest-crd.yaml +++ b/internal/embed/infrastructure/base/templates/purchaserequest-crd.yaml @@ -174,32 +174,63 @@ spec: properties: conditions: items: + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: - description: Last time the condition transitioned. + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. format: date-time type: string message: - description: Human-readable message with details. + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer reason: - description: Machine-readable reason for the condition. + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ type: string status: - description: Status of the condition. + description: status of the condition, one of True, False, Unknown. enum: - "True" - "False" - Unknown type: string type: - description: Condition type. + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string required: + - lastTransitionTime + - message + - reason - status - type type: object type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map observedGeneration: format: int64 type: integer diff --git a/internal/embed/infrastructure/base/templates/serviceoffer-crd.yaml b/internal/embed/infrastructure/base/templates/serviceoffer-crd.yaml index 9bac5643..bc54aaa8 100644 --- a/internal/embed/infrastructure/base/templates/serviceoffer-crd.yaml +++ b/internal/embed/infrastructure/base/templates/serviceoffer-crd.yaml @@ -32,6 +32,10 @@ spec: - jsonPath: .status.conditions[?(@.type=="Ready")].status name: Ready type: string + - jsonPath: .status.conditions[?(@.type=="Paused")].status + name: Paused + priority: 1 + type: boolean - jsonPath: .metadata.creationTimestamp name: Age type: date @@ -101,6 +105,19 @@ spec: description: URL path prefix for the HTTPRoute, defaults to /services/. pattern: ^/[a-zA-Z0-9/_.-]*$ type: string + paused: + default: false + description: |- + Paused, when true, signals the controller to tear down the + published route and stop serving traffic for this offer. The + ServiceOffer CR itself is preserved (annotations, status, + payment config) so resume is a single-byte spec flip. + + Compare Deployment.spec.paused, CronJob.spec.suspend — typed + spec fields are the K8s api-conventions canonical way to gate + reconciler behaviour. The legacy obol.org/paused annotation is + still honoured for one release; see IsPaused(). + type: boolean payment: properties: asset: @@ -319,34 +336,65 @@ spec: conditions: description: |- Condition types: ModelReady, UpstreamHealthy, PaymentGateReady, - RoutePublished, Registered, Ready. + RoutePublished, Registered, Paused, Ready. items: + description: Condition contains details for one aspect of the current + state of this API Resource. properties: lastTransitionTime: - description: Last time the condition transitioned. + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. format: date-time type: string message: - description: Human-readable message with details. + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer reason: - description: Machine-readable reason for the condition. + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ type: string status: - description: Status of the condition. + description: status of the condition, one of True, False, Unknown. enum: - "True" - "False" - Unknown type: string type: - description: Condition type. + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ type: string required: + - lastTransitionTime + - message + - reason - status - type type: object type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map endpoint: description: The public endpoint URL once the route is published. type: string diff --git a/internal/monetizeapi/types.go b/internal/monetizeapi/types.go index ac104b7c..49f41b0f 100644 --- a/internal/monetizeapi/types.go +++ b/internal/monetizeapi/types.go @@ -71,6 +71,7 @@ var ( // +kubebuilder:printcolumn:name="Price",type=string,JSONPath=`.spec.payment.price.perRequest` // +kubebuilder:printcolumn:name="Network",type=string,JSONPath=`.spec.payment.network` // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` +// +kubebuilder:printcolumn:name="Paused",type=boolean,JSONPath=`.status.conditions[?(@.type=="Paused")].status`,priority=1 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` // ServiceOffer declares a compute service that can be exposed publicly, @@ -126,6 +127,20 @@ type ServiceOfferSpec struct { // ERC-8004 registration metadata. Field names align with the // AgentRegistration document schema (ERC-8004 spec). Registration ServiceOfferRegistration `json:"registration,omitempty"` + + // Paused, when true, signals the controller to tear down the + // published route and stop serving traffic for this offer. The + // ServiceOffer CR itself is preserved (annotations, status, + // payment config) so resume is a single-byte spec flip. + // + // Compare Deployment.spec.paused, CronJob.spec.suspend — typed + // spec fields are the K8s api-conventions canonical way to gate + // reconciler behaviour. The legacy obol.org/paused annotation is + // still honoured for one release; see IsPaused(). + // + // +optional + // +kubebuilder:default=false + Paused bool `json:"paused,omitempty"` } // ServiceOfferAgent is populated when Spec.Type == "agent". The controller @@ -266,8 +281,14 @@ type ServiceOfferService struct { type ServiceOfferStatus struct { // Condition types: ModelReady, UpstreamHealthy, PaymentGateReady, - // RoutePublished, Registered, Ready. - Conditions []Condition `json:"conditions,omitempty"` + // RoutePublished, Registered, Paused, Ready. + // + // +listType=map + // +listMapKey=type + // +optional + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` // The public endpoint URL once the route is published. Endpoint string `json:"endpoint,omitempty"` // ERC-8004 agent NFT token ID after on-chain registration. @@ -293,22 +314,6 @@ type ServiceOfferAgentResolution struct { Endpoint string `json:"endpoint,omitempty"` } -type Condition struct { - // Condition type. - // +kubebuilder:validation:Required - Type string `json:"type"` - // Status of the condition. - // +kubebuilder:validation:Required - // +kubebuilder:validation:Enum=True;False;Unknown - Status string `json:"status"` - // Machine-readable reason for the condition. - Reason string `json:"reason,omitempty"` - // Human-readable message with details. - Message string `json:"message,omitempty"` - // Last time the condition transitioned. - LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` -} - // ── RegistrationRequest ───────────────────────────────────────────────────── // +kubebuilder:object:root=true @@ -402,7 +407,24 @@ func (o *ServiceOffer) IsAgent() bool { return o.Spec.Type == "agent" } +// IsPaused returns true if the offer is paused via spec.paused +// (preferred) or the legacy obol.org/paused annotation. The +// annotation read will be dropped in v0.11.0 — migrate now. func (o *ServiceOffer) IsPaused() bool { + if o.Spec.Paused { + return true + } + return o.Annotations != nil && o.Annotations[PausedAnnotation] == "true" +} + +// IsPausedByAnnotation returns true when the offer is paused only +// because of the legacy obol.org/paused annotation (spec.paused is +// false). Callers use this to emit a one-time deprecation log so +// operators know to migrate to spec.paused before v0.11.0. +func (o *ServiceOffer) IsPausedByAnnotation() bool { + if o.Spec.Paused { + return false + } return o.Annotations != nil && o.Annotations[PausedAnnotation] == "true" } @@ -519,8 +541,13 @@ type PurchasePayment struct { } type PurchaseRequestStatus struct { - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - Conditions []Condition `json:"conditions,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // +listType=map + // +listMapKey=type + // +optional + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` // LiteLLM model name (paid/). PublicModel string `json:"publicModel,omitempty"` Remaining int `json:"remaining,omitempty"` @@ -612,8 +639,13 @@ type AgentStatus struct { WalletAddress string `json:"walletAddress,omitempty"` // Cluster-internal URL for the agent runtime (e.g. // http://hermes.agent-quant.svc.cluster.local:8642). - Endpoint string `json:"endpoint,omitempty"` - Conditions []Condition `json:"conditions,omitempty"` + Endpoint string `json:"endpoint,omitempty"` + // +listType=map + // +listMapKey=type + // +optional + // +patchStrategy=merge + // +patchMergeKey=type + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } func (a *Agent) EffectiveRuntime() string { diff --git a/internal/monetizeapi/zz_generated.deepcopy.go b/internal/monetizeapi/zz_generated.deepcopy.go index ca9fdb32..482cee16 100644 --- a/internal/monetizeapi/zz_generated.deepcopy.go +++ b/internal/monetizeapi/zz_generated.deepcopy.go @@ -7,6 +7,7 @@ package monetizeapi import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -204,7 +205,7 @@ func (in *AgentStatus) DeepCopyInto(out *AgentStatus) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]Condition, len(*in)) + *out = make([]v1.Condition, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -236,22 +237,6 @@ func (in *AgentWallet) DeepCopy() *AgentWallet { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Condition) DeepCopyInto(out *Condition) { - *out = *in - in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Condition. -func (in *Condition) DeepCopy() *Condition { - if in == nil { - return nil - } - out := new(Condition) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PurchaseAutoRefill) DeepCopyInto(out *PurchaseAutoRefill) { *out = *in @@ -370,7 +355,7 @@ func (in *PurchaseRequestStatus) DeepCopyInto(out *PurchaseRequestStatus) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]Condition, len(*in)) + *out = make([]v1.Condition, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -737,7 +722,7 @@ func (in *ServiceOfferStatus) DeepCopyInto(out *ServiceOfferStatus) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]Condition, len(*in)) + *out = make([]v1.Condition, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } diff --git a/internal/serviceoffercontroller/agent.go b/internal/serviceoffercontroller/agent.go index 9235121a..bb86c53d 100644 --- a/internal/serviceoffercontroller/agent.go +++ b/internal/serviceoffercontroller/agent.go @@ -6,11 +6,11 @@ import ( "fmt" "log" "strings" - "time" "github.com/ObolNetwork/obol-stack/internal/monetizeapi" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -567,32 +567,18 @@ func (c *Controller) isAgentDeploymentReady(ctx context.Context, namespace strin // same as the rest of the controller; Go resolves them by receiver.) // setAgentCondition is the AgentStatus equivalent of setCondition in -// render.go. Kept separate from the ServiceOffer flavour because the -// existing helper is typed against ServiceOfferStatus; refactoring both -// onto a shared []Condition helper is a clean follow-up but out of scope -// for the agent reconciler skeleton. +// render.go. Wraps apimeta.SetStatusCondition and threads +// status.ObservedGeneration into ObservedGeneration so observers can +// tell stale-view conditions from current-view (api-conventions). func setAgentCondition(status *monetizeapi.AgentStatus, conditionType, conditionStatus, reason, message string) { - now := metav1.NewTime(time.Now().UTC()) - for i := range status.Conditions { - if status.Conditions[i].Type != conditionType { - continue - } - if status.Conditions[i].Status != conditionStatus { - status.Conditions[i].LastTransitionTime = now - } - status.Conditions[i].Status = conditionStatus - status.Conditions[i].Reason = reason - status.Conditions[i].Message = message - if status.Conditions[i].LastTransitionTime.IsZero() { - status.Conditions[i].LastTransitionTime = now - } - return + cond := metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionStatus(conditionStatus), + Reason: reason, + Message: message, } - status.Conditions = append(status.Conditions, monetizeapi.Condition{ - Type: conditionType, - Status: conditionStatus, - Reason: reason, - Message: message, - LastTransitionTime: now, - }) + if status.ObservedGeneration > 0 { + cond.ObservedGeneration = status.ObservedGeneration + } + apimeta.SetStatusCondition(&status.Conditions, cond) } diff --git a/internal/serviceoffercontroller/agent_test.go b/internal/serviceoffercontroller/agent_test.go index d6717a07..0040882b 100644 --- a/internal/serviceoffercontroller/agent_test.go +++ b/internal/serviceoffercontroller/agent_test.go @@ -454,7 +454,7 @@ func getAgent(t *testing.T, c *Controller, namespace, name string) *monetizeapi. return &a } -func agentCondition(t *testing.T, a *monetizeapi.Agent, condType string) monetizeapi.Condition { +func agentCondition(t *testing.T, a *monetizeapi.Agent, condType string) metav1.Condition { t.Helper() for _, c := range a.Status.Conditions { if c.Type == condType { @@ -462,5 +462,5 @@ func agentCondition(t *testing.T, a *monetizeapi.Agent, condType string) monetiz } } t.Fatalf("missing agent condition %q", condType) - return monetizeapi.Condition{} + return metav1.Condition{} } diff --git a/internal/serviceoffercontroller/controller.go b/internal/serviceoffercontroller/controller.go index fac586b0..f3c84e4b 100644 --- a/internal/serviceoffercontroller/controller.go +++ b/internal/serviceoffercontroller/controller.go @@ -77,6 +77,12 @@ type Controller struct { pendingAuths sync.Map // key: "ns/name" → []map[string]string + // loggedPausedAnnotationMigration tracks "ns/name" → struct{} for + // offers that have already had their annotation-based-pause read + // logged once, so we don't spam the controller log every reconcile. + // Will be removed alongside the annotation read in v0.11.0. + loggedPausedAnnotationMigration sync.Map + httpClient *http.Client // litellmURLOverride is used in tests to point at a local httptest server @@ -431,6 +437,16 @@ func (c *Controller) reconcileOffer(ctx context.Context, key string) error { status.ObservedGeneration = offer.Generation status.Endpoint = offer.EffectivePath() + // Mirror the (spec.paused | legacy annotation) intent as a typed + // status condition so observers can read pause state from the + // canonical status block, not by parsing other-condition reasons. + paused := offer.IsPaused() + pausedByAnnotation := offer.IsPausedByAnnotation() + c.recordPausedCondition(&status, offer, paused, pausedByAnnotation) + if pausedByAnnotation { + c.logPausedAnnotationDeprecationOnce(offer) + } + if offer.IsAgent() { ready, resolveErr := c.resolveAgentOffer(ctx, offer, &status) if resolveErr != nil { @@ -441,7 +457,11 @@ func (c *Controller) reconcileOffer(ctx context.Context, key string) error { setCondition(&status, "UpstreamHealthy", "False", "WaitingForAgent", "Referenced Agent is not yet Ready") setCondition(&status, "PaymentGateReady", "False", "WaitingForAgent", "Referenced Agent is not yet Ready") setCondition(&status, "RoutePublished", "False", "WaitingForAgent", "Referenced Agent is not yet Ready") - setCondition(&status, "Ready", "False", "WaitingForAgent", "Referenced Agent is not yet Ready") + // Ready is now a pure rollup computed at end-of-reconcile from + // the predicate conditions above. Don't set it independently + // here; the rollup will naturally evaluate to False because + // the predicates are False. + rollupReady(&status, offer.Generation) return c.updateOfferStatus(ctx, raw, status) } } @@ -455,7 +475,7 @@ func (c *Controller) reconcileOffer(ctx context.Context, key string) error { return err } - if offer.IsPaused() { + if paused { if err := c.deleteRouteChildren(ctx, offer); err != nil { return err } @@ -479,16 +499,7 @@ func (c *Controller) reconcileOffer(ctx context.Context, key string) error { return err } - ready := isConditionTrue(status, "ModelReady") && - isConditionTrue(status, "UpstreamHealthy") && - isConditionTrue(status, "PaymentGateReady") && - isConditionTrue(status, "RoutePublished") && - isConditionTrue(status, "Registered") - if ready { - setCondition(&status, "Ready", "True", "Reconciled", "Offer reconciled successfully") - } else { - setCondition(&status, "Ready", "False", "Reconciling", "Offer is not fully reconciled yet") - } + ready := rollupReady(&status, offer.Generation) if err := c.updateOfferStatus(ctx, raw, status); err != nil { return err @@ -1393,3 +1404,56 @@ func httpRouteAccepted(route *unstructured.Unstructured) bool { func md5Sum(content string) [16]byte { return md5.Sum([]byte(content)) } + +// recordPausedCondition writes the typed Paused condition to status, +// distinguishing whether the pause came from spec.paused (the canonical +// path) or the legacy obol.org/paused annotation (one-release back-compat). +// The condition mirrors observed state so observers can read pause from +// status.conditions[type==Paused], not by parsing other-condition reasons. +func (c *Controller) recordPausedCondition(status *monetizeapi.ServiceOfferStatus, offer *monetizeapi.ServiceOffer, paused, byAnnotation bool) { + switch { + case paused && byAnnotation: + setConditionWithGeneration(status, "Paused", "True", "PausedByAnnotation", + "Offer paused via legacy obol.org/paused annotation (migrate to spec.paused before v0.11.0)", + offer.Generation) + case paused: + setConditionWithGeneration(status, "Paused", "True", "PausedBySpec", + "Offer paused via spec.paused=true", + offer.Generation) + default: + setConditionWithGeneration(status, "Paused", "False", "NotPaused", + "Offer is not paused", offer.Generation) + } +} + +// logPausedAnnotationDeprecationOnce emits a single warning per offer +// when the controller observes pause state via the legacy annotation +// instead of spec.paused. The dedupe map prevents log spam across +// reconcile passes. Will be removed alongside the annotation read in +// v0.11.0. +func (c *Controller) logPausedAnnotationDeprecationOnce(offer *monetizeapi.ServiceOffer) { + key := offer.Namespace + "/" + offer.Name + if _, loaded := c.loggedPausedAnnotationMigration.LoadOrStore(key, struct{}{}); loaded { + return + } + log.Printf("ServiceOffer %s: paused via legacy obol.org/paused annotation; migrate to spec.paused. Annotation read will be removed in v0.11.0", key) +} + +// rollupReady computes the canonical Ready condition as the AND of the +// predicate conditions and writes it back to status. Returns the final +// boolean so callers can decide whether to requeue. Centralising this +// removes drift between independent setCondition("Ready", ...) call +// sites that previously made Ready disagree with the predicates. +func rollupReady(status *monetizeapi.ServiceOfferStatus, generation int64) bool { + ready := isConditionTrue(*status, "ModelReady") && + isConditionTrue(*status, "UpstreamHealthy") && + isConditionTrue(*status, "PaymentGateReady") && + isConditionTrue(*status, "RoutePublished") && + isConditionTrue(*status, "Registered") + if ready { + setConditionWithGeneration(status, "Ready", "True", "Reconciled", "Offer reconciled successfully", generation) + } else { + setConditionWithGeneration(status, "Ready", "False", "Reconciling", "Offer is not fully reconciled yet", generation) + } + return ready +} diff --git a/internal/serviceoffercontroller/controller_test.go b/internal/serviceoffercontroller/controller_test.go index 8d308b0b..7a98cdd0 100644 --- a/internal/serviceoffercontroller/controller_test.go +++ b/internal/serviceoffercontroller/controller_test.go @@ -104,7 +104,7 @@ func TestPurchaseReadyRequiresRuntimePoolToMatchSpec(t *testing.T) { func TestApplySharedRegistrationStatus_NonOwnerUsesSharedAgent(t *testing.T) { status := &monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "RoutePublished", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "RoutePublished", Status: "True"}}, } owner := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "alpha", Namespace: "demo"}} offer := &monetizeapi.ServiceOffer{ObjectMeta: metav1.ObjectMeta{Name: "beta", Namespace: "demo"}} diff --git a/internal/serviceoffercontroller/identity_render_test.go b/internal/serviceoffercontroller/identity_render_test.go index 1e8fd2b9..0cbef5f6 100644 --- a/internal/serviceoffercontroller/identity_render_test.go +++ b/internal/serviceoffercontroller/identity_render_test.go @@ -21,7 +21,7 @@ func readyOffer(name string) *monetizeapi.ServiceOffer { o.Spec.Registration.Name = name o.Spec.Registration.Skills = []string{"chat/general"} for _, t := range []string{"ModelReady", "UpstreamHealthy", "PaymentGateReady", "RoutePublished"} { - o.Status.Conditions = append(o.Status.Conditions, monetizeapi.Condition{Type: t, Status: "True"}) + o.Status.Conditions = append(o.Status.Conditions, metav1.Condition{Type: t, Status: "True"}) } return o } diff --git a/internal/serviceoffercontroller/paused_condition_test.go b/internal/serviceoffercontroller/paused_condition_test.go new file mode 100644 index 00000000..319540ff --- /dev/null +++ b/internal/serviceoffercontroller/paused_condition_test.go @@ -0,0 +1,178 @@ +package serviceoffercontroller + +import ( + "strings" + "testing" + + "github.com/ObolNetwork/obol-stack/internal/monetizeapi" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// These tests cover the CRD-design fixes in the +// spec.paused + metav1.Condition PR. The behaviour they pin down is +// also exercised end-to-end by render_test.go / controller_test.go, +// but this file isolates each invariant so a future refactor that +// breaks one of them fails with a name that matches the invariant. + +// --- IsPaused: spec field wins, annotation is back-compat ------------------- + +func TestIsPaused_SpecPausedTakesPrecedence(t *testing.T) { + offer := &monetizeapi.ServiceOffer{ + Spec: monetizeapi.ServiceOfferSpec{Paused: true}, + } + if !offer.IsPaused() { + t.Fatalf("spec.paused=true should pause") + } + if offer.IsPausedByAnnotation() { + t.Fatalf("spec.paused=true must not register as paused-by-annotation") + } +} + +func TestIsPaused_LegacyAnnotationStillHonoured(t *testing.T) { + offer := &monetizeapi.ServiceOffer{} + offer.Annotations = map[string]string{monetizeapi.PausedAnnotation: "true"} + if !offer.IsPaused() { + t.Fatalf("legacy obol.org/paused=true should still pause for back-compat") + } + if !offer.IsPausedByAnnotation() { + t.Fatalf("annotation-only pause must register as paused-by-annotation so deprecation log fires") + } +} + +func TestIsPaused_NotPaused(t *testing.T) { + offer := &monetizeapi.ServiceOffer{} + if offer.IsPaused() { + t.Fatalf("default ServiceOffer must not be paused") + } +} + +// --- recordPausedCondition writes a typed status condition ------------------ + +func TestRecordPausedCondition_SpecPath(t *testing.T) { + c := &Controller{} + offer := &monetizeapi.ServiceOffer{ + Spec: monetizeapi.ServiceOfferSpec{Paused: true}, + } + offer.Generation = 7 + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 7} + + c.recordPausedCondition(&status, offer, offer.IsPaused(), offer.IsPausedByAnnotation()) + + cond := apimeta.FindStatusCondition(status.Conditions, "Paused") + if cond == nil { + t.Fatalf("Paused condition was not appended to status") + } + if cond.Status != metav1.ConditionTrue { + t.Fatalf("Paused.Status = %q, want True", cond.Status) + } + if cond.Reason != "PausedBySpec" { + t.Fatalf("Paused.Reason = %q, want PausedBySpec", cond.Reason) + } + if cond.ObservedGeneration != 7 { + t.Fatalf("Paused.ObservedGeneration = %d, want 7", cond.ObservedGeneration) + } +} + +func TestRecordPausedCondition_AnnotationPath(t *testing.T) { + c := &Controller{} + offer := &monetizeapi.ServiceOffer{} + offer.Generation = 3 + offer.Annotations = map[string]string{monetizeapi.PausedAnnotation: "true"} + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 3} + + c.recordPausedCondition(&status, offer, offer.IsPaused(), offer.IsPausedByAnnotation()) + + cond := apimeta.FindStatusCondition(status.Conditions, "Paused") + if cond == nil { + t.Fatalf("Paused condition missing") + } + if cond.Reason != "PausedByAnnotation" { + t.Fatalf("Paused.Reason = %q, want PausedByAnnotation (so operators see the deprecation)", cond.Reason) + } + if !strings.Contains(cond.Message, "v0.11.0") { + t.Fatalf("Paused.Message %q should mention the v0.11.0 deprecation cut", cond.Message) + } +} + +func TestRecordPausedCondition_NotPaused(t *testing.T) { + c := &Controller{} + offer := &monetizeapi.ServiceOffer{} + offer.Generation = 1 + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 1} + + c.recordPausedCondition(&status, offer, false, false) + + cond := apimeta.FindStatusCondition(status.Conditions, "Paused") + if cond == nil { + t.Fatalf("Paused=False condition should be present (api-conventions: explicit False, not absent)") + } + if cond.Status != metav1.ConditionFalse { + t.Fatalf("Paused.Status = %q, want False", cond.Status) + } + if cond.Reason != "NotPaused" { + t.Fatalf("Paused.Reason = %q, want NotPaused", cond.Reason) + } +} + +// --- setCondition populates ObservedGeneration ------------------------------ + +func TestSetCondition_PopulatesObservedGeneration(t *testing.T) { + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 42} + setCondition(&status, "Ready", "True", "Reconciled", "ok") + + cond := apimeta.FindStatusCondition(status.Conditions, "Ready") + if cond == nil { + t.Fatalf("Ready not set") + } + if cond.ObservedGeneration != 42 { + t.Fatalf("ObservedGeneration = %d, want 42 (must inherit from status.ObservedGeneration)", cond.ObservedGeneration) + } +} + +func TestSetConditionWithGeneration_PinsExplicitValue(t *testing.T) { + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 1} + setConditionWithGeneration(&status, "Ready", "False", "Reconciling", "wait", 9) + + cond := apimeta.FindStatusCondition(status.Conditions, "Ready") + if cond == nil || cond.ObservedGeneration != 9 { + t.Fatalf("explicit generation arg should win over status.ObservedGeneration; got cond=%+v", cond) + } +} + +// --- rollupReady is the AND of the five predicate conditions ---------------- + +func TestRollupReady_AllTrue(t *testing.T) { + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 4} + for _, t := range []string{"ModelReady", "UpstreamHealthy", "PaymentGateReady", "RoutePublished", "Registered"} { + setCondition(&status, t, "True", "ok", "ok") + } + + if !rollupReady(&status, 4) { + t.Fatalf("rollupReady should be true when all five predicates are True") + } + ready := apimeta.FindStatusCondition(status.Conditions, "Ready") + if ready == nil || ready.Status != metav1.ConditionTrue { + t.Fatalf("Ready condition should be True, got %+v", ready) + } +} + +func TestRollupReady_OneFalseFlipsReadyFalse(t *testing.T) { + status := monetizeapi.ServiceOfferStatus{ObservedGeneration: 5} + for _, t := range []string{"ModelReady", "UpstreamHealthy", "PaymentGateReady", "RoutePublished"} { + setCondition(&status, t, "True", "ok", "ok") + } + // One predicate False — Ready must NOT be True. + setCondition(&status, "Registered", "False", "Pending", "wait") + + if rollupReady(&status, 5) { + t.Fatalf("rollupReady must be false when any predicate is False") + } + ready := apimeta.FindStatusCondition(status.Conditions, "Ready") + if ready == nil || ready.Status != metav1.ConditionFalse { + t.Fatalf("Ready should be False, got %+v", ready) + } + if ready.ObservedGeneration != 5 { + t.Fatalf("Ready.ObservedGeneration should propagate the generation arg, got %d", ready.ObservedGeneration) + } +} diff --git a/internal/serviceoffercontroller/purchase.go b/internal/serviceoffercontroller/purchase.go index f7c3d60d..3881a5e7 100644 --- a/internal/serviceoffercontroller/purchase.go +++ b/internal/serviceoffercontroller/purchase.go @@ -13,6 +13,7 @@ import ( "github.com/ObolNetwork/obol-stack/internal/monetizeapi" "k8s.io/apimachinery/pkg/api/equality" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -55,7 +56,7 @@ func (c *Controller) reconcilePurchase(ctx context.Context, key string) error { status = monetizeapi.PurchaseRequestStatus{} } status.ObservedGeneration = pr.Generation - status.Conditions = append([]monetizeapi.Condition{}, status.Conditions...) + status.Conditions = append([]metav1.Condition{}, status.Conditions...) // Stage 1: Probe if !purchaseConditionIsTrue(status.Conditions, "Probed") { @@ -380,33 +381,36 @@ func (c *Controller) updatePurchaseStatus(ctx context.Context, raw *unstructured return err } -func purchaseConditionIsTrue(conditions []monetizeapi.Condition, condType string) bool { - for _, c := range conditions { - if c.Type == condType { - return c.Status == "True" - } - } - return false +func purchaseConditionIsTrue(conditions []metav1.Condition, condType string) bool { + return apimeta.IsStatusConditionTrue(conditions, condType) } -func setPurchaseCondition(conditions *[]monetizeapi.Condition, condType, status, reason, message string) { - now := metav1.Now() - for i, c := range *conditions { - if c.Type == condType { - if c.Status != status { - (*conditions)[i].LastTransitionTime = now - } - (*conditions)[i].Status = status - (*conditions)[i].Reason = reason - (*conditions)[i].Message = message - return - } - } - *conditions = append(*conditions, monetizeapi.Condition{ - Type: condType, - Status: status, - Reason: reason, - Message: message, - LastTransitionTime: now, +// setPurchaseCondition uses apimeta.SetStatusCondition for SSA-safe +// dedupe-by-type and LastTransitionTime semantics. Callers that have +// the PurchaseRequest's observedGeneration handy should pass it +// through; the lifecycle reconciler stamps it on each cycle so we +// take it from the in-progress status. +func setPurchaseCondition(conditions *[]metav1.Condition, condType, status, reason, message string) { + apimeta.SetStatusCondition(conditions, metav1.Condition{ + Type: condType, + Status: metav1.ConditionStatus(status), + Reason: reason, + Message: message, }) } + +// setPurchaseConditionWithGeneration is the generation-aware variant. +// New call sites (and any sites that already plumb the PurchaseRequest +// in) should prefer this so observers can detect stale-view conditions. +func setPurchaseConditionWithGeneration(conditions *[]metav1.Condition, condType, status, reason, message string, generation int64) { + cond := metav1.Condition{ + Type: condType, + Status: metav1.ConditionStatus(status), + Reason: reason, + Message: message, + } + if generation > 0 { + cond.ObservedGeneration = generation + } + apimeta.SetStatusCondition(conditions, cond) +} diff --git a/internal/serviceoffercontroller/purchase_lifecycle_test.go b/internal/serviceoffercontroller/purchase_lifecycle_test.go index 65f96bd2..496cbdb7 100644 --- a/internal/serviceoffercontroller/purchase_lifecycle_test.go +++ b/internal/serviceoffercontroller/purchase_lifecycle_test.go @@ -217,7 +217,7 @@ func getPurchaseRequest(t *testing.T, c *Controller, namespace, name string) *mo return &pr } -func purchaseCondition(t *testing.T, pr *monetizeapi.PurchaseRequest, condType string) monetizeapi.Condition { +func purchaseCondition(t *testing.T, pr *monetizeapi.PurchaseRequest, condType string) metav1.Condition { t.Helper() for _, condition := range pr.Status.Conditions { @@ -226,7 +226,7 @@ func purchaseCondition(t *testing.T, pr *monetizeapi.PurchaseRequest, condType s } } t.Fatalf("missing purchase condition %q", condType) - return monetizeapi.Condition{} + return metav1.Condition{} } func seedBuyerConfigMaps(t *testing.T, c *Controller, entries map[string]string) { @@ -551,7 +551,7 @@ func TestReconcilePurchaseReadyKeepsReadyWhileUpdatingLiveSpend(t *testing.T) { Remaining: 3, Spent: 0, PublicModel: "paid/qwen3.5:9b", - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "Probed", Status: "True", Reason: "Validated"}, {Type: "AuthsLoaded", Status: "True", Reason: "Loaded"}, {Type: "Configured", Status: "True", Reason: "Written"}, @@ -638,7 +638,7 @@ func TestReconcilePurchaseConfigureRebuildsPendingAuthsFromSpecAfterRestart(t *t }, Status: monetizeapi.PurchaseRequestStatus{ ObservedGeneration: 1, - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "Probed", Status: "True", Reason: "Validated"}, {Type: "AuthsLoaded", Status: "True", Reason: "Loaded"}, }, @@ -746,7 +746,7 @@ func TestUpdatePurchaseStatusNoOpWhenUnchanged(t *testing.T) { Status: monetizeapi.PurchaseRequestStatus{ ObservedGeneration: 1, Remaining: 3, - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "Ready", Status: "True", Reason: "Reconciled", Message: "ok"}, }, }, @@ -858,7 +858,7 @@ func TestReconcileDeletingPurchaseDrainsUntilRemainingZero(t *testing.T) { ObservedGeneration: 1, Remaining: 2, Spent: 1, - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "Configured", Status: "True", Reason: "Written"}, {Type: "Ready", Status: "True", Reason: "Reconciled"}, }, @@ -978,7 +978,7 @@ func TestReconcileDeletingPurchaseWaitsForRuntimeStatusToDisappear(t *testing.T) ObservedGeneration: 2, Remaining: 0, Spent: 7, - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "Configured", Status: "True", Reason: "Written"}, {Type: "Ready", Status: "True", Reason: "Reconciled"}, }, diff --git a/internal/serviceoffercontroller/purchase_pure_test.go b/internal/serviceoffercontroller/purchase_pure_test.go index a05fd5e9..9bb4d27a 100644 --- a/internal/serviceoffercontroller/purchase_pure_test.go +++ b/internal/serviceoffercontroller/purchase_pure_test.go @@ -5,12 +5,13 @@ import ( "testing" "github.com/ObolNetwork/obol-stack/internal/monetizeapi" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // --- purchaseConditionIsTrue ------------------------------------------------ func TestPurchaseConditionIsTrue(t *testing.T) { - conds := []monetizeapi.Condition{ + conds := []metav1.Condition{ {Type: "Signed", Status: "True"}, {Type: "Configured", Status: "False"}, {Type: "Ready", Status: "Unknown"}, @@ -35,7 +36,7 @@ func TestPurchaseConditionIsTrue(t *testing.T) { // --- setPurchaseCondition --------------------------------------------------- func TestSetPurchaseCondition_AppendsNew(t *testing.T) { - var conds []monetizeapi.Condition + var conds []metav1.Condition setPurchaseCondition(&conds, "Signed", "True", "SignedOK", "ok") @@ -51,7 +52,7 @@ func TestSetPurchaseCondition_AppendsNew(t *testing.T) { } func TestSetPurchaseCondition_UpdatesExistingNoStatusChange(t *testing.T) { - conds := []monetizeapi.Condition{ + conds := []metav1.Condition{ {Type: "Signed", Status: "True", Reason: "Old", Message: "old msg"}, } // Capture the original timestamp (zero value is fine — we just want it to remain unchanged). @@ -76,7 +77,7 @@ func TestSetPurchaseCondition_UpdatesExistingNoStatusChange(t *testing.T) { } func TestSetPurchaseCondition_StatusFlipBumpsTimestamp(t *testing.T) { - conds := []monetizeapi.Condition{ + conds := []metav1.Condition{ {Type: "Signed", Status: "False"}, } diff --git a/internal/serviceoffercontroller/render.go b/internal/serviceoffercontroller/render.go index a733213b..f597b7b4 100644 --- a/internal/serviceoffercontroller/render.go +++ b/internal/serviceoffercontroller/render.go @@ -9,11 +9,11 @@ import ( "sort" "strconv" "strings" - "time" "github.com/ObolNetwork/obol-stack/internal/erc8004" "github.com/ObolNetwork/obol-stack/internal/monetizeapi" "github.com/ObolNetwork/obol-stack/internal/schemas" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" @@ -565,39 +565,37 @@ func ownerRefMapFor(apiVersion, kind, name string, uid types.UID) map[string]any } } +// setCondition is the back-compat shim that callers without easy +// access to offer.Generation use. It defers to +// setConditionWithGeneration, passing status.ObservedGeneration so +// existing call sites preserve the most recently observed generation +// without a signature change. New code paths should call +// setConditionWithGeneration directly. func setCondition(status *monetizeapi.ServiceOfferStatus, conditionType, conditionStatus, reason, message string) { - now := metav1.NewTime(time.Now().UTC()) - for i := range status.Conditions { - if status.Conditions[i].Type != conditionType { - continue - } - if status.Conditions[i].Status != conditionStatus { - status.Conditions[i].LastTransitionTime = now - } - status.Conditions[i].Status = conditionStatus - status.Conditions[i].Reason = reason - status.Conditions[i].Message = message - if status.Conditions[i].LastTransitionTime.IsZero() { - status.Conditions[i].LastTransitionTime = now - } - return - } - status.Conditions = append(status.Conditions, monetizeapi.Condition{ - Type: conditionType, - Status: conditionStatus, - Reason: reason, - Message: message, - LastTransitionTime: now, - }) + setConditionWithGeneration(status, conditionType, conditionStatus, reason, message, status.ObservedGeneration) +} + +// setConditionWithGeneration updates (or appends) a condition entry +// of the given type using metav1.Condition semantics. Wraps +// apimeta.SetStatusCondition (dedupe-by-type, LastTransitionTime +// flips, ObservedGeneration preserved when caller leaves it unset) +// instead of hand-rolling the merge — that hand-rolled loop was the +// drift surface this PR removes. +func setConditionWithGeneration(status *monetizeapi.ServiceOfferStatus, conditionType, conditionStatus, reason, message string, generation int64) { + cond := metav1.Condition{ + Type: conditionType, + Status: metav1.ConditionStatus(conditionStatus), + Reason: reason, + Message: message, + } + if generation > 0 { + cond.ObservedGeneration = generation + } + apimeta.SetStatusCondition(&status.Conditions, cond) } func isConditionTrue(status monetizeapi.ServiceOfferStatus, conditionType string) bool { - for _, condition := range status.Conditions { - if condition.Type == conditionType { - return condition.Status == "True" - } - } - return false + return apimeta.IsStatusConditionTrue(status.Conditions, conditionType) } func buildActiveRegistrationDocument(owner *monetizeapi.ServiceOffer, offers []*monetizeapi.ServiceOffer, baseURL, agentID string) erc8004.AgentRegistration { diff --git a/internal/serviceoffercontroller/render_test.go b/internal/serviceoffercontroller/render_test.go index eb71891c..d8805481 100644 --- a/internal/serviceoffercontroller/render_test.go +++ b/internal/serviceoffercontroller/render_test.go @@ -106,7 +106,7 @@ func TestBuildReferenceGrant(t *testing.T) { func TestSetConditionUpdatesExistingEntry(t *testing.T) { status := monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{ + Conditions: []metav1.Condition{{ Type: "Ready", Status: "False", }}, @@ -162,7 +162,7 @@ func TestBuildAgentIdentityRegistrationHTTPRoute(t *testing.T) { } func TestBuildActiveRegistrationDocument(t *testing.T) { - readyConditions := []monetizeapi.Condition{ + readyConditions := []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, @@ -283,7 +283,7 @@ func TestBuildActiveRegistrationDocument_KeepsOperatorDescription(t *testing.T) }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, @@ -388,7 +388,7 @@ func TestBuildRegistrationServices_IncludesOwnerWhenOwnerNotYetPublished(t *test }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, @@ -410,7 +410,7 @@ func TestBuildRegistrationServices_IncludesOwnerWhenOwnerNotYetPublished(t *test } func TestBuildRegistrationConfigMap_PublishesAggregatedAgentRegistration(t *testing.T) { - readyConditions := []monetizeapi.Condition{ + readyConditions := []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, @@ -548,13 +548,13 @@ func TestBuildSkillCatalogMarkdown(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } notReadyOffer := &monetizeapi.ServiceOffer{ ObjectMeta: metav1.ObjectMeta{Name: "pending", Namespace: "llm"}, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "False"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "False"}}, }, } @@ -615,13 +615,13 @@ func TestBuildServiceCatalogJSON(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } notReadyOffer := &monetizeapi.ServiceOffer{ ObjectMeta: metav1.ObjectMeta{Name: "pending", Namespace: "demo"}, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "False"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "False"}}, }, } @@ -689,7 +689,7 @@ func TestBuildServiceCatalogJSON_AgentOfferUsesResolvedModel(t *testing.T) { Model: "qwen3.5:9b", Runtime: "hermes", }, - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } @@ -722,7 +722,7 @@ func TestBuildServiceCatalogJSON_AgentOfferUsesResolvedModel(t *testing.T) { // nil offers, paused offers, and offers with a DeletionTimestamp must never // leak onto the public storefront, even if they carry Ready=True. func TestBuildServiceCatalogJSON_ExcludesNonReady(t *testing.T) { - readyCond := []monetizeapi.Condition{{Type: "Ready", Status: "True"}} + readyCond := []metav1.Condition{{Type: "Ready", Status: "True"}} deleting := metav1.Now() offers := []*monetizeapi.ServiceOffer{ @@ -744,7 +744,7 @@ func TestBuildServiceCatalogJSON_ExcludesNonReady(t *testing.T) { { ObjectMeta: metav1.ObjectMeta{Name: "not-ready-svc", Namespace: "llm"}, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "False"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "False"}}, }, }, { @@ -780,7 +780,7 @@ func TestBuildServiceCatalogJSON_ExcludesNonReady(t *testing.T) { // store yields items in arbitrary order, so without a sort the public // storefront reorders itself between reconciles. func TestBuildServiceCatalogJSON_SortOrder(t *testing.T) { - readyCond := []monetizeapi.Condition{{Type: "Ready", Status: "True"}} + readyCond := []metav1.Condition{{Type: "Ready", Status: "True"}} makeOffer := func(name string) *monetizeapi.ServiceOffer { return &monetizeapi.ServiceOffer{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "llm"}, @@ -830,7 +830,7 @@ func TestBuildServiceCatalogJSON_PerMTokPricing(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } @@ -878,7 +878,7 @@ func TestBuildServiceCatalogJSON_FallbackDescription(t *testing.T) { // Spec.Registration.Description intentionally omitted. }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } @@ -908,7 +908,7 @@ func TestBuildServiceCatalogJSON_BaseURLTrailingSlash(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } @@ -947,7 +947,7 @@ func TestBuildServiceCatalogJSON_AssetAndCAIP2Defaults(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } @@ -1034,7 +1034,7 @@ func TestBuildServiceCatalogJSON_ExplicitOBOLToken(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "Ready", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "Ready", Status: "True"}}, }, } @@ -1132,7 +1132,7 @@ func TestOfferOperationallyReady_IncludesAwaitingExternalRegistration(t *testing awaiting := &monetizeapi.ServiceOffer{ ObjectMeta: metav1.ObjectMeta{Name: "aeon", Namespace: "llm"}, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, @@ -1157,7 +1157,7 @@ func TestOfferOperationallyReady_RejectsRealNotReady(t *testing.T) { notUsable := &monetizeapi.ServiceOffer{ ObjectMeta: metav1.ObjectMeta{Name: "broken", Namespace: "llm"}, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "False", Reason: "Unhealthy"}, {Type: "PaymentGateReady", Status: "False", Reason: "WaitingForUpstream"}, @@ -1196,7 +1196,7 @@ func TestBuildServiceCatalogJSON_IncludesPendingRegistrationOffers(t *testing.T) }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, @@ -1237,7 +1237,7 @@ func TestBuildServiceCatalogJSON_RegistrationPendingFalseForFullyReady(t *testin }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{ + Conditions: []metav1.Condition{ {Type: "ModelReady", Status: "True"}, {Type: "UpstreamHealthy", Status: "True"}, {Type: "PaymentGateReady", Status: "True"}, diff --git a/internal/x402/network_norm_test.go b/internal/x402/network_norm_test.go index 97696627..52eccb53 100644 --- a/internal/x402/network_norm_test.go +++ b/internal/x402/network_norm_test.go @@ -33,7 +33,7 @@ func TestRoutesFromStore_NetworkCAIP2Normalization(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "RoutePublished", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "RoutePublished", Status: "True"}}, }, }), } diff --git a/internal/x402/serviceoffer_source_test.go b/internal/x402/serviceoffer_source_test.go index 9733095e..96e47ba8 100644 --- a/internal/x402/serviceoffer_source_test.go +++ b/internal/x402/serviceoffer_source_test.go @@ -21,7 +21,7 @@ func TestRoutesFromStore(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "RoutePublished", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "RoutePublished", Status: "True"}}, }, }), mustOfferObject(t, monetizeapi.ServiceOffer{ @@ -34,7 +34,7 @@ func TestRoutesFromStore(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "RoutePublished", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "RoutePublished", Status: "True"}}, }, }), mustOfferObject(t, monetizeapi.ServiceOffer{ @@ -47,7 +47,7 @@ func TestRoutesFromStore(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "RoutePublished", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "RoutePublished", Status: "True"}}, }, }), } @@ -101,7 +101,7 @@ func TestRoutesFromStore_IgnoresUnpublishedOffers(t *testing.T) { }, }, Status: monetizeapi.ServiceOfferStatus{ - Conditions: []monetizeapi.Condition{{Type: "PaymentGateReady", Status: "True"}}, + Conditions: []metav1.Condition{{Type: "PaymentGateReady", Status: "True"}}, }, }), }