diff --git a/cmd/thv-operator/DESIGN.md b/cmd/thv-operator/DESIGN.md
index 8f2ac745da..8739275bda 100644
--- a/cmd/thv-operator/DESIGN.md
+++ b/cmd/thv-operator/DESIGN.md
@@ -29,40 +29,17 @@ When building operators, the decision of when to use a `podTemplateSpec` and whe
### Status Management Design
-**Decision**: Use batched status updates via StatusCollector pattern instead of individual field updates.
+**Decision**: Use standard Kubernetes workload status pattern matching MCPServer — flat `Phase` + `Ready` condition + `ReadyReplicas` + `URL`.
**Rationale**:
-- Prevents race conditions between multiple status updates
-- Reduces API server load with fewer update calls
-- Ensures consistent status across reconciliation cycles
-- Handles resource version conflicts gracefully
+- Consistency with MCPServer and standard Kubernetes workload patterns
+- Enables `kubectl wait --for=condition=Ready` and standard monitoring
+- The operator only needs to track deployment readiness, not internal registry server state
+- Tracking internal sync/API states would require the operator to call the registry server, which with auth enabled is not feasible
-**Implementation**: StatusCollector interface collects all changes and applies them atomically.
+**Implementation**: Controller sets `Phase`, `Message`, `URL`, `ReadyReplicas`, and a `Ready` condition directly based on the API deployment's readiness. The latest resource version is refetched before status updates to avoid conflicts.
-### Sync Operation Design
-
-**Decision**: Separate sync decision logic from sync execution with clear interfaces.
-
-**Rationale**:
-- Testability: Mock sync decisions independently from execution
-- Flexibility: Different sync strategies without changing core logic
-- Maintainability: Clear separation of concerns
-
-**Key Patterns**:
-- Idempotent operations for safe retry
-- Manual vs automatic sync distinction
-- Data preservation on failures
-
-### Storage Architecture
-
-**Decision**: Abstract storage via StorageManager interface with ConfigMap as default implementation.
-
-**Rationale**:
-- Future flexibility: Easy addition of new storage backends (OCI, databases)
-- Testability: Mock storage for unit tests
-- Consistency: Single interface for all storage operations
-
-**Current Implementation**: ConfigMap-based with owner references for automatic cleanup.
+**History**: The original design used a `StatusCollector` pattern (`mcpregistrystatus` package) that batched status changes from multiple independent sources — an `APIStatusCollector` for deployment state and originally a sync collector — then applied them atomically via a single `Status().Update()`. A `StatusDeriver` computed the overall phase from sub-phases (`SyncPhase` + `APIPhase` → `MCPRegistryPhase`). This was removed because with sync operations moved to the registry server itself, only one status source remained (deployment readiness), making the batching/derivation indirection unnecessary. The new approach produces the same number of API server calls with less abstraction.
### Registry API Service Pattern
@@ -78,27 +55,21 @@ When building operators, the decision of when to use a `podTemplateSpec` and whe
### Error Handling Strategy
-**Decision**: Structured error types with progressive retry backoff.
+**Decision**: Structured error types (`registryapi.Error`) with condition metadata.
**Rationale**:
- Different error types need different handling strategies
-- Progressive backoff prevents thundering herd problems
-- Structured errors enable better observability
+- Structured errors carry `ConditionReason` for setting Kubernetes conditions with specific failure reasons (e.g., `ConfigMapFailed`, `DeploymentFailed`)
+- Enables better observability via condition reasons
-**Implementation**: 5m initial retry, exponential backoff with cap, manual sync bypass.
+**Implementation**: `registryapi.Error` carries `ConditionReason` and `Message`. The controller uses `errors.As` to extract structured fields when available, falling back to generic `NotReady` reason for unstructured errors.
### Performance Design Decisions
#### Resource Optimization
-- **Status Updates**: Batched to reduce API calls (implemented)
-- **Source Fetching**: Planned caching to avoid repeated downloads
+- **Status Updates**: Single refetch-then-update per reconciliation cycle
- **API Deployment**: Lazy creation only when needed (implemented)
-#### Memory Management
-- **Git Operations**: Shallow clones to minimize disk usage (implemented)
-- **Large Registries**: Stream processing planned for future
-- **Status Objects**: Efficient field-level updates (implemented)
-
### Security Architecture
#### Permission Model
diff --git a/cmd/thv-operator/api/v1alpha1/mcpregistry_types.go b/cmd/thv-operator/api/v1alpha1/mcpregistry_types.go
index 8f3880007f..101bcdb822 100644
--- a/cmd/thv-operator/api/v1alpha1/mcpregistry_types.go
+++ b/cmd/thv-operator/api/v1alpha1/mcpregistry_types.go
@@ -492,203 +492,66 @@ type MCPRegistryOAuthProviderConfig struct {
// MCPRegistryStatus defines the observed state of MCPRegistry
type MCPRegistryStatus struct {
- // ObservedGeneration reflects the generation most recently observed by the controller
- // +optional
- ObservedGeneration int64 `json:"observedGeneration,omitempty"`
-
- // Phase represents the current overall phase of the MCPRegistry
- // Derived from sync and API status
- // +optional
- Phase MCPRegistryPhase `json:"phase,omitempty"`
-
- // Message provides additional information about the current phase
- // +optional
- Message string `json:"message,omitempty"`
-
- // SyncStatus provides detailed information about data synchronization
- // +optional
- SyncStatus *SyncStatus `json:"syncStatus,omitempty"`
-
- // APIStatus provides detailed information about the API service
- // +optional
- APIStatus *APIStatus `json:"apiStatus,omitempty"`
-
- // LastAppliedFilterHash is the hash of the last applied filter
- // +optional
- LastAppliedFilterHash string `json:"lastAppliedFilterHash,omitempty"`
-
- // StorageRef is a reference to the internal storage location
- // +optional
- StorageRef *StorageReference `json:"storageRef,omitempty"`
-
- // LastManualSyncTrigger tracks the last processed manual sync annotation value
- // Used to detect new manual sync requests via toolhive.stacklok.dev/sync-trigger annotation
- // +optional
- LastManualSyncTrigger string `json:"lastManualSyncTrigger,omitempty"`
-
// Conditions represent the latest available observations of the MCPRegistry's state
- // +optional
// +listType=map
// +listMapKey=type
- Conditions []metav1.Condition `json:"conditions,omitempty"`
-}
-
-// SyncStatus provides detailed information about data synchronization
-type SyncStatus struct {
- // Phase represents the current synchronization phase
- // +kubebuilder:validation:Enum=Syncing;Complete;Failed
- Phase SyncPhase `json:"phase"`
-
- // Message provides additional information about the sync status
- // +optional
- Message string `json:"message,omitempty"`
-
- // LastAttempt is the timestamp of the last sync attempt
- // +optional
- LastAttempt *metav1.Time `json:"lastAttempt,omitempty"`
-
- // AttemptCount is the number of sync attempts since last success
// +optional
- // +kubebuilder:validation:Minimum=0
- AttemptCount int32 `json:"attemptCount,omitempty"`
-
- // LastSyncTime is the timestamp of the last successful sync
- // +optional
- LastSyncTime *metav1.Time `json:"lastSyncTime,omitempty"`
+ Conditions []metav1.Condition `json:"conditions,omitempty"`
- // LastSyncHash is the hash of the last successfully synced data
- // Used to detect changes in source data
+ // ObservedGeneration reflects the generation most recently observed by the controller
// +optional
- LastSyncHash string `json:"lastSyncHash,omitempty"`
+ ObservedGeneration int64 `json:"observedGeneration,omitempty"`
- // ServerCount is the total number of servers in the registry
+ // Phase represents the current overall phase of the MCPRegistry
// +optional
- // +kubebuilder:validation:Minimum=0
- ServerCount int32 `json:"serverCount,omitempty"`
-}
-
-// APIStatus provides detailed information about the API service
-type APIStatus struct {
- // Phase represents the current API service phase
- // +kubebuilder:validation:Enum=NotStarted;Deploying;Ready;Unhealthy;Error
- Phase APIPhase `json:"phase"`
+ Phase MCPRegistryPhase `json:"phase,omitempty"`
- // Message provides additional information about the API status
+ // Message provides additional information about the current phase
// +optional
Message string `json:"message,omitempty"`
- // Endpoint is the URL where the API is accessible
+ // URL is the URL where the registry API can be accessed
// +optional
- Endpoint string `json:"endpoint,omitempty"`
+ URL string `json:"url,omitempty"`
- // ReadySince is the timestamp when the API became ready
+ // ReadyReplicas is the number of ready registry API replicas
// +optional
- ReadySince *metav1.Time `json:"readySince,omitempty"`
-}
-
-// SyncPhase represents the data synchronization state
-// +kubebuilder:validation:Enum=Syncing;Complete;Failed
-type SyncPhase string
-
-const (
- // SyncPhaseSyncing means sync is currently in progress
- SyncPhaseSyncing SyncPhase = "Syncing"
-
- // SyncPhaseComplete means sync completed successfully
- SyncPhaseComplete SyncPhase = "Complete"
-
- // SyncPhaseFailed means sync failed
- SyncPhaseFailed SyncPhase = "Failed"
-)
-
-// APIPhase represents the API service state
-// +kubebuilder:validation:Enum=NotStarted;Deploying;Ready;Unhealthy;Error
-type APIPhase string
-
-const (
- // APIPhaseNotStarted means API deployment has not been created
- APIPhaseNotStarted APIPhase = "NotStarted"
-
- // APIPhaseDeploying means API is being deployed
- APIPhaseDeploying APIPhase = "Deploying"
-
- // APIPhaseReady means API is ready to serve requests
- APIPhaseReady APIPhase = "Ready"
-
- // APIPhaseUnhealthy means API is deployed but not healthy
- APIPhaseUnhealthy APIPhase = "Unhealthy"
-
- // APIPhaseError means API deployment failed
- APIPhaseError APIPhase = "Error"
-)
-
-// StorageReference defines a reference to internal storage
-type StorageReference struct {
- // Type is the storage type (configmap)
- // +kubebuilder:validation:Enum=configmap
- Type string `json:"type"`
-
- // ConfigMapRef is a reference to a ConfigMap storage
- // Only used when Type is "configmap"
- // +optional
- ConfigMapRef *corev1.LocalObjectReference `json:"configMapRef,omitempty"`
+ ReadyReplicas int32 `json:"readyReplicas,omitempty"`
}
// MCPRegistryPhase represents the phase of the MCPRegistry
-// +kubebuilder:validation:Enum=Pending;Ready;Failed;Syncing;Terminating
+// +kubebuilder:validation:Enum=Pending;Running;Failed;Terminating
type MCPRegistryPhase string
const (
// MCPRegistryPhasePending means the MCPRegistry is being initialized
MCPRegistryPhasePending MCPRegistryPhase = "Pending"
- // MCPRegistryPhaseReady means the MCPRegistry is ready and operational
- MCPRegistryPhaseReady MCPRegistryPhase = "Ready"
+ // MCPRegistryPhaseRunning means the MCPRegistry is running and operational
+ MCPRegistryPhaseRunning MCPRegistryPhase = "Running"
// MCPRegistryPhaseFailed means the MCPRegistry has failed
MCPRegistryPhaseFailed MCPRegistryPhase = "Failed"
- // MCPRegistryPhaseSyncing means the MCPRegistry is currently syncing data
- MCPRegistryPhaseSyncing MCPRegistryPhase = "Syncing"
-
// MCPRegistryPhaseTerminating means the MCPRegistry is being deleted
MCPRegistryPhaseTerminating MCPRegistryPhase = "Terminating"
)
-// Condition types for MCPRegistry
+// Condition reasons for MCPRegistry
const (
- // ConditionSourceAvailable indicates whether the source is available and accessible
- ConditionSourceAvailable = "SourceAvailable"
+ // ConditionReasonRegistryReady indicates the MCPRegistry is ready
+ ConditionReasonRegistryReady = "Ready"
- // ConditionDataValid indicates whether the registry data is valid
- ConditionDataValid = "DataValid"
-
- // ConditionSyncSuccessful indicates whether the last sync was successful
- ConditionSyncSuccessful = "SyncSuccessful"
-
- // ConditionAPIReady indicates whether the registry API is ready
- ConditionAPIReady = "APIReady"
-
- // ConditionRegistryPodTemplateValid indicates whether the PodTemplateSpec is valid
- ConditionRegistryPodTemplateValid = "PodTemplateValid"
-)
-
-// Condition reasons for MCPRegistry PodTemplateSpec validation
-const (
- // ConditionReasonRegistryPodTemplateValid indicates PodTemplateSpec validation succeeded
- ConditionReasonRegistryPodTemplateValid = "ValidPodTemplateSpec"
-
- // ConditionReasonRegistryPodTemplateInvalid indicates PodTemplateSpec validation failed
- ConditionReasonRegistryPodTemplateInvalid = "InvalidPodTemplateSpec"
+ // ConditionReasonRegistryNotReady indicates the MCPRegistry is not ready
+ ConditionReasonRegistryNotReady = "NotReady"
)
//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
-//+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
-//+kubebuilder:printcolumn:name="Sync",type="string",JSONPath=".status.syncStatus.phase"
-//+kubebuilder:printcolumn:name="API",type="string",JSONPath=".status.apiStatus.phase"
-//+kubebuilder:printcolumn:name="Servers",type="integer",JSONPath=".status.syncStatus.serverCount"
-//+kubebuilder:printcolumn:name="Last Sync",type="date",JSONPath=".status.syncStatus.lastSyncTime"
+//+kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase"
+//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
+//+kubebuilder:printcolumn:name="Replicas",type="integer",JSONPath=".status.readyReplicas"
+//+kubebuilder:printcolumn:name="URL",type="string",JSONPath=".status.url"
//+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
//+kubebuilder:resource:shortName=mcpreg;registry,scope=Namespaced,categories=toolhive
//nolint:lll
@@ -722,48 +585,6 @@ func (r *MCPRegistry) GetAPIResourceName() string {
return fmt.Sprintf("%s-api", r.Name)
}
-// DeriveOverallPhase determines the overall MCPRegistry phase based on sync and API status
-func (r *MCPRegistry) DeriveOverallPhase() MCPRegistryPhase {
- syncStatus := r.Status.SyncStatus
- apiStatus := r.Status.APIStatus
-
- // Default phases if status not set
- var syncPhase SyncPhase
- if syncStatus != nil {
- syncPhase = syncStatus.Phase
- }
-
- apiPhase := APIPhaseNotStarted
- if apiStatus != nil {
- apiPhase = apiStatus.Phase
- }
-
- // If sync failed, overall is Failed
- if syncPhase == SyncPhaseFailed {
- return MCPRegistryPhaseFailed
- }
-
- // If sync in progress, overall is Syncing
- if syncPhase == SyncPhaseSyncing {
- return MCPRegistryPhaseSyncing
- }
-
- // If sync is complete (no sync needed), check API status
- if syncPhase == SyncPhaseComplete {
- switch apiPhase {
- case APIPhaseReady:
- return MCPRegistryPhaseReady
- case APIPhaseError:
- return MCPRegistryPhaseFailed
- case APIPhaseNotStarted, APIPhaseDeploying, APIPhaseUnhealthy:
- return MCPRegistryPhasePending // API still starting/not healthy
- }
- }
-
- // Default to pending for initial states
- return MCPRegistryPhasePending
-}
-
func init() {
SchemeBuilder.Register(&MCPRegistry{}, &MCPRegistryList{})
}
diff --git a/cmd/thv-operator/api/v1alpha1/mcpregistry_types_test.go b/cmd/thv-operator/api/v1alpha1/mcpregistry_types_test.go
deleted file mode 100644
index 40635d8cf6..0000000000
--- a/cmd/thv-operator/api/v1alpha1/mcpregistry_types_test.go
+++ /dev/null
@@ -1,273 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-package v1alpha1
-
-import (
- "testing"
-
- "github.com/stretchr/testify/assert"
- metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-)
-
-func TestMCPRegistry_DeriveOverallPhase(t *testing.T) {
- t.Parallel()
-
- tests := []struct {
- name string
- syncStatus *SyncStatus
- apiStatus *APIStatus
- expectedPhase MCPRegistryPhase
- description string
- }{
- // No status set (initial state)
- {
- name: "no_status_set",
- syncStatus: nil,
- apiStatus: nil,
- expectedPhase: MCPRegistryPhasePending,
- description: "Default to pending when no status is set",
- },
-
- // Sync Failed cases
- {
- name: "sync_failed_api_notstarted",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseFailed,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseNotStarted,
- },
- expectedPhase: MCPRegistryPhaseFailed,
- description: "Sync failed should result in Failed regardless of API status",
- },
- {
- name: "sync_failed_api_ready",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseFailed,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseReady,
- },
- expectedPhase: MCPRegistryPhaseFailed,
- description: "Sync failed should result in Failed even when API is ready",
- },
-
- // Sync Syncing cases
- {
- name: "sync_syncing_api_notstarted",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseSyncing,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseNotStarted,
- },
- expectedPhase: MCPRegistryPhaseSyncing,
- description: "Sync in progress should result in Syncing regardless of API status",
- },
- {
- name: "sync_syncing_api_ready",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseSyncing,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseReady,
- },
- expectedPhase: MCPRegistryPhaseSyncing,
- description: "Sync in progress should result in Syncing even when API is ready",
- },
-
- // Sync Complete + API combinations
- {
- name: "sync_complete_api_ready",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseComplete,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseReady,
- },
- expectedPhase: MCPRegistryPhaseReady,
- description: "Sync complete + API ready should result in Ready",
- },
- {
- name: "sync_complete_api_error",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseComplete,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseError,
- },
- expectedPhase: MCPRegistryPhaseFailed,
- description: "Sync complete + API error should result in Failed",
- },
- {
- name: "sync_complete_api_notstarted",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseComplete,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseNotStarted,
- },
- expectedPhase: MCPRegistryPhasePending,
- description: "Sync complete + API not started should result in Pending",
- },
- {
- name: "sync_complete_api_deploying",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseComplete,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseDeploying,
- },
- expectedPhase: MCPRegistryPhasePending,
- description: "Sync complete + API deploying should result in Pending",
- },
- {
- name: "sync_complete_api_unhealthy",
- syncStatus: &SyncStatus{
- Phase: SyncPhaseComplete,
- },
- apiStatus: &APIStatus{
- Phase: APIPhaseUnhealthy,
- },
- expectedPhase: MCPRegistryPhasePending,
- description: "Sync complete + API unhealthy should result in Pending",
- },
-
- // Partial status combinations (one nil, one set)
- {
- name: "sync_complete_api_nil",
- syncStatus: &SyncStatus{Phase: SyncPhaseComplete},
- apiStatus: nil,
- expectedPhase: MCPRegistryPhasePending,
- description: "Sync complete + API nil should result in Pending (API defaults to NotStarted)",
- },
- {
- name: "sync_nil_api_ready",
- syncStatus: nil,
- apiStatus: &APIStatus{Phase: APIPhaseReady},
- expectedPhase: MCPRegistryPhasePending,
- description: "Sync nil + API ready should result in Pending",
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- t.Parallel()
-
- // Create MCPRegistry with the specified status
- registry := &MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "test-namespace",
- },
- Status: MCPRegistryStatus{
- SyncStatus: tt.syncStatus,
- APIStatus: tt.apiStatus,
- },
- }
-
- // Call DeriveOverallPhase and verify result
- actualPhase := registry.DeriveOverallPhase()
-
- assert.Equal(t, tt.expectedPhase, actualPhase,
- "Test case: %s\nDescription: %s\nSync phase: %v\nAPI phase: %v",
- tt.name, tt.description,
- getSyncPhaseString(tt.syncStatus),
- getAPIPhaseString(tt.apiStatus))
- })
- }
-}
-
-// Helper function to get sync phase as string for better test output
-func getSyncPhaseString(syncStatus *SyncStatus) string {
- if syncStatus == nil {
- return "nil"
- }
- return string(syncStatus.Phase)
-}
-
-// Helper function to get API phase as string for better test output
-func getAPIPhaseString(apiStatus *APIStatus) string {
- if apiStatus == nil {
- return "nil (defaults to NotStarted)"
- }
- return string(apiStatus.Phase)
-}
-
-// TestMCPRegistry_DeriveOverallPhase_EdgeCases tests additional edge cases and regression scenarios
-func TestMCPRegistry_DeriveOverallPhase_EdgeCases(t *testing.T) {
- t.Parallel()
-
- t.Run("regression_test_complete_ready_becomes_ready", func(t *testing.T) {
- t.Parallel()
- // This is the specific regression test for the bug fix where
- // syncPhase=Complete + apiPhase=Ready was incorrectly returning Pending
- registry := &MCPRegistry{
- Status: MCPRegistryStatus{
- SyncStatus: &SyncStatus{Phase: SyncPhaseComplete},
- APIStatus: &APIStatus{Phase: APIPhaseReady},
- },
- }
-
- phase := registry.DeriveOverallPhase()
- assert.Equal(t, MCPRegistryPhaseReady, phase,
- "The specific case syncPhase=Complete + apiPhase=Ready should result in Ready phase")
- })
-
- t.Run("all_api_phases_with_failed_sync", func(t *testing.T) {
- t.Parallel()
- // Verify that sync failed always results in overall failed regardless of API phase
- apiPhases := []APIPhase{
- APIPhaseNotStarted,
- APIPhaseDeploying,
- APIPhaseReady,
- APIPhaseUnhealthy,
- APIPhaseError,
- }
-
- for _, apiPhase := range apiPhases {
- t.Run(string(apiPhase), func(t *testing.T) {
- t.Parallel()
- registry := &MCPRegistry{
- Status: MCPRegistryStatus{
- SyncStatus: &SyncStatus{Phase: SyncPhaseFailed},
- APIStatus: &APIStatus{Phase: apiPhase},
- },
- }
-
- phase := registry.DeriveOverallPhase()
- assert.Equal(t, MCPRegistryPhaseFailed, phase,
- "Sync failed should always result in Failed phase regardless of API phase: %s", apiPhase)
- })
- }
- })
-
- t.Run("all_api_phases_with_syncing", func(t *testing.T) {
- t.Parallel()
- // Verify that sync in progress always results in overall syncing regardless of API phase
- apiPhases := []APIPhase{
- APIPhaseNotStarted,
- APIPhaseDeploying,
- APIPhaseReady,
- APIPhaseUnhealthy,
- APIPhaseError,
- }
-
- for _, apiPhase := range apiPhases {
- t.Run(string(apiPhase), func(t *testing.T) {
- t.Parallel()
- registry := &MCPRegistry{
- Status: MCPRegistryStatus{
- SyncStatus: &SyncStatus{Phase: SyncPhaseSyncing},
- APIStatus: &APIStatus{Phase: apiPhase},
- },
- }
-
- phase := registry.DeriveOverallPhase()
- assert.Equal(t, MCPRegistryPhaseSyncing, phase,
- "Sync in progress should always result in Syncing phase regardless of API phase: %s", apiPhase)
- })
- }
- })
-}
diff --git a/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go
index d833e6ff92..7357d67d4e 100644
--- a/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go
+++ b/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go
@@ -1,5 +1,21 @@
//go:build !ignore_autogenerated
+/*
+Copyright 2025 Stacklok
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
// Code generated by controller-gen. DO NOT EDIT.
package v1alpha1
@@ -25,25 +41,6 @@ func (in *APISource) DeepCopy() *APISource {
return out
}
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *APIStatus) DeepCopyInto(out *APIStatus) {
- *out = *in
- if in.ReadySince != nil {
- in, out := &in.ReadySince, &out.ReadySince
- *out = (*in).DeepCopy()
- }
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIStatus.
-func (in *APIStatus) DeepCopy() *APIStatus {
- if in == nil {
- return nil
- }
- out := new(APIStatus)
- in.DeepCopyInto(out)
- return out
-}
-
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *AWSStsConfig) DeepCopyInto(out *AWSStsConfig) {
*out = *in
@@ -1352,21 +1349,6 @@ func (in *MCPRegistrySpec) DeepCopy() *MCPRegistrySpec {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *MCPRegistryStatus) DeepCopyInto(out *MCPRegistryStatus) {
*out = *in
- if in.SyncStatus != nil {
- in, out := &in.SyncStatus, &out.SyncStatus
- *out = new(SyncStatus)
- (*in).DeepCopyInto(*out)
- }
- if in.APIStatus != nil {
- in, out := &in.APIStatus, &out.APIStatus
- *out = new(APIStatus)
- (*in).DeepCopyInto(*out)
- }
- if in.StorageRef != nil {
- in, out := &in.StorageRef, &out.StorageRef
- *out = new(StorageReference)
- (*in).DeepCopyInto(*out)
- }
if in.Conditions != nil {
in, out := &in.Conditions, &out.Conditions
*out = make([]v1.Condition, len(*in))
@@ -2703,26 +2685,6 @@ func (in *SessionStorageConfig) DeepCopy() *SessionStorageConfig {
return out
}
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *StorageReference) DeepCopyInto(out *StorageReference) {
- *out = *in
- if in.ConfigMapRef != nil {
- in, out := &in.ConfigMapRef, &out.ConfigMapRef
- *out = new(corev1.LocalObjectReference)
- **out = **in
- }
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StorageReference.
-func (in *StorageReference) DeepCopy() *StorageReference {
- if in == nil {
- return nil
- }
- out := new(StorageReference)
- in.DeepCopyInto(out)
- return out
-}
-
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *SyncPolicy) DeepCopyInto(out *SyncPolicy) {
*out = *in
@@ -2738,29 +2700,6 @@ func (in *SyncPolicy) DeepCopy() *SyncPolicy {
return out
}
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *SyncStatus) DeepCopyInto(out *SyncStatus) {
- *out = *in
- if in.LastAttempt != nil {
- in, out := &in.LastAttempt, &out.LastAttempt
- *out = (*in).DeepCopy()
- }
- if in.LastSyncTime != nil {
- in, out := &in.LastSyncTime, &out.LastSyncTime
- *out = (*in).DeepCopy()
- }
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SyncStatus.
-func (in *SyncStatus) DeepCopy() *SyncStatus {
- if in == nil {
- return nil
- }
- out := new(SyncStatus)
- in.DeepCopyInto(out)
- return out
-}
-
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *TagFilter) DeepCopyInto(out *TagFilter) {
*out = *in
diff --git a/cmd/thv-operator/controllers/mcpregistry_controller.go b/cmd/thv-operator/controllers/mcpregistry_controller.go
index 232c9e853a..a8a1bfcce4 100644
--- a/cmd/thv-operator/controllers/mcpregistry_controller.go
+++ b/cmd/thv-operator/controllers/mcpregistry_controller.go
@@ -5,6 +5,7 @@ package controllers
import (
"context"
+ "errors"
"fmt"
"time"
@@ -21,7 +22,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
- "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus"
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi"
)
@@ -97,24 +97,23 @@ func (r *MCPRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request)
ctxLogger.Error(err, "Failed to get MCPRegistry")
return ctrl.Result{}, err
}
- // Safe access to nested status fields
- var apiPhase any
- if mcpRegistry.Status.APIStatus != nil {
- apiPhase = mcpRegistry.Status.APIStatus.Phase
- }
- apiEndpoint := ""
- if mcpRegistry.Status.APIStatus != nil {
- apiEndpoint = mcpRegistry.Status.APIStatus.Endpoint
- }
- ctxLogger.Info("Reconciling MCPRegistry", "MCPRegistry.Name", mcpRegistry.Name, "phase", mcpRegistry.Status.Phase,
- "apiPhase", apiPhase, "apiEndpoint", apiEndpoint)
+ ctxLogger.Info("Reconciling MCPRegistry", "MCPRegistry.Name", mcpRegistry.Name,
+ "phase", mcpRegistry.Status.Phase, "url", mcpRegistry.Status.URL)
// Validate PodTemplateSpec early - before other operations
+ var podTemplateCondition *metav1.Condition
if mcpRegistry.HasPodTemplateSpec() {
- // Validate PodTemplateSpec early - before other operations
- // This ensures we fail fast if the spec is invalid
- if !r.validateAndUpdatePodTemplateStatus(ctx, mcpRegistry) {
+ valid, cond := r.validatePodTemplate(mcpRegistry)
+ podTemplateCondition = cond
+ if !valid {
+ // Write status immediately for the failure case since we return early
+ mcpRegistry.Status.Phase = mcpv1alpha1.MCPRegistryPhaseFailed
+ mcpRegistry.Status.Message = fmt.Sprintf("Invalid PodTemplateSpec: %v", cond.Message)
+ meta.SetStatusCondition(&mcpRegistry.Status.Conditions, *cond)
+ if statusErr := r.Status().Update(ctx, mcpRegistry); statusErr != nil {
+ ctxLogger.Error(statusErr, "Failed to update MCPRegistry status with PodTemplateSpec validation")
+ }
// Invalid PodTemplateSpec - return without error to avoid infinite retries
// The user must fix the spec and the next reconciliation will retry
return ctrl.Result{}, nil
@@ -162,114 +161,42 @@ func (r *MCPRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return ctrl.Result{}, nil
}
- // 3. Create status manager for batched updates with separation of concerns
- statusManager := mcpregistrystatus.NewStatusManager(mcpRegistry)
-
- // Initialize result
- result := ctrl.Result{}
- err = nil
-
- // 4. Reconcile API service
+ // 3. Reconcile API service - capture error for status update
+ var reconcileErr error
if apiErr := r.registryAPIManager.ReconcileAPIService(ctx, mcpRegistry); apiErr != nil {
ctxLogger.Error(apiErr, "Failed to reconcile API service")
- // Set API status with detailed error message from structured error
- statusManager.API().SetAPIStatus(mcpv1alpha1.APIPhaseError, apiErr.Message, "")
- statusManager.API().SetAPIReadyCondition(apiErr.ConditionReason, apiErr.Message, metav1.ConditionFalse)
- err = apiErr
- } else {
- // API reconciliation successful - check readiness and set appropriate status
- isReady := r.registryAPIManager.IsAPIReady(ctx, mcpRegistry)
- if isReady {
- // In-cluster endpoint (simplified form works for internal access)
- endpoint := fmt.Sprintf("http://%s.%s:8080",
- mcpRegistry.GetAPIResourceName(), mcpRegistry.Namespace)
- statusManager.API().SetAPIStatus(mcpv1alpha1.APIPhaseReady,
- "Registry API is ready and serving requests", endpoint)
- statusManager.API().SetAPIReadyCondition("APIReady",
- "Registry API is ready and serving requests", metav1.ConditionTrue)
- } else {
- statusManager.API().SetAPIStatus(mcpv1alpha1.APIPhaseDeploying,
- "Registry API deployment is not ready yet", "")
- statusManager.API().SetAPIReadyCondition("APINotReady",
- "Registry API deployment is not ready yet", metav1.ConditionFalse)
- }
+ reconcileErr = apiErr
}
- // 5. Check if we need to requeue for API readiness
- if err == nil && !r.registryAPIManager.IsAPIReady(ctx, mcpRegistry) {
- ctxLogger.Info("API not ready yet, scheduling requeue to check readiness")
- if result.RequeueAfter == 0 || result.RequeueAfter > time.Second*30 {
- result.RequeueAfter = time.Second * 30
+ // 4. Determine and persist status
+ isReady, statusUpdateErr := r.updateRegistryStatus(ctx, mcpRegistry, reconcileErr, podTemplateCondition)
+ if statusUpdateErr != nil {
+ ctxLogger.Error(statusUpdateErr, "Failed to update registry status")
+ // Return the status update error only if there was no main reconciliation error
+ if reconcileErr == nil {
+ reconcileErr = statusUpdateErr
}
}
- // 6. Derive overall phase and message from API status
- statusDeriver := mcpregistrystatus.NewDefaultStatusDeriver()
- r.deriveOverallStatus(ctx, mcpRegistry, statusManager, statusDeriver)
-
- // 7. Apply all status changes in a single batch update
- if statusUpdateErr := r.applyStatusUpdates(ctx, r.Client, mcpRegistry, statusManager); statusUpdateErr != nil {
- ctxLogger.Error(statusUpdateErr, "Failed to apply batched status update")
- // Return the status update error only if there was no main reconciliation error
- if err == nil {
- err = statusUpdateErr
- }
+ // 5. Determine requeue based on phase
+ result := ctrl.Result{}
+ if reconcileErr == nil && !isReady {
+ ctxLogger.Info("API not ready yet, scheduling requeue to check readiness")
+ result.RequeueAfter = time.Second * 30
}
+
// Log reconciliation completion
- if err != nil {
- ctxLogger.Error(err, "Reconciliation completed with error",
+ if reconcileErr != nil {
+ ctxLogger.Error(reconcileErr, "Reconciliation completed with error",
"MCPRegistry.Name", mcpRegistry.Name, "requeueAfter", result.RequeueAfter)
} else {
- var apiPhase string
- if mcpRegistry.Status.APIStatus != nil {
- apiPhase = string(mcpRegistry.Status.APIStatus.Phase)
- }
-
ctxLogger.Info("Reconciliation completed successfully",
"MCPRegistry.Name", mcpRegistry.Name,
"phase", mcpRegistry.Status.Phase,
- "apiPhase", apiPhase,
"requeueAfter", result.RequeueAfter)
}
- return result, err
-}
-
-// finalizeMCPRegistry performs the finalizer logic for the MCPRegistry
-func (r *MCPRegistryReconciler) finalizeMCPRegistry(ctx context.Context, registry *mcpv1alpha1.MCPRegistry) error {
- ctxLogger := log.FromContext(ctx)
-
- // Update the MCPRegistry status to indicate termination - immediate update needed since object is being deleted
- registry.Status.Phase = mcpv1alpha1.MCPRegistryPhaseTerminating
- registry.Status.Message = "MCPRegistry is being terminated"
- if err := r.Status().Update(ctx, registry); err != nil {
- ctxLogger.Error(err, "Failed to update MCPRegistry status during finalization")
- return err
- }
-
- ctxLogger.Info("MCPRegistry finalization completed", "registry", registry.Name)
- return nil
-}
-
-// deriveOverallStatus determines the overall MCPRegistry phase and message based on API status
-func (*MCPRegistryReconciler) deriveOverallStatus(
- ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry,
- statusManager mcpregistrystatus.StatusManager, statusDeriver mcpregistrystatus.StatusDeriver) {
- ctxLogger := log.FromContext(ctx)
-
- apiStatus := statusManager.API().Status()
- if apiStatus == nil {
- apiStatus = mcpRegistry.Status.APIStatus
- }
- // Use the StatusDeriver to determine the overall phase and message
- // based on current API status
- derivedPhase, derivedMessage := statusDeriver.DeriveOverallStatus(apiStatus)
-
- // Only update phase and message if they've changed
- statusManager.SetOverallStatus(derivedPhase, derivedMessage)
- ctxLogger.Info("Updated overall status", "apiStatus", apiStatus,
- "oldPhase", mcpRegistry.Status.Phase, "newPhase", derivedPhase,
- "oldMessage", mcpRegistry.Status.Message, "newMessage", derivedMessage)
+ return result, reconcileErr
}
// SetupWithManager sets up the controller with the Manager.
@@ -285,101 +212,115 @@ func (r *MCPRegistryReconciler) SetupWithManager(mgr ctrl.Manager) error {
Complete(r)
}
-// Apply applies all collected status changes in a single batch update.
-// Only actual changes are applied to the status to avoid unnecessary reconciliations
-func (*MCPRegistryReconciler) applyStatusUpdates(
- ctx context.Context, k8sClient client.Client,
- mcpRegistry *mcpv1alpha1.MCPRegistry, statusManager mcpregistrystatus.StatusManager) error {
+// updateRegistryStatus determines the MCPRegistry phase from the API deployment state
+// and persists it with a single status update. Returns whether the API is ready and any
+// error from the status update.
+func (r *MCPRegistryReconciler) updateRegistryStatus(
+ ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry, reconcileErr error, podTemplateCond *metav1.Condition,
+) (bool, error) {
+ // Refetch the latest version to avoid conflicts
+ latest := &mcpv1alpha1.MCPRegistry{}
+ if err := r.Get(ctx, client.ObjectKeyFromObject(mcpRegistry), latest); err != nil {
+ return false, fmt.Errorf("failed to fetch latest MCPRegistry version: %w", err)
+ }
- ctxLogger := log.FromContext(ctx)
+ var isReady bool
- // Refetch the latest version of the resource to avoid conflicts
- latestRegistry := &mcpv1alpha1.MCPRegistry{}
- if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(mcpRegistry), latestRegistry); err != nil {
- ctxLogger.Error(err, "Failed to fetch latest MCPRegistry version for status update")
- return fmt.Errorf("failed to fetch latest MCPRegistry version: %w", err)
- }
- latestRegistryStatus := latestRegistry.Status
- hasUpdates := false
+ if reconcileErr != nil {
+ latest.Status.Phase = mcpv1alpha1.MCPRegistryPhaseFailed
+ latest.Status.ReadyReplicas = 0
+ // Use structured error fields if available
+ var apiErr *registryapi.Error
+ if errors.As(reconcileErr, &apiErr) {
+ latest.Status.Message = apiErr.Message
+ setRegistryReadyCondition(latest, metav1.ConditionFalse, apiErr.ConditionReason, apiErr.Message)
+ } else {
+ latest.Status.Message = reconcileErr.Error()
+ setRegistryReadyCondition(latest, metav1.ConditionFalse,
+ mcpv1alpha1.ConditionReasonRegistryNotReady, reconcileErr.Error())
+ }
+ } else {
+ var readyReplicas int32
+ isReady, readyReplicas = r.registryAPIManager.GetAPIStatus(ctx, mcpRegistry)
+ latest.Status.ReadyReplicas = readyReplicas
- // Apply status changes from status manager
- hasUpdates = statusManager.UpdateStatus(ctx, &latestRegistryStatus) || hasUpdates
+ if isReady {
+ endpoint := fmt.Sprintf("http://%s.%s:8080",
+ mcpRegistry.GetAPIResourceName(), mcpRegistry.Namespace)
+ latest.Status.Phase = mcpv1alpha1.MCPRegistryPhaseRunning
+ latest.Status.Message = "Registry API is ready and serving requests"
+ latest.Status.URL = endpoint
+ setRegistryReadyCondition(latest, metav1.ConditionTrue,
+ mcpv1alpha1.ConditionReasonRegistryReady, "Registry API is ready and serving requests")
+ } else {
+ latest.Status.Phase = mcpv1alpha1.MCPRegistryPhasePending
+ latest.Status.Message = "Registry API deployment is not ready yet"
+ setRegistryReadyCondition(latest, metav1.ConditionFalse,
+ mcpv1alpha1.ConditionReasonRegistryNotReady, "Registry API deployment is not ready yet")
+ }
+ }
- // Update ObservedGeneration to reflect that we've processed this generation
- if latestRegistryStatus.ObservedGeneration != mcpRegistry.Generation {
- latestRegistryStatus.ObservedGeneration = mcpRegistry.Generation
- hasUpdates = true
+ // Apply PodTemplate condition if present
+ if podTemplateCond != nil {
+ meta.SetStatusCondition(&latest.Status.Conditions, *podTemplateCond)
}
- // Single status update using the latest version
- if hasUpdates {
- latestRegistry.Status = latestRegistryStatus
- if err := k8sClient.Status().Update(ctx, latestRegistry); err != nil {
- ctxLogger.Error(err, "Failed to apply batched status update")
- return fmt.Errorf("failed to apply batched status update: %w", err)
- }
- var apiPhase string
- if latestRegistryStatus.APIStatus != nil {
- apiPhase = string(latestRegistryStatus.APIStatus.Phase)
- }
- ctxLogger.V(1).Info("Applied batched status updates",
- "phase", latestRegistryStatus.Phase,
- "apiPhase", apiPhase,
- "message", latestRegistryStatus.Message,
- "conditionsCount", len(latestRegistryStatus.Conditions))
- } else {
- ctxLogger.V(1).Info("No batched status updates applied")
+ latest.Status.ObservedGeneration = latest.Generation
+ if err := r.Status().Update(ctx, latest); err != nil {
+ return false, err
}
+ return isReady, nil
+}
- return nil
+// setRegistryReadyCondition sets the top-level Ready condition on an MCPRegistry.
+func setRegistryReadyCondition(registry *mcpv1alpha1.MCPRegistry, status metav1.ConditionStatus, reason, message string) {
+ meta.SetStatusCondition(®istry.Status.Conditions, metav1.Condition{
+ Type: mcpv1alpha1.ConditionTypeReady,
+ Status: status,
+ Reason: reason,
+ Message: message,
+ ObservedGeneration: registry.Generation,
+ })
}
-// validateAndUpdatePodTemplateStatus validates the PodTemplateSpec and updates the MCPRegistry status
-// with appropriate conditions. Returns true if validation passes, false otherwise.
-func (r *MCPRegistryReconciler) validateAndUpdatePodTemplateStatus(
- ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry,
-) bool {
+// finalizeMCPRegistry performs the finalizer logic for the MCPRegistry
+func (r *MCPRegistryReconciler) finalizeMCPRegistry(ctx context.Context, registry *mcpv1alpha1.MCPRegistry) error {
ctxLogger := log.FromContext(ctx)
- // Validate the PodTemplateSpec by attempting to parse it
+ // Update the MCPRegistry status to indicate termination - immediate update needed since object is being deleted
+ registry.Status.Phase = mcpv1alpha1.MCPRegistryPhaseTerminating
+ registry.Status.Message = "MCPRegistry is being terminated"
+ setRegistryReadyCondition(registry, metav1.ConditionFalse,
+ mcpv1alpha1.ConditionReasonRegistryNotReady, "MCPRegistry is being terminated")
+ if err := r.Status().Update(ctx, registry); err != nil {
+ ctxLogger.Error(err, "Failed to update MCPRegistry status during finalization")
+ return err
+ }
+
+ ctxLogger.Info("MCPRegistry finalization completed", "registry", registry.Name)
+ return nil
+}
+
+// validatePodTemplate validates the PodTemplateSpec and returns a condition reflecting the result.
+// Returns true if validation passes, and a condition to apply during the next status update.
+func (*MCPRegistryReconciler) validatePodTemplate(
+ mcpRegistry *mcpv1alpha1.MCPRegistry,
+) (bool, *metav1.Condition) {
err := registryapi.ValidatePodTemplateSpec(mcpRegistry.GetPodTemplateSpecRaw())
if err != nil {
- // Set phase and message
- mcpRegistry.Status.Phase = mcpv1alpha1.MCPRegistryPhaseFailed
- mcpRegistry.Status.Message = fmt.Sprintf("Invalid PodTemplateSpec: %v", err)
-
- // Set condition for invalid PodTemplateSpec
- meta.SetStatusCondition(&mcpRegistry.Status.Conditions, metav1.Condition{
- Type: mcpv1alpha1.ConditionRegistryPodTemplateValid,
+ return false, &metav1.Condition{
+ Type: mcpv1alpha1.ConditionPodTemplateValid,
Status: metav1.ConditionFalse,
ObservedGeneration: mcpRegistry.Generation,
- Reason: mcpv1alpha1.ConditionReasonRegistryPodTemplateInvalid,
+ Reason: mcpv1alpha1.ConditionReasonPodTemplateInvalid,
Message: fmt.Sprintf("Failed to parse PodTemplateSpec: %v. Deployment blocked until fixed.", err),
- })
-
- // Update status with the condition
- if statusErr := r.Status().Update(ctx, mcpRegistry); statusErr != nil {
- ctxLogger.Error(statusErr, "Failed to update MCPRegistry status with PodTemplateSpec validation")
- return false
}
-
- ctxLogger.Error(err, "PodTemplateSpec validation failed")
- return false
}
-
- // Set condition for valid PodTemplateSpec
- meta.SetStatusCondition(&mcpRegistry.Status.Conditions, metav1.Condition{
- Type: mcpv1alpha1.ConditionRegistryPodTemplateValid,
+ return true, &metav1.Condition{
+ Type: mcpv1alpha1.ConditionPodTemplateValid,
Status: metav1.ConditionTrue,
ObservedGeneration: mcpRegistry.Generation,
- Reason: mcpv1alpha1.ConditionReasonRegistryPodTemplateValid,
+ Reason: mcpv1alpha1.ConditionReasonPodTemplateValid,
Message: "PodTemplateSpec is valid",
- })
-
- // Update status with the condition
- if statusErr := r.Status().Update(ctx, mcpRegistry); statusErr != nil {
- ctxLogger.Error(statusErr, "Failed to update MCPRegistry status with PodTemplateSpec validation")
}
-
- return true
}
diff --git a/cmd/thv-operator/controllers/mcpregistry_controller_test.go b/cmd/thv-operator/controllers/mcpregistry_controller_test.go
index 98e635bcf0..8b61605fe4 100644
--- a/cmd/thv-operator/controllers/mcpregistry_controller_test.go
+++ b/cmd/thv-operator/controllers/mcpregistry_controller_test.go
@@ -23,7 +23,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
- "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus"
+ "github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi"
registryapimocks "github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi/mocks"
)
@@ -195,7 +195,7 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
var updated mcpv1alpha1.MCPRegistry
require.NoError(t, fakeClient.Get(t.Context(),
types.NamespacedName{Name: registryName, Namespace: registryNamespace}, &updated))
- cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionRegistryPodTemplateValid)
+ cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionPodTemplateValid)
require.NotNil(t, cond, "PodTemplateValid condition must be set")
assert.Equal(t, metav1.ConditionFalse, cond.Status)
assert.Equal(t, mcpv1alpha1.MCPRegistryPhaseFailed, updated.Status.Phase)
@@ -219,7 +219,7 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
},
configureMocks: func(mock *registryapimocks.MockManager) {
mock.EXPECT().ReconcileAPIService(gomock.Any(), gomock.Any()).Return(nil)
- mock.EXPECT().IsAPIReady(gomock.Any(), gomock.Any()).Return(true).Times(2)
+ mock.EXPECT().GetAPIStatus(gomock.Any(), gomock.Any()).Return(true, int32(1))
},
expResult: ctrl.Result{},
expErr: nil,
@@ -228,7 +228,7 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
var updated mcpv1alpha1.MCPRegistry
require.NoError(t, fakeClient.Get(t.Context(),
types.NamespacedName{Name: registryName, Namespace: registryNamespace}, &updated))
- cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionRegistryPodTemplateValid)
+ cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionPodTemplateValid)
require.NotNil(t, cond, "PodTemplateValid condition must be set")
assert.Equal(t, metav1.ConditionTrue, cond.Status)
},
@@ -246,15 +246,15 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
},
configureMocks: func(mock *registryapimocks.MockManager) {
mock.EXPECT().ReconcileAPIService(gomock.Any(), gomock.Any()).Return(
- &mcpregistrystatus.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
+ ®istryapi.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
)
- // err != nil in Reconcile → IsAPIReady is never called.
+ // reconcileErr != nil → IsAPIReady and GetReadyReplicas are never called.
},
expResult: ctrl.Result{},
- expErr: &mcpregistrystatus.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
+ expErr: ®istryapi.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
},
{
- // applyStatusUpdates writes APIPhaseDeploying; deriveOverallStatus sets PhasePending.
+ // updateRegistryStatus sets Phase=Pending when API is not ready.
// Reconcile also schedules a requeue because IsAPIReady returns false.
name: "api_reconcile_success_api_not_ready",
setup: func(t *testing.T, s *runtime.Scheme) (*fake.ClientBuilder, *mcpv1alpha1.MCPRegistry) {
@@ -268,8 +268,7 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
},
configureMocks: func(mock *registryapimocks.MockManager) {
mock.EXPECT().ReconcileAPIService(gomock.Any(), gomock.Any()).Return(nil)
- // Called twice: once in the success branch, once in the requeue check.
- mock.EXPECT().IsAPIReady(gomock.Any(), gomock.Any()).Return(false).Times(2)
+ mock.EXPECT().GetAPIStatus(gomock.Any(), gomock.Any()).Return(false, int32(0))
},
expResult: ctrl.Result{RequeueAfter: 30 * time.Second},
expErr: nil,
@@ -278,13 +277,12 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
var updated mcpv1alpha1.MCPRegistry
require.NoError(t, fakeClient.Get(t.Context(),
types.NamespacedName{Name: registryName, Namespace: registryNamespace}, &updated))
- require.NotNil(t, updated.Status.APIStatus)
- assert.Equal(t, mcpv1alpha1.APIPhaseDeploying, updated.Status.APIStatus.Phase)
assert.Equal(t, mcpv1alpha1.MCPRegistryPhasePending, updated.Status.Phase)
+ assert.Equal(t, int32(0), updated.Status.ReadyReplicas)
},
},
{
- // applyStatusUpdates writes APIPhaseReady; deriveOverallStatus sets PhaseReady.
+ // updateRegistryStatus sets Phase=Running when API is ready.
// No requeue because IsAPIReady returns true.
name: "api_reconcile_success_api_ready",
setup: func(t *testing.T, s *runtime.Scheme) (*fake.ClientBuilder, *mcpv1alpha1.MCPRegistry) {
@@ -298,8 +296,7 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
},
configureMocks: func(mock *registryapimocks.MockManager) {
mock.EXPECT().ReconcileAPIService(gomock.Any(), gomock.Any()).Return(nil)
- // Called twice: once in the success branch, once in the requeue check.
- mock.EXPECT().IsAPIReady(gomock.Any(), gomock.Any()).Return(true).Times(2)
+ mock.EXPECT().GetAPIStatus(gomock.Any(), gomock.Any()).Return(true, int32(1))
},
expResult: ctrl.Result{},
expErr: nil,
@@ -308,15 +305,14 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
var updated mcpv1alpha1.MCPRegistry
require.NoError(t, fakeClient.Get(t.Context(),
types.NamespacedName{Name: registryName, Namespace: registryNamespace}, &updated))
- require.NotNil(t, updated.Status.APIStatus)
- assert.Equal(t, mcpv1alpha1.APIPhaseReady, updated.Status.APIStatus.Phase)
- assert.Equal(t, mcpv1alpha1.MCPRegistryPhaseReady, updated.Status.Phase)
+ assert.Equal(t, mcpv1alpha1.MCPRegistryPhaseRunning, updated.Status.Phase)
+ assert.Equal(t, int32(1), updated.Status.ReadyReplicas)
},
},
{
- // When ReconcileAPIService fails, applyStatusUpdates should still persist
- // APIStatus.Phase=Error and set the APIReady condition to False.
- name: "api_reconcile_error_updates_api_error_status",
+ // When ReconcileAPIService fails, updateRegistryStatus sets Phase=Failed
+ // and the Ready condition to False with the structured error reason.
+ name: "api_reconcile_error_updates_failed_status",
setup: func(t *testing.T, s *runtime.Scheme) (*fake.ClientBuilder, *mcpv1alpha1.MCPRegistry) {
t.Helper()
mcpRegistry := newMCPRegistryWithFinalizer(registryName, registryNamespace)
@@ -328,27 +324,28 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
},
configureMocks: func(mock *registryapimocks.MockManager) {
mock.EXPECT().ReconcileAPIService(gomock.Any(), gomock.Any()).Return(
- &mcpregistrystatus.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
+ ®istryapi.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
)
- // err != nil → IsAPIReady is never called.
+ // reconcileErr != nil → IsAPIReady and GetReadyReplicas are never called.
},
expResult: ctrl.Result{},
- expErr: &mcpregistrystatus.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
+ expErr: ®istryapi.Error{Message: "deploy failed", ConditionReason: "DeployFailed"},
assertRegistry: func(t *testing.T, fakeClient client.Client) {
t.Helper()
var updated mcpv1alpha1.MCPRegistry
require.NoError(t, fakeClient.Get(t.Context(),
types.NamespacedName{Name: registryName, Namespace: registryNamespace}, &updated))
- require.NotNil(t, updated.Status.APIStatus)
- assert.Equal(t, mcpv1alpha1.APIPhaseError, updated.Status.APIStatus.Phase)
- cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionAPIReady)
- require.NotNil(t, cond, "APIReady condition must be set")
+ assert.Equal(t, mcpv1alpha1.MCPRegistryPhaseFailed, updated.Status.Phase)
+ assert.Equal(t, "deploy failed", updated.Status.Message)
+ cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionTypeReady)
+ require.NotNil(t, cond, "Ready condition must be set")
assert.Equal(t, metav1.ConditionFalse, cond.Status)
+ assert.Equal(t, "DeployFailed", cond.Reason)
},
},
{
- // When the API is ready, the endpoint in APIStatus should follow the in-cluster
- // URL format and the APIReady condition should be True.
+ // When the API is ready, the URL should follow the in-cluster format
+ // and the Ready condition should be True.
name: "api_reconcile_success_api_ready_checks_endpoint_and_condition",
setup: func(t *testing.T, s *runtime.Scheme) (*fake.ClientBuilder, *mcpv1alpha1.MCPRegistry) {
t.Helper()
@@ -361,8 +358,7 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
},
configureMocks: func(mock *registryapimocks.MockManager) {
mock.EXPECT().ReconcileAPIService(gomock.Any(), gomock.Any()).Return(nil)
- // Called twice: once in the success branch, once in the requeue check.
- mock.EXPECT().IsAPIReady(gomock.Any(), gomock.Any()).Return(true).Times(2)
+ mock.EXPECT().GetAPIStatus(gomock.Any(), gomock.Any()).Return(true, int32(2))
},
expResult: ctrl.Result{},
expErr: nil,
@@ -371,10 +367,10 @@ func TestMCPRegistryReconciler_Reconcile(t *testing.T) {
var updated mcpv1alpha1.MCPRegistry
require.NoError(t, fakeClient.Get(t.Context(),
types.NamespacedName{Name: registryName, Namespace: registryNamespace}, &updated))
- require.NotNil(t, updated.Status.APIStatus)
- assert.Equal(t, "http://test-registry-api.default:8080", updated.Status.APIStatus.Endpoint)
- cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionAPIReady)
- require.NotNil(t, cond, "APIReady condition must be set")
+ assert.Equal(t, "http://test-registry-api.default:8080", updated.Status.URL)
+ assert.Equal(t, int32(2), updated.Status.ReadyReplicas)
+ cond := k8smeta.FindStatusCondition(updated.Status.Conditions, mcpv1alpha1.ConditionTypeReady)
+ require.NotNil(t, cond, "Ready condition must be set")
assert.Equal(t, metav1.ConditionTrue, cond.Status)
},
},
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/collector.go b/cmd/thv-operator/pkg/mcpregistrystatus/collector.go
deleted file mode 100644
index 9920ea29bd..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/collector.go
+++ /dev/null
@@ -1,164 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-// Package mcpregistrystatus provides status management and batched updates for MCPRegistry resources.
-package mcpregistrystatus
-
-import (
- "context"
-
- "k8s.io/apimachinery/pkg/api/meta"
- metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
- "sigs.k8s.io/controller-runtime/pkg/log"
-
- mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
-)
-
-// StatusCollector collects status changes during reconciliation
-// and applies them in a single batch update at the end.
-// It implements the StatusManager interface.
-type StatusCollector struct {
- mcpRegistry *mcpv1alpha1.MCPRegistry
- hasChanges bool
- phase *mcpv1alpha1.MCPRegistryPhase
- message *string
- apiStatus *mcpv1alpha1.APIStatus
- conditions map[string]metav1.Condition
- apiCollector *apiStatusCollector
-}
-
-// apiStatusCollector implements APIStatusCollector
-type apiStatusCollector struct {
- parent *StatusCollector
-}
-
-// NewStatusManager creates a new StatusManager for the given MCPRegistry resource.
-func NewStatusManager(mcpRegistry *mcpv1alpha1.MCPRegistry) StatusManager {
- return newStatusCollector(mcpRegistry)
-}
-
-// newStatusCollector creates the internal StatusCollector implementation
-func newStatusCollector(mcpRegistry *mcpv1alpha1.MCPRegistry) *StatusCollector {
- collector := &StatusCollector{
- mcpRegistry: mcpRegistry,
- conditions: make(map[string]metav1.Condition),
- }
- collector.apiCollector = &apiStatusCollector{parent: collector}
- return collector
-}
-
-// SetPhase sets the phase to be updated.
-func (s *StatusCollector) SetPhase(phase mcpv1alpha1.MCPRegistryPhase) {
- s.phase = &phase
- s.hasChanges = true
-}
-
-// SetMessage sets the message to be updated.
-func (s *StatusCollector) SetMessage(message string) {
- s.message = &message
- s.hasChanges = true
-}
-
-// SetCondition sets a general condition with the specified type, reason, message, and status
-func (s *StatusCollector) SetCondition(conditionType, reason, message string, status metav1.ConditionStatus) {
- s.conditions[conditionType] = metav1.Condition{
- Type: conditionType,
- Status: status,
- Reason: reason,
- Message: message,
- }
- s.hasChanges = true
-}
-
-// SetAPIReadyCondition adds or updates the API ready condition.
-func (s *StatusCollector) SetAPIReadyCondition(reason, message string, status metav1.ConditionStatus) {
- s.SetCondition(mcpv1alpha1.ConditionAPIReady, reason, message, status)
-}
-
-// SetAPIStatus sets the detailed API status.
-func (s *StatusCollector) SetAPIStatus(phase mcpv1alpha1.APIPhase, message string, endpoint string) {
- s.apiStatus = &mcpv1alpha1.APIStatus{
- Phase: phase,
- Message: message,
- Endpoint: endpoint,
- }
-
- // Set ReadySince timestamp when API becomes ready
- if phase == mcpv1alpha1.APIPhaseReady &&
- (s.mcpRegistry.Status.APIStatus == nil || s.mcpRegistry.Status.APIStatus.Phase != mcpv1alpha1.APIPhaseReady) {
- now := metav1.Now()
- s.apiStatus.ReadySince = &now
- } else if s.mcpRegistry.Status.APIStatus != nil && s.mcpRegistry.Status.APIStatus.ReadySince != nil {
- // Preserve existing ReadySince if already set and still ready
- s.apiStatus.ReadySince = s.mcpRegistry.Status.APIStatus.ReadySince
- }
-
- s.hasChanges = true
-}
-
-// UpdateStatus applies all collected status changes in a single batch update.
-// Requires the MCPRegistryStatus being the updated version from the cluster
-func (s *StatusCollector) UpdateStatus(ctx context.Context, mcpRegistryStatus *mcpv1alpha1.MCPRegistryStatus) bool {
-
- ctxLogger := log.FromContext(ctx)
-
- if s.hasChanges {
- // Apply phase change
- if s.phase != nil {
- mcpRegistryStatus.Phase = *s.phase
- }
-
- // Apply message change
- if s.message != nil {
- mcpRegistryStatus.Message = *s.message
- }
-
- // Apply API status change
- if s.apiStatus != nil {
- mcpRegistryStatus.APIStatus = s.apiStatus
- }
-
- // Apply condition changes
- for _, condition := range s.conditions {
- meta.SetStatusCondition(&mcpRegistryStatus.Conditions, condition)
- }
-
- ctxLogger.V(1).Info("Batched status update applied",
- "phase", s.phase,
- "message", s.message,
- "conditionsCount", len(s.conditions))
- return true
- }
- ctxLogger.V(1).Info("No batched status update needed")
- return false
-}
-
-// StatusManager interface methods
-
-// API returns the API status collector
-func (s *StatusCollector) API() APIStatusCollector {
- return s.apiCollector
-}
-
-// SetOverallStatus sets the overall phase and message explicitly (for special cases)
-func (s *StatusCollector) SetOverallStatus(phase mcpv1alpha1.MCPRegistryPhase, message string) {
- s.SetPhase(phase)
- s.SetMessage(message)
-}
-
-// APIStatusCollector implementation
-
-// Status returns the API status
-func (ac *apiStatusCollector) Status() *mcpv1alpha1.APIStatus {
- return ac.parent.apiStatus
-}
-
-// SetAPIStatus delegates to the parent's SetAPIStatus method
-func (ac *apiStatusCollector) SetAPIStatus(phase mcpv1alpha1.APIPhase, message string, endpoint string) {
- ac.parent.SetAPIStatus(phase, message, endpoint)
-}
-
-// SetAPIReadyCondition delegates to the parent's SetAPIReadyCondition method
-func (ac *apiStatusCollector) SetAPIReadyCondition(reason, message string, status metav1.ConditionStatus) {
- ac.parent.SetAPIReadyCondition(reason, message, status)
-}
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/collector_test.go b/cmd/thv-operator/pkg/mcpregistrystatus/collector_test.go
deleted file mode 100644
index c525a3ba03..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/collector_test.go
+++ /dev/null
@@ -1,443 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-package mcpregistrystatus
-
-import (
- "context"
- "testing"
-
- "github.com/stretchr/testify/assert"
- "github.com/stretchr/testify/require"
- metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
- "k8s.io/apimachinery/pkg/runtime"
- "sigs.k8s.io/controller-runtime/pkg/client/fake"
-
- mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
-)
-
-func TestNewStatusManager(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "default",
- },
- }
-
- statusManager := NewStatusManager(registry)
-
- assert.NotNil(t, statusManager)
- sc := statusManager.(*StatusCollector)
- assert.Equal(t, registry, sc.mcpRegistry)
- assert.False(t, sc.hasChanges)
- assert.Empty(t, sc.conditions)
-}
-
-func TestStatusCollector_SetPhase(t *testing.T) {
- t.Parallel()
-
- tests := []struct {
- name string
- phase mcpv1alpha1.MCPRegistryPhase
- }{
- {
- name: "set pending phase",
- phase: mcpv1alpha1.MCPRegistryPhasePending,
- },
- {
- name: "set ready phase",
- phase: mcpv1alpha1.MCPRegistryPhaseReady,
- },
- {
- name: "set failed phase",
- phase: mcpv1alpha1.MCPRegistryPhaseFailed,
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
-
- collector.SetPhase(tt.phase)
-
- assert.True(t, collector.hasChanges)
- assert.NotNil(t, collector.phase)
- assert.Equal(t, tt.phase, *collector.phase)
- })
- }
-}
-
-func TestStatusCollector_SetMessage(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
- testMessage := "Test message"
-
- collector.SetMessage(testMessage)
-
- assert.True(t, collector.hasChanges)
- assert.NotNil(t, collector.message)
- assert.Equal(t, testMessage, *collector.message)
-}
-
-func TestStatusCollector_SetAPIReadyCondition(t *testing.T) {
- t.Parallel()
-
- tests := []struct {
- name string
- reason string
- message string
- status metav1.ConditionStatus
- expectKey string
- }{
- {
- name: "API ready condition true",
- reason: "APIReady",
- message: "API is ready",
- status: metav1.ConditionTrue,
- expectKey: mcpv1alpha1.ConditionAPIReady,
- },
- {
- name: "API ready condition false",
- reason: "APINotReady",
- message: "API is not ready",
- status: metav1.ConditionFalse,
- expectKey: mcpv1alpha1.ConditionAPIReady,
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
-
- collector.SetAPIReadyCondition(tt.reason, tt.message, tt.status)
-
- assert.True(t, collector.hasChanges)
- assert.Contains(t, collector.conditions, tt.expectKey)
-
- condition := collector.conditions[tt.expectKey]
- assert.Equal(t, tt.expectKey, condition.Type)
- assert.Equal(t, tt.reason, condition.Reason)
- assert.Equal(t, tt.message, condition.Message)
- assert.Equal(t, tt.status, condition.Status)
- })
- }
-}
-func TestStatusCollector_SetAPIStatus(t *testing.T) {
- t.Parallel()
-
- tests := []struct {
- name string
- phase mcpv1alpha1.APIPhase
- message string
- endpoint string
- }{
- {
- name: "API status ready",
- phase: mcpv1alpha1.APIPhaseReady,
- message: "API is ready",
- endpoint: "http://test-api.default.svc.cluster.local:8080",
- },
- {
- name: "API status deploying",
- phase: mcpv1alpha1.APIPhaseDeploying,
- message: "API is deploying",
- endpoint: "",
- },
- {
- name: "API status error",
- phase: mcpv1alpha1.APIPhaseError,
- message: "Deployment failed",
- endpoint: "",
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
-
- collector.SetAPIStatus(tt.phase, tt.message, tt.endpoint)
-
- assert.True(t, collector.hasChanges)
- assert.NotNil(t, collector.apiStatus)
-
- apiStatus := collector.apiStatus
- assert.Equal(t, tt.phase, apiStatus.Phase)
- assert.Equal(t, tt.message, apiStatus.Message)
- assert.Equal(t, tt.endpoint, apiStatus.Endpoint)
- })
- }
-}
-
-func TestStatusCollector_SetAPIStatus_ReadySince(t *testing.T) {
- t.Parallel()
-
- t.Run("sets ReadySince when becoming ready", func(t *testing.T) {
- t.Parallel()
- registry := &mcpv1alpha1.MCPRegistry{
- Status: mcpv1alpha1.MCPRegistryStatus{
- APIStatus: &mcpv1alpha1.APIStatus{
- Phase: mcpv1alpha1.APIPhaseDeploying,
- },
- },
- }
- collector := NewStatusManager(registry).(*StatusCollector)
-
- collector.SetAPIStatus(mcpv1alpha1.APIPhaseReady, "API is ready", "http://test.com")
-
- assert.NotNil(t, collector.apiStatus.ReadySince)
- })
-
- t.Run("preserves ReadySince when already ready", func(t *testing.T) {
- t.Parallel()
- readySince := metav1.Now()
- registry := &mcpv1alpha1.MCPRegistry{
- Status: mcpv1alpha1.MCPRegistryStatus{
- APIStatus: &mcpv1alpha1.APIStatus{
- Phase: mcpv1alpha1.APIPhaseReady,
- ReadySince: &readySince,
- },
- },
- }
- collector := NewStatusManager(registry).(*StatusCollector)
-
- collector.SetAPIStatus(mcpv1alpha1.APIPhaseReady, "API is ready", "http://test.com")
-
- assert.Equal(t, &readySince, collector.apiStatus.ReadySince)
- })
-
- t.Run("clears ReadySince when not ready", func(t *testing.T) {
- t.Parallel()
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
-
- collector.SetAPIStatus(mcpv1alpha1.APIPhaseError, "API failed", "")
-
- assert.Nil(t, collector.apiStatus.ReadySince)
- })
-}
-
-func TestStatusCollector_Apply(t *testing.T) {
- t.Parallel()
-
- ctx := context.Background()
-
- // Create scheme and fake client
- scheme := runtime.NewScheme()
- require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
- k8sClient := fake.NewClientBuilder().WithScheme(scheme).Build()
-
- // Create test registry
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "default",
- },
- Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhasePending,
- Message: "Initial state",
- },
- }
-
- // Create registry in fake client
- require.NoError(t, k8sClient.Create(ctx, registry))
-
- t.Run("applies no changes when hasChanges is false", func(t *testing.T) {
- t.Parallel()
- collector := NewStatusManager(registry).(*StatusCollector)
-
- hasUpdates := collector.UpdateStatus(ctx, ®istry.Status)
- assert.False(t, hasUpdates)
- })
-
- t.Run("verifies hasChanges behavior", func(t *testing.T) {
- t.Parallel()
- collector := NewStatusManager(registry).(*StatusCollector)
-
- // Initially no changes
- assert.False(t, collector.hasChanges)
-
- // Setting a value should mark as having changes
- collector.SetPhase(mcpv1alpha1.MCPRegistryPhaseReady)
- assert.True(t, collector.hasChanges)
- })
-
- t.Run("verifies status field collection", func(t *testing.T) {
- t.Parallel()
- collector := NewStatusManager(registry).(*StatusCollector)
-
- // Set various status fields
- collector.SetPhase(mcpv1alpha1.MCPRegistryPhaseReady)
- collector.SetMessage("Registry is ready")
- collector.SetAPIStatus(mcpv1alpha1.APIPhaseReady, "API ready", "http://test-api.default.svc.cluster.local:8080")
- collector.SetAPIReadyCondition("APIReady", "API is ready", metav1.ConditionTrue)
-
- // Verify all fields are collected
- assert.True(t, collector.hasChanges)
- assert.NotNil(t, collector.phase)
- assert.Equal(t, mcpv1alpha1.MCPRegistryPhaseReady, *collector.phase)
- assert.NotNil(t, collector.message)
- assert.Equal(t, "Registry is ready", *collector.message)
- assert.NotNil(t, collector.apiStatus)
- assert.Equal(t, mcpv1alpha1.APIPhaseReady, collector.apiStatus.Phase)
- assert.Equal(t, "http://test-api.default.svc.cluster.local:8080", collector.apiStatus.Endpoint)
- assert.Len(t, collector.conditions, 1)
- assert.Contains(t, collector.conditions, mcpv1alpha1.ConditionAPIReady)
- })
-}
-
-func TestStatusCollector_NoChanges(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
-
- // Initially no changes
- assert.False(t, collector.hasChanges)
-
- // After setting values, should have changes
- collector.SetPhase(mcpv1alpha1.MCPRegistryPhaseReady)
- assert.True(t, collector.hasChanges)
-}
-
-func TestStatusCollector_MultipleConditions(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{}
- collector := NewStatusManager(registry).(*StatusCollector)
-
- // Add condition
- collector.SetAPIReadyCondition("APIReady", "API is ready", metav1.ConditionTrue)
-
- // Should have the condition
- assert.Len(t, collector.conditions, 1)
- assert.Contains(t, collector.conditions, mcpv1alpha1.ConditionAPIReady)
-}
-
-func TestStatusCollector_NoUpdates(t *testing.T) {
- t.Parallel()
-
- ctx := context.Background()
-
- // Create scheme
- scheme := runtime.NewScheme()
- require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
-
- t.Run("error fetching latest registry", func(t *testing.T) {
- t.Parallel()
-
- // Create collector with registry that doesn't exist in client
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "nonexistent-registry",
- Namespace: "default",
- },
- }
-
- collector := newStatusCollector(registry) // No changes
- hasUpdates := collector.UpdateStatus(ctx, ®istry.Status)
- assert.False(t, hasUpdates)
-
- })
-
-}
-
-func TestStatusCollector_InterfaceMethods(t *testing.T) {
- t.Parallel()
- t.Run("API method returns API collector", func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "default",
- },
- }
- collector := newStatusCollector(registry)
-
- apiCollector := collector.API()
- assert.NotNil(t, apiCollector)
- assert.IsType(t, &apiStatusCollector{}, apiCollector)
- })
-
- t.Run("SetOverallStatus delegates correctly", func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "default",
- },
- }
- collector := newStatusCollector(registry)
-
- collector.SetOverallStatus(mcpv1alpha1.MCPRegistryPhaseReady, "Test message")
-
- assert.True(t, collector.hasChanges)
- assert.Equal(t, mcpv1alpha1.MCPRegistryPhaseReady, *collector.phase)
- assert.Equal(t, "Test message", *collector.message)
- })
-}
-
-func TestAPIStatusCollector_Methods(t *testing.T) {
- t.Parallel()
-
- t.Run("SetAPIStatus delegates correctly", func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "default",
- },
- }
- collector := newStatusCollector(registry)
- apiCollector := collector.API()
-
- apiCollector.SetAPIStatus(
- mcpv1alpha1.APIPhaseReady,
- "API is ready",
- "http://example.com",
- )
-
- assert.True(t, collector.hasChanges)
- assert.NotNil(t, collector.apiStatus)
- assert.Equal(t, mcpv1alpha1.APIPhaseReady, collector.apiStatus.Phase)
- assert.Equal(t, "API is ready", collector.apiStatus.Message)
- assert.Equal(t, "http://example.com", collector.apiStatus.Endpoint)
- })
-
- t.Run("SetAPIReadyCondition delegates correctly", func(t *testing.T) {
- t.Parallel()
-
- registry := &mcpv1alpha1.MCPRegistry{
- ObjectMeta: metav1.ObjectMeta{
- Name: "test-registry",
- Namespace: "default",
- },
- }
- collector := newStatusCollector(registry)
- apiCollector := collector.API()
-
- apiCollector.SetAPIReadyCondition("APIReady", "API is ready", metav1.ConditionTrue)
-
- assert.True(t, collector.hasChanges)
- assert.Contains(t, collector.conditions, mcpv1alpha1.ConditionAPIReady)
- condition := collector.conditions[mcpv1alpha1.ConditionAPIReady]
- assert.Equal(t, metav1.ConditionTrue, condition.Status)
- assert.Equal(t, "APIReady", condition.Reason)
- assert.Equal(t, "API is ready", condition.Message)
- })
-}
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/deriver.go b/cmd/thv-operator/pkg/mcpregistrystatus/deriver.go
deleted file mode 100644
index 38aa51fb9e..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/deriver.go
+++ /dev/null
@@ -1,43 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-// Package mcpregistrystatus provides status management for MCPRegistry resources.
-package mcpregistrystatus
-
-import (
- "fmt"
-
- mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
-)
-
-// DefaultStatusDeriver implements the StatusDeriver interface
-type DefaultStatusDeriver struct{}
-
-// NewDefaultStatusDeriver creates a new DefaultStatusDeriver
-func NewDefaultStatusDeriver() StatusDeriver {
- return &DefaultStatusDeriver{}
-}
-
-// DeriveOverallStatus derives the overall MCPRegistry phase and message from component statuses
-func (*DefaultStatusDeriver) DeriveOverallStatus(
- apiStatus *mcpv1alpha1.APIStatus) (mcpv1alpha1.MCPRegistryPhase, string) {
- // Handle API failures
- if apiStatus != nil && apiStatus.Phase == mcpv1alpha1.APIPhaseError {
- return mcpv1alpha1.MCPRegistryPhaseFailed, fmt.Sprintf("API deployment failed: %s", apiStatus.Message)
- }
-
- // If API is not ready, return pending
- if apiStatus != nil && apiStatus.Phase != mcpv1alpha1.APIPhaseReady {
- return mcpv1alpha1.MCPRegistryPhasePending, "API is not ready"
- }
-
- // Check if API is ready
- apiReady := apiStatus != nil && apiStatus.Phase == mcpv1alpha1.APIPhaseReady
-
- if apiReady {
- return mcpv1alpha1.MCPRegistryPhaseReady, "Registry is ready and API is serving requests"
- }
-
- // Default to pending for initial state or unknown combinations
- return mcpv1alpha1.MCPRegistryPhasePending, "Registry initialization in progress"
-}
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/deriver_test.go b/cmd/thv-operator/pkg/mcpregistrystatus/deriver_test.go
deleted file mode 100644
index 8c317fe36c..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/deriver_test.go
+++ /dev/null
@@ -1,18 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-package mcpregistrystatus
-
-import (
- "testing"
-
- "github.com/stretchr/testify/assert"
-)
-
-func TestNewDefaultStatusDeriver(t *testing.T) {
- t.Parallel()
-
- deriver := NewDefaultStatusDeriver()
- assert.NotNil(t, deriver)
- assert.IsType(t, &DefaultStatusDeriver{}, deriver)
-}
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/mocks/mock_collector.go b/cmd/thv-operator/pkg/mcpregistrystatus/mocks/mock_collector.go
deleted file mode 100644
index 96840d04bc..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/mocks/mock_collector.go
+++ /dev/null
@@ -1,197 +0,0 @@
-// Code generated by MockGen. DO NOT EDIT.
-// Source: types.go
-//
-// Generated by this command:
-//
-// mockgen -destination=mocks/mock_collector.go -package=mocks -source=types.go APIStatusCollector,StatusDeriver,StatusManager
-//
-
-// Package mocks is a generated GoMock package.
-package mocks
-
-import (
- context "context"
- reflect "reflect"
-
- v1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
- mcpregistrystatus "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus"
- gomock "go.uber.org/mock/gomock"
- v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-)
-
-// MockAPIStatusCollector is a mock of APIStatusCollector interface.
-type MockAPIStatusCollector struct {
- ctrl *gomock.Controller
- recorder *MockAPIStatusCollectorMockRecorder
- isgomock struct{}
-}
-
-// MockAPIStatusCollectorMockRecorder is the mock recorder for MockAPIStatusCollector.
-type MockAPIStatusCollectorMockRecorder struct {
- mock *MockAPIStatusCollector
-}
-
-// NewMockAPIStatusCollector creates a new mock instance.
-func NewMockAPIStatusCollector(ctrl *gomock.Controller) *MockAPIStatusCollector {
- mock := &MockAPIStatusCollector{ctrl: ctrl}
- mock.recorder = &MockAPIStatusCollectorMockRecorder{mock}
- return mock
-}
-
-// EXPECT returns an object that allows the caller to indicate expected use.
-func (m *MockAPIStatusCollector) EXPECT() *MockAPIStatusCollectorMockRecorder {
- return m.recorder
-}
-
-// SetAPIReadyCondition mocks base method.
-func (m *MockAPIStatusCollector) SetAPIReadyCondition(reason, message string, status v1.ConditionStatus) {
- m.ctrl.T.Helper()
- m.ctrl.Call(m, "SetAPIReadyCondition", reason, message, status)
-}
-
-// SetAPIReadyCondition indicates an expected call of SetAPIReadyCondition.
-func (mr *MockAPIStatusCollectorMockRecorder) SetAPIReadyCondition(reason, message, status any) *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAPIReadyCondition", reflect.TypeOf((*MockAPIStatusCollector)(nil).SetAPIReadyCondition), reason, message, status)
-}
-
-// SetAPIStatus mocks base method.
-func (m *MockAPIStatusCollector) SetAPIStatus(phase v1alpha1.APIPhase, message, endpoint string) {
- m.ctrl.T.Helper()
- m.ctrl.Call(m, "SetAPIStatus", phase, message, endpoint)
-}
-
-// SetAPIStatus indicates an expected call of SetAPIStatus.
-func (mr *MockAPIStatusCollectorMockRecorder) SetAPIStatus(phase, message, endpoint any) *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAPIStatus", reflect.TypeOf((*MockAPIStatusCollector)(nil).SetAPIStatus), phase, message, endpoint)
-}
-
-// Status mocks base method.
-func (m *MockAPIStatusCollector) Status() *v1alpha1.APIStatus {
- m.ctrl.T.Helper()
- ret := m.ctrl.Call(m, "Status")
- ret0, _ := ret[0].(*v1alpha1.APIStatus)
- return ret0
-}
-
-// Status indicates an expected call of Status.
-func (mr *MockAPIStatusCollectorMockRecorder) Status() *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Status", reflect.TypeOf((*MockAPIStatusCollector)(nil).Status))
-}
-
-// MockStatusDeriver is a mock of StatusDeriver interface.
-type MockStatusDeriver struct {
- ctrl *gomock.Controller
- recorder *MockStatusDeriverMockRecorder
- isgomock struct{}
-}
-
-// MockStatusDeriverMockRecorder is the mock recorder for MockStatusDeriver.
-type MockStatusDeriverMockRecorder struct {
- mock *MockStatusDeriver
-}
-
-// NewMockStatusDeriver creates a new mock instance.
-func NewMockStatusDeriver(ctrl *gomock.Controller) *MockStatusDeriver {
- mock := &MockStatusDeriver{ctrl: ctrl}
- mock.recorder = &MockStatusDeriverMockRecorder{mock}
- return mock
-}
-
-// EXPECT returns an object that allows the caller to indicate expected use.
-func (m *MockStatusDeriver) EXPECT() *MockStatusDeriverMockRecorder {
- return m.recorder
-}
-
-// DeriveOverallStatus mocks base method.
-func (m *MockStatusDeriver) DeriveOverallStatus(apiStatus *v1alpha1.APIStatus) (v1alpha1.MCPRegistryPhase, string) {
- m.ctrl.T.Helper()
- ret := m.ctrl.Call(m, "DeriveOverallStatus", apiStatus)
- ret0, _ := ret[0].(v1alpha1.MCPRegistryPhase)
- ret1, _ := ret[1].(string)
- return ret0, ret1
-}
-
-// DeriveOverallStatus indicates an expected call of DeriveOverallStatus.
-func (mr *MockStatusDeriverMockRecorder) DeriveOverallStatus(apiStatus any) *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeriveOverallStatus", reflect.TypeOf((*MockStatusDeriver)(nil).DeriveOverallStatus), apiStatus)
-}
-
-// MockStatusManager is a mock of StatusManager interface.
-type MockStatusManager struct {
- ctrl *gomock.Controller
- recorder *MockStatusManagerMockRecorder
- isgomock struct{}
-}
-
-// MockStatusManagerMockRecorder is the mock recorder for MockStatusManager.
-type MockStatusManagerMockRecorder struct {
- mock *MockStatusManager
-}
-
-// NewMockStatusManager creates a new mock instance.
-func NewMockStatusManager(ctrl *gomock.Controller) *MockStatusManager {
- mock := &MockStatusManager{ctrl: ctrl}
- mock.recorder = &MockStatusManagerMockRecorder{mock}
- return mock
-}
-
-// EXPECT returns an object that allows the caller to indicate expected use.
-func (m *MockStatusManager) EXPECT() *MockStatusManagerMockRecorder {
- return m.recorder
-}
-
-// API mocks base method.
-func (m *MockStatusManager) API() mcpregistrystatus.APIStatusCollector {
- m.ctrl.T.Helper()
- ret := m.ctrl.Call(m, "API")
- ret0, _ := ret[0].(mcpregistrystatus.APIStatusCollector)
- return ret0
-}
-
-// API indicates an expected call of API.
-func (mr *MockStatusManagerMockRecorder) API() *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "API", reflect.TypeOf((*MockStatusManager)(nil).API))
-}
-
-// SetCondition mocks base method.
-func (m *MockStatusManager) SetCondition(conditionType, reason, message string, status v1.ConditionStatus) {
- m.ctrl.T.Helper()
- m.ctrl.Call(m, "SetCondition", conditionType, reason, message, status)
-}
-
-// SetCondition indicates an expected call of SetCondition.
-func (mr *MockStatusManagerMockRecorder) SetCondition(conditionType, reason, message, status any) *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetCondition", reflect.TypeOf((*MockStatusManager)(nil).SetCondition), conditionType, reason, message, status)
-}
-
-// SetOverallStatus mocks base method.
-func (m *MockStatusManager) SetOverallStatus(phase v1alpha1.MCPRegistryPhase, message string) {
- m.ctrl.T.Helper()
- m.ctrl.Call(m, "SetOverallStatus", phase, message)
-}
-
-// SetOverallStatus indicates an expected call of SetOverallStatus.
-func (mr *MockStatusManagerMockRecorder) SetOverallStatus(phase, message any) *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetOverallStatus", reflect.TypeOf((*MockStatusManager)(nil).SetOverallStatus), phase, message)
-}
-
-// UpdateStatus mocks base method.
-func (m *MockStatusManager) UpdateStatus(ctx context.Context, mcpRegistryStatus *v1alpha1.MCPRegistryStatus) bool {
- m.ctrl.T.Helper()
- ret := m.ctrl.Call(m, "UpdateStatus", ctx, mcpRegistryStatus)
- ret0, _ := ret[0].(bool)
- return ret0
-}
-
-// UpdateStatus indicates an expected call of UpdateStatus.
-func (mr *MockStatusManagerMockRecorder) UpdateStatus(ctx, mcpRegistryStatus any) *gomock.Call {
- mr.mock.ctrl.T.Helper()
- return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateStatus", reflect.TypeOf((*MockStatusManager)(nil).UpdateStatus), ctx, mcpRegistryStatus)
-}
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/types.go b/cmd/thv-operator/pkg/mcpregistrystatus/types.go
deleted file mode 100644
index 79474eb558..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/types.go
+++ /dev/null
@@ -1,64 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-// Package mcpregistrystatus provides status management for MCPRegistry resources.
-package mcpregistrystatus
-
-import (
- "context"
-
- metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
- mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
-)
-
-// Error represents a structured error with condition information for operator components
-type Error struct {
- Err error
- Message string
- ConditionType string
- ConditionReason string
-}
-
-func (e *Error) Error() string {
- return e.Message
-}
-
-func (e *Error) Unwrap() error {
- return e.Err
-}
-
-//go:generate mockgen -destination=mocks/mock_collector.go -package=mocks -source=types.go APIStatusCollector,StatusDeriver,StatusManager
-
-// APIStatusCollector handles API-related status updates
-type APIStatusCollector interface {
- // Status returns the API status
- Status() *mcpv1alpha1.APIStatus
-
- // SetAPIStatus sets the detailed API status
- SetAPIStatus(phase mcpv1alpha1.APIPhase, message string, endpoint string)
-
- // SetAPIReadyCondition sets the API ready condition with the specified reason, message, and status
- SetAPIReadyCondition(reason, message string, status metav1.ConditionStatus)
-}
-
-// StatusDeriver handles overall status derivation logic
-type StatusDeriver interface {
- // DeriveOverallStatus derives the overall MCPRegistry phase and message from component statuses
- DeriveOverallStatus(apiStatus *mcpv1alpha1.APIStatus) (mcpv1alpha1.MCPRegistryPhase, string)
-}
-
-// StatusManager orchestrates all status updates and provides access to domain-specific collectors
-type StatusManager interface {
- // API returns the API status collector
- API() APIStatusCollector
-
- // SetOverallStatus sets the overall phase and message explicitly (for special cases)
- SetOverallStatus(phase mcpv1alpha1.MCPRegistryPhase, message string)
-
- // SetCondition sets a general condition
- SetCondition(conditionType, reason, message string, status metav1.ConditionStatus)
-
- // UpdateStatus updates the status of the MCPRegistry if any change happened
- UpdateStatus(ctx context.Context, mcpRegistryStatus *mcpv1alpha1.MCPRegistryStatus) bool
-}
diff --git a/cmd/thv-operator/pkg/mcpregistrystatus/types_test.go b/cmd/thv-operator/pkg/mcpregistrystatus/types_test.go
deleted file mode 100644
index 5b5353512c..0000000000
--- a/cmd/thv-operator/pkg/mcpregistrystatus/types_test.go
+++ /dev/null
@@ -1,171 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
-// SPDX-License-Identifier: Apache-2.0
-
-package mcpregistrystatus
-
-import (
- "errors"
- "testing"
-
- "github.com/stretchr/testify/assert"
-)
-
-func TestError_Error(t *testing.T) {
- t.Parallel()
-
- tests := []struct {
- name string
- err *Error
- expected string
- }{
- {
- name: "normal message",
- err: &Error{
- Err: errors.New("underlying error"),
- Message: "custom error message",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- },
- expected: "custom error message",
- },
- {
- name: "empty message",
- err: &Error{
- Err: errors.New("underlying error"),
- Message: "",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- },
- expected: "",
- },
- {
- name: "message with special characters",
- err: &Error{
- Err: errors.New("underlying error"),
- Message: "Error: 50% of deployments failed\nRetry needed",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- },
- expected: "Error: 50% of deployments failed\nRetry needed",
- },
- {
- name: "nil underlying error",
- err: &Error{
- Err: nil,
- Message: "custom message without underlying error",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- },
- expected: "custom message without underlying error",
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- t.Parallel()
-
- result := tt.err.Error()
- assert.Equal(t, tt.expected, result)
- })
- }
-}
-
-func TestError_Unwrap(t *testing.T) {
- t.Parallel()
-
- tests := []struct {
- name string
- err *Error
- expected error
- }{
- {
- name: "normal underlying error",
- err: &Error{
- Err: errors.New("underlying error"),
- Message: "custom error message",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- },
- expected: errors.New("underlying error"),
- },
- {
- name: "nil underlying error",
- err: &Error{
- Err: nil,
- Message: "custom error message",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- },
- expected: nil,
- },
- }
-
- for _, tt := range tests {
- t.Run(tt.name, func(t *testing.T) {
- t.Parallel()
-
- result := tt.err.Unwrap()
- if tt.expected == nil {
- assert.Nil(t, result)
- } else {
- assert.Equal(t, tt.expected.Error(), result.Error())
- }
- })
- }
-}
-
-func TestError_Interface(t *testing.T) {
- t.Parallel()
-
- // Test that Error implements the error interface
- var _ error = &Error{}
-
- // Test error chaining with errors.Is and errors.As
- originalErr := errors.New("original error")
- wrappedErr := &Error{
- Err: originalErr,
- Message: "wrapped error",
- ConditionType: "TestCondition",
- ConditionReason: "TestReason",
- }
-
- // Test errors.Is
- assert.True(t, errors.Is(wrappedErr, originalErr))
-
- // Test errors.As
- var targetErr *Error
- assert.True(t, errors.As(wrappedErr, &targetErr))
- assert.Equal(t, "wrapped error", targetErr.Message)
- assert.Equal(t, "TestCondition", targetErr.ConditionType)
- assert.Equal(t, "TestReason", targetErr.ConditionReason)
-}
-
-func TestError_Fields(t *testing.T) {
- t.Parallel()
-
- originalErr := errors.New("original error")
- err := &Error{
- Err: originalErr,
- Message: "custom message",
- ConditionType: "SyncFailed",
- ConditionReason: "NetworkError",
- }
-
- // Test that all fields are accessible and correct
- assert.Equal(t, originalErr, err.Err)
- assert.Equal(t, "custom message", err.Message)
- assert.Equal(t, "SyncFailed", err.ConditionType)
- assert.Equal(t, "NetworkError", err.ConditionReason)
-}
-
-func TestError_ZeroValue(t *testing.T) {
- t.Parallel()
-
- // Test zero value behavior
- var err Error
-
- assert.Equal(t, "", err.Error())
- assert.Nil(t, err.Unwrap())
- assert.Equal(t, "", err.ConditionType)
- assert.Equal(t, "", err.ConditionReason)
-}
diff --git a/cmd/thv-operator/pkg/registryapi/manager.go b/cmd/thv-operator/pkg/registryapi/manager.go
index 35d1b753dd..5509c6825d 100644
--- a/cmd/thv-operator/pkg/registryapi/manager.go
+++ b/cmd/thv-operator/pkg/registryapi/manager.go
@@ -15,7 +15,6 @@ import (
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/kubernetes"
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/kubernetes/configmaps"
- "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus"
"github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi/config"
)
@@ -43,7 +42,7 @@ func NewManager(
// checking readiness, and updating the MCPRegistry status with deployment references and endpoint information.
func (m *manager) ReconcileAPIService(
ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry,
-) *mcpregistrystatus.Error {
+) *Error {
ctxLogger := log.FromContext(ctx).WithValues("mcpregistry", mcpRegistry.Name)
ctxLogger.Info("Reconciling API service")
@@ -54,10 +53,9 @@ func (m *manager) ReconcileAPIService(
err := m.ensureRegistryServerConfigConfigMap(ctx, mcpRegistry, configManager)
if err != nil {
ctxLogger.Error(err, "Failed to ensure registry server config config map")
- return &mcpregistrystatus.Error{
+ return &Error{
Err: err,
Message: fmt.Sprintf("Failed to ensure registry server config config map: %v", err),
- ConditionType: mcpv1alpha1.ConditionAPIReady,
ConditionReason: "ConfigMapFailed",
}
}
@@ -66,10 +64,9 @@ func (m *manager) ReconcileAPIService(
err = m.ensureRBACResources(ctx, mcpRegistry)
if err != nil {
ctxLogger.Error(err, "Failed to ensure RBAC resources")
- return &mcpregistrystatus.Error{
+ return &Error{
Err: err,
Message: fmt.Sprintf("Failed to ensure RBAC resources: %v", err),
- ConditionType: mcpv1alpha1.ConditionAPIReady,
ConditionReason: "RBACFailed",
}
}
@@ -79,10 +76,9 @@ func (m *manager) ReconcileAPIService(
err = m.ensurePGPassSecret(ctx, mcpRegistry)
if err != nil {
ctxLogger.Error(err, "Failed to ensure pgpass secret")
- return &mcpregistrystatus.Error{
+ return &Error{
Err: err,
Message: fmt.Sprintf("Failed to ensure pgpass secret: %v", err),
- ConditionType: mcpv1alpha1.ConditionAPIReady,
ConditionReason: "PGPassSecretFailed",
}
}
@@ -92,10 +88,9 @@ func (m *manager) ReconcileAPIService(
deployment, err := m.ensureDeployment(ctx, mcpRegistry, configManager)
if err != nil {
ctxLogger.Error(err, "Failed to ensure deployment")
- return &mcpregistrystatus.Error{
+ return &Error{
Err: err,
Message: fmt.Sprintf("Failed to ensure deployment: %v", err),
- ConditionType: mcpv1alpha1.ConditionAPIReady,
ConditionReason: "DeploymentFailed",
}
}
@@ -104,10 +99,9 @@ func (m *manager) ReconcileAPIService(
_, err = m.ensureService(ctx, mcpRegistry)
if err != nil {
ctxLogger.Error(err, "Failed to ensure service")
- return &mcpregistrystatus.Error{
+ return &Error{
Err: err,
Message: fmt.Sprintf("Failed to ensure service: %v", err),
- ConditionType: mcpv1alpha1.ConditionAPIReady,
ConditionReason: "ServiceFailed",
}
}
@@ -149,6 +143,46 @@ func (m *manager) IsAPIReady(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRe
return m.CheckAPIReadiness(ctx, deployment)
}
+// GetReadyReplicas returns the number of ready replicas for the registry API deployment.
+// Returns 0 if the deployment is not found or an error occurs.
+func (m *manager) GetReadyReplicas(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) int32 {
+ ctxLogger := log.FromContext(ctx).WithValues("mcpregistry", mcpRegistry.Name)
+
+ deploymentName := mcpRegistry.GetAPIResourceName()
+ deployment := &appsv1.Deployment{}
+
+ err := m.client.Get(ctx, client.ObjectKey{
+ Name: deploymentName,
+ Namespace: mcpRegistry.Namespace,
+ }, deployment)
+
+ if err != nil {
+ ctxLogger.V(1).Info("API deployment not found for ready replicas check", "error", err)
+ return 0
+ }
+
+ return deployment.Status.ReadyReplicas
+}
+
+// GetAPIStatus returns the readiness state and ready replica count from a single Deployment fetch.
+func (m *manager) GetAPIStatus(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) (bool, int32) {
+ ctxLogger := log.FromContext(ctx).WithValues("mcpregistry", mcpRegistry.Name)
+
+ deploymentName := mcpRegistry.GetAPIResourceName()
+ deployment := &appsv1.Deployment{}
+
+ err := m.client.Get(ctx, client.ObjectKey{
+ Name: deploymentName,
+ Namespace: mcpRegistry.Namespace,
+ }, deployment)
+ if err != nil {
+ ctxLogger.V(1).Info("API deployment not found", "error", err)
+ return false, 0
+ }
+
+ return m.CheckAPIReadiness(ctx, deployment), deployment.Status.ReadyReplicas
+}
+
// getConfigMapName generates the ConfigMap name for registry storage
// This mirrors the logic in ConfigMapStorageManager to maintain consistency
func getConfigMapName(mcpRegistry *mcpv1alpha1.MCPRegistry) string {
diff --git a/cmd/thv-operator/pkg/registryapi/mocks/mock_manager.go b/cmd/thv-operator/pkg/registryapi/mocks/mock_manager.go
index 18c0ae40b4..e041af5184 100644
--- a/cmd/thv-operator/pkg/registryapi/mocks/mock_manager.go
+++ b/cmd/thv-operator/pkg/registryapi/mocks/mock_manager.go
@@ -14,7 +14,7 @@ import (
reflect "reflect"
v1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
- mcpregistrystatus "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus"
+ registryapi "github.com/stacklok/toolhive/cmd/thv-operator/pkg/registryapi"
gomock "go.uber.org/mock/gomock"
v1 "k8s.io/api/apps/v1"
)
@@ -57,6 +57,35 @@ func (mr *MockManagerMockRecorder) CheckAPIReadiness(ctx, deployment any) *gomoc
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckAPIReadiness", reflect.TypeOf((*MockManager)(nil).CheckAPIReadiness), ctx, deployment)
}
+// GetAPIStatus mocks base method.
+func (m *MockManager) GetAPIStatus(ctx context.Context, mcpRegistry *v1alpha1.MCPRegistry) (bool, int32) {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GetAPIStatus", ctx, mcpRegistry)
+ ret0, _ := ret[0].(bool)
+ ret1, _ := ret[1].(int32)
+ return ret0, ret1
+}
+
+// GetAPIStatus indicates an expected call of GetAPIStatus.
+func (mr *MockManagerMockRecorder) GetAPIStatus(ctx, mcpRegistry any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAPIStatus", reflect.TypeOf((*MockManager)(nil).GetAPIStatus), ctx, mcpRegistry)
+}
+
+// GetReadyReplicas mocks base method.
+func (m *MockManager) GetReadyReplicas(ctx context.Context, mcpRegistry *v1alpha1.MCPRegistry) int32 {
+ m.ctrl.T.Helper()
+ ret := m.ctrl.Call(m, "GetReadyReplicas", ctx, mcpRegistry)
+ ret0, _ := ret[0].(int32)
+ return ret0
+}
+
+// GetReadyReplicas indicates an expected call of GetReadyReplicas.
+func (mr *MockManagerMockRecorder) GetReadyReplicas(ctx, mcpRegistry any) *gomock.Call {
+ mr.mock.ctrl.T.Helper()
+ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetReadyReplicas", reflect.TypeOf((*MockManager)(nil).GetReadyReplicas), ctx, mcpRegistry)
+}
+
// IsAPIReady mocks base method.
func (m *MockManager) IsAPIReady(ctx context.Context, mcpRegistry *v1alpha1.MCPRegistry) bool {
m.ctrl.T.Helper()
@@ -72,10 +101,10 @@ func (mr *MockManagerMockRecorder) IsAPIReady(ctx, mcpRegistry any) *gomock.Call
}
// ReconcileAPIService mocks base method.
-func (m *MockManager) ReconcileAPIService(ctx context.Context, mcpRegistry *v1alpha1.MCPRegistry) *mcpregistrystatus.Error {
+func (m *MockManager) ReconcileAPIService(ctx context.Context, mcpRegistry *v1alpha1.MCPRegistry) *registryapi.Error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ReconcileAPIService", ctx, mcpRegistry)
- ret0, _ := ret[0].(*mcpregistrystatus.Error)
+ ret0, _ := ret[0].(*registryapi.Error)
return ret0
}
diff --git a/cmd/thv-operator/pkg/registryapi/types.go b/cmd/thv-operator/pkg/registryapi/types.go
index 5812966c21..fb8980c0f8 100644
--- a/cmd/thv-operator/pkg/registryapi/types.go
+++ b/cmd/thv-operator/pkg/registryapi/types.go
@@ -9,7 +9,6 @@ import (
appsv1 "k8s.io/api/apps/v1"
mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
- "github.com/stacklok/toolhive/cmd/thv-operator/pkg/mcpregistrystatus"
)
const (
@@ -90,18 +89,39 @@ const (
gitAuthSecretsBasePath = "/secrets"
)
+// Error represents a structured error with condition information for operator components
+type Error struct {
+ Err error
+ Message string
+ ConditionReason string
+}
+
+func (e *Error) Error() string {
+ return e.Message
+}
+
+func (e *Error) Unwrap() error {
+ return e.Err
+}
+
//go:generate mockgen -destination=mocks/mock_manager.go -package=mocks -source=types.go Manager
// Manager handles registry API deployment operations
type Manager interface {
// ReconcileAPIService orchestrates the deployment, service creation, and readiness checking for the registry API
- ReconcileAPIService(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) *mcpregistrystatus.Error
+ ReconcileAPIService(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) *Error
// CheckAPIReadiness verifies that the deployed registry-API Deployment is ready
CheckAPIReadiness(ctx context.Context, deployment *appsv1.Deployment) bool
// IsAPIReady checks if the registry API deployment is ready and serving requests
IsAPIReady(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) bool
+
+ // GetReadyReplicas returns the number of ready replicas for the registry API deployment
+ GetReadyReplicas(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) int32
+
+ // GetAPIStatus returns the readiness state and ready replica count from a single Deployment fetch
+ GetAPIStatus(ctx context.Context, mcpRegistry *mcpv1alpha1.MCPRegistry) (ready bool, readyReplicas int32)
}
// GetServiceAccountName returns the service account name for a given MCPRegistry.
diff --git a/cmd/thv-operator/pkg/validation/image_validation.go b/cmd/thv-operator/pkg/validation/image_validation.go
index b6a2027ed5..30725a33fe 100644
--- a/cmd/thv-operator/pkg/validation/image_validation.go
+++ b/cmd/thv-operator/pkg/validation/image_validation.go
@@ -198,7 +198,7 @@ func (v *RegistryEnforcingValidator) checkImageInRegistry(
image string,
) (bool, error) {
// Only check registries that are ready
- if mcpRegistry.Status.Phase != mcpv1alpha1.MCPRegistryPhaseReady {
+ if mcpRegistry.Status.Phase != mcpv1alpha1.MCPRegistryPhaseRunning {
return false, nil
}
diff --git a/cmd/thv-operator/pkg/validation/image_validation_test.go b/cmd/thv-operator/pkg/validation/image_validation_test.go
index 76ede4408d..bb54451b50 100644
--- a/cmd/thv-operator/pkg/validation/image_validation_test.go
+++ b/cmd/thv-operator/pkg/validation/image_validation_test.go
@@ -186,7 +186,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: false,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -206,7 +206,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -237,7 +237,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -268,7 +268,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -301,7 +301,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -356,7 +356,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
&mcpv1alpha1.MCPRegistry{
@@ -368,7 +368,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: false,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -410,7 +410,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
&mcpv1alpha1.MCPRegistry{
@@ -422,7 +422,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: false,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -455,7 +455,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
&mcpv1alpha1.MCPRegistry{
@@ -467,7 +467,7 @@ func TestRegistryEnforcingValidator_ValidateImage(t *testing.T) {
EnforceServers: false,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -588,7 +588,7 @@ func TestCheckImageInRegistry(t *testing.T) {
Namespace: "test-namespace",
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
image: "docker.io/toolhive/test:v1.0.0",
@@ -602,7 +602,7 @@ func TestCheckImageInRegistry(t *testing.T) {
Namespace: "test-namespace",
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
configMap: &corev1.ConfigMap{
@@ -625,7 +625,7 @@ func TestCheckImageInRegistry(t *testing.T) {
Namespace: "test-namespace",
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
configMap: &corev1.ConfigMap{
@@ -648,7 +648,7 @@ func TestCheckImageInRegistry(t *testing.T) {
Namespace: "test-namespace",
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
configMap: &corev1.ConfigMap{
@@ -830,7 +830,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
&mcpv1alpha1.MCPRegistry{
@@ -842,7 +842,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -878,7 +878,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: false,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -903,7 +903,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -941,7 +941,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -968,7 +968,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
&mcpv1alpha1.MCPRegistry{
@@ -980,7 +980,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
@@ -1023,7 +1023,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
&mcpv1alpha1.MCPRegistry{
@@ -1035,7 +1035,7 @@ func TestRegistryEnforcingValidator_ValidateImageWithRegistryLabel(t *testing.T)
EnforceServers: true,
},
Status: mcpv1alpha1.MCPRegistryStatus{
- Phase: mcpv1alpha1.MCPRegistryPhaseReady,
+ Phase: mcpv1alpha1.MCPRegistryPhaseRunning,
},
},
},
diff --git a/cmd/thv-operator/test-integration/mcp-registry/pvc_source_test.go b/cmd/thv-operator/test-integration/mcp-registry/pvc_source_test.go
index daba227040..0ec5bed3fb 100644
--- a/cmd/thv-operator/test-integration/mcp-registry/pvc_source_test.go
+++ b/cmd/thv-operator/test-integration/mcp-registry/pvc_source_test.go
@@ -82,7 +82,7 @@ var _ = Describe("MCPRegistry PVC Source", Label("k8s", "registry", "pvc"), func
By("Waiting for registry to initialize")
statusHelper.WaitForPhaseAny(registry.Name, []mcpv1alpha1.MCPRegistryPhase{
- mcpv1alpha1.MCPRegistryPhaseReady,
+ mcpv1alpha1.MCPRegistryPhaseRunning,
mcpv1alpha1.MCPRegistryPhasePending,
}, MediumTimeout)
@@ -189,7 +189,7 @@ var _ = Describe("MCPRegistry PVC Source", Label("k8s", "registry", "pvc"), func
By("Waiting for registry to initialize")
statusHelper.WaitForPhaseAny(registry.Name, []mcpv1alpha1.MCPRegistryPhase{
- mcpv1alpha1.MCPRegistryPhaseReady,
+ mcpv1alpha1.MCPRegistryPhaseRunning,
mcpv1alpha1.MCPRegistryPhasePending,
}, MediumTimeout)
diff --git a/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go
index b610b014e0..f1b357b43f 100644
--- a/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go
+++ b/cmd/thv-operator/test-integration/mcp-registry/registry_helpers.go
@@ -396,8 +396,7 @@ func (h *MCPRegistryTestHelper) WaitForRegistryInitialization(registryName strin
ginkgo.By("waiting for controller to process and verify initial status")
statusHelper.WaitForPhaseAny(registryName, []mcpv1alpha1.MCPRegistryPhase{
mcpv1alpha1.MCPRegistryPhasePending,
- mcpv1alpha1.MCPRegistryPhaseReady,
- mcpv1alpha1.MCPRegistryPhaseSyncing,
+ mcpv1alpha1.MCPRegistryPhaseRunning,
}, MediumTimeout)
}
diff --git a/cmd/thv-operator/test-integration/mcp-registry/registry_lifecycle_test.go b/cmd/thv-operator/test-integration/mcp-registry/registry_lifecycle_test.go
index d295723752..1d22980a0b 100644
--- a/cmd/thv-operator/test-integration/mcp-registry/registry_lifecycle_test.go
+++ b/cmd/thv-operator/test-integration/mcp-registry/registry_lifecycle_test.go
@@ -127,12 +127,7 @@ var _ = Describe("MCPRegistry Lifecycle Management", Label("k8s", "registry"), f
Create(registryHelper)
// Wait for registry to be ready
- statusHelper.WaitForPhaseAny(registry.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseReady, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
-
- // Store initial storage reference for cleanup verification
- status, err := registryHelper.GetRegistryStatus(registry.Name)
- Expect(err).NotTo(HaveOccurred())
- initialStorageRef := status.StorageRef
+ statusHelper.WaitForPhaseAny(registry.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseRunning, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
// Delete the registry
Expect(registryHelper.DeleteRegistry(registry.Name)).To(Succeed())
@@ -140,15 +135,6 @@ var _ = Describe("MCPRegistry Lifecycle Management", Label("k8s", "registry"), f
// Verify graceful deletion process
statusHelper.WaitForPhase(registry.Name, mcpv1alpha1.MCPRegistryPhaseTerminating, QuickTimeout)
- // Verify cleanup of associated resources (if any storage was created)
- if initialStorageRef != nil && initialStorageRef.ConfigMapRef != nil {
- timingHelper.WaitForControllerReconciliation(func() interface{} {
- _, err := configMapHelper.GetConfigMap(initialStorageRef.ConfigMapRef.Name)
- // Storage ConfigMap should be cleaned up or marked for deletion
- return errors.IsNotFound(err)
- }).Should(BeTrue())
- }
-
// Verify complete deletion
timingHelper.WaitForControllerReconciliation(func() interface{} {
_, err := registryHelper.GetRegistry(registry.Name)
@@ -197,8 +183,8 @@ var _ = Describe("MCPRegistry Lifecycle Management", Label("k8s", "registry"), f
Create(registryHelper)
// Both should become ready independently
- statusHelper.WaitForPhaseAny(registry1.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseReady, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
- statusHelper.WaitForPhaseAny(registry2.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseReady, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
+ statusHelper.WaitForPhaseAny(registry1.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseRunning, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
+ statusHelper.WaitForPhaseAny(registry2.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseRunning, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
// Verify they operate independently
Expect(registry1.Spec.Registries[0].SyncPolicy.Interval).To(Equal("1h"))
@@ -223,7 +209,7 @@ var _ = Describe("MCPRegistry Lifecycle Management", Label("k8s", "registry"), f
Create(registryHelper)
// Both should become ready
- statusHelper.WaitForPhaseAny(registry1.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseReady, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
+ statusHelper.WaitForPhaseAny(registry1.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseRunning, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
By("verifying registry servers config ConfigMap is created")
serverConfigMap1 := testHelpers.waitForAndGetServerConfigMap(registry1.Name)
@@ -277,7 +263,7 @@ var _ = Describe("MCPRegistry Lifecycle Management", Label("k8s", "registry"), f
Expect(errors.IsAlreadyExists(err)).To(BeTrue())
// Original registry should still be functional
- statusHelper.WaitForPhaseAny(registry1.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseReady, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
+ statusHelper.WaitForPhaseAny(registry1.Name, []mcpv1alpha1.MCPRegistryPhase{mcpv1alpha1.MCPRegistryPhaseRunning, mcpv1alpha1.MCPRegistryPhasePending}, MediumTimeout)
})
})
})
diff --git a/cmd/thv-operator/test-integration/mcp-registry/registry_server_rbac_test.go b/cmd/thv-operator/test-integration/mcp-registry/registry_server_rbac_test.go
index e520cde2e4..c316fae27d 100644
--- a/cmd/thv-operator/test-integration/mcp-registry/registry_server_rbac_test.go
+++ b/cmd/thv-operator/test-integration/mcp-registry/registry_server_rbac_test.go
@@ -50,7 +50,7 @@ var _ = Describe("MCPRegistry RBAC Resources", Label("k8s", "registry", "rbac"),
// Wait for registry to be reconciled
statusHelper.WaitForPhaseAny(registry.Name, []mcpv1alpha1.MCPRegistryPhase{
- mcpv1alpha1.MCPRegistryPhaseReady,
+ mcpv1alpha1.MCPRegistryPhaseRunning,
mcpv1alpha1.MCPRegistryPhasePending,
}, MediumTimeout)
diff --git a/cmd/thv-operator/test-integration/mcp-registry/registryserver_config_test.go b/cmd/thv-operator/test-integration/mcp-registry/registryserver_config_test.go
index 9ac5019b82..bcc2eb9a3d 100644
--- a/cmd/thv-operator/test-integration/mcp-registry/registryserver_config_test.go
+++ b/cmd/thv-operator/test-integration/mcp-registry/registryserver_config_test.go
@@ -137,20 +137,15 @@ var _ = Describe("MCPRegistry Server Config (Consolidated)", Label("k8s", "regis
// but verify that sync is complete and API deployment is in progress
Expect(registry.Status.Phase).To(BeElementOf(
mcpv1alpha1.MCPRegistryPhasePending, // API deployment in progress
- mcpv1alpha1.MCPRegistryPhaseReady, // If somehow API becomes ready
+ mcpv1alpha1.MCPRegistryPhaseRunning, // If somehow API becomes ready
))
// Verify ObservedGeneration is set after reconciliation
Expect(registry.Status.ObservedGeneration).To(Equal(registry.Generation))
- // Verify API status exists and shows deployment
- Expect(registry.Status.APIStatus).NotTo(BeNil())
- Expect(registry.Status.APIStatus.Phase).To(BeElementOf(
- mcpv1alpha1.APIPhaseDeploying, // Deployment created but not ready
- mcpv1alpha1.APIPhaseReady, // If somehow becomes ready
- ))
- if registry.Status.APIStatus.Phase == mcpv1alpha1.APIPhaseReady {
- Expect(registry.Status.APIStatus.Endpoint).To(Equal(fmt.Sprintf("http://%s.%s.svc.cluster.local:8080", apiResourceName, testNamespace)))
+ // Verify phase and URL
+ if registry.Status.Phase == mcpv1alpha1.MCPRegistryPhaseRunning {
+ Expect(registry.Status.URL).To(Equal(fmt.Sprintf("http://%s.%s.svc.cluster.local:8080", apiResourceName, testNamespace)))
}
By("verifying registry server config ConfigMap is created")
@@ -1036,17 +1031,17 @@ func (h *serverConfigTestHelpers) verifyPodTemplateValidCondition(registryName s
if err != nil {
return false
}
- condition := meta.FindStatusCondition(updatedRegistry.Status.Conditions, mcpv1alpha1.ConditionRegistryPodTemplateValid)
+ condition := meta.FindStatusCondition(updatedRegistry.Status.Conditions, mcpv1alpha1.ConditionPodTemplateValid)
if condition == nil {
return false
}
if expectedValid {
return condition.Status == metav1.ConditionTrue &&
- condition.Reason == mcpv1alpha1.ConditionReasonRegistryPodTemplateValid
+ condition.Reason == mcpv1alpha1.ConditionReasonPodTemplateValid
}
return condition.Status == metav1.ConditionFalse &&
- condition.Reason == mcpv1alpha1.ConditionReasonRegistryPodTemplateInvalid
+ condition.Reason == mcpv1alpha1.ConditionReasonPodTemplateInvalid
}, MediumTimeout, DefaultPollingInterval).Should(BeTrue(),
fmt.Sprintf("PodTemplateValid condition should be %v", expectedValid))
}
diff --git a/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go b/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go
index 2400301fda..250b7e1a1a 100644
--- a/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go
+++ b/cmd/thv-operator/test-integration/mcp-registry/status_helpers.go
@@ -77,42 +77,6 @@ func (h *StatusTestHelper) WaitForConditionReason(registryName, conditionType, e
"MCPRegistry %s condition %s should have reason %s", registryName, conditionType, expectedReason)
}
-// WaitForServerCount waits for the registry to report a specific server count
-func (h *StatusTestHelper) WaitForServerCount(registryName string, expectedCount int32, timeout time.Duration) {
- gomega.Eventually(func() int32 {
- status, err := h.registryHelper.GetRegistryStatus(registryName)
- if err != nil {
- return -1
- }
- return status.SyncStatus.ServerCount
- }, timeout, time.Second).Should(gomega.Equal(expectedCount),
- "MCPRegistry %s should have server count %d", registryName, expectedCount)
-}
-
-// WaitForLastSyncTime waits for the registry to update its last sync time
-func (h *StatusTestHelper) WaitForLastSyncTime(registryName string, afterTime time.Time, timeout time.Duration) {
- gomega.Eventually(func() bool {
- status, err := h.registryHelper.GetRegistryStatus(registryName)
- if err != nil || status.SyncStatus.LastSyncTime == nil {
- return false
- }
- return status.SyncStatus.LastSyncTime.After(afterTime)
- }, timeout, time.Second).Should(gomega.BeTrue(),
- "MCPRegistry %s should update last sync time after %s", registryName, afterTime)
-}
-
-// WaitForLastSyncHash waits for the registry to have a non-empty last sync hash
-func (h *StatusTestHelper) WaitForLastSyncHash(registryName string, timeout time.Duration) {
- gomega.Eventually(func() string {
- status, err := h.registryHelper.GetRegistryStatus(registryName)
- if err != nil {
- return ""
- }
- return status.SyncStatus.LastSyncHash
- }, timeout, time.Second).ShouldNot(gomega.BeEmpty(),
- "MCPRegistry %s should have a last sync hash", registryName)
-}
-
// WaitForSyncCompletion waits for a sync operation to complete (either success or failure)
func (h *StatusTestHelper) WaitForSyncCompletion(registryName string, timeout time.Duration) {
gomega.Eventually(func() bool {
@@ -123,20 +87,8 @@ func (h *StatusTestHelper) WaitForSyncCompletion(registryName string, timeout ti
// Check if sync is no longer in progress
phase := registry.Status.Phase
- return phase == mcpv1alpha1.MCPRegistryPhaseReady ||
+ return phase == mcpv1alpha1.MCPRegistryPhaseRunning ||
phase == mcpv1alpha1.MCPRegistryPhaseFailed
}, timeout, time.Second).Should(gomega.BeTrue(),
"MCPRegistry %s sync operation should complete", registryName)
}
-
-// WaitForManualSyncProcessed waits for a manual sync annotation to be processed
-func (h *StatusTestHelper) WaitForManualSyncProcessed(registryName, triggerValue string, timeout time.Duration) {
- gomega.Eventually(func() string {
- status, err := h.registryHelper.GetRegistryStatus(registryName)
- if err != nil {
- return ""
- }
- return status.LastManualSyncTrigger
- }, timeout, time.Second).Should(gomega.Equal(triggerValue),
- "MCPRegistry %s should process manual sync trigger %s", registryName, triggerValue)
-}
diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpregistries.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpregistries.yaml
index 90552fb5fa..06cadf9cc3 100644
--- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpregistries.yaml
+++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpregistries.yaml
@@ -21,20 +21,17 @@ spec:
versions:
- additionalPrinterColumns:
- jsonPath: .status.phase
- name: Phase
+ name: Status
type: string
- - jsonPath: .status.syncStatus.phase
- name: Sync
+ - jsonPath: .status.conditions[?(@.type=='Ready')].status
+ name: Ready
type: string
- - jsonPath: .status.apiStatus.phase
- name: API
- type: string
- - jsonPath: .status.syncStatus.serverCount
- name: Servers
+ - jsonPath: .status.readyReplicas
+ name: Replicas
type: integer
- - jsonPath: .status.syncStatus.lastSyncTime
- name: Last Sync
- type: date
+ - jsonPath: .status.url
+ name: URL
+ type: string
- jsonPath: .metadata.creationTimestamp
name: Age
type: date
@@ -674,40 +671,6 @@ spec:
status:
description: MCPRegistryStatus defines the observed state of MCPRegistry
properties:
- apiStatus:
- description: APIStatus provides detailed information about the API
- service
- properties:
- endpoint:
- description: Endpoint is the URL where the API is accessible
- type: string
- message:
- description: Message provides additional information about the
- API status
- type: string
- phase:
- allOf:
- - enum:
- - NotStarted
- - Deploying
- - Ready
- - Unhealthy
- - Error
- - enum:
- - NotStarted
- - Deploying
- - Ready
- - Unhealthy
- - Error
- description: Phase represents the current API service phase
- type: string
- readySince:
- description: ReadySince is the timestamp when the API became ready
- format: date-time
- type: string
- required:
- - phase
- type: object
conditions:
description: Conditions represent the latest available observations
of the MCPRegistry's state
@@ -769,15 +732,6 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
- lastAppliedFilterHash:
- description: LastAppliedFilterHash is the hash of the last applied
- filter
- type: string
- lastManualSyncTrigger:
- description: |-
- LastManualSyncTrigger tracks the last processed manual sync annotation value
- Used to detect new manual sync requests via toolhive.stacklok.dev/sync-trigger annotation
- type: string
message:
description: Message provides additional information about the current
phase
@@ -788,91 +742,20 @@ spec:
format: int64
type: integer
phase:
- description: |-
- Phase represents the current overall phase of the MCPRegistry
- Derived from sync and API status
+ description: Phase represents the current overall phase of the MCPRegistry
enum:
- Pending
- - Ready
+ - Running
- Failed
- - Syncing
- Terminating
type: string
- storageRef:
- description: StorageRef is a reference to the internal storage location
- properties:
- configMapRef:
- description: |-
- ConfigMapRef is a reference to a ConfigMap storage
- Only used when Type is "configmap"
- properties:
- name:
- default: ""
- description: |-
- Name of the referent.
- This field is effectively required, but due to backwards compatibility is
- allowed to be empty. Instances of this type with an empty value here are
- almost certainly wrong.
- More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
- type: string
- type: object
- x-kubernetes-map-type: atomic
- type:
- description: Type is the storage type (configmap)
- enum:
- - configmap
- type: string
- required:
- - type
- type: object
- syncStatus:
- description: SyncStatus provides detailed information about data synchronization
- properties:
- attemptCount:
- description: AttemptCount is the number of sync attempts since
- last success
- format: int32
- minimum: 0
- type: integer
- lastAttempt:
- description: LastAttempt is the timestamp of the last sync attempt
- format: date-time
- type: string
- lastSyncHash:
- description: |-
- LastSyncHash is the hash of the last successfully synced data
- Used to detect changes in source data
- type: string
- lastSyncTime:
- description: LastSyncTime is the timestamp of the last successful
- sync
- format: date-time
- type: string
- message:
- description: Message provides additional information about the
- sync status
- type: string
- phase:
- allOf:
- - enum:
- - Syncing
- - Complete
- - Failed
- - enum:
- - Syncing
- - Complete
- - Failed
- description: Phase represents the current synchronization phase
- type: string
- serverCount:
- description: ServerCount is the total number of servers in the
- registry
- format: int32
- minimum: 0
- type: integer
- required:
- - phase
- type: object
+ readyReplicas:
+ description: ReadyReplicas is the number of ready registry API replicas
+ format: int32
+ type: integer
+ url:
+ description: URL is the URL where the registry API can be accessed
+ type: string
type: object
type: object
x-kubernetes-validations:
diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpregistries.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpregistries.yaml
index 1321f5cd96..f53c579eac 100644
--- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpregistries.yaml
+++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpregistries.yaml
@@ -24,20 +24,17 @@ spec:
versions:
- additionalPrinterColumns:
- jsonPath: .status.phase
- name: Phase
+ name: Status
type: string
- - jsonPath: .status.syncStatus.phase
- name: Sync
+ - jsonPath: .status.conditions[?(@.type=='Ready')].status
+ name: Ready
type: string
- - jsonPath: .status.apiStatus.phase
- name: API
- type: string
- - jsonPath: .status.syncStatus.serverCount
- name: Servers
+ - jsonPath: .status.readyReplicas
+ name: Replicas
type: integer
- - jsonPath: .status.syncStatus.lastSyncTime
- name: Last Sync
- type: date
+ - jsonPath: .status.url
+ name: URL
+ type: string
- jsonPath: .metadata.creationTimestamp
name: Age
type: date
@@ -677,40 +674,6 @@ spec:
status:
description: MCPRegistryStatus defines the observed state of MCPRegistry
properties:
- apiStatus:
- description: APIStatus provides detailed information about the API
- service
- properties:
- endpoint:
- description: Endpoint is the URL where the API is accessible
- type: string
- message:
- description: Message provides additional information about the
- API status
- type: string
- phase:
- allOf:
- - enum:
- - NotStarted
- - Deploying
- - Ready
- - Unhealthy
- - Error
- - enum:
- - NotStarted
- - Deploying
- - Ready
- - Unhealthy
- - Error
- description: Phase represents the current API service phase
- type: string
- readySince:
- description: ReadySince is the timestamp when the API became ready
- format: date-time
- type: string
- required:
- - phase
- type: object
conditions:
description: Conditions represent the latest available observations
of the MCPRegistry's state
@@ -772,15 +735,6 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
- lastAppliedFilterHash:
- description: LastAppliedFilterHash is the hash of the last applied
- filter
- type: string
- lastManualSyncTrigger:
- description: |-
- LastManualSyncTrigger tracks the last processed manual sync annotation value
- Used to detect new manual sync requests via toolhive.stacklok.dev/sync-trigger annotation
- type: string
message:
description: Message provides additional information about the current
phase
@@ -791,91 +745,20 @@ spec:
format: int64
type: integer
phase:
- description: |-
- Phase represents the current overall phase of the MCPRegistry
- Derived from sync and API status
+ description: Phase represents the current overall phase of the MCPRegistry
enum:
- Pending
- - Ready
+ - Running
- Failed
- - Syncing
- Terminating
type: string
- storageRef:
- description: StorageRef is a reference to the internal storage location
- properties:
- configMapRef:
- description: |-
- ConfigMapRef is a reference to a ConfigMap storage
- Only used when Type is "configmap"
- properties:
- name:
- default: ""
- description: |-
- Name of the referent.
- This field is effectively required, but due to backwards compatibility is
- allowed to be empty. Instances of this type with an empty value here are
- almost certainly wrong.
- More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
- type: string
- type: object
- x-kubernetes-map-type: atomic
- type:
- description: Type is the storage type (configmap)
- enum:
- - configmap
- type: string
- required:
- - type
- type: object
- syncStatus:
- description: SyncStatus provides detailed information about data synchronization
- properties:
- attemptCount:
- description: AttemptCount is the number of sync attempts since
- last success
- format: int32
- minimum: 0
- type: integer
- lastAttempt:
- description: LastAttempt is the timestamp of the last sync attempt
- format: date-time
- type: string
- lastSyncHash:
- description: |-
- LastSyncHash is the hash of the last successfully synced data
- Used to detect changes in source data
- type: string
- lastSyncTime:
- description: LastSyncTime is the timestamp of the last successful
- sync
- format: date-time
- type: string
- message:
- description: Message provides additional information about the
- sync status
- type: string
- phase:
- allOf:
- - enum:
- - Syncing
- - Complete
- - Failed
- - enum:
- - Syncing
- - Complete
- - Failed
- description: Phase represents the current synchronization phase
- type: string
- serverCount:
- description: ServerCount is the total number of servers in the
- registry
- format: int32
- minimum: 0
- type: integer
- required:
- - phase
- type: object
+ readyReplicas:
+ description: ReadyReplicas is the number of ready registry API replicas
+ format: int32
+ type: integer
+ url:
+ description: URL is the URL where the registry API can be accessed
+ type: string
type: object
type: object
x-kubernetes-validations:
diff --git a/docs/arch/06-registry-system.md b/docs/arch/06-registry-system.md
index dda217a82c..db6e8bd676 100644
--- a/docs/arch/06-registry-system.md
+++ b/docs/arch/06-registry-system.md
@@ -633,25 +633,25 @@ curl http://localhost:8080/api/v1/registry
**Status fields:**
```yaml
status:
- phase: Ready
- syncStatus:
- phase: Complete
- message: "Successfully synced registry"
- lastSyncTime: "2025-10-13T12:00:00Z"
- lastSyncHash: "abc123def456"
- apiStatus:
- phase: Ready
- endpoint: "http://company-registry-api.default.svc.cluster.local:8080"
+ phase: Running
+ message: "Registry API is ready and serving requests"
+ url: "http://company-registry-api.default.svc.cluster.local:8080"
+ readyReplicas: 1
+ observedGeneration: 1
+ conditions:
+ - type: Ready
+ status: "True"
+ reason: Ready
+ message: "Registry API is ready and serving requests"
```
**Phases:**
-- `Pending` - Initial state
-- `Syncing` - Fetching registry data
-- `Ready` - Registry available
-- `Failed` - Sync failed
+- `Pending` - Initial state, deployment not ready yet
+- `Running` - Registry API is ready and serving requests
+- `Failed` - Deployment or reconciliation failed
- `Terminating` - Registry being deleted
-**Implementation**: `cmd/thv-operator/pkg/mcpregistrystatus/`
+**Implementation**: `cmd/thv-operator/controllers/mcpregistry_controller.go`
### Storage
diff --git a/docs/arch/09-operator-architecture.md b/docs/arch/09-operator-architecture.md
index 5f4d461a01..34f26f8d9f 100644
--- a/docs/arch/09-operator-architecture.md
+++ b/docs/arch/09-operator-architecture.md
@@ -482,11 +482,11 @@ spec:
### Status Management
-**Pattern**: Batched updates via StatusCollector
+**Pattern**: Direct status update matching MCPServer workload pattern
-**Why**: Prevents race conditions, reduces API calls
+**Why**: Simple Phase + Ready condition + ReadyReplicas + URL, enables `kubectl wait --for=condition=Ready`
-**Implementation**: `cmd/thv-operator/pkg/mcpregistrystatus/`
+**Implementation**: `cmd/thv-operator/controllers/mcpregistry_controller.go`
## MCPRegistry Controller
@@ -527,7 +527,7 @@ graph TB
**Storage Manager**: `cmd/thv-operator/pkg/sources/storage_manager.go`
- Creates ConfigMap with key `registry.json` containing full registry data
-- Sync metadata (timestamp, hash, attempt count) stored in MCPRegistry CRD status field `SyncStatus`
+- Sync operations are handled by the registry server itself
**Interface**: `cmd/thv-operator/pkg/sources/types.go`
@@ -546,7 +546,7 @@ data:
{ full registry data }
```
-Sync metadata (timestamp, hash, attempt count) is stored in the MCPRegistry CRD status field `SyncStatus`, not in the ConfigMap.
+Sync operations are handled by the registry server, not the operator.
### Sync Policy
diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md
index 7e37163e3d..8240817a8c 100644
--- a/docs/operator/crd-api.md
+++ b/docs/operator/crd-api.md
@@ -755,27 +755,6 @@ _Appears in:_
-#### api.v1alpha1.APIPhase
-
-_Underlying type:_ _string_
-
-APIPhase represents the API service state
-
-_Validation:_
-- Enum: [NotStarted Deploying Ready Unhealthy Error]
-
-_Appears in:_
-- [api.v1alpha1.APIStatus](#apiv1alpha1apistatus)
-
-| Field | Description |
-| --- | --- |
-| `NotStarted` | APIPhaseNotStarted means API deployment has not been created
|
-| `Deploying` | APIPhaseDeploying means API is being deployed
|
-| `Ready` | APIPhaseReady means API is ready to serve requests
|
-| `Unhealthy` | APIPhaseUnhealthy means API is deployed but not healthy
|
-| `Error` | APIPhaseError means API deployment failed
|
-
-
#### api.v1alpha1.APISource
@@ -794,25 +773,6 @@ _Appears in:_
| `endpoint` _string_ | Endpoint is the base API URL (without path)
The controller will append the appropriate paths:
Phase 1 (ToolHive API):
- /v0/servers - List all servers (single response, no pagination)
- /v0/servers/\{name\} - Get specific server (future)
- /v0/info - Get registry metadata (future)
Example: "http://my-registry-api.default.svc.cluster.local/api" | | MinLength: 1
Pattern: `^https?://.*`
Required: \{\}
|
-#### api.v1alpha1.APIStatus
-
-
-
-APIStatus provides detailed information about the API service
-
-
-
-_Appears in:_
-- [api.v1alpha1.MCPRegistryStatus](#apiv1alpha1mcpregistrystatus)
-
-| Field | Description | Default | Validation |
-| --- | --- | --- | --- |
-| `phase` _[api.v1alpha1.APIPhase](#apiv1alpha1apiphase)_ | Phase represents the current API service phase | | Enum: [NotStarted Deploying Ready Unhealthy Error]
|
-| `message` _string_ | Message provides additional information about the API status | | Optional: \{\}
|
-| `endpoint` _string_ | Endpoint is the URL where the API is accessible | | Optional: \{\}
|
-| `readySince` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#time-v1-meta)_ | ReadySince is the timestamp when the API became ready | | Optional: \{\}
|
-
-
#### api.v1alpha1.AWSStsConfig
@@ -1962,7 +1922,7 @@ _Underlying type:_ _string_
MCPRegistryPhase represents the phase of the MCPRegistry
_Validation:_
-- Enum: [Pending Ready Failed Syncing Terminating]
+- Enum: [Pending Running Failed Terminating]
_Appears in:_
- [api.v1alpha1.MCPRegistryStatus](#apiv1alpha1mcpregistrystatus)
@@ -1970,9 +1930,8 @@ _Appears in:_
| Field | Description |
| --- | --- |
| `Pending` | MCPRegistryPhasePending means the MCPRegistry is being initialized
|
-| `Ready` | MCPRegistryPhaseReady means the MCPRegistry is ready and operational
|
+| `Running` | MCPRegistryPhaseRunning means the MCPRegistry is running and operational
|
| `Failed` | MCPRegistryPhaseFailed means the MCPRegistry has failed
|
-| `Syncing` | MCPRegistryPhaseSyncing means the MCPRegistry is currently syncing data
|
| `Terminating` | MCPRegistryPhaseTerminating means the MCPRegistry is being deleted
|
@@ -2010,15 +1969,12 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
+| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPRegistry's state | | Optional: \{\}
|
| `observedGeneration` _integer_ | ObservedGeneration reflects the generation most recently observed by the controller | | Optional: \{\}
|
-| `phase` _[api.v1alpha1.MCPRegistryPhase](#apiv1alpha1mcpregistryphase)_ | Phase represents the current overall phase of the MCPRegistry
Derived from sync and API status | | Enum: [Pending Ready Failed Syncing Terminating]
Optional: \{\}
|
+| `phase` _[api.v1alpha1.MCPRegistryPhase](#apiv1alpha1mcpregistryphase)_ | Phase represents the current overall phase of the MCPRegistry | | Enum: [Pending Running Failed Terminating]
Optional: \{\}
|
| `message` _string_ | Message provides additional information about the current phase | | Optional: \{\}
|
-| `syncStatus` _[api.v1alpha1.SyncStatus](#apiv1alpha1syncstatus)_ | SyncStatus provides detailed information about data synchronization | | Optional: \{\}
|
-| `apiStatus` _[api.v1alpha1.APIStatus](#apiv1alpha1apistatus)_ | APIStatus provides detailed information about the API service | | Optional: \{\}
|
-| `lastAppliedFilterHash` _string_ | LastAppliedFilterHash is the hash of the last applied filter | | Optional: \{\}
|
-| `storageRef` _[api.v1alpha1.StorageReference](#apiv1alpha1storagereference)_ | StorageRef is a reference to the internal storage location | | Optional: \{\}
|
-| `lastManualSyncTrigger` _string_ | LastManualSyncTrigger tracks the last processed manual sync annotation value
Used to detect new manual sync requests via toolhive.stacklok.dev/sync-trigger annotation | | Optional: \{\}
|
-| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPRegistry's state | | Optional: \{\}
|
+| `url` _string_ | URL is the URL where the registry API can be accessed | | Optional: \{\}
|
+| `readyReplicas` _integer_ | ReadyReplicas is the number of ready registry API replicas | | Optional: \{\}
|
#### api.v1alpha1.MCPRemoteProxy
@@ -3073,42 +3029,6 @@ _Appears in:_
| `passwordRef` _[api.v1alpha1.SecretKeyRef](#apiv1alpha1secretkeyref)_ | PasswordRef is a reference to a Secret key containing the Redis password | | Optional: \{\}
|
-#### api.v1alpha1.StorageReference
-
-
-
-StorageReference defines a reference to internal storage
-
-
-
-_Appears in:_
-- [api.v1alpha1.MCPRegistryStatus](#apiv1alpha1mcpregistrystatus)
-
-| Field | Description | Default | Validation |
-| --- | --- | --- | --- |
-| `type` _string_ | Type is the storage type (configmap) | | Enum: [configmap]
|
-| `configMapRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#localobjectreference-v1-core)_ | ConfigMapRef is a reference to a ConfigMap storage
Only used when Type is "configmap" | | Optional: \{\}
|
-
-
-#### api.v1alpha1.SyncPhase
-
-_Underlying type:_ _string_
-
-SyncPhase represents the data synchronization state
-
-_Validation:_
-- Enum: [Syncing Complete Failed]
-
-_Appears in:_
-- [api.v1alpha1.SyncStatus](#apiv1alpha1syncstatus)
-
-| Field | Description |
-| --- | --- |
-| `Syncing` | SyncPhaseSyncing means sync is currently in progress
|
-| `Complete` | SyncPhaseComplete means sync completed successfully
|
-| `Failed` | SyncPhaseFailed means sync failed
|
-
-
#### api.v1alpha1.SyncPolicy
@@ -3128,28 +3048,6 @@ _Appears in:_
| `interval` _string_ | Interval is the sync interval for automatic synchronization (Go duration format)
Examples: "1h", "30m", "24h" | | Pattern: `^([0-9]+(\.[0-9]+)?(ns\|us\|µs\|ms\|s\|m\|h))+$`
Required: \{\}
|
-#### api.v1alpha1.SyncStatus
-
-
-
-SyncStatus provides detailed information about data synchronization
-
-
-
-_Appears in:_
-- [api.v1alpha1.MCPRegistryStatus](#apiv1alpha1mcpregistrystatus)
-
-| Field | Description | Default | Validation |
-| --- | --- | --- | --- |
-| `phase` _[api.v1alpha1.SyncPhase](#apiv1alpha1syncphase)_ | Phase represents the current synchronization phase | | Enum: [Syncing Complete Failed]
|
-| `message` _string_ | Message provides additional information about the sync status | | Optional: \{\}
|
-| `lastAttempt` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#time-v1-meta)_ | LastAttempt is the timestamp of the last sync attempt | | Optional: \{\}
|
-| `attemptCount` _integer_ | AttemptCount is the number of sync attempts since last success | | Minimum: 0
Optional: \{\}
|
-| `lastSyncTime` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#time-v1-meta)_ | LastSyncTime is the timestamp of the last successful sync | | Optional: \{\}
|
-| `lastSyncHash` _string_ | LastSyncHash is the hash of the last successfully synced data
Used to detect changes in source data | | Optional: \{\}
|
-| `serverCount` _integer_ | ServerCount is the total number of servers in the registry | | Minimum: 0
Optional: \{\}
|
-
-
#### api.v1alpha1.TagFilter