From 08de8fad2602c2887f13a505daa712cdb11c90c0 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Wed, 22 Oct 2025 13:29:50 +0100 Subject: [PATCH 1/8] feat(kubevirt): Add VM management toolset with plan-based creation Introduces a new KubeVirt toolset providing virtual machine management capabilities through MCP tools. The vm_create tool generates comprehensive creation plans with pre-creation validation of instance types, preferences, and container disk images, enabling AI assistants to help users create VirtualMachines with appropriate resource configurations. The tool supports: - Workload specification via OS names or container disk URLs - Auto-selection of instance types based on size/performance hints - DataSource integration for common OS images - Comprehensive validation and planning before resource creation Assisted-By: Claude Signed-off-by: Lee Yarwood --- internal/tools/update-readme/main.go | 1 + pkg/kubernetes-mcp-server/cmd/root_test.go | 2 +- pkg/kubernetes/kubernetes.go | 9 + pkg/mcp/modules.go | 14 +- pkg/toolsets/kubevirt/toolset.go | 34 + pkg/toolsets/kubevirt/vm/create/plan.tmpl | 99 +++ pkg/toolsets/kubevirt/vm/create/tool.go | 771 ++++++++++++++++++ pkg/toolsets/kubevirt/vm/create/tool_test.go | 205 +++++ .../kubevirt/vm/troubleshoot/plan.tmpl | 188 +++++ pkg/toolsets/kubevirt/vm/troubleshoot/tool.go | 98 +++ .../kubevirt/vm/troubleshoot/tool_test.go | 110 +++ 11 files changed, 1526 insertions(+), 5 deletions(-) create mode 100644 pkg/toolsets/kubevirt/toolset.go create mode 100644 pkg/toolsets/kubevirt/vm/create/plan.tmpl create mode 100644 pkg/toolsets/kubevirt/vm/create/tool.go create mode 100644 pkg/toolsets/kubevirt/vm/create/tool_test.go create mode 100644 pkg/toolsets/kubevirt/vm/troubleshoot/plan.tmpl create mode 100644 pkg/toolsets/kubevirt/vm/troubleshoot/tool.go create mode 100644 pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go diff --git a/internal/tools/update-readme/main.go b/internal/tools/update-readme/main.go index 79cf7325..d400afe4 100644 --- a/internal/tools/update-readme/main.go +++ b/internal/tools/update-readme/main.go @@ -17,6 +17,7 @@ import ( _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/core" _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/helm" _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kiali" + _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt" ) type OpenShift struct{} diff --git a/pkg/kubernetes-mcp-server/cmd/root_test.go b/pkg/kubernetes-mcp-server/cmd/root_test.go index a464daab..d85eb37c 100644 --- a/pkg/kubernetes-mcp-server/cmd/root_test.go +++ b/pkg/kubernetes-mcp-server/cmd/root_test.go @@ -137,7 +137,7 @@ func TestToolsets(t *testing.T) { rootCmd := NewMCPServer(ioStreams) rootCmd.SetArgs([]string{"--help"}) o, err := captureOutput(rootCmd.Execute) // --help doesn't use logger/klog, cobra prints directly to stdout - if !strings.Contains(o, "Comma-separated list of MCP toolsets to use (available toolsets: config, core, helm, kiali).") { + if !strings.Contains(o, "Comma-separated list of MCP toolsets to use (available toolsets: config, core, helm, kiali, kubevirt).") { t.Fatalf("Expected all available toolsets, got %s %v", o, err) } }) diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index 7de8d6ff..b3e2a264 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -2,6 +2,7 @@ package kubernetes import ( "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" "github.com/containers/kubernetes-mcp-server/pkg/helm" "github.com/containers/kubernetes-mcp-server/pkg/kiali" @@ -31,6 +32,14 @@ func (k *Kubernetes) AccessControlClientset() *AccessControlClientset { return k.manager.accessControlClientSet } +// RESTConfig returns the Kubernetes REST configuration +func (k *Kubernetes) RESTConfig() *rest.Config { + if k.manager == nil { + return nil + } + return k.manager.cfg +} + var Scheme = scheme.Scheme var ParameterCodec = runtime.NewParameterCodec(Scheme) diff --git a/pkg/mcp/modules.go b/pkg/mcp/modules.go index 464eefc8..c8595e99 100644 --- a/pkg/mcp/modules.go +++ b/pkg/mcp/modules.go @@ -1,6 +1,12 @@ package mcp -import _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/config" -import _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/core" -import _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/helm" -import _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kiali" +import ( + _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/config" + _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/core" + + _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/helm" + + _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kiali" + + _ "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt" +) diff --git a/pkg/toolsets/kubevirt/toolset.go b/pkg/toolsets/kubevirt/toolset.go new file mode 100644 index 00000000..f8b21137 --- /dev/null +++ b/pkg/toolsets/kubevirt/toolset.go @@ -0,0 +1,34 @@ +package kubevirt + +import ( + "slices" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" + "github.com/containers/kubernetes-mcp-server/pkg/toolsets" + vm_create "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/create" + vm_troubleshoot "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/troubleshoot" +) + +type Toolset struct{} + +var _ api.Toolset = (*Toolset)(nil) + +func (t *Toolset) GetName() string { + return "kubevirt" +} + +func (t *Toolset) GetDescription() string { + return "KubeVirt virtual machine management tools" +} + +func (t *Toolset) GetTools(o internalk8s.Openshift) []api.ServerTool { + return slices.Concat( + vm_create.Tools(), + vm_troubleshoot.Tools(), + ) +} + +func init() { + toolsets.Register(&Toolset{}) +} diff --git a/pkg/toolsets/kubevirt/vm/create/plan.tmpl b/pkg/toolsets/kubevirt/vm/create/plan.tmpl new file mode 100644 index 00000000..758b0ee0 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/create/plan.tmpl @@ -0,0 +1,99 @@ +# VirtualMachine Creation Plan + +**IMPORTANT**: Always use `runStrategy` instead of the deprecated `running` field when creating VirtualMachines. + +Use the `resources_create_or_update` tool with the following YAML: + +```yaml +apiVersion: kubevirt.io/v1 +kind: VirtualMachine +metadata: + name: {{.Name}} + namespace: {{.Namespace}} +spec: + runStrategy: Halted +{{- if .Instancetype}} + instancetype: + name: {{.Instancetype}} + kind: VirtualMachineClusterInstancetype +{{- end}} +{{- if .Preference}} + preference: + name: {{.Preference}} + kind: VirtualMachineClusterPreference +{{- end}} +{{- if .UseDataSource}} + dataVolumeTemplates: + - metadata: + name: {{.Name}}-rootdisk + spec: + sourceRef: + kind: DataSource + name: {{.DataSourceName}} + namespace: {{.DataSourceNamespace}} + storage: + resources: + requests: + storage: 30Gi +{{- end}} + template: + spec: + domain: + devices: + disks: + - name: {{.Name}}-rootdisk +{{- if not .Instancetype}} + memory: + guest: 2Gi +{{- end}} + volumes: + - name: {{.Name}}-rootdisk +{{- if .UseDataSource}} + dataVolume: + name: {{.Name}}-rootdisk +{{- else}} + containerDisk: + image: {{.ContainerDisk}} +{{- end}} +``` + +## Run Strategy Options + +The VM is created with `runStrategy: Halted` (stopped state). You can modify the `runStrategy` field to control the VM's execution: + +- **`Halted`** - VM is stopped and will not run +- **`Always`** - VM should always be running (restarts automatically) +- **`RerunOnFailure`** - Restart the VM only if it fails +- **`Manual`** - Manual start/stop control via `virtctl start/stop` +- **`Once`** - Run the VM once, then stop when it terminates + +To start the VM after creation, change `runStrategy: Halted` to `runStrategy: Always` or use the Manual strategy and start it with virtctl. + +## Verification + +After creating the VirtualMachine, verify it was created successfully: + +Use the `resources_get` tool: +- **apiVersion**: `kubevirt.io/v1` +- **kind**: `VirtualMachine` +- **namespace**: `{{.Namespace}}` +- **name**: `{{.Name}}` + +Check the resource details for any warnings or errors in the status conditions. + +## Troubleshooting + +If the VirtualMachine fails to create or start: + +1. **Check the VM resource details and events**: + - Use `resources_get` tool with apiVersion `kubevirt.io/v1`, kind `VirtualMachine`, namespace `{{.Namespace}}`, name `{{.Name}}` + - Look for error messages in the status conditions + +2. **Verify instance type exists** (if specified): + - Use `resources_get` tool with apiVersion `instancetype.kubevirt.io/v1beta1`, kind `VirtualMachineClusterInstancetype`, name `{{.Instancetype}}` + +3. **Verify preference exists** (if specified): + - Use `resources_get` tool with apiVersion `instancetype.kubevirt.io/v1beta1`, kind `VirtualMachineClusterPreference`, name `{{.Preference}}` + +4. **Check KubeVirt installation**: + - Use `pods_list` tool with namespace `kubevirt` diff --git a/pkg/toolsets/kubevirt/vm/create/tool.go b/pkg/toolsets/kubevirt/vm/create/tool.go new file mode 100644 index 00000000..287f03a4 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/create/tool.go @@ -0,0 +1,771 @@ +package create + +import ( + _ "embed" + "fmt" + "strings" + "text/template" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + "github.com/google/jsonschema-go/jsonschema" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/utils/ptr" +) + +const ( + defaultInstancetypeLabel = "instancetype.kubevirt.io/default-instancetype" + defaultPreferenceLabel = "instancetype.kubevirt.io/default-preference" +) + +//go:embed plan.tmpl +var planTemplate string + +func Tools() []api.ServerTool { + return []api.ServerTool{ + { + Tool: api.Tool{ + Name: "vm_create", + Description: "Generate a comprehensive creation plan for a VirtualMachine, including pre-creation checks for instance types, preferences, and container disk images", + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "namespace": { + Type: "string", + Description: "The namespace for the virtual machine", + }, + "name": { + Type: "string", + Description: "The name of the virtual machine", + }, + "workload": { + Type: "string", + Description: "The workload for the VM. Accepts OS names (e.g., 'fedora' (default), 'ubuntu', 'centos', 'centos-stream', 'debian', 'rhel', 'opensuse', 'opensuse-tumbleweed', 'opensuse-leap') or full container disk image URLs", + Examples: []interface{}{"fedora", "ubuntu", "centos", "debian", "rhel", "quay.io/containerdisks/fedora:latest"}, + }, + "instancetype": { + Type: "string", + Description: "Optional instance type name for the VM (e.g., 'u1.small', 'u1.medium', 'u1.large')", + }, + "preference": { + Type: "string", + Description: "Optional preference name for the VM", + }, + "size": { + Type: "string", + Description: "Optional workload size hint for the VM (e.g., 'small', 'medium', 'large', 'xlarge'). Used to auto-select an appropriate instance type if not explicitly specified.", + Examples: []interface{}{"small", "medium", "large"}, + }, + "performance": { + Type: "string", + Description: "Optional performance family hint for the VM instance type (e.g., 'u1' for general-purpose, 'o1' for overcommitted, 'c1' for compute-optimized, 'm1' for memory-optimized). Defaults to 'u1' (general-purpose) if not specified.", + Examples: []interface{}{"general-purpose", "overcommitted", "compute-optimized", "memory-optimized"}, + }, + }, + Required: []string{"namespace", "name"}, + }, + Annotations: api.ToolAnnotations{ + Title: "Virtual Machine: Create", + ReadOnlyHint: ptr.To(true), + DestructiveHint: ptr.To(false), + IdempotentHint: ptr.To(true), + OpenWorldHint: ptr.To(false), + }, + }, + Handler: create, + }, + } +} + +type vmParams struct { + Namespace string + Name string + ContainerDisk string + Instancetype string + Preference string + UseDataSource bool + DataSourceName string + DataSourceNamespace string +} + +type DataSourceInfo struct { + Name string + Namespace string + Source string + DefaultInstancetype string + DefaultPreference string +} + +type PreferenceInfo struct { + Name string +} + +type InstancetypeInfo struct { + Name string + Labels map[string]string +} + +func create(params api.ToolHandlerParams) (*api.ToolCallResult, error) { + // Parse and validate input parameters + createParams, err := parseCreateParameters(params) + if err != nil { + return api.NewToolCallResult("", err), nil + } + + // Search for available DataSources + dataSources, _ := searchDataSources(params, createParams.Workload) + + // Match DataSource based on workload input + matchedDataSource := matchDataSource(dataSources, createParams.Workload) + + // Resolve preference from DataSource defaults or cluster resources + preference := resolvePreference(params, createParams.Preference, matchedDataSource, createParams.Workload, createParams.Namespace) + + // Resolve instancetype from DataSource defaults or size/performance hints + instancetype := resolveInstancetype(params, createParams, matchedDataSource) + + // Build template parameters from resolved resources + templateParams := buildTemplateParams(createParams, matchedDataSource, instancetype, preference) + + // Render the VM creation plan template + result, err := renderTemplate(templateParams) + if err != nil { + return api.NewToolCallResult("", err), nil + } + + return api.NewToolCallResult(result, nil), nil +} + +// createParameters holds parsed input parameters for VM creation +type createParameters struct { + Namespace string + Name string + Workload string + Instancetype string + Preference string + Size string + Performance string +} + +// parseCreateParameters parses and validates input parameters +func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, error) { + namespace, err := getRequiredString(params, "namespace") + if err != nil { + return nil, err + } + + name, err := getRequiredString(params, "name") + if err != nil { + return nil, err + } + + workload := getOptionalString(params, "workload") + if workload == "" { + workload = "fedora" // Default to fedora if not specified + } + + performance := normalizePerformance(getOptionalString(params, "performance")) + + return &createParameters{ + Namespace: namespace, + Name: name, + Workload: workload, + Instancetype: getOptionalString(params, "instancetype"), + Preference: getOptionalString(params, "preference"), + Size: getOptionalString(params, "size"), + Performance: performance, + }, nil +} + +// matchDataSource finds a DataSource that matches the workload input +func matchDataSource(dataSources []DataSourceInfo, workload string) *DataSourceInfo { + normalizedInput := strings.ToLower(strings.TrimSpace(workload)) + + // First try exact match + for i := range dataSources { + ds := &dataSources[i] + if strings.EqualFold(ds.Name, normalizedInput) || strings.EqualFold(ds.Name, workload) { + return ds + } + } + + // If no exact match, try partial matching (e.g., "rhel" matches "rhel9") + // Only match against real DataSources with namespaces, not built-in containerdisks + for i := range dataSources { + ds := &dataSources[i] + // Only do partial matching for real DataSources (those with namespaces) + if ds.Namespace != "" && strings.Contains(strings.ToLower(ds.Name), normalizedInput) { + return ds + } + } + + return nil +} + +// resolvePreference determines the preference to use from DataSource defaults or cluster resources +func resolvePreference(params api.ToolHandlerParams, explicitPreference string, matchedDataSource *DataSourceInfo, workload string, namespace string) string { + // Use explicitly specified preference if provided + if explicitPreference != "" { + return explicitPreference + } + + // Use DataSource default preference if available + if matchedDataSource != nil && matchedDataSource.DefaultPreference != "" { + return matchedDataSource.DefaultPreference + } + + // Try to match preference name against the workload input + preferences := searchPreferences(params, namespace) + normalizedInput := strings.ToLower(strings.TrimSpace(workload)) + + for i := range preferences { + pref := &preferences[i] + // Common patterns: "fedora", "rhel.9", "ubuntu", etc. + if strings.Contains(strings.ToLower(pref.Name), normalizedInput) { + return pref.Name + } + } + + return "" +} + +// resolveInstancetype determines the instancetype to use from DataSource defaults or size/performance hints +func resolveInstancetype(params api.ToolHandlerParams, createParams *createParameters, matchedDataSource *DataSourceInfo) string { + // Use explicitly specified instancetype if provided + if createParams.Instancetype != "" { + return createParams.Instancetype + } + + // Use DataSource default instancetype if available (when size not specified) + if createParams.Size == "" && matchedDataSource != nil && matchedDataSource.DefaultInstancetype != "" { + return matchedDataSource.DefaultInstancetype + } + + // Match instancetype based on size and performance hints + if createParams.Size != "" { + return matchInstancetypeBySize(params, createParams.Size, createParams.Performance, createParams.Namespace) + } + + return "" +} + +// matchInstancetypeBySize finds an instancetype that matches the size and performance hints +func matchInstancetypeBySize(params api.ToolHandlerParams, size, performance, namespace string) string { + instancetypes := searchInstancetypes(params, namespace) + normalizedSize := strings.ToLower(strings.TrimSpace(size)) + normalizedPerformance := strings.ToLower(strings.TrimSpace(performance)) + + // Filter instance types by size + candidatesBySize := filterInstancetypesBySize(instancetypes, normalizedSize) + if len(candidatesBySize) == 0 { + return "" + } + + // Try to match by performance family prefix (e.g., "u1.small") + for i := range candidatesBySize { + it := &candidatesBySize[i] + if strings.HasPrefix(strings.ToLower(it.Name), normalizedPerformance+".") { + return it.Name + } + } + + // Try to match by performance family label + for i := range candidatesBySize { + it := &candidatesBySize[i] + if it.Labels != nil { + if class, ok := it.Labels["instancetype.kubevirt.io/class"]; ok { + if strings.EqualFold(class, normalizedPerformance) { + return it.Name + } + } + } + } + + // Fall back to first candidate that matches size + return candidatesBySize[0].Name +} + +// filterInstancetypesBySize filters instancetypes that contain the size hint in their name +func filterInstancetypesBySize(instancetypes []InstancetypeInfo, normalizedSize string) []InstancetypeInfo { + var candidates []InstancetypeInfo + for i := range instancetypes { + it := &instancetypes[i] + if strings.Contains(strings.ToLower(it.Name), normalizedSize) { + candidates = append(candidates, *it) + } + } + return candidates +} + +// buildTemplateParams constructs the template parameters for VM creation +func buildTemplateParams(createParams *createParameters, matchedDataSource *DataSourceInfo, instancetype, preference string) vmParams { + params := vmParams{ + Namespace: createParams.Namespace, + Name: createParams.Name, + Instancetype: instancetype, + Preference: preference, + } + + if matchedDataSource != nil && matchedDataSource.Namespace != "" { + // Use the matched DataSource (real cluster DataSource with namespace) + params.UseDataSource = true + params.DataSourceName = matchedDataSource.Name + params.DataSourceNamespace = matchedDataSource.Namespace + } else if matchedDataSource != nil { + // Matched a built-in containerdisk (no namespace) + params.ContainerDisk = matchedDataSource.Source + } else { + // No match, resolve container disk image from workload name + params.ContainerDisk = resolveContainerDisk(createParams.Workload) + } + + return params +} + +// renderTemplate renders the VM creation plan template +func renderTemplate(templateParams vmParams) (string, error) { + tmpl, err := template.New("vm").Parse(planTemplate) + if err != nil { + return "", fmt.Errorf("failed to parse template: %w", err) + } + + var result strings.Builder + if err := tmpl.Execute(&result, templateParams); err != nil { + return "", fmt.Errorf("failed to render template: %w", err) + } + + return result.String(), nil +} + +// Helper functions + +func normalizePerformance(performance string) string { + // Normalize to lowercase and trim spaces + normalized := strings.ToLower(strings.TrimSpace(performance)) + + // Map natural language terms to instance type prefixes + performanceMap := map[string]string{ + "general-purpose": "u1", + "generalpurpose": "u1", + "general": "u1", + "overcommitted": "o1", + "compute": "c1", + "compute-optimized": "c1", + "computeoptimized": "c1", + "memory-optimized": "m1", + "memoryoptimized": "m1", + "memory": "m1", + "u1": "u1", + "o1": "o1", + "c1": "c1", + "m1": "m1", + } + + // Look up the mapping + if prefix, exists := performanceMap[normalized]; exists { + return prefix + } + + // Default to "u1" (general-purpose) if not recognized or empty + return "u1" +} + +func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { + args := params.GetArguments() + val, ok := args[key] + if !ok { + return "", fmt.Errorf("%s parameter required", key) + } + str, ok := val.(string) + if !ok { + return "", fmt.Errorf("%s parameter must be a string", key) + } + return str, nil +} + +func getOptionalString(params api.ToolHandlerParams, key string) string { + args := params.GetArguments() + val, ok := args[key] + if !ok { + return "" + } + str, ok := val.(string) + if !ok { + return "" + } + return str +} + +// resolveContainerDisk resolves OS names to container disk images from quay.io/containerdisks +func resolveContainerDisk(input string) string { + // If input already looks like a container image, return as-is + if strings.Contains(input, "/") || strings.Contains(input, ":") { + return input + } + + // Common OS name mappings to containerdisk images + osMap := map[string]string{ + "fedora": "quay.io/containerdisks/fedora:latest", + "ubuntu": "quay.io/containerdisks/ubuntu:24.04", + "centos": "quay.io/containerdisks/centos-stream:9-latest", + "centos-stream": "quay.io/containerdisks/centos-stream:9-latest", + "debian": "quay.io/containerdisks/debian:latest", + "opensuse": "quay.io/containerdisks/opensuse-tumbleweed:1.0.0", + "opensuse-tumbleweed": "quay.io/containerdisks/opensuse-tumbleweed:1.0.0", + "opensuse-leap": "quay.io/containerdisks/opensuse-leap:15.6", + // NOTE: The following RHEL images could not be verified due to authentication requirements. + "rhel8": "registry.redhat.io/rhel8/rhel-guest-image:latest", + "rhel9": "registry.redhat.io/rhel9/rhel-guest-image:latest", + "rhel10": "registry.redhat.io/rhel10/rhel-guest-image:latest", + } + + // Normalize input to lowercase for lookup + normalized := strings.ToLower(strings.TrimSpace(input)) + + // Look up the OS name + if containerDisk, exists := osMap[normalized]; exists { + return containerDisk + } + + // If no match found, return the input as-is (assume it's a valid container image URL) + return input +} + +// getDefaultContainerDisks returns a list of common containerdisk images +func getDefaultContainerDisks() []DataSourceInfo { + return []DataSourceInfo{ + { + Name: "fedora", + Source: "quay.io/containerdisks/fedora:latest", + }, + { + Name: "ubuntu", + Source: "quay.io/containerdisks/ubuntu:24.04", + }, + { + Name: "centos-stream", + Source: "quay.io/containerdisks/centos-stream:9-latest", + }, + { + Name: "debian", + Source: "quay.io/containerdisks/debian:latest", + }, + { + Name: "rhel8", + Source: "registry.redhat.io/rhel8/rhel-guest-image:latest", + }, + { + Name: "rhel9", + Source: "registry.redhat.io/rhel9/rhel-guest-image:latest", + }, + { + Name: "rhel10", + Source: "registry.redhat.io/rhel10/rhel-guest-image:latest", + }, + } +} + +// searchDataSources searches for DataSource resources in the cluster +func searchDataSources(params api.ToolHandlerParams, query string) ([]DataSourceInfo, error) { + // Get dynamic client for querying DataSources + restConfig := params.RESTConfig() + if restConfig == nil { + return nil, fmt.Errorf("REST config is nil") + } + + dynamicClient, err := dynamic.NewForConfig(restConfig) + if err != nil { + // Return just the built-in containerdisk images + return getDefaultContainerDisks(), nil + } + + // DataSource GVR for CDI + dataSourceGVR := schema.GroupVersionResource{ + Group: "cdi.kubevirt.io", + Version: "v1beta1", + Resource: "datasources", + } + + // Collect DataSources from well-known namespaces and all namespaces + results := collectDataSources(params, dynamicClient, dataSourceGVR) + + // Add common containerdisk images + results = append(results, getDefaultContainerDisks()...) + + // Return helpful message if no sources found + if len(results) == 0 { + return []DataSourceInfo{ + { + Name: "No sources available", + Namespace: "", + Source: "No DataSources or containerdisks found", + }, + }, nil + } + + return results, nil +} + +// collectDataSources collects DataSources from well-known namespaces and all namespaces +func collectDataSources(params api.ToolHandlerParams, dynamicClient dynamic.Interface, gvr schema.GroupVersionResource) []DataSourceInfo { + var results []DataSourceInfo + + // Try to list DataSources from well-known namespaces first + wellKnownNamespaces := []string{ + "openshift-virtualization-os-images", + "kubevirt-os-images", + } + + for _, ns := range wellKnownNamespaces { + dsInfos, err := listDataSourcesFromNamespace(params, dynamicClient, gvr, ns) + if err == nil { + results = append(results, dsInfos...) + } + } + + // List DataSources from all namespaces + list, err := dynamicClient.Resource(gvr).List(params.Context, metav1.ListOptions{}) + if err != nil { + // If we found DataSources from well-known namespaces but couldn't list all, return what we have + if len(results) > 0 { + return results + } + // DataSources might not be available, return helpful message + return []DataSourceInfo{ + { + Name: "No DataSources found", + Namespace: "", + Source: "CDI may not be installed or DataSources are not available in this cluster", + }, + } + } + + // Deduplicate and add DataSources from all namespaces + results = deduplicateAndMergeDataSources(results, list.Items) + + return results +} + +// deduplicateAndMergeDataSources merges new DataSources with existing ones, avoiding duplicates +func deduplicateAndMergeDataSources(existing []DataSourceInfo, items []unstructured.Unstructured) []DataSourceInfo { + // Create a map to track already seen DataSources + seen := make(map[string]bool) + for _, ds := range existing { + key := ds.Namespace + "/" + ds.Name + seen[key] = true + } + + // Add new DataSources that haven't been seen + for _, item := range items { + name := item.GetName() + namespace := item.GetNamespace() + key := namespace + "/" + name + + // Skip if we've already added this DataSource + if seen[key] { + continue + } + + labels := item.GetLabels() + source := extractDataSourceInfo(&item) + + // Extract default instancetype and preference from labels + defaultInstancetype := "" + defaultPreference := "" + if labels != nil { + defaultInstancetype = labels[defaultInstancetypeLabel] + defaultPreference = labels[defaultPreferenceLabel] + } + + existing = append(existing, DataSourceInfo{ + Name: name, + Namespace: namespace, + Source: source, + DefaultInstancetype: defaultInstancetype, + DefaultPreference: defaultPreference, + }) + } + + return existing +} + +// listDataSourcesFromNamespace lists DataSources from a specific namespace +func listDataSourcesFromNamespace(params api.ToolHandlerParams, dynamicClient dynamic.Interface, gvr schema.GroupVersionResource, namespace string) ([]DataSourceInfo, error) { + var results []DataSourceInfo + list, err := dynamicClient.Resource(gvr).Namespace(namespace).List(params.Context, metav1.ListOptions{}) + if err != nil { + return nil, err + } + + for _, item := range list.Items { + name := item.GetName() + ns := item.GetNamespace() + labels := item.GetLabels() + + // Extract source information from the DataSource spec + source := extractDataSourceInfo(&item) + + // Extract default instancetype and preference from labels + defaultInstancetype := "" + defaultPreference := "" + if labels != nil { + defaultInstancetype = labels[defaultInstancetypeLabel] + defaultPreference = labels[defaultPreferenceLabel] + } + + results = append(results, DataSourceInfo{ + Name: name, + Namespace: ns, + Source: source, + DefaultInstancetype: defaultInstancetype, + DefaultPreference: defaultPreference, + }) + } + + return results, nil +} + +// searchPreferences searches for both cluster-wide and namespaced VirtualMachinePreference resources +func searchPreferences(params api.ToolHandlerParams, namespace string) []PreferenceInfo { + // Handle nil or invalid clients gracefully (e.g., in test environments) + if params.Kubernetes == nil { + return []PreferenceInfo{} + } + + restConfig := params.RESTConfig() + if restConfig == nil { + return []PreferenceInfo{} + } + + dynamicClient, err := dynamic.NewForConfig(restConfig) + if err != nil { + return []PreferenceInfo{} + } + + var results []PreferenceInfo + + // Search for cluster-wide VirtualMachineClusterPreferences + clusterPreferenceGVR := schema.GroupVersionResource{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Resource: "virtualmachineclusterpreferences", + } + + clusterList, err := dynamicClient.Resource(clusterPreferenceGVR).List(params.Context, metav1.ListOptions{}) + if err == nil { + for _, item := range clusterList.Items { + results = append(results, PreferenceInfo{ + Name: item.GetName(), + }) + } + } + + // Search for namespaced VirtualMachinePreferences + namespacedPreferenceGVR := schema.GroupVersionResource{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Resource: "virtualmachinepreferences", + } + + namespacedList, err := dynamicClient.Resource(namespacedPreferenceGVR).Namespace(namespace).List(params.Context, metav1.ListOptions{}) + if err == nil { + for _, item := range namespacedList.Items { + results = append(results, PreferenceInfo{ + Name: item.GetName(), + }) + } + } + + return results +} + +// searchInstancetypes searches for both cluster-wide and namespaced VirtualMachineInstancetype resources +func searchInstancetypes(params api.ToolHandlerParams, namespace string) []InstancetypeInfo { + // Handle nil or invalid clients gracefully (e.g., in test environments) + if params.Kubernetes == nil { + return []InstancetypeInfo{} + } + + restConfig := params.RESTConfig() + if restConfig == nil { + return []InstancetypeInfo{} + } + + dynamicClient, err := dynamic.NewForConfig(restConfig) + if err != nil { + return []InstancetypeInfo{} + } + + var results []InstancetypeInfo + + // Search for cluster-wide VirtualMachineClusterInstancetypes + clusterInstancetypeGVR := schema.GroupVersionResource{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Resource: "virtualmachineclusterinstancetypes", + } + + clusterList, err := dynamicClient.Resource(clusterInstancetypeGVR).List(params.Context, metav1.ListOptions{}) + if err == nil { + for _, item := range clusterList.Items { + results = append(results, InstancetypeInfo{ + Name: item.GetName(), + Labels: item.GetLabels(), + }) + } + } + + // Search for namespaced VirtualMachineInstancetypes + namespacedInstancetypeGVR := schema.GroupVersionResource{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Resource: "virtualmachineinstancetypes", + } + + namespacedList, err := dynamicClient.Resource(namespacedInstancetypeGVR).Namespace(namespace).List(params.Context, metav1.ListOptions{}) + if err == nil { + for _, item := range namespacedList.Items { + results = append(results, InstancetypeInfo{ + Name: item.GetName(), + Labels: item.GetLabels(), + }) + } + } + + return results +} + +// extractDataSourceInfo extracts source information from a DataSource object +func extractDataSourceInfo(obj *unstructured.Unstructured) string { + // Try to get the source from spec.source + spec, found, err := unstructured.NestedMap(obj.Object, "spec", "source") + if err != nil || !found { + return "unknown source" + } + + // Check for PVC source + if pvcInfo, found, _ := unstructured.NestedMap(spec, "pvc"); found { + if pvcName, found, _ := unstructured.NestedString(pvcInfo, "name"); found { + if pvcNamespace, found, _ := unstructured.NestedString(pvcInfo, "namespace"); found { + return fmt.Sprintf("PVC: %s/%s", pvcNamespace, pvcName) + } + return fmt.Sprintf("PVC: %s", pvcName) + } + } + + // Check for registry source + if registryInfo, found, _ := unstructured.NestedMap(spec, "registry"); found { + if url, found, _ := unstructured.NestedString(registryInfo, "url"); found { + return fmt.Sprintf("Registry: %s", url) + } + } + + // Check for http source + if url, found, _ := unstructured.NestedString(spec, "http", "url"); found { + return fmt.Sprintf("HTTP: %s", url) + } + + return "DataSource (type unknown)" +} diff --git a/pkg/toolsets/kubevirt/vm/create/tool_test.go b/pkg/toolsets/kubevirt/vm/create/tool_test.go new file mode 100644 index 00000000..7d3a834e --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/create/tool_test.go @@ -0,0 +1,205 @@ +package create + +import ( + "context" + "strings" + "testing" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" +) + +type mockToolCallRequest struct { + arguments map[string]interface{} +} + +func (m *mockToolCallRequest) GetArguments() map[string]any { + return m.arguments +} + +func TestCreate(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + wantErr bool + checkFunc func(t *testing.T, result string) + }{ + { + name: "creates VM with basic settings", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + "workload": "fedora", + }, + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "VirtualMachine Creation Plan") { + t.Errorf("Expected 'VirtualMachine Creation Plan' header in result") + } + if !strings.Contains(result, "name: test-vm") { + t.Errorf("Expected VM name test-vm in YAML") + } + if !strings.Contains(result, "namespace: test-ns") { + t.Errorf("Expected namespace test-ns in YAML") + } + if !strings.Contains(result, "quay.io/containerdisks/fedora:latest") { + t.Errorf("Expected fedora container disk in result") + } + if !strings.Contains(result, "guest: 2Gi") { + t.Errorf("Expected guest: 2Gi in YAML manifest") + } + }, + }, + { + name: "creates VM with instancetype", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + "workload": "ubuntu", + "instancetype": "u1.medium", + }, + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "name: u1.medium") { + t.Errorf("Expected instance type in YAML manifest") + } + if !strings.Contains(result, "kind: VirtualMachineClusterInstancetype") { + t.Errorf("Expected VirtualMachineClusterInstancetype in YAML manifest") + } + // When instancetype is set, memory should not be in the YAML resources section + if strings.Contains(result, "resources:\n requests:\n memory:") { + t.Errorf("Should not have memory resources when instancetype is specified") + } + }, + }, + { + name: "creates VM with preference", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + "workload": "rhel", + "preference": "rhel.9", + }, + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "name: rhel.9") { + t.Errorf("Expected preference in YAML manifest") + } + if !strings.Contains(result, "kind: VirtualMachineClusterPreference") { + t.Errorf("Expected VirtualMachineClusterPreference in YAML manifest") + } + }, + }, + { + name: "creates VM with custom container disk", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + "workload": "quay.io/myrepo/myimage:v1.0", + }, + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "quay.io/myrepo/myimage:v1.0") { + t.Errorf("Expected custom container disk in YAML") + } + }, + }, + { + name: "missing namespace", + args: map[string]interface{}{ + "name": "test-vm", + "workload": "fedora", + }, + wantErr: true, + }, + { + name: "missing name", + args: map[string]interface{}{ + "namespace": "test-ns", + "workload": "fedora", + }, + wantErr: true, + }, + { + name: "missing workload defaults to fedora", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + }, + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "quay.io/containerdisks/fedora:latest") { + t.Errorf("Expected default fedora container disk in result") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := api.ToolHandlerParams{ + Context: context.Background(), + Kubernetes: &internalk8s.Kubernetes{}, + ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, + } + + result, err := create(params) + if err != nil { + t.Errorf("create() unexpected Go error: %v", err) + return + } + + if result == nil { + t.Error("Expected non-nil result") + return + } + + if tt.wantErr { + if result.Error == nil { + t.Error("Expected error in result.Error, got nil") + } + } else { + if result.Error != nil { + t.Errorf("Expected no error in result, got: %v", result.Error) + } + if result.Content == "" { + t.Error("Expected non-empty result content") + } + if tt.checkFunc != nil { + tt.checkFunc(t, result.Content) + } + } + }) + } +} + +func TestResolveContainerDisk(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + {"fedora", "fedora", "quay.io/containerdisks/fedora:latest"}, + {"ubuntu", "ubuntu", "quay.io/containerdisks/ubuntu:24.04"}, + {"rhel8", "rhel8", "registry.redhat.io/rhel8/rhel-guest-image:latest"}, + {"rhel9", "rhel9", "registry.redhat.io/rhel9/rhel-guest-image:latest"}, + {"rhel10", "rhel10", "registry.redhat.io/rhel10/rhel-guest-image:latest"}, + {"centos", "centos", "quay.io/containerdisks/centos-stream:9-latest"}, + {"centos-stream", "centos-stream", "quay.io/containerdisks/centos-stream:9-latest"}, + {"debian", "debian", "quay.io/containerdisks/debian:latest"}, + {"case insensitive", "FEDORA", "quay.io/containerdisks/fedora:latest"}, + {"with whitespace", " ubuntu ", "quay.io/containerdisks/ubuntu:24.04"}, + {"custom image", "quay.io/myrepo/myimage:v1", "quay.io/myrepo/myimage:v1"}, + {"with tag", "myimage:latest", "myimage:latest"}, + {"unknown OS", "customos", "customos"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := resolveContainerDisk(tt.input) + if result != tt.expected { + t.Errorf("resolveContainerDisk(%s) = %s, want %s", tt.input, result, tt.expected) + } + }) + } +} diff --git a/pkg/toolsets/kubevirt/vm/troubleshoot/plan.tmpl b/pkg/toolsets/kubevirt/vm/troubleshoot/plan.tmpl new file mode 100644 index 00000000..abc9e22a --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/troubleshoot/plan.tmpl @@ -0,0 +1,188 @@ +# VirtualMachine Troubleshooting Guide + +## VM: {{.Name}} (namespace: {{.Namespace}}) + +Follow these steps to diagnose issues with the VirtualMachine: + +--- + +## Step 1: Check VirtualMachine Status + +Use the `resources_get` tool to inspect the VirtualMachine: +- **apiVersion**: `kubevirt.io/v1` +- **kind**: `VirtualMachine` +- **namespace**: `{{.Namespace}}` +- **name**: `{{.Name}}` + +**What to look for:** +- `status.printableStatus` - Should be "Running" for a healthy VM +- `status.ready` - Should be `true` +- `status.conditions` - Look for conditions with `status: "False"` or error messages +- `spec.runStrategy` - Check if it's "Always", "Manual", "Halted", or "RerunOnFailure" + +--- + +## Step 2: Check VirtualMachineInstance Status + +If the VM exists but isn't running, check if a VirtualMachineInstance was created: + +Use the `resources_get` tool: +- **apiVersion**: `kubevirt.io/v1` +- **kind**: `VirtualMachineInstance` +- **namespace**: `{{.Namespace}}` +- **name**: `{{.Name}}` + +**What to look for:** +- `status.phase` - Should be "Running" for a healthy VMI +- `status.conditions` - Check for "Ready" condition with `status: "True"` +- `status.guestOSInfo` - Confirms guest agent is running +- If VMI doesn't exist and VM runStrategy is "Always", this indicates a problem + +--- + +## Step 3: Check DataVolume Status (if applicable) + +If the VM uses DataVolumeTemplates, check their status: + +Use the `resources_list` tool: +- **apiVersion**: `cdi.kubevirt.io/v1beta1` +- **kind**: `DataVolume` +- **namespace**: `{{.Namespace}}` + +Look for DataVolumes with names starting with `{{.Name}}-` + +**What to look for:** +- `status.phase` - Should be "Succeeded" when ready +- `status.progress` - Shows import/clone progress (e.g., "100.0%") +- Common issues: + - Phase "Pending" - Waiting for resources + - Phase "ImportScheduled" or "ImportInProgress" - Still importing + - Phase "Failed" - Check `status.conditions` for error details + +### Check Underlying PersistentVolumeClaims + +DataVolumes create PVCs to provision storage. Check the PVC status: + +Use the `resources_list` tool: +- **apiVersion**: `v1` +- **kind**: `PersistentVolumeClaim` +- **namespace**: `{{.Namespace}}` + +Look for PVCs with names matching the DataVolume names (typically `{{.Name}}-*`) + +Or inspect a specific PVC with `resources_get`: +- **apiVersion**: `v1` +- **kind**: `PersistentVolumeClaim` +- **namespace**: `{{.Namespace}}` +- **name**: (name from DataVolume or VM volumes) + +**What to look for:** +- `status.phase` - Should be "Bound" when ready +- `spec.storageClassName` - Verify the storage class exists and is available +- `status.capacity.storage` - Confirms allocated storage size +- Common PVC issues: + - Phase "Pending" - Storage class not available, insufficient storage, or provisioner issues + - Missing PVC - DataVolume creation may have failed + - Incorrect size - Check if requested size matches available storage + +**Check Storage Class:** + +If PVC is stuck in "Pending", verify the storage class exists: + +Use the `resources_get` tool: +- **apiVersion**: `storage.k8s.io/v1` +- **kind**: `StorageClass` +- **name**: (from PVC `spec.storageClassName`) + +Ensure the storage class provisioner is healthy and has capacity. + +--- + +## Step 4: Check virt-launcher Pod + +The virt-launcher pod runs the actual VM. Find and inspect it: + +Use the `pods_list_in_namespace` tool: +- **namespace**: `{{.Namespace}}` +- **labelSelector**: `kubevirt.io=virt-launcher,vm.kubevirt.io/name={{.Name}}` + +**What to look for:** +- Pod should be in "Running" phase +- All containers should be ready (e.g., "2/2") +- Check pod events and conditions for errors + +If pod exists, get detailed status with `pods_get`: +- **namespace**: `{{.Namespace}}` +- **name**: `virt-launcher-{{.Name}}-xxxxx` (use actual pod name from list) + +Get pod logs with `pods_log`: +- **namespace**: `{{.Namespace}}` +- **name**: `virt-launcher-{{.Name}}-xxxxx` +- **container**: `compute` (main VM container) + +--- + +## Step 5: Check Events + +Events provide crucial diagnostic information: + +Use the `events_list` tool: +- **namespace**: `{{.Namespace}}` + +Filter output for events related to `{{.Name}}` - look for warnings or errors. + +--- + +## Step 6: Check Instance Type and Preference (if used) + +If the VM uses instance types or preferences, verify they exist: + +For instance types, use `resources_get`: +- **apiVersion**: `instancetype.kubevirt.io/v1beta1` +- **kind**: `VirtualMachineClusterInstancetype` +- **name**: (check VM spec for instancetype name) + +For preferences, use `resources_get`: +- **apiVersion**: `instancetype.kubevirt.io/v1beta1` +- **kind**: `VirtualMachineClusterPreference` +- **name**: (check VM spec for preference name) + +--- + +## Common Issues and Solutions + +### VM stuck in "Stopped" or "Halted" +- Check `spec.runStrategy` - if "Halted", the VM is intentionally stopped +- Change runStrategy to "Always" to start the VM + +### VMI doesn't exist +- Check VM conditions for admission errors +- Verify instance type and preference exist +- Check resource quotas in the namespace + +### DataVolume stuck in "ImportInProgress" +- Check CDI controller pods in `cdi` namespace +- Verify source image is accessible +- Check PVC storage class exists and has available capacity + +### virt-launcher pod in CrashLoopBackOff +- Check pod logs for container `compute` +- Common causes: + - Insufficient resources (CPU/memory) + - Invalid VM configuration + - Storage issues (PVC not available) + +### VM starts but guest doesn't boot +- Check virt-launcher logs for QEMU errors +- Verify boot disk is properly configured +- Check if guest agent is installed (for cloud images) +- Ensure correct architecture (amd64 vs arm64) + +--- + +## Additional Resources + +For more detailed diagnostics: +- Check KubeVirt components: `pods_list` in `kubevirt` namespace +- Check CDI components: `pods_list` in `cdi` namespace (if using DataVolumes) +- Review resource consumption: `pods_top` for the virt-launcher pod diff --git a/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go b/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go new file mode 100644 index 00000000..7e0f8ead --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go @@ -0,0 +1,98 @@ +package troubleshoot + +import ( + _ "embed" + "fmt" + "strings" + "text/template" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + "github.com/google/jsonschema-go/jsonschema" + "k8s.io/utils/ptr" +) + +//go:embed plan.tmpl +var planTemplate string + +func Tools() []api.ServerTool { + return []api.ServerTool{ + { + Tool: api.Tool{ + Name: "vm_troubleshoot", + Description: "Generate a comprehensive troubleshooting guide for a VirtualMachine, providing step-by-step instructions to diagnose common issues", + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "namespace": { + Type: "string", + Description: "The namespace of the virtual machine", + }, + "name": { + Type: "string", + Description: "The name of the virtual machine", + }, + }, + Required: []string{"namespace", "name"}, + }, + Annotations: api.ToolAnnotations{ + Title: "Virtual Machine: Troubleshoot", + ReadOnlyHint: ptr.To(true), + DestructiveHint: ptr.To(false), + IdempotentHint: ptr.To(true), + OpenWorldHint: ptr.To(false), + }, + }, + Handler: troubleshoot, + }, + } +} + +type troubleshootParams struct { + Namespace string + Name string +} + +func troubleshoot(params api.ToolHandlerParams) (*api.ToolCallResult, error) { + // Parse required parameters + namespace, err := getRequiredString(params, "namespace") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + name, err := getRequiredString(params, "name") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + // Prepare template parameters + templateParams := troubleshootParams{ + Namespace: namespace, + Name: name, + } + + // Render template + tmpl, err := template.New("troubleshoot").Parse(planTemplate) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to parse template: %w", err)), nil + } + + var result strings.Builder + if err := tmpl.Execute(&result, templateParams); err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to render template: %w", err)), nil + } + + return api.NewToolCallResult(result.String(), nil), nil +} + +func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { + args := params.GetArguments() + val, ok := args[key] + if !ok { + return "", fmt.Errorf("%s parameter required", key) + } + str, ok := val.(string) + if !ok { + return "", fmt.Errorf("%s parameter must be a string", key) + } + return str, nil +} diff --git a/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go b/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go new file mode 100644 index 00000000..8d371d42 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go @@ -0,0 +1,110 @@ +package troubleshoot + +import ( + "context" + "strings" + "testing" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" +) + +type mockToolCallRequest struct { + arguments map[string]interface{} +} + +func (m *mockToolCallRequest) GetArguments() map[string]any { + return m.arguments +} + +func TestTroubleshoot(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + wantErr bool + checkFunc func(t *testing.T, result string) + }{ + { + name: "generates troubleshooting guide", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + }, + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "VirtualMachine Troubleshooting Guide") { + t.Errorf("Expected troubleshooting guide header") + } + if !strings.Contains(result, "test-vm") { + t.Errorf("Expected VM name in guide") + } + if !strings.Contains(result, "test-ns") { + t.Errorf("Expected namespace in guide") + } + if !strings.Contains(result, "Step 1: Check VirtualMachine Status") { + t.Errorf("Expected step 1 header") + } + if !strings.Contains(result, "resources_get") { + t.Errorf("Expected resources_get tool reference") + } + if !strings.Contains(result, "VirtualMachineInstance") { + t.Errorf("Expected VMI section") + } + if !strings.Contains(result, "virt-launcher") { + t.Errorf("Expected virt-launcher pod section") + } + }, + }, + { + name: "missing namespace", + args: map[string]interface{}{ + "name": "test-vm", + }, + wantErr: true, + }, + { + name: "missing name", + args: map[string]interface{}{ + "namespace": "test-ns", + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := api.ToolHandlerParams{ + Context: context.Background(), + Kubernetes: &internalk8s.Kubernetes{}, + ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, + } + + result, err := troubleshoot(params) + if err != nil { + t.Errorf("troubleshoot() unexpected Go error: %v", err) + return + } + + if result == nil { + t.Error("Expected non-nil result") + return + } + + if tt.wantErr { + if result.Error == nil { + t.Error("Expected error in result.Error, got nil") + } + } else { + if result.Error != nil { + t.Errorf("Expected no error in result, got: %v", result.Error) + } + if result.Content == "" { + t.Error("Expected non-empty result content") + } + if tt.checkFunc != nil { + tt.checkFunc(t, result.Content) + } + } + }) + } +} From edeed84b51ac258fef3435dad54e36be960c53c9 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Fri, 7 Nov 2025 13:20:43 +0000 Subject: [PATCH 2/8] feat(kubevirt): Add vm_start and vm_stop tools Add lifecycle management tools for starting and stopping VirtualMachines. These tools provide simple, single-action operations that prevent destructive workarounds like delete/recreate. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/toolsets/kubevirt/toolset.go | 4 + pkg/toolsets/kubevirt/vm/start/tool.go | 123 ++++++++++++++++++++ pkg/toolsets/kubevirt/vm/start/tool_test.go | 101 ++++++++++++++++ pkg/toolsets/kubevirt/vm/stop/tool.go | 123 ++++++++++++++++++++ pkg/toolsets/kubevirt/vm/stop/tool_test.go | 101 ++++++++++++++++ 5 files changed, 452 insertions(+) create mode 100644 pkg/toolsets/kubevirt/vm/start/tool.go create mode 100644 pkg/toolsets/kubevirt/vm/start/tool_test.go create mode 100644 pkg/toolsets/kubevirt/vm/stop/tool.go create mode 100644 pkg/toolsets/kubevirt/vm/stop/tool_test.go diff --git a/pkg/toolsets/kubevirt/toolset.go b/pkg/toolsets/kubevirt/toolset.go index f8b21137..41257960 100644 --- a/pkg/toolsets/kubevirt/toolset.go +++ b/pkg/toolsets/kubevirt/toolset.go @@ -7,6 +7,8 @@ import ( internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" "github.com/containers/kubernetes-mcp-server/pkg/toolsets" vm_create "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/create" + vm_start "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/start" + vm_stop "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/stop" vm_troubleshoot "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/troubleshoot" ) @@ -25,6 +27,8 @@ func (t *Toolset) GetDescription() string { func (t *Toolset) GetTools(o internalk8s.Openshift) []api.ServerTool { return slices.Concat( vm_create.Tools(), + vm_start.Tools(), + vm_stop.Tools(), vm_troubleshoot.Tools(), ) } diff --git a/pkg/toolsets/kubevirt/vm/start/tool.go b/pkg/toolsets/kubevirt/vm/start/tool.go new file mode 100644 index 00000000..b2784d2f --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/start/tool.go @@ -0,0 +1,123 @@ +package start + +import ( + "fmt" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + "github.com/containers/kubernetes-mcp-server/pkg/output" + "github.com/google/jsonschema-go/jsonschema" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/utils/ptr" +) + +func Tools() []api.ServerTool { + return []api.ServerTool{ + { + Tool: api.Tool{ + Name: "vm_start", + Description: "Start a halted or stopped VirtualMachine by changing its runStrategy to Always", + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "namespace": { + Type: "string", + Description: "The namespace of the virtual machine", + }, + "name": { + Type: "string", + Description: "The name of the virtual machine to start", + }, + }, + Required: []string{"namespace", "name"}, + }, + Annotations: api.ToolAnnotations{ + Title: "Virtual Machine: Start", + ReadOnlyHint: ptr.To(false), + DestructiveHint: ptr.To(false), + IdempotentHint: ptr.To(true), + OpenWorldHint: ptr.To(false), + }, + }, + Handler: start, + }, + } +} + +func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) { + // Parse required parameters + namespace, err := getRequiredString(params, "namespace") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + name, err := getRequiredString(params, "name") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + // Get dynamic client + restConfig := params.RESTConfig() + if restConfig == nil { + return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil + } + + dynamicClient, err := dynamic.NewForConfig(restConfig) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil + } + + // Get the current VM + gvr := schema.GroupVersionResource{ + Group: "kubevirt.io", + Version: "v1", + Resource: "virtualmachines", + } + + vm, err := dynamicClient.Resource(gvr).Namespace(namespace).Get( + params.Context, + name, + metav1.GetOptions{}, + ) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to get VirtualMachine: %w", err)), nil + } + + // Update runStrategy to Always + if err := unstructured.SetNestedField(vm.Object, "Always", "spec", "runStrategy"); err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to set runStrategy: %w", err)), nil + } + + // Update the VM + updatedVM, err := dynamicClient.Resource(gvr).Namespace(namespace).Update( + params.Context, + vm, + metav1.UpdateOptions{}, + ) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to update VirtualMachine: %w", err)), nil + } + + // Format the output + marshalledYaml, err := output.MarshalYaml(updatedVM) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to marshal VirtualMachine: %w", err)), nil + } + + return api.NewToolCallResult("# VirtualMachine started successfully\n"+marshalledYaml, nil), nil +} + +func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { + args := params.GetArguments() + val, ok := args[key] + if !ok { + return "", fmt.Errorf("%s parameter required", key) + } + str, ok := val.(string) + if !ok { + return "", fmt.Errorf("%s parameter must be a string", key) + } + return str, nil +} diff --git a/pkg/toolsets/kubevirt/vm/start/tool_test.go b/pkg/toolsets/kubevirt/vm/start/tool_test.go new file mode 100644 index 00000000..6c207c90 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/start/tool_test.go @@ -0,0 +1,101 @@ +package start + +import ( + "context" + "testing" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" +) + +type mockToolCallRequest struct { + arguments map[string]interface{} +} + +func (m *mockToolCallRequest) GetArguments() map[string]any { + return m.arguments +} + +func TestStartParameterValidation(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + wantErr bool + errMsg string + }{ + { + name: "missing namespace parameter", + args: map[string]interface{}{ + "name": "test-vm", + }, + wantErr: true, + errMsg: "namespace parameter required", + }, + { + name: "missing name parameter", + args: map[string]interface{}{ + "namespace": "test-ns", + }, + wantErr: true, + errMsg: "name parameter required", + }, + { + name: "invalid namespace type", + args: map[string]interface{}{ + "namespace": 123, + "name": "test-vm", + }, + wantErr: true, + errMsg: "namespace parameter must be a string", + }, + { + name: "invalid name type", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": 456, + }, + wantErr: true, + errMsg: "name parameter must be a string", + }, + { + name: "valid parameters - cluster interaction expected", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + }, + wantErr: true, // Will fail due to missing cluster connection, but parameters are valid + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := api.ToolHandlerParams{ + Context: context.Background(), + Kubernetes: &internalk8s.Kubernetes{}, + ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, + } + + result, err := start(params) + if err != nil { + t.Errorf("start() unexpected Go error: %v", err) + return + } + + if result == nil { + t.Error("Expected non-nil result") + return + } + + // For parameter validation errors, check the error message + if tt.wantErr && tt.errMsg != "" { + if result.Error == nil { + t.Error("Expected error in result.Error, got nil") + return + } + if result.Error.Error() != tt.errMsg { + t.Errorf("Expected error message %q, got %q", tt.errMsg, result.Error.Error()) + } + } + }) + } +} diff --git a/pkg/toolsets/kubevirt/vm/stop/tool.go b/pkg/toolsets/kubevirt/vm/stop/tool.go new file mode 100644 index 00000000..6ab03485 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/stop/tool.go @@ -0,0 +1,123 @@ +package stop + +import ( + "fmt" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + "github.com/containers/kubernetes-mcp-server/pkg/output" + "github.com/google/jsonschema-go/jsonschema" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/utils/ptr" +) + +func Tools() []api.ServerTool { + return []api.ServerTool{ + { + Tool: api.Tool{ + Name: "vm_stop", + Description: "Stop a running VirtualMachine by changing its runStrategy to Halted", + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "namespace": { + Type: "string", + Description: "The namespace of the virtual machine", + }, + "name": { + Type: "string", + Description: "The name of the virtual machine to stop", + }, + }, + Required: []string{"namespace", "name"}, + }, + Annotations: api.ToolAnnotations{ + Title: "Virtual Machine: Stop", + ReadOnlyHint: ptr.To(false), + DestructiveHint: ptr.To(false), + IdempotentHint: ptr.To(true), + OpenWorldHint: ptr.To(false), + }, + }, + Handler: stop, + }, + } +} + +func stop(params api.ToolHandlerParams) (*api.ToolCallResult, error) { + // Parse required parameters + namespace, err := getRequiredString(params, "namespace") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + name, err := getRequiredString(params, "name") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + // Get dynamic client + restConfig := params.RESTConfig() + if restConfig == nil { + return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil + } + + dynamicClient, err := dynamic.NewForConfig(restConfig) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil + } + + // Get the current VM + gvr := schema.GroupVersionResource{ + Group: "kubevirt.io", + Version: "v1", + Resource: "virtualmachines", + } + + vm, err := dynamicClient.Resource(gvr).Namespace(namespace).Get( + params.Context, + name, + metav1.GetOptions{}, + ) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to get VirtualMachine: %w", err)), nil + } + + // Update runStrategy to Halted + if err := unstructured.SetNestedField(vm.Object, "Halted", "spec", "runStrategy"); err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to set runStrategy: %w", err)), nil + } + + // Update the VM + updatedVM, err := dynamicClient.Resource(gvr).Namespace(namespace).Update( + params.Context, + vm, + metav1.UpdateOptions{}, + ) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to update VirtualMachine: %w", err)), nil + } + + // Format the output + marshalledYaml, err := output.MarshalYaml(updatedVM) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to marshal VirtualMachine: %w", err)), nil + } + + return api.NewToolCallResult("# VirtualMachine stopped successfully\n"+marshalledYaml, nil), nil +} + +func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { + args := params.GetArguments() + val, ok := args[key] + if !ok { + return "", fmt.Errorf("%s parameter required", key) + } + str, ok := val.(string) + if !ok { + return "", fmt.Errorf("%s parameter must be a string", key) + } + return str, nil +} diff --git a/pkg/toolsets/kubevirt/vm/stop/tool_test.go b/pkg/toolsets/kubevirt/vm/stop/tool_test.go new file mode 100644 index 00000000..bcb8a206 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/stop/tool_test.go @@ -0,0 +1,101 @@ +package stop + +import ( + "context" + "testing" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" +) + +type mockToolCallRequest struct { + arguments map[string]interface{} +} + +func (m *mockToolCallRequest) GetArguments() map[string]any { + return m.arguments +} + +func TestStopParameterValidation(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + wantErr bool + errMsg string + }{ + { + name: "missing namespace parameter", + args: map[string]interface{}{ + "name": "test-vm", + }, + wantErr: true, + errMsg: "namespace parameter required", + }, + { + name: "missing name parameter", + args: map[string]interface{}{ + "namespace": "test-ns", + }, + wantErr: true, + errMsg: "name parameter required", + }, + { + name: "invalid namespace type", + args: map[string]interface{}{ + "namespace": 123, + "name": "test-vm", + }, + wantErr: true, + errMsg: "namespace parameter must be a string", + }, + { + name: "invalid name type", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": 456, + }, + wantErr: true, + errMsg: "name parameter must be a string", + }, + { + name: "valid parameters - cluster interaction expected", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + }, + wantErr: true, // Will fail due to missing cluster connection, but parameters are valid + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := api.ToolHandlerParams{ + Context: context.Background(), + Kubernetes: &internalk8s.Kubernetes{}, + ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, + } + + result, err := stop(params) + if err != nil { + t.Errorf("stop() unexpected Go error: %v", err) + return + } + + if result == nil { + t.Error("Expected non-nil result") + return + } + + // For parameter validation errors, check the error message + if tt.wantErr && tt.errMsg != "" { + if result.Error == nil { + t.Error("Expected error in result.Error, got nil") + return + } + if result.Error.Error() != tt.errMsg { + t.Errorf("Expected error message %q, got %q", tt.errMsg, result.Error.Error()) + } + } + }) + } +} From 429030e98f9b1b3c6e6f969fa5b7683e37f520cc Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Fri, 7 Nov 2025 13:23:07 +0000 Subject: [PATCH 3/8] feat(kubevirt): Add autostart parameter to vm_create Add optional autostart parameter to vm_create tool that sets runStrategy to Always instead of Halted, allowing VMs to be created and started in a single operation. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/toolsets/kubevirt/vm/create/tool.go | 63 ++++++-- pkg/toolsets/kubevirt/vm/create/tool_test.go | 157 +++++++++---------- pkg/toolsets/kubevirt/vm/create/vm.yaml.tmpl | 50 ++++++ 3 files changed, 177 insertions(+), 93 deletions(-) create mode 100644 pkg/toolsets/kubevirt/vm/create/vm.yaml.tmpl diff --git a/pkg/toolsets/kubevirt/vm/create/tool.go b/pkg/toolsets/kubevirt/vm/create/tool.go index 287f03a4..33c8fb5a 100644 --- a/pkg/toolsets/kubevirt/vm/create/tool.go +++ b/pkg/toolsets/kubevirt/vm/create/tool.go @@ -7,6 +7,7 @@ import ( "text/template" "github.com/containers/kubernetes-mcp-server/pkg/api" + "github.com/containers/kubernetes-mcp-server/pkg/output" "github.com/google/jsonschema-go/jsonschema" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -20,15 +21,15 @@ const ( defaultPreferenceLabel = "instancetype.kubevirt.io/default-preference" ) -//go:embed plan.tmpl -var planTemplate string +//go:embed vm.yaml.tmpl +var vmYamlTemplate string func Tools() []api.ServerTool { return []api.ServerTool{ { Tool: api.Tool{ Name: "vm_create", - Description: "Generate a comprehensive creation plan for a VirtualMachine, including pre-creation checks for instance types, preferences, and container disk images", + Description: "Create a VirtualMachine in the cluster with the specified configuration, automatically resolving instance types, preferences, and container disk images. VM will be created in Halted state by default; use autostart parameter to start it immediately.", InputSchema: &jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ @@ -63,13 +64,17 @@ func Tools() []api.ServerTool { Description: "Optional performance family hint for the VM instance type (e.g., 'u1' for general-purpose, 'o1' for overcommitted, 'c1' for compute-optimized, 'm1' for memory-optimized). Defaults to 'u1' (general-purpose) if not specified.", Examples: []interface{}{"general-purpose", "overcommitted", "compute-optimized", "memory-optimized"}, }, + "autostart": { + Type: "boolean", + Description: "Optional flag to automatically start the VM after creation (sets runStrategy to Always instead of Halted). Defaults to false.", + }, }, Required: []string{"namespace", "name"}, }, Annotations: api.ToolAnnotations{ Title: "Virtual Machine: Create", - ReadOnlyHint: ptr.To(true), - DestructiveHint: ptr.To(false), + ReadOnlyHint: ptr.To(false), + DestructiveHint: ptr.To(true), IdempotentHint: ptr.To(true), OpenWorldHint: ptr.To(false), }, @@ -88,6 +93,7 @@ type vmParams struct { UseDataSource bool DataSourceName string DataSourceNamespace string + RunStrategy string } type DataSourceInfo struct { @@ -129,13 +135,25 @@ func create(params api.ToolHandlerParams) (*api.ToolCallResult, error) { // Build template parameters from resolved resources templateParams := buildTemplateParams(createParams, matchedDataSource, instancetype, preference) - // Render the VM creation plan template - result, err := renderTemplate(templateParams) + // Render the VM YAML + vmYaml, err := renderVMYaml(templateParams) if err != nil { return api.NewToolCallResult("", err), nil } - return api.NewToolCallResult(result, nil), nil + // Create the VM in the cluster + resources, err := params.ResourcesCreateOrUpdate(params, vmYaml) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to create VirtualMachine: %w", err)), nil + } + + // Format the output + marshalledYaml, err := output.MarshalYaml(resources) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to marshal created VirtualMachine: %w", err)), nil + } + + return api.NewToolCallResult("# VirtualMachine created successfully\n"+marshalledYaml, nil), nil } // createParameters holds parsed input parameters for VM creation @@ -147,6 +165,7 @@ type createParameters struct { Preference string Size string Performance string + Autostart bool } // parseCreateParameters parses and validates input parameters @@ -167,6 +186,7 @@ func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, err } performance := normalizePerformance(getOptionalString(params, "performance")) + autostart := getOptionalBool(params, "autostart") return &createParameters{ Namespace: namespace, @@ -176,6 +196,7 @@ func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, err Preference: getOptionalString(params, "preference"), Size: getOptionalString(params, "size"), Performance: performance, + Autostart: autostart, }, nil } @@ -301,11 +322,18 @@ func filterInstancetypesBySize(instancetypes []InstancetypeInfo, normalizedSize // buildTemplateParams constructs the template parameters for VM creation func buildTemplateParams(createParams *createParameters, matchedDataSource *DataSourceInfo, instancetype, preference string) vmParams { + // Determine runStrategy based on autostart parameter + runStrategy := "Halted" + if createParams.Autostart { + runStrategy = "Always" + } + params := vmParams{ Namespace: createParams.Namespace, Name: createParams.Name, Instancetype: instancetype, Preference: preference, + RunStrategy: runStrategy, } if matchedDataSource != nil && matchedDataSource.Namespace != "" { @@ -324,9 +352,9 @@ func buildTemplateParams(createParams *createParameters, matchedDataSource *Data return params } -// renderTemplate renders the VM creation plan template -func renderTemplate(templateParams vmParams) (string, error) { - tmpl, err := template.New("vm").Parse(planTemplate) +// renderVMYaml renders the VM YAML from template +func renderVMYaml(templateParams vmParams) (string, error) { + tmpl, err := template.New("vm").Parse(vmYamlTemplate) if err != nil { return "", fmt.Errorf("failed to parse template: %w", err) } @@ -398,6 +426,19 @@ func getOptionalString(params api.ToolHandlerParams, key string) string { return str } +func getOptionalBool(params api.ToolHandlerParams, key string) bool { + args := params.GetArguments() + val, ok := args[key] + if !ok { + return false + } + b, ok := val.(bool) + if !ok { + return false + } + return b +} + // resolveContainerDisk resolves OS names to container disk images from quay.io/containerdisks func resolveContainerDisk(input string) string { // If input already looks like a container image, return as-is diff --git a/pkg/toolsets/kubevirt/vm/create/tool_test.go b/pkg/toolsets/kubevirt/vm/create/tool_test.go index 7d3a834e..00742bc0 100644 --- a/pkg/toolsets/kubevirt/vm/create/tool_test.go +++ b/pkg/toolsets/kubevirt/vm/create/tool_test.go @@ -1,40 +1,33 @@ package create import ( - "context" "strings" "testing" - - "github.com/containers/kubernetes-mcp-server/pkg/api" - internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" ) -type mockToolCallRequest struct { - arguments map[string]interface{} -} - -func (m *mockToolCallRequest) GetArguments() map[string]any { - return m.arguments -} - -func TestCreate(t *testing.T) { +// Test the YAML rendering directly without creating resources +func TestRenderVMYaml(t *testing.T) { tests := []struct { name string - args map[string]interface{} + params vmParams wantErr bool checkFunc func(t *testing.T, result string) }{ { - name: "creates VM with basic settings", - args: map[string]interface{}{ - "namespace": "test-ns", - "name": "test-vm", - "workload": "fedora", + name: "renders VM with basic settings", + params: vmParams{ + Namespace: "test-ns", + Name: "test-vm", + ContainerDisk: "quay.io/containerdisks/fedora:latest", + RunStrategy: "Halted", }, wantErr: false, checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "VirtualMachine Creation Plan") { - t.Errorf("Expected 'VirtualMachine Creation Plan' header in result") + if !strings.Contains(result, "apiVersion: kubevirt.io/v1") { + t.Errorf("Expected apiVersion in YAML") + } + if !strings.Contains(result, "kind: VirtualMachine") { + t.Errorf("Expected kind VirtualMachine in YAML") } if !strings.Contains(result, "name: test-vm") { t.Errorf("Expected VM name test-vm in YAML") @@ -51,12 +44,13 @@ func TestCreate(t *testing.T) { }, }, { - name: "creates VM with instancetype", - args: map[string]interface{}{ - "namespace": "test-ns", - "name": "test-vm", - "workload": "ubuntu", - "instancetype": "u1.medium", + name: "renders VM with instancetype", + params: vmParams{ + Namespace: "test-ns", + Name: "test-vm", + ContainerDisk: "quay.io/containerdisks/ubuntu:24.04", + Instancetype: "u1.medium", + RunStrategy: "Halted", }, wantErr: false, checkFunc: func(t *testing.T, result string) { @@ -66,19 +60,20 @@ func TestCreate(t *testing.T) { if !strings.Contains(result, "kind: VirtualMachineClusterInstancetype") { t.Errorf("Expected VirtualMachineClusterInstancetype in YAML manifest") } - // When instancetype is set, memory should not be in the YAML resources section - if strings.Contains(result, "resources:\n requests:\n memory:") { - t.Errorf("Should not have memory resources when instancetype is specified") + // When instancetype is set, memory should not be in the YAML + if strings.Contains(result, "guest: 2Gi") { + t.Errorf("Should not have guest memory when instancetype is specified") } }, }, { - name: "creates VM with preference", - args: map[string]interface{}{ - "namespace": "test-ns", - "name": "test-vm", - "workload": "rhel", - "preference": "rhel.9", + name: "renders VM with preference", + params: vmParams{ + Namespace: "test-ns", + Name: "test-vm", + ContainerDisk: "registry.redhat.io/rhel9/rhel-guest-image:latest", + Preference: "rhel.9", + RunStrategy: "Halted", }, wantErr: false, checkFunc: func(t *testing.T, result string) { @@ -91,11 +86,12 @@ func TestCreate(t *testing.T) { }, }, { - name: "creates VM with custom container disk", - args: map[string]interface{}{ - "namespace": "test-ns", - "name": "test-vm", - "workload": "quay.io/myrepo/myimage:v1.0", + name: "renders VM with custom container disk", + params: vmParams{ + Namespace: "test-ns", + Name: "test-vm", + ContainerDisk: "quay.io/myrepo/myimage:v1.0", + RunStrategy: "Halted", }, wantErr: false, checkFunc: func(t *testing.T, result string) { @@ -105,31 +101,43 @@ func TestCreate(t *testing.T) { }, }, { - name: "missing namespace", - args: map[string]interface{}{ - "name": "test-vm", - "workload": "fedora", + name: "renders VM with DataSource", + params: vmParams{ + Namespace: "test-ns", + Name: "test-vm", + UseDataSource: true, + DataSourceName: "fedora", + DataSourceNamespace: "openshift-virtualization-os-images", + RunStrategy: "Halted", }, - wantErr: true, - }, - { - name: "missing name", - args: map[string]interface{}{ - "namespace": "test-ns", - "workload": "fedora", + wantErr: false, + checkFunc: func(t *testing.T, result string) { + if !strings.Contains(result, "dataVolumeTemplates") { + t.Errorf("Expected dataVolumeTemplates in YAML") + } + if !strings.Contains(result, "kind: DataSource") { + t.Errorf("Expected DataSource kind in YAML") + } + if !strings.Contains(result, "name: fedora") { + t.Errorf("Expected DataSource name in YAML") + } + if !strings.Contains(result, "openshift-virtualization-os-images") { + t.Errorf("Expected DataSource namespace in YAML") + } }, - wantErr: true, }, { - name: "missing workload defaults to fedora", - args: map[string]interface{}{ - "namespace": "test-ns", - "name": "test-vm", + name: "renders VM with autostart (runStrategy Always)", + params: vmParams{ + Namespace: "test-ns", + Name: "test-vm", + ContainerDisk: "quay.io/containerdisks/fedora:latest", + RunStrategy: "Always", }, wantErr: false, checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "quay.io/containerdisks/fedora:latest") { - t.Errorf("Expected default fedora container disk in result") + if !strings.Contains(result, "runStrategy: Always") { + t.Errorf("Expected runStrategy: Always in YAML") } }, }, @@ -137,36 +145,21 @@ func TestCreate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - params := api.ToolHandlerParams{ - Context: context.Background(), - Kubernetes: &internalk8s.Kubernetes{}, - ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, - } - - result, err := create(params) - if err != nil { - t.Errorf("create() unexpected Go error: %v", err) - return - } - - if result == nil { - t.Error("Expected non-nil result") - return - } + result, err := renderVMYaml(tt.params) if tt.wantErr { - if result.Error == nil { - t.Error("Expected error in result.Error, got nil") + if err == nil { + t.Error("Expected error, got nil") } } else { - if result.Error != nil { - t.Errorf("Expected no error in result, got: %v", result.Error) + if err != nil { + t.Errorf("Expected no error, got: %v", err) } - if result.Content == "" { - t.Error("Expected non-empty result content") + if result == "" { + t.Error("Expected non-empty result") } if tt.checkFunc != nil { - tt.checkFunc(t, result.Content) + tt.checkFunc(t, result) } } }) diff --git a/pkg/toolsets/kubevirt/vm/create/vm.yaml.tmpl b/pkg/toolsets/kubevirt/vm/create/vm.yaml.tmpl new file mode 100644 index 00000000..9982d4a9 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/create/vm.yaml.tmpl @@ -0,0 +1,50 @@ +apiVersion: kubevirt.io/v1 +kind: VirtualMachine +metadata: + name: {{.Name}} + namespace: {{.Namespace}} +spec: + runStrategy: {{.RunStrategy}} +{{- if .Instancetype}} + instancetype: + name: {{.Instancetype}} + kind: VirtualMachineClusterInstancetype +{{- end}} +{{- if .Preference}} + preference: + name: {{.Preference}} + kind: VirtualMachineClusterPreference +{{- end}} +{{- if .UseDataSource}} + dataVolumeTemplates: + - metadata: + name: {{.Name}}-rootdisk + spec: + sourceRef: + kind: DataSource + name: {{.DataSourceName}} + namespace: {{.DataSourceNamespace}} + storage: + resources: + requests: + storage: 30Gi +{{- end}} + template: + spec: + domain: + devices: + disks: + - name: {{.Name}}-rootdisk +{{- if not .Instancetype}} + memory: + guest: 2Gi +{{- end}} + volumes: + - name: {{.Name}}-rootdisk +{{- if .UseDataSource}} + dataVolume: + name: {{.Name}}-rootdisk +{{- else}} + containerDisk: + image: {{.ContainerDisk}} +{{- end}} From 5edf6f5ee6e2925e74d9825364d85e29b250d90f Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Mon, 10 Nov 2025 11:49:09 +0000 Subject: [PATCH 4/8] refactor(api): Add parameter helper methods to ToolHandlerParams Add GetRequiredString, GetOptionalString, and GetOptionalBool methods to ToolHandlerParams type to eliminate code duplication across kubevirt VM tools. These methods provide a cleaner, reusable API for extracting parameters from tool call arguments. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/api/toolsets.go | 46 +++++++++++++++ pkg/toolsets/kubevirt/vm/create/tool.go | 58 +++---------------- pkg/toolsets/kubevirt/vm/start/tool.go | 17 +----- pkg/toolsets/kubevirt/vm/stop/tool.go | 17 +----- pkg/toolsets/kubevirt/vm/troubleshoot/tool.go | 17 +----- 5 files changed, 60 insertions(+), 95 deletions(-) diff --git a/pkg/api/toolsets.go b/pkg/api/toolsets.go index c5960e3f..24f61618 100644 --- a/pkg/api/toolsets.go +++ b/pkg/api/toolsets.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "fmt" internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" "github.com/containers/kubernetes-mcp-server/pkg/output" @@ -71,6 +72,51 @@ type ToolHandlerParams struct { ListOutput output.Output } +// GetRequiredString extracts a required string parameter from the tool call arguments. +// Returns an error if the parameter is missing or not a string. +func (p ToolHandlerParams) GetRequiredString(key string) (string, error) { + args := p.GetArguments() + val, ok := args[key] + if !ok { + return "", fmt.Errorf("%s parameter required", key) + } + str, ok := val.(string) + if !ok { + return "", fmt.Errorf("%s parameter must be a string", key) + } + return str, nil +} + +// GetOptionalString extracts an optional string parameter from the tool call arguments. +// Returns an empty string if the parameter is missing or not a string. +func (p ToolHandlerParams) GetOptionalString(key string) string { + args := p.GetArguments() + val, ok := args[key] + if !ok { + return "" + } + str, ok := val.(string) + if !ok { + return "" + } + return str +} + +// GetOptionalBool extracts an optional boolean parameter from the tool call arguments. +// Returns false if the parameter is missing or not a boolean. +func (p ToolHandlerParams) GetOptionalBool(key string) bool { + args := p.GetArguments() + val, ok := args[key] + if !ok { + return false + } + b, ok := val.(bool) + if !ok { + return false + } + return b +} + type ToolHandlerFunc func(params ToolHandlerParams) (*ToolCallResult, error) type Tool struct { diff --git a/pkg/toolsets/kubevirt/vm/create/tool.go b/pkg/toolsets/kubevirt/vm/create/tool.go index 33c8fb5a..5ccd5437 100644 --- a/pkg/toolsets/kubevirt/vm/create/tool.go +++ b/pkg/toolsets/kubevirt/vm/create/tool.go @@ -170,33 +170,30 @@ type createParameters struct { // parseCreateParameters parses and validates input parameters func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, error) { - namespace, err := getRequiredString(params, "namespace") + namespace, err := params.GetRequiredString("namespace") if err != nil { return nil, err } - name, err := getRequiredString(params, "name") + name, err := params.GetRequiredString("name") if err != nil { return nil, err } - workload := getOptionalString(params, "workload") + workload := params.GetOptionalString("workload") if workload == "" { workload = "fedora" // Default to fedora if not specified } - performance := normalizePerformance(getOptionalString(params, "performance")) - autostart := getOptionalBool(params, "autostart") - return &createParameters{ Namespace: namespace, Name: name, Workload: workload, - Instancetype: getOptionalString(params, "instancetype"), - Preference: getOptionalString(params, "preference"), - Size: getOptionalString(params, "size"), - Performance: performance, - Autostart: autostart, + Instancetype: params.GetOptionalString("instancetype"), + Preference: params.GetOptionalString("preference"), + Size: params.GetOptionalString("size"), + Performance: normalizePerformance(params.GetOptionalString("performance")), + Autostart: params.GetOptionalBool("autostart"), }, nil } @@ -400,45 +397,6 @@ func normalizePerformance(performance string) string { return "u1" } -func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { - args := params.GetArguments() - val, ok := args[key] - if !ok { - return "", fmt.Errorf("%s parameter required", key) - } - str, ok := val.(string) - if !ok { - return "", fmt.Errorf("%s parameter must be a string", key) - } - return str, nil -} - -func getOptionalString(params api.ToolHandlerParams, key string) string { - args := params.GetArguments() - val, ok := args[key] - if !ok { - return "" - } - str, ok := val.(string) - if !ok { - return "" - } - return str -} - -func getOptionalBool(params api.ToolHandlerParams, key string) bool { - args := params.GetArguments() - val, ok := args[key] - if !ok { - return false - } - b, ok := val.(bool) - if !ok { - return false - } - return b -} - // resolveContainerDisk resolves OS names to container disk images from quay.io/containerdisks func resolveContainerDisk(input string) string { // If input already looks like a container image, return as-is diff --git a/pkg/toolsets/kubevirt/vm/start/tool.go b/pkg/toolsets/kubevirt/vm/start/tool.go index b2784d2f..3295c914 100644 --- a/pkg/toolsets/kubevirt/vm/start/tool.go +++ b/pkg/toolsets/kubevirt/vm/start/tool.go @@ -48,12 +48,12 @@ func Tools() []api.ServerTool { func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) { // Parse required parameters - namespace, err := getRequiredString(params, "namespace") + namespace, err := params.GetRequiredString("namespace") if err != nil { return api.NewToolCallResult("", err), nil } - name, err := getRequiredString(params, "name") + name, err := params.GetRequiredString("name") if err != nil { return api.NewToolCallResult("", err), nil } @@ -108,16 +108,3 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("# VirtualMachine started successfully\n"+marshalledYaml, nil), nil } - -func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { - args := params.GetArguments() - val, ok := args[key] - if !ok { - return "", fmt.Errorf("%s parameter required", key) - } - str, ok := val.(string) - if !ok { - return "", fmt.Errorf("%s parameter must be a string", key) - } - return str, nil -} diff --git a/pkg/toolsets/kubevirt/vm/stop/tool.go b/pkg/toolsets/kubevirt/vm/stop/tool.go index 6ab03485..6267b269 100644 --- a/pkg/toolsets/kubevirt/vm/stop/tool.go +++ b/pkg/toolsets/kubevirt/vm/stop/tool.go @@ -48,12 +48,12 @@ func Tools() []api.ServerTool { func stop(params api.ToolHandlerParams) (*api.ToolCallResult, error) { // Parse required parameters - namespace, err := getRequiredString(params, "namespace") + namespace, err := params.GetRequiredString("namespace") if err != nil { return api.NewToolCallResult("", err), nil } - name, err := getRequiredString(params, "name") + name, err := params.GetRequiredString("name") if err != nil { return api.NewToolCallResult("", err), nil } @@ -108,16 +108,3 @@ func stop(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("# VirtualMachine stopped successfully\n"+marshalledYaml, nil), nil } - -func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { - args := params.GetArguments() - val, ok := args[key] - if !ok { - return "", fmt.Errorf("%s parameter required", key) - } - str, ok := val.(string) - if !ok { - return "", fmt.Errorf("%s parameter must be a string", key) - } - return str, nil -} diff --git a/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go b/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go index 7e0f8ead..036bb49e 100644 --- a/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go +++ b/pkg/toolsets/kubevirt/vm/troubleshoot/tool.go @@ -54,12 +54,12 @@ type troubleshootParams struct { func troubleshoot(params api.ToolHandlerParams) (*api.ToolCallResult, error) { // Parse required parameters - namespace, err := getRequiredString(params, "namespace") + namespace, err := params.GetRequiredString("namespace") if err != nil { return api.NewToolCallResult("", err), nil } - name, err := getRequiredString(params, "name") + name, err := params.GetRequiredString("name") if err != nil { return api.NewToolCallResult("", err), nil } @@ -83,16 +83,3 @@ func troubleshoot(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult(result.String(), nil), nil } - -func getRequiredString(params api.ToolHandlerParams, key string) (string, error) { - args := params.GetArguments() - val, ok := args[key] - if !ok { - return "", fmt.Errorf("%s parameter required", key) - } - str, ok := val.(string) - if !ok { - return "", fmt.Errorf("%s parameter must be a string", key) - } - return str, nil -} From 35b63ec25b416c881905c605b92bb16574b0a473 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Mon, 10 Nov 2025 11:51:57 +0000 Subject: [PATCH 5/8] refactor(api): Add default value parameter to GetOptionalString Extend GetOptionalString method to accept a default value parameter, allowing callers to specify what value to return when a parameter is missing or invalid. This simplifies code by eliminating post-call default value checks. Use variadic parameters to make the default value argument optional in GetOptionalString. Callers can now either provide a default value or omit it to get empty string behavior. This provides more flexibility and cleaner call sites when empty string is the desired default. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/api/toolsets.go | 11 +++++++++-- pkg/toolsets/kubevirt/vm/create/tool.go | 8 ++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/api/toolsets.go b/pkg/api/toolsets.go index 24f61618..f3679e4d 100644 --- a/pkg/api/toolsets.go +++ b/pkg/api/toolsets.go @@ -88,15 +88,22 @@ func (p ToolHandlerParams) GetRequiredString(key string) (string, error) { } // GetOptionalString extracts an optional string parameter from the tool call arguments. -// Returns an empty string if the parameter is missing or not a string. -func (p ToolHandlerParams) GetOptionalString(key string) string { +// Returns the provided default value if the parameter is missing or not a string. +// If no default value is provided, returns an empty string. +func (p ToolHandlerParams) GetOptionalString(key string, defaultValue ...string) string { args := p.GetArguments() val, ok := args[key] if !ok { + if len(defaultValue) > 0 { + return defaultValue[0] + } return "" } str, ok := val.(string) if !ok { + if len(defaultValue) > 0 { + return defaultValue[0] + } return "" } return str diff --git a/pkg/toolsets/kubevirt/vm/create/tool.go b/pkg/toolsets/kubevirt/vm/create/tool.go index 5ccd5437..c509c8ff 100644 --- a/pkg/toolsets/kubevirt/vm/create/tool.go +++ b/pkg/toolsets/kubevirt/vm/create/tool.go @@ -19,6 +19,7 @@ import ( const ( defaultInstancetypeLabel = "instancetype.kubevirt.io/default-instancetype" defaultPreferenceLabel = "instancetype.kubevirt.io/default-preference" + defaultWorkload = "fedora" ) //go:embed vm.yaml.tmpl @@ -180,15 +181,10 @@ func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, err return nil, err } - workload := params.GetOptionalString("workload") - if workload == "" { - workload = "fedora" // Default to fedora if not specified - } - return &createParameters{ Namespace: namespace, Name: name, - Workload: workload, + Workload: params.GetOptionalString("workload", defaultWorkload), Instancetype: params.GetOptionalString("instancetype"), Preference: params.GetOptionalString("preference"), Size: params.GetOptionalString("size"), From 1432202627574c4db82015caaf9111f6d093238a Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Mon, 10 Nov 2025 12:04:53 +0000 Subject: [PATCH 6/8] refactor(kubevirt): Move tests to external packages and test public API Refactor all kubevirt VM tool tests to use external test packages (_test suffix) and test only public behavior through the Tools() API. This ensures tests verify the public interface rather than implementation details. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/toolsets/kubevirt/vm/create/tool_test.go | 238 ++++++------------ pkg/toolsets/kubevirt/vm/start/tool_test.go | 15 +- pkg/toolsets/kubevirt/vm/stop/tool_test.go | 15 +- .../kubevirt/vm/troubleshoot/tool_test.go | 15 +- 4 files changed, 107 insertions(+), 176 deletions(-) diff --git a/pkg/toolsets/kubevirt/vm/create/tool_test.go b/pkg/toolsets/kubevirt/vm/create/tool_test.go index 00742bc0..0a19aa8f 100644 --- a/pkg/toolsets/kubevirt/vm/create/tool_test.go +++ b/pkg/toolsets/kubevirt/vm/create/tool_test.go @@ -1,197 +1,101 @@ -package create +package create_test import ( - "strings" + "context" "testing" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" + "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/create" ) -// Test the YAML rendering directly without creating resources -func TestRenderVMYaml(t *testing.T) { +type mockToolCallRequest struct { + arguments map[string]interface{} +} + +func (m *mockToolCallRequest) GetArguments() map[string]any { + return m.arguments +} + +func TestCreateParameterValidation(t *testing.T) { tests := []struct { - name string - params vmParams - wantErr bool - checkFunc func(t *testing.T, result string) + name string + args map[string]interface{} + wantErr bool + errMsg string }{ { - name: "renders VM with basic settings", - params: vmParams{ - Namespace: "test-ns", - Name: "test-vm", - ContainerDisk: "quay.io/containerdisks/fedora:latest", - RunStrategy: "Halted", - }, - wantErr: false, - checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "apiVersion: kubevirt.io/v1") { - t.Errorf("Expected apiVersion in YAML") - } - if !strings.Contains(result, "kind: VirtualMachine") { - t.Errorf("Expected kind VirtualMachine in YAML") - } - if !strings.Contains(result, "name: test-vm") { - t.Errorf("Expected VM name test-vm in YAML") - } - if !strings.Contains(result, "namespace: test-ns") { - t.Errorf("Expected namespace test-ns in YAML") - } - if !strings.Contains(result, "quay.io/containerdisks/fedora:latest") { - t.Errorf("Expected fedora container disk in result") - } - if !strings.Contains(result, "guest: 2Gi") { - t.Errorf("Expected guest: 2Gi in YAML manifest") - } - }, - }, - { - name: "renders VM with instancetype", - params: vmParams{ - Namespace: "test-ns", - Name: "test-vm", - ContainerDisk: "quay.io/containerdisks/ubuntu:24.04", - Instancetype: "u1.medium", - RunStrategy: "Halted", - }, - wantErr: false, - checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "name: u1.medium") { - t.Errorf("Expected instance type in YAML manifest") - } - if !strings.Contains(result, "kind: VirtualMachineClusterInstancetype") { - t.Errorf("Expected VirtualMachineClusterInstancetype in YAML manifest") - } - // When instancetype is set, memory should not be in the YAML - if strings.Contains(result, "guest: 2Gi") { - t.Errorf("Should not have guest memory when instancetype is specified") - } + name: "missing namespace parameter", + args: map[string]interface{}{ + "name": "test-vm", }, + wantErr: true, + errMsg: "namespace parameter required", }, { - name: "renders VM with preference", - params: vmParams{ - Namespace: "test-ns", - Name: "test-vm", - ContainerDisk: "registry.redhat.io/rhel9/rhel-guest-image:latest", - Preference: "rhel.9", - RunStrategy: "Halted", - }, - wantErr: false, - checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "name: rhel.9") { - t.Errorf("Expected preference in YAML manifest") - } - if !strings.Contains(result, "kind: VirtualMachineClusterPreference") { - t.Errorf("Expected VirtualMachineClusterPreference in YAML manifest") - } - }, - }, - { - name: "renders VM with custom container disk", - params: vmParams{ - Namespace: "test-ns", - Name: "test-vm", - ContainerDisk: "quay.io/myrepo/myimage:v1.0", - RunStrategy: "Halted", - }, - wantErr: false, - checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "quay.io/myrepo/myimage:v1.0") { - t.Errorf("Expected custom container disk in YAML") - } + name: "missing name parameter", + args: map[string]interface{}{ + "namespace": "test-ns", }, + wantErr: true, + errMsg: "name parameter required", }, { - name: "renders VM with DataSource", - params: vmParams{ - Namespace: "test-ns", - Name: "test-vm", - UseDataSource: true, - DataSourceName: "fedora", - DataSourceNamespace: "openshift-virtualization-os-images", - RunStrategy: "Halted", - }, - wantErr: false, - checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "dataVolumeTemplates") { - t.Errorf("Expected dataVolumeTemplates in YAML") - } - if !strings.Contains(result, "kind: DataSource") { - t.Errorf("Expected DataSource kind in YAML") - } - if !strings.Contains(result, "name: fedora") { - t.Errorf("Expected DataSource name in YAML") - } - if !strings.Contains(result, "openshift-virtualization-os-images") { - t.Errorf("Expected DataSource namespace in YAML") - } + name: "invalid namespace type", + args: map[string]interface{}{ + "namespace": 123, + "name": "test-vm", }, + wantErr: true, + errMsg: "namespace parameter must be a string", }, { - name: "renders VM with autostart (runStrategy Always)", - params: vmParams{ - Namespace: "test-ns", - Name: "test-vm", - ContainerDisk: "quay.io/containerdisks/fedora:latest", - RunStrategy: "Always", - }, - wantErr: false, - checkFunc: func(t *testing.T, result string) { - if !strings.Contains(result, "runStrategy: Always") { - t.Errorf("Expected runStrategy: Always in YAML") - } + name: "invalid name type", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": 456, }, + wantErr: true, + errMsg: "name parameter must be a string", }, } + // Get the tool through the public API + tools := create.Tools() + if len(tools) != 1 { + t.Fatalf("Expected 1 tool, got %d", len(tools)) + } + vmCreateTool := tools[0] + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := renderVMYaml(tt.params) + params := api.ToolHandlerParams{ + Context: context.Background(), + Kubernetes: &internalk8s.Kubernetes{}, + ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, + } - if tt.wantErr { - if err == nil { - t.Error("Expected error, got nil") - } - } else { - if err != nil { - t.Errorf("Expected no error, got: %v", err) - } - if result == "" { - t.Error("Expected non-empty result") - } - if tt.checkFunc != nil { - tt.checkFunc(t, result) - } + // Call through the public Handler interface + result, err := vmCreateTool.Handler(params) + if err != nil { + t.Errorf("Handler() unexpected Go error: %v", err) + return } - }) - } -} -func TestResolveContainerDisk(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - {"fedora", "fedora", "quay.io/containerdisks/fedora:latest"}, - {"ubuntu", "ubuntu", "quay.io/containerdisks/ubuntu:24.04"}, - {"rhel8", "rhel8", "registry.redhat.io/rhel8/rhel-guest-image:latest"}, - {"rhel9", "rhel9", "registry.redhat.io/rhel9/rhel-guest-image:latest"}, - {"rhel10", "rhel10", "registry.redhat.io/rhel10/rhel-guest-image:latest"}, - {"centos", "centos", "quay.io/containerdisks/centos-stream:9-latest"}, - {"centos-stream", "centos-stream", "quay.io/containerdisks/centos-stream:9-latest"}, - {"debian", "debian", "quay.io/containerdisks/debian:latest"}, - {"case insensitive", "FEDORA", "quay.io/containerdisks/fedora:latest"}, - {"with whitespace", " ubuntu ", "quay.io/containerdisks/ubuntu:24.04"}, - {"custom image", "quay.io/myrepo/myimage:v1", "quay.io/myrepo/myimage:v1"}, - {"with tag", "myimage:latest", "myimage:latest"}, - {"unknown OS", "customos", "customos"}, - } + if result == nil { + t.Error("Expected non-nil result") + return + } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := resolveContainerDisk(tt.input) - if result != tt.expected { - t.Errorf("resolveContainerDisk(%s) = %s, want %s", tt.input, result, tt.expected) + // For parameter validation errors, check the error message + if tt.wantErr && tt.errMsg != "" { + if result.Error == nil { + t.Error("Expected error in result.Error, got nil") + return + } + if result.Error.Error() != tt.errMsg { + t.Errorf("Expected error message %q, got %q", tt.errMsg, result.Error.Error()) + } } }) } diff --git a/pkg/toolsets/kubevirt/vm/start/tool_test.go b/pkg/toolsets/kubevirt/vm/start/tool_test.go index 6c207c90..5acd1b88 100644 --- a/pkg/toolsets/kubevirt/vm/start/tool_test.go +++ b/pkg/toolsets/kubevirt/vm/start/tool_test.go @@ -1,4 +1,4 @@ -package start +package start_test import ( "context" @@ -6,6 +6,7 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/api" internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" + "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/start" ) type mockToolCallRequest struct { @@ -67,6 +68,13 @@ func TestStartParameterValidation(t *testing.T) { }, } + // Get the tool through the public API + tools := start.Tools() + if len(tools) != 1 { + t.Fatalf("Expected 1 tool, got %d", len(tools)) + } + vmStartTool := tools[0] + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { params := api.ToolHandlerParams{ @@ -75,9 +83,10 @@ func TestStartParameterValidation(t *testing.T) { ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, } - result, err := start(params) + // Call through the public Handler interface + result, err := vmStartTool.Handler(params) if err != nil { - t.Errorf("start() unexpected Go error: %v", err) + t.Errorf("Handler() unexpected Go error: %v", err) return } diff --git a/pkg/toolsets/kubevirt/vm/stop/tool_test.go b/pkg/toolsets/kubevirt/vm/stop/tool_test.go index bcb8a206..abbd67eb 100644 --- a/pkg/toolsets/kubevirt/vm/stop/tool_test.go +++ b/pkg/toolsets/kubevirt/vm/stop/tool_test.go @@ -1,4 +1,4 @@ -package stop +package stop_test import ( "context" @@ -6,6 +6,7 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/api" internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" + "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/stop" ) type mockToolCallRequest struct { @@ -67,6 +68,13 @@ func TestStopParameterValidation(t *testing.T) { }, } + // Get the tool through the public API + tools := stop.Tools() + if len(tools) != 1 { + t.Fatalf("Expected 1 tool, got %d", len(tools)) + } + vmStopTool := tools[0] + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { params := api.ToolHandlerParams{ @@ -75,9 +83,10 @@ func TestStopParameterValidation(t *testing.T) { ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, } - result, err := stop(params) + // Call through the public Handler interface + result, err := vmStopTool.Handler(params) if err != nil { - t.Errorf("stop() unexpected Go error: %v", err) + t.Errorf("Handler() unexpected Go error: %v", err) return } diff --git a/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go b/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go index 8d371d42..303e6ae4 100644 --- a/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go +++ b/pkg/toolsets/kubevirt/vm/troubleshoot/tool_test.go @@ -1,4 +1,4 @@ -package troubleshoot +package troubleshoot_test import ( "context" @@ -7,6 +7,7 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/api" internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" + "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/troubleshoot" ) type mockToolCallRequest struct { @@ -71,6 +72,13 @@ func TestTroubleshoot(t *testing.T) { }, } + // Get the tool through the public API + tools := troubleshoot.Tools() + if len(tools) != 1 { + t.Fatalf("Expected 1 tool, got %d", len(tools)) + } + vmTroubleshootTool := tools[0] + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { params := api.ToolHandlerParams{ @@ -79,9 +87,10 @@ func TestTroubleshoot(t *testing.T) { ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, } - result, err := troubleshoot(params) + // Call through the public Handler interface + result, err := vmTroubleshootTool.Handler(params) if err != nil { - t.Errorf("troubleshoot() unexpected Go error: %v", err) + t.Errorf("Handler() unexpected Go error: %v", err) return } From 49f08ad2d7b56177531c543d4e56ecfdd8d86e5c Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 13 Nov 2025 15:03:29 +0000 Subject: [PATCH 7/8] feat(kubevirt): Add vm_delete tool Introduces a dedicated vm_delete tool to simplify VirtualMachine deletion. Based on agent evaluation results, multiple agents were searching for a vm_delete tool instead of using the generic resources_delete tool. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/toolsets/kubevirt/toolset.go | 2 + pkg/toolsets/kubevirt/vm/delete/tool.go | 88 +++++++++++++++ pkg/toolsets/kubevirt/vm/delete/tool_test.go | 110 +++++++++++++++++++ 3 files changed, 200 insertions(+) create mode 100644 pkg/toolsets/kubevirt/vm/delete/tool.go create mode 100644 pkg/toolsets/kubevirt/vm/delete/tool_test.go diff --git a/pkg/toolsets/kubevirt/toolset.go b/pkg/toolsets/kubevirt/toolset.go index 41257960..db50fd35 100644 --- a/pkg/toolsets/kubevirt/toolset.go +++ b/pkg/toolsets/kubevirt/toolset.go @@ -7,6 +7,7 @@ import ( internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" "github.com/containers/kubernetes-mcp-server/pkg/toolsets" vm_create "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/create" + vm_delete "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/delete" vm_start "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/start" vm_stop "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/stop" vm_troubleshoot "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/troubleshoot" @@ -27,6 +28,7 @@ func (t *Toolset) GetDescription() string { func (t *Toolset) GetTools(o internalk8s.Openshift) []api.ServerTool { return slices.Concat( vm_create.Tools(), + vm_delete.Tools(), vm_start.Tools(), vm_stop.Tools(), vm_troubleshoot.Tools(), diff --git a/pkg/toolsets/kubevirt/vm/delete/tool.go b/pkg/toolsets/kubevirt/vm/delete/tool.go new file mode 100644 index 00000000..c73fa978 --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/delete/tool.go @@ -0,0 +1,88 @@ +package delete + +import ( + "fmt" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + "github.com/google/jsonschema-go/jsonschema" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + "k8s.io/utils/ptr" +) + +func Tools() []api.ServerTool { + return []api.ServerTool{ + { + Tool: api.Tool{ + Name: "vm_delete", + Description: "Delete a VirtualMachine in the current cluster by providing its namespace and name", + InputSchema: &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "namespace": { + Type: "string", + Description: "The namespace of the virtual machine", + }, + "name": { + Type: "string", + Description: "The name of the virtual machine to delete", + }, + }, + Required: []string{"namespace", "name"}, + }, + Annotations: api.ToolAnnotations{ + Title: "Virtual Machine: Delete", + ReadOnlyHint: ptr.To(false), + DestructiveHint: ptr.To(true), + IdempotentHint: ptr.To(true), + OpenWorldHint: ptr.To(false), + }, + }, + Handler: deleteVM, + }, + } +} + +func deleteVM(params api.ToolHandlerParams) (*api.ToolCallResult, error) { + // Parse required parameters + namespace, err := params.GetRequiredString("namespace") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + name, err := params.GetRequiredString("name") + if err != nil { + return api.NewToolCallResult("", err), nil + } + + // Get dynamic client + restConfig := params.RESTConfig() + if restConfig == nil { + return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil + } + + dynamicClient, err := dynamic.NewForConfig(restConfig) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil + } + + // Define the VirtualMachine GVR + gvr := schema.GroupVersionResource{ + Group: "kubevirt.io", + Version: "v1", + Resource: "virtualmachines", + } + + // Delete the VM + err = dynamicClient.Resource(gvr).Namespace(namespace).Delete( + params.Context, + name, + metav1.DeleteOptions{}, + ) + if err != nil { + return api.NewToolCallResult("", fmt.Errorf("failed to delete VirtualMachine: %w", err)), nil + } + + return api.NewToolCallResult(fmt.Sprintf("# VirtualMachine deleted successfully\nVirtualMachine '%s' in namespace '%s' has been deleted.", name, namespace), nil), nil +} diff --git a/pkg/toolsets/kubevirt/vm/delete/tool_test.go b/pkg/toolsets/kubevirt/vm/delete/tool_test.go new file mode 100644 index 00000000..afeb8cbe --- /dev/null +++ b/pkg/toolsets/kubevirt/vm/delete/tool_test.go @@ -0,0 +1,110 @@ +package delete_test + +import ( + "context" + "testing" + + "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" + "github.com/containers/kubernetes-mcp-server/pkg/toolsets/kubevirt/vm/delete" +) + +type mockToolCallRequest struct { + arguments map[string]interface{} +} + +func (m *mockToolCallRequest) GetArguments() map[string]any { + return m.arguments +} + +func TestDeleteParameterValidation(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + wantErr bool + errMsg string + }{ + { + name: "missing namespace parameter", + args: map[string]interface{}{ + "name": "test-vm", + }, + wantErr: true, + errMsg: "namespace parameter required", + }, + { + name: "missing name parameter", + args: map[string]interface{}{ + "namespace": "test-ns", + }, + wantErr: true, + errMsg: "name parameter required", + }, + { + name: "invalid namespace type", + args: map[string]interface{}{ + "namespace": 123, + "name": "test-vm", + }, + wantErr: true, + errMsg: "namespace parameter must be a string", + }, + { + name: "invalid name type", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": 456, + }, + wantErr: true, + errMsg: "name parameter must be a string", + }, + { + name: "valid parameters - cluster interaction expected", + args: map[string]interface{}{ + "namespace": "test-ns", + "name": "test-vm", + }, + wantErr: true, // Will fail due to missing cluster connection, but parameters are valid + }, + } + + // Get the tool through the public API + tools := delete.Tools() + if len(tools) != 1 { + t.Fatalf("Expected 1 tool, got %d", len(tools)) + } + vmDeleteTool := tools[0] + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := api.ToolHandlerParams{ + Context: context.Background(), + Kubernetes: &internalk8s.Kubernetes{}, + ToolCallRequest: &mockToolCallRequest{arguments: tt.args}, + } + + // Call through the public Handler interface + result, err := vmDeleteTool.Handler(params) + if err != nil { + t.Errorf("Handler() unexpected Go error: %v", err) + return + } + + if result == nil { + t.Error("Expected non-nil result") + return + } + + // For parameter validation errors, check the error message + if tt.wantErr && tt.errMsg != "" { + if result.Error == nil { + t.Error("Expected error in result.Error, got nil") + return + } + if result.Error.Error() != tt.errMsg { + t.Errorf("Expected error message %q, got %q", tt.errMsg, result.Error.Error()) + } + } + }) + } +} From 9fa0abfd69cae3ede01b0168929645071714e373 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Fri, 14 Nov 2025 16:49:31 +0000 Subject: [PATCH 8/8] refactor(kubevirt): Enforce access control in toolset API Remove direct RESTConfig() exposure from Kubernetes type. Update all kubevirt tools to use GVK instead of GVR with ResourcesList. This prevents toolsets from bypassing the denied resources configuration and ensures all API access goes through the access control layer. Assisted-By: Claude Signed-off-by: Lee Yarwood --- pkg/kubernetes/kubernetes.go | 9 -- pkg/kubernetes/resources.go | 11 +- pkg/toolsets/kubevirt/vm/create/tool.go | 163 ++++++++++++------------ pkg/toolsets/kubevirt/vm/delete/tool.go | 31 +---- pkg/toolsets/kubevirt/vm/start/tool.go | 51 ++++---- pkg/toolsets/kubevirt/vm/stop/tool.go | 51 ++++---- 6 files changed, 141 insertions(+), 175 deletions(-) diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index b3e2a264..7de8d6ff 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -2,7 +2,6 @@ package kubernetes import ( "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/rest" "github.com/containers/kubernetes-mcp-server/pkg/helm" "github.com/containers/kubernetes-mcp-server/pkg/kiali" @@ -32,14 +31,6 @@ func (k *Kubernetes) AccessControlClientset() *AccessControlClientset { return k.manager.accessControlClientSet } -// RESTConfig returns the Kubernetes REST configuration -func (k *Kubernetes) RESTConfig() *rest.Config { - if k.manager == nil { - return nil - } - return k.manager.cfg -} - var Scheme = scheme.Scheme var ParameterCodec = runtime.NewParameterCodec(Scheme) diff --git a/pkg/kubernetes/resources.go b/pkg/kubernetes/resources.go index 1f559e12..9ca66c0c 100644 --- a/pkg/kubernetes/resources.go +++ b/pkg/kubernetes/resources.go @@ -3,10 +3,11 @@ package kubernetes import ( "context" "fmt" - "k8s.io/apimachinery/pkg/runtime" "regexp" "strings" + "k8s.io/apimachinery/pkg/runtime" + "github.com/containers/kubernetes-mcp-server/pkg/version" authv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,6 +47,10 @@ func (k *Kubernetes) ResourcesList(ctx context.Context, gvk *schema.GroupVersion } func (k *Kubernetes) ResourcesGet(ctx context.Context, gvk *schema.GroupVersionKind, namespace, name string) (*unstructured.Unstructured, error) { + if k.manager == nil { + return nil, fmt.Errorf("kubernetes manager not initialized") + } + gvr, err := k.resourceFor(gvk) if err != nil { return nil, err @@ -73,6 +78,10 @@ func (k *Kubernetes) ResourcesCreateOrUpdate(ctx context.Context, resource strin } func (k *Kubernetes) ResourcesDelete(ctx context.Context, gvk *schema.GroupVersionKind, namespace, name string) error { + if k.manager == nil { + return fmt.Errorf("kubernetes manager not initialized") + } + gvr, err := k.resourceFor(gvk) if err != nil { return err diff --git a/pkg/toolsets/kubevirt/vm/create/tool.go b/pkg/toolsets/kubevirt/vm/create/tool.go index c509c8ff..94dc9d53 100644 --- a/pkg/toolsets/kubevirt/vm/create/tool.go +++ b/pkg/toolsets/kubevirt/vm/create/tool.go @@ -7,12 +7,11 @@ import ( "text/template" "github.com/containers/kubernetes-mcp-server/pkg/api" + internalk8s "github.com/containers/kubernetes-mcp-server/pkg/kubernetes" "github.com/containers/kubernetes-mcp-server/pkg/output" "github.com/google/jsonschema-go/jsonschema" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" "k8s.io/utils/ptr" ) @@ -464,27 +463,15 @@ func getDefaultContainerDisks() []DataSourceInfo { // searchDataSources searches for DataSource resources in the cluster func searchDataSources(params api.ToolHandlerParams, query string) ([]DataSourceInfo, error) { - // Get dynamic client for querying DataSources - restConfig := params.RESTConfig() - if restConfig == nil { - return nil, fmt.Errorf("REST config is nil") - } - - dynamicClient, err := dynamic.NewForConfig(restConfig) - if err != nil { - // Return just the built-in containerdisk images - return getDefaultContainerDisks(), nil - } - - // DataSource GVR for CDI - dataSourceGVR := schema.GroupVersionResource{ - Group: "cdi.kubevirt.io", - Version: "v1beta1", - Resource: "datasources", + // DataSource GVK for CDI + dataSourceGVK := schema.GroupVersionKind{ + Group: "cdi.kubevirt.io", + Version: "v1beta1", + Kind: "DataSource", } // Collect DataSources from well-known namespaces and all namespaces - results := collectDataSources(params, dynamicClient, dataSourceGVR) + results := collectDataSources(params, dataSourceGVK) // Add common containerdisk images results = append(results, getDefaultContainerDisks()...) @@ -504,7 +491,7 @@ func searchDataSources(params api.ToolHandlerParams, query string) ([]DataSource } // collectDataSources collects DataSources from well-known namespaces and all namespaces -func collectDataSources(params api.ToolHandlerParams, dynamicClient dynamic.Interface, gvr schema.GroupVersionResource) []DataSourceInfo { +func collectDataSources(params api.ToolHandlerParams, gvk schema.GroupVersionKind) []DataSourceInfo { var results []DataSourceInfo // Try to list DataSources from well-known namespaces first @@ -514,14 +501,14 @@ func collectDataSources(params api.ToolHandlerParams, dynamicClient dynamic.Inte } for _, ns := range wellKnownNamespaces { - dsInfos, err := listDataSourcesFromNamespace(params, dynamicClient, gvr, ns) + dsInfos, err := listDataSourcesFromNamespace(params, gvk, ns) if err == nil { results = append(results, dsInfos...) } } // List DataSources from all namespaces - list, err := dynamicClient.Resource(gvr).List(params.Context, metav1.ListOptions{}) + listResult, err := params.ResourcesList(params.Context, &gvk, "", internalk8s.ResourceListOptions{}) if err != nil { // If we found DataSources from well-known namespaces but couldn't list all, return what we have if len(results) > 0 { @@ -537,6 +524,22 @@ func collectDataSources(params api.ToolHandlerParams, dynamicClient dynamic.Inte } } + // Convert to UnstructuredList + list, ok := listResult.(*unstructured.UnstructuredList) + if !ok { + // If we found DataSources from well-known namespaces but couldn't convert the result, return what we have + if len(results) > 0 { + return results + } + return []DataSourceInfo{ + { + Name: "No DataSources found", + Namespace: "", + Source: "CDI may not be installed or DataSources are not available in this cluster", + }, + } + } + // Deduplicate and add DataSources from all namespaces results = deduplicateAndMergeDataSources(results, list.Items) @@ -587,13 +590,19 @@ func deduplicateAndMergeDataSources(existing []DataSourceInfo, items []unstructu } // listDataSourcesFromNamespace lists DataSources from a specific namespace -func listDataSourcesFromNamespace(params api.ToolHandlerParams, dynamicClient dynamic.Interface, gvr schema.GroupVersionResource, namespace string) ([]DataSourceInfo, error) { +func listDataSourcesFromNamespace(params api.ToolHandlerParams, gvk schema.GroupVersionKind, namespace string) ([]DataSourceInfo, error) { var results []DataSourceInfo - list, err := dynamicClient.Resource(gvr).Namespace(namespace).List(params.Context, metav1.ListOptions{}) + listResult, err := params.ResourcesList(params.Context, &gvk, namespace, internalk8s.ResourceListOptions{}) if err != nil { return nil, err } + // Convert to UnstructuredList + list, ok := listResult.(*unstructured.UnstructuredList) + if !ok { + return nil, fmt.Errorf("unexpected result type: %T", listResult) + } + for _, item := range list.Items { name := item.GetName() ns := item.GetNamespace() @@ -629,47 +638,41 @@ func searchPreferences(params api.ToolHandlerParams, namespace string) []Prefere return []PreferenceInfo{} } - restConfig := params.RESTConfig() - if restConfig == nil { - return []PreferenceInfo{} - } - - dynamicClient, err := dynamic.NewForConfig(restConfig) - if err != nil { - return []PreferenceInfo{} - } - var results []PreferenceInfo // Search for cluster-wide VirtualMachineClusterPreferences - clusterPreferenceGVR := schema.GroupVersionResource{ - Group: "instancetype.kubevirt.io", - Version: "v1beta1", - Resource: "virtualmachineclusterpreferences", + clusterPreferenceGVK := schema.GroupVersionKind{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Kind: "VirtualMachineClusterPreference", } - clusterList, err := dynamicClient.Resource(clusterPreferenceGVR).List(params.Context, metav1.ListOptions{}) + clusterListResult, err := params.ResourcesList(params.Context, &clusterPreferenceGVK, "", internalk8s.ResourceListOptions{}) if err == nil { - for _, item := range clusterList.Items { - results = append(results, PreferenceInfo{ - Name: item.GetName(), - }) + if clusterList, ok := clusterListResult.(*unstructured.UnstructuredList); ok { + for _, item := range clusterList.Items { + results = append(results, PreferenceInfo{ + Name: item.GetName(), + }) + } } } // Search for namespaced VirtualMachinePreferences - namespacedPreferenceGVR := schema.GroupVersionResource{ - Group: "instancetype.kubevirt.io", - Version: "v1beta1", - Resource: "virtualmachinepreferences", + namespacedPreferenceGVK := schema.GroupVersionKind{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Kind: "VirtualMachinePreference", } - namespacedList, err := dynamicClient.Resource(namespacedPreferenceGVR).Namespace(namespace).List(params.Context, metav1.ListOptions{}) + namespacedListResult, err := params.ResourcesList(params.Context, &namespacedPreferenceGVK, namespace, internalk8s.ResourceListOptions{}) if err == nil { - for _, item := range namespacedList.Items { - results = append(results, PreferenceInfo{ - Name: item.GetName(), - }) + if namespacedList, ok := namespacedListResult.(*unstructured.UnstructuredList); ok { + for _, item := range namespacedList.Items { + results = append(results, PreferenceInfo{ + Name: item.GetName(), + }) + } } } @@ -683,49 +686,43 @@ func searchInstancetypes(params api.ToolHandlerParams, namespace string) []Insta return []InstancetypeInfo{} } - restConfig := params.RESTConfig() - if restConfig == nil { - return []InstancetypeInfo{} - } - - dynamicClient, err := dynamic.NewForConfig(restConfig) - if err != nil { - return []InstancetypeInfo{} - } - var results []InstancetypeInfo // Search for cluster-wide VirtualMachineClusterInstancetypes - clusterInstancetypeGVR := schema.GroupVersionResource{ - Group: "instancetype.kubevirt.io", - Version: "v1beta1", - Resource: "virtualmachineclusterinstancetypes", + clusterInstancetypeGVK := schema.GroupVersionKind{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Kind: "VirtualMachineClusterInstancetype", } - clusterList, err := dynamicClient.Resource(clusterInstancetypeGVR).List(params.Context, metav1.ListOptions{}) + clusterListResult, err := params.ResourcesList(params.Context, &clusterInstancetypeGVK, "", internalk8s.ResourceListOptions{}) if err == nil { - for _, item := range clusterList.Items { - results = append(results, InstancetypeInfo{ - Name: item.GetName(), - Labels: item.GetLabels(), - }) + if clusterList, ok := clusterListResult.(*unstructured.UnstructuredList); ok { + for _, item := range clusterList.Items { + results = append(results, InstancetypeInfo{ + Name: item.GetName(), + Labels: item.GetLabels(), + }) + } } } // Search for namespaced VirtualMachineInstancetypes - namespacedInstancetypeGVR := schema.GroupVersionResource{ - Group: "instancetype.kubevirt.io", - Version: "v1beta1", - Resource: "virtualmachineinstancetypes", + namespacedInstancetypeGVK := schema.GroupVersionKind{ + Group: "instancetype.kubevirt.io", + Version: "v1beta1", + Kind: "VirtualMachineInstancetype", } - namespacedList, err := dynamicClient.Resource(namespacedInstancetypeGVR).Namespace(namespace).List(params.Context, metav1.ListOptions{}) + namespacedListResult, err := params.ResourcesList(params.Context, &namespacedInstancetypeGVK, namespace, internalk8s.ResourceListOptions{}) if err == nil { - for _, item := range namespacedList.Items { - results = append(results, InstancetypeInfo{ - Name: item.GetName(), - Labels: item.GetLabels(), - }) + if namespacedList, ok := namespacedListResult.(*unstructured.UnstructuredList); ok { + for _, item := range namespacedList.Items { + results = append(results, InstancetypeInfo{ + Name: item.GetName(), + Labels: item.GetLabels(), + }) + } } } diff --git a/pkg/toolsets/kubevirt/vm/delete/tool.go b/pkg/toolsets/kubevirt/vm/delete/tool.go index c73fa978..79fbd0b5 100644 --- a/pkg/toolsets/kubevirt/vm/delete/tool.go +++ b/pkg/toolsets/kubevirt/vm/delete/tool.go @@ -5,9 +5,7 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/api" "github.com/google/jsonschema-go/jsonschema" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" "k8s.io/utils/ptr" ) @@ -56,30 +54,15 @@ func deleteVM(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("", err), nil } - // Get dynamic client - restConfig := params.RESTConfig() - if restConfig == nil { - return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil + // Define the VirtualMachine GVK + gvk := schema.GroupVersionKind{ + Group: "kubevirt.io", + Version: "v1", + Kind: "VirtualMachine", } - dynamicClient, err := dynamic.NewForConfig(restConfig) - if err != nil { - return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil - } - - // Define the VirtualMachine GVR - gvr := schema.GroupVersionResource{ - Group: "kubevirt.io", - Version: "v1", - Resource: "virtualmachines", - } - - // Delete the VM - err = dynamicClient.Resource(gvr).Namespace(namespace).Delete( - params.Context, - name, - metav1.DeleteOptions{}, - ) + // Delete the VM using the access-controlled method + err = params.ResourcesDelete(params.Context, &gvk, namespace, name) if err != nil { return api.NewToolCallResult("", fmt.Errorf("failed to delete VirtualMachine: %w", err)), nil } diff --git a/pkg/toolsets/kubevirt/vm/start/tool.go b/pkg/toolsets/kubevirt/vm/start/tool.go index 3295c914..5064c8e5 100644 --- a/pkg/toolsets/kubevirt/vm/start/tool.go +++ b/pkg/toolsets/kubevirt/vm/start/tool.go @@ -6,10 +6,8 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/api" "github.com/containers/kubernetes-mcp-server/pkg/output" "github.com/google/jsonschema-go/jsonschema" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" "k8s.io/utils/ptr" ) @@ -58,29 +56,15 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("", err), nil } - // Get dynamic client - restConfig := params.RESTConfig() - if restConfig == nil { - return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil + // Define the VirtualMachine GVK + gvk := schema.GroupVersionKind{ + Group: "kubevirt.io", + Version: "v1", + Kind: "VirtualMachine", } - dynamicClient, err := dynamic.NewForConfig(restConfig) - if err != nil { - return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil - } - - // Get the current VM - gvr := schema.GroupVersionResource{ - Group: "kubevirt.io", - Version: "v1", - Resource: "virtualmachines", - } - - vm, err := dynamicClient.Resource(gvr).Namespace(namespace).Get( - params.Context, - name, - metav1.GetOptions{}, - ) + // Get the current VM using access-controlled method + vm, err := params.ResourcesGet(params.Context, &gvk, namespace, name) if err != nil { return api.NewToolCallResult("", fmt.Errorf("failed to get VirtualMachine: %w", err)), nil } @@ -90,15 +74,15 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("", fmt.Errorf("failed to set runStrategy: %w", err)), nil } - // Update the VM - updatedVM, err := dynamicClient.Resource(gvr).Namespace(namespace).Update( - params.Context, - vm, - metav1.UpdateOptions{}, - ) + // Update the VM using access-controlled method + updatedVMs, err := params.ResourcesCreateOrUpdate(params.Context, mustMarshalYAML(vm)) if err != nil { return api.NewToolCallResult("", fmt.Errorf("failed to update VirtualMachine: %w", err)), nil } + if len(updatedVMs) == 0 { + return api.NewToolCallResult("", fmt.Errorf("no VirtualMachine returned after update")), nil + } + updatedVM := updatedVMs[0] // Format the output marshalledYaml, err := output.MarshalYaml(updatedVM) @@ -108,3 +92,12 @@ func start(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("# VirtualMachine started successfully\n"+marshalledYaml, nil), nil } + +// mustMarshalYAML marshals an unstructured object to YAML string +func mustMarshalYAML(obj *unstructured.Unstructured) string { + yaml, err := output.MarshalYaml(obj) + if err != nil { + panic(fmt.Sprintf("failed to marshal object to YAML: %v", err)) + } + return yaml +} diff --git a/pkg/toolsets/kubevirt/vm/stop/tool.go b/pkg/toolsets/kubevirt/vm/stop/tool.go index 6267b269..b78d022f 100644 --- a/pkg/toolsets/kubevirt/vm/stop/tool.go +++ b/pkg/toolsets/kubevirt/vm/stop/tool.go @@ -6,10 +6,8 @@ import ( "github.com/containers/kubernetes-mcp-server/pkg/api" "github.com/containers/kubernetes-mcp-server/pkg/output" "github.com/google/jsonschema-go/jsonschema" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" "k8s.io/utils/ptr" ) @@ -58,29 +56,15 @@ func stop(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("", err), nil } - // Get dynamic client - restConfig := params.RESTConfig() - if restConfig == nil { - return api.NewToolCallResult("", fmt.Errorf("failed to get REST config")), nil + // Define the VirtualMachine GVK + gvk := schema.GroupVersionKind{ + Group: "kubevirt.io", + Version: "v1", + Kind: "VirtualMachine", } - dynamicClient, err := dynamic.NewForConfig(restConfig) - if err != nil { - return api.NewToolCallResult("", fmt.Errorf("failed to create dynamic client: %w", err)), nil - } - - // Get the current VM - gvr := schema.GroupVersionResource{ - Group: "kubevirt.io", - Version: "v1", - Resource: "virtualmachines", - } - - vm, err := dynamicClient.Resource(gvr).Namespace(namespace).Get( - params.Context, - name, - metav1.GetOptions{}, - ) + // Get the current VM using access-controlled method + vm, err := params.ResourcesGet(params.Context, &gvk, namespace, name) if err != nil { return api.NewToolCallResult("", fmt.Errorf("failed to get VirtualMachine: %w", err)), nil } @@ -90,15 +74,15 @@ func stop(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("", fmt.Errorf("failed to set runStrategy: %w", err)), nil } - // Update the VM - updatedVM, err := dynamicClient.Resource(gvr).Namespace(namespace).Update( - params.Context, - vm, - metav1.UpdateOptions{}, - ) + // Update the VM using access-controlled method + updatedVMs, err := params.ResourcesCreateOrUpdate(params.Context, mustMarshalYAML(vm)) if err != nil { return api.NewToolCallResult("", fmt.Errorf("failed to update VirtualMachine: %w", err)), nil } + if len(updatedVMs) == 0 { + return api.NewToolCallResult("", fmt.Errorf("no VirtualMachine returned after update")), nil + } + updatedVM := updatedVMs[0] // Format the output marshalledYaml, err := output.MarshalYaml(updatedVM) @@ -108,3 +92,12 @@ func stop(params api.ToolHandlerParams) (*api.ToolCallResult, error) { return api.NewToolCallResult("# VirtualMachine stopped successfully\n"+marshalledYaml, nil), nil } + +// mustMarshalYAML marshals an unstructured object to YAML string +func mustMarshalYAML(obj *unstructured.Unstructured) string { + yaml, err := output.MarshalYaml(obj) + if err != nil { + panic(fmt.Sprintf("failed to marshal object to YAML: %v", err)) + } + return yaml +}