-
Notifications
You must be signed in to change notification settings - Fork 142
Unified workload manager #2487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
amirejaz
wants to merge
20
commits into
main
Choose a base branch
from
unified-workload-manager
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Unified workload manager #2487
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
fb159b1
unify workload management across CLI and Kubernetes
amirejaz 480db30
removed unnecessary files
amirejaz 0b09d38
unified workload with separate workloads for cli and k8s
amirejaz 030368d
merged remote main branch into local
amirejaz 068e2ce
refactor the constructor and fix tests
amirejaz c58ce7c
Merge remote-tracking branch 'origin/main' into unified-workload-manager
amirejaz 31e40e7
adds more tests
amirejaz 2c921c7
fixed thv listing
amirejaz 1d6d807
checks the kubernetes client runtime instead of the environment
amirejaz bec49a2
Merge remote-tracking branch 'origin/main' into unified-workload-manager
amirejaz 26a37ad
fix e2e tests
amirejaz 9dd801e
Merge remote-tracking branch 'origin/main' into unified-workload-manager
amirejaz f3fd09f
improves test coverage
amirejaz 846fe38
Merge remote-tracking branch 'origin/main' into unified-workload-manager
amirejaz b70ac65
refactor the k8s manager into separate package
amirejaz dd7c0ed
removed logs fns
amirejaz d121268
Merge remote-tracking branch 'origin/main' into unified-workload-manager
amirejaz 0608b7a
use pkg/k8s package for client and namespace
amirejaz ed6b6d9
moved discoverer creation to factory inside pkg
amirejaz 152a921
Merge remote-tracking branch 'origin/main' into unified-workload-manager
amirejaz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package aggregator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| rt "github.com/stacklok/toolhive/pkg/container/runtime" | ||
| "github.com/stacklok/toolhive/pkg/groups" | ||
| "github.com/stacklok/toolhive/pkg/vmcp/config" | ||
| "github.com/stacklok/toolhive/pkg/workloads" | ||
| "github.com/stacklok/toolhive/pkg/workloads/k8s" | ||
| ) | ||
|
|
||
| // NewBackendDiscoverer creates a BackendDiscoverer based on the runtime environment. | ||
| // It automatically detects whether to use CLI (Docker/Podman) or Kubernetes discoverer | ||
| // and creates the appropriate workloads manager. | ||
| // | ||
| // Parameters: | ||
| // - ctx: Context for creating managers | ||
| // - groupsManager: Manager for group operations (must already be initialized) | ||
| // - authConfig: Outgoing authentication configuration for discovered backends | ||
| // | ||
| // Returns: | ||
| // - BackendDiscoverer: The appropriate discoverer for the current runtime | ||
| // - error: If manager creation fails | ||
| func NewBackendDiscoverer( | ||
| ctx context.Context, | ||
| groupsManager groups.Manager, | ||
| authConfig *config.OutgoingAuthConfig, | ||
| ) (BackendDiscoverer, error) { | ||
| if rt.IsKubernetesRuntime() { | ||
| k8sWorkloadsManager, err := k8s.NewManagerFromContext(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create Kubernetes workloads manager: %w", err) | ||
| } | ||
| return NewK8SBackendDiscoverer(k8sWorkloadsManager, groupsManager, authConfig), nil | ||
| } | ||
|
|
||
| cliWorkloadsManager, err := workloads.NewManager(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create CLI workloads manager: %w", err) | ||
| } | ||
| return NewCLIBackendDiscoverer(cliWorkloadsManager, groupsManager, authConfig), nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,160 @@ | ||
| // Package aggregator provides platform-agnostic backend discovery. | ||
| // This file contains the Kubernetes-specific discoverer implementation. | ||
| package aggregator | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" | ||
| "github.com/stacklok/toolhive/pkg/groups" | ||
| "github.com/stacklok/toolhive/pkg/logger" | ||
| "github.com/stacklok/toolhive/pkg/vmcp" | ||
| "github.com/stacklok/toolhive/pkg/vmcp/config" | ||
| "github.com/stacklok/toolhive/pkg/workloads/k8s" | ||
| ) | ||
|
|
||
| // k8sBackendDiscoverer discovers backend MCP servers from Kubernetes pods/services in a group. | ||
| // This is the Kubernetes version of BackendDiscoverer (not implemented yet). | ||
| // k8sBackendDiscoverer discovers backend MCP servers from Kubernetes workloads (MCPServer CRDs). | ||
| // It works with k8s.Manager and k8s.Workload. | ||
| type k8sBackendDiscoverer struct { | ||
| // TODO: Add Kubernetes client and group CRD interfaces | ||
| workloadsManager k8s.Manager | ||
| groupsManager groups.Manager | ||
| authConfig *config.OutgoingAuthConfig | ||
| } | ||
|
|
||
| // NewK8sBackendDiscoverer creates a new Kubernetes-based backend discoverer. | ||
| // It discovers workloads from Kubernetes MCPServer resources managed by the operator. | ||
| func NewK8sBackendDiscoverer() BackendDiscoverer { | ||
| return &k8sBackendDiscoverer{} | ||
| // NewK8SBackendDiscoverer creates a new Kubernetes-based backend discoverer. | ||
| // It discovers workloads from MCPServer CRDs managed by the ToolHive operator in Kubernetes. | ||
| // | ||
| // The authConfig parameter configures authentication for discovered backends. | ||
| // If nil, backends will have no authentication configured. | ||
| // | ||
| // This is the Kubernetes-specific constructor. For CLI workloads, use NewCLIBackendDiscoverer. | ||
| func NewK8SBackendDiscoverer( | ||
| workloadsManager k8s.Manager, | ||
| groupsManager groups.Manager, | ||
| authConfig *config.OutgoingAuthConfig, | ||
| ) BackendDiscoverer { | ||
| return &k8sBackendDiscoverer{ | ||
| workloadsManager: workloadsManager, | ||
| groupsManager: groupsManager, | ||
| authConfig: authConfig, | ||
| } | ||
| } | ||
|
|
||
| // Discover finds all backend workloads in the specified Kubernetes group. | ||
| // The groupRef is the MCPGroup name. | ||
| func (*k8sBackendDiscoverer) Discover(_ context.Context, _ string) ([]vmcp.Backend, error) { | ||
| // TODO: Implement Kubernetes backend discovery | ||
| // 1. Query MCPGroup CRD by name | ||
| // 2. List MCPServer resources with matching group label | ||
| // 3. Filter for ready/running MCPServers | ||
| // 4. Build service URLs (http://service-name.namespace.svc.cluster.local:port) | ||
| // 5. Extract transport type from MCPServer spec | ||
| // 6. Return vmcp.Backend list | ||
| return nil, fmt.Errorf("kubernetes backend discovery not yet implemented") | ||
| // Discover finds all backend workloads in the specified group. | ||
| func (d *k8sBackendDiscoverer) Discover(ctx context.Context, groupRef string) ([]vmcp.Backend, error) { | ||
| logger.Infof("Discovering Kubernetes backends in group %s", groupRef) | ||
|
|
||
| // Verify that the group exists | ||
| exists, err := d.groupsManager.Exists(ctx, groupRef) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to check if group exists: %w", err) | ||
| } | ||
| if !exists { | ||
| return nil, fmt.Errorf("group %s not found", groupRef) | ||
| } | ||
|
|
||
| // Get all workload names in the group | ||
| workloadNames, err := d.workloadsManager.ListWorkloadsInGroup(ctx, groupRef) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to list workloads in group: %w", err) | ||
| } | ||
|
|
||
| if len(workloadNames) == 0 { | ||
| logger.Infof("No workloads found in group %s", groupRef) | ||
| return []vmcp.Backend{}, nil | ||
| } | ||
|
|
||
| logger.Debugf("Found %d workloads in group %s, discovering backends", len(workloadNames), groupRef) | ||
|
|
||
| // Query each workload and convert to backend | ||
| var backends []vmcp.Backend | ||
| for _, name := range workloadNames { | ||
| workload, err := d.workloadsManager.GetWorkload(ctx, name) | ||
| if err != nil { | ||
| logger.Warnf("Failed to get workload %s: %v, skipping", name, err) | ||
| continue | ||
| } | ||
|
|
||
| backend := d.convertK8SWorkload(workload, groupRef) | ||
| if backend != nil { | ||
| backends = append(backends, *backend) | ||
| } | ||
| } | ||
|
|
||
| if len(backends) == 0 { | ||
| logger.Infof("No accessible backends found in group %s (all workloads lack URLs)", groupRef) | ||
| return []vmcp.Backend{}, nil | ||
| } | ||
|
|
||
| logger.Infof("Discovered %d backends in group %s", len(backends), groupRef) | ||
| return backends, nil | ||
| } | ||
|
|
||
| // convertK8SWorkload converts a k8s.Workload to a vmcp.Backend. | ||
| func (d *k8sBackendDiscoverer) convertK8SWorkload(workload k8s.Workload, groupRef string) *vmcp.Backend { | ||
| // Skip workloads without a URL (not accessible) | ||
| if workload.URL == "" { | ||
| logger.Debugf("Skipping workload %s without URL", workload.Name) | ||
| return nil | ||
| } | ||
|
|
||
| // Map workload phase to backend health status | ||
| healthStatus := mapK8SWorkloadPhaseToHealth(workload.Phase) | ||
|
|
||
| // Convert k8s.Workload to vmcp.Backend | ||
| transportType := workload.ProxyMode | ||
| if transportType == "" { | ||
| // Fallback to TransportType if ProxyMode is not set (for direct transports) | ||
| transportType = workload.TransportType.String() | ||
| } | ||
|
|
||
| backend := vmcp.Backend{ | ||
| ID: workload.Name, | ||
| Name: workload.Name, | ||
| BaseURL: workload.URL, | ||
| TransportType: transportType, | ||
| HealthStatus: healthStatus, | ||
| Metadata: make(map[string]string), | ||
| } | ||
|
|
||
| // Apply authentication configuration if provided | ||
| authStrategy, authMetadata := d.authConfig.ResolveForBackend(workload.Name) | ||
| backend.AuthStrategy = authStrategy | ||
| backend.AuthMetadata = authMetadata | ||
| if authStrategy != "" { | ||
| logger.Debugf("Backend %s configured with auth strategy: %s", workload.Name, authStrategy) | ||
| } | ||
|
|
||
| // Copy user labels to metadata first | ||
| for k, v := range workload.Labels { | ||
| backend.Metadata[k] = v | ||
| } | ||
|
|
||
| // Set system metadata (these override user labels to prevent conflicts) | ||
| backend.Metadata["group"] = groupRef | ||
| backend.Metadata["tool_type"] = workload.ToolType | ||
| backend.Metadata["workload_phase"] = string(workload.Phase) | ||
| backend.Metadata["namespace"] = workload.Namespace | ||
|
|
||
| logger.Debugf("Discovered backend %s: %s (%s) with health status %s", | ||
| backend.ID, backend.BaseURL, backend.TransportType, backend.HealthStatus) | ||
|
|
||
| return &backend | ||
| } | ||
|
|
||
| // mapK8SWorkloadPhaseToHealth converts a MCPServerPhase to a backend health status. | ||
| func mapK8SWorkloadPhaseToHealth(phase mcpv1alpha1.MCPServerPhase) vmcp.BackendHealthStatus { | ||
| switch phase { | ||
| case mcpv1alpha1.MCPServerPhaseRunning: | ||
| return vmcp.BackendHealthy | ||
| case mcpv1alpha1.MCPServerPhaseFailed: | ||
| return vmcp.BackendUnhealthy | ||
| case mcpv1alpha1.MCPServerPhaseTerminating: | ||
| return vmcp.BackendUnhealthy | ||
| case mcpv1alpha1.MCPServerPhasePending: | ||
| return vmcp.BackendUnknown | ||
| default: | ||
| return vmcp.BackendUnknown | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic of the k8s discoverer and cli discoverer are very similar. This makes me wonder if we could have a single discoverer type, and create an interface which encapsulates the operation they perform on the underlying runtime. Let's chat through this if you have some time.