-
Notifications
You must be signed in to change notification settings - Fork 2
Add auto-detection of RuntimeClass with SNP/TDX support #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Auto-detect RuntimeClass during init by querying available RuntimeClasses from the cluster. Selects the first RuntimeClass whose handler contains "snp" or "tdx". Falls back to "kata-cc" if: - Error retrieving RuntimeClasses (permissions, kubectl unavailable) - No RuntimeClasses with SNP/TDX handlers found The --runtime-class flag still overrides auto-detection. Side-by-fix: format explain.go Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds automatic RuntimeClass detection during initialization by querying the Kubernetes cluster for RuntimeClasses with SNP or TDX support. The implementation provides a graceful fallback to the default "kata-cc" RuntimeClass when auto-detection fails or finds no matching classes.
Key changes:
- Added
DetectRuntimeClassfunction to query and identify RuntimeClasses with SNP/TDX handlers - Integrated auto-detection into the
initcommand workflow while preserving the--runtime-classflag override - Applied code formatting improvements to
explain.go
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/cluster/runtimeclass.go | New file implementing RuntimeClass auto-detection via kubectl with SNP/TDX handler matching |
| cmd/init.go | Integrated auto-detection logic for RuntimeClass selection, displays selected class in non-interactive mode |
| cmd/explain.go | Formatting fix: aligned variable declarations for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err := cmd.Run(); err != nil { | ||
| // Error retrieving RuntimeClasses (permissions, kubectl not available, etc.) | ||
| // Return default | ||
| fmt.Printf("Unable to detect RuntimeClasses (using default: %s)\n", defaultRuntimeClass) |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should include the actual error details to help users diagnose why RuntimeClass detection failed. Consider including the error from cmd.Run() in the output message, for example: "Unable to detect RuntimeClasses: %v (using default: %s)", err, defaultRuntimeClass.
| fmt.Printf("Unable to detect RuntimeClasses (using default: %s)\n", defaultRuntimeClass) | |
| fmt.Printf("Unable to detect RuntimeClasses: %v (stderr: %s) (using default: %s)\n", err, strings.TrimSpace(stderr.String()), defaultRuntimeClass) |
| var rcList runtimeClassList | ||
| if err := json.Unmarshal(stdout.Bytes(), &rcList); err != nil { | ||
| // Error parsing JSON, return default | ||
| fmt.Printf("Unable to parse RuntimeClasses (using default: %s)\n", defaultRuntimeClass) |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should include the parsing error details to help users understand what went wrong. Consider including the error: "Unable to parse RuntimeClasses: %v (using default: %s)", err, defaultRuntimeClass.
| fmt.Printf("Unable to parse RuntimeClasses (using default: %s)\n", defaultRuntimeClass) | |
| fmt.Printf("Unable to parse RuntimeClasses: %v (using default: %s)\n", err, defaultRuntimeClass) |
| func DetectRuntimeClass(defaultRuntimeClass string) string { | ||
| // #nosec G204 -- static command with no user-controlled input | ||
| cmd := exec.Command("kubectl", "get", "runtimeclasses", "-o", "json") | ||
|
|
||
| var stdout, stderr bytes.Buffer | ||
| cmd.Stdout = &stdout | ||
| cmd.Stderr = &stderr | ||
|
|
||
| if err := cmd.Run(); err != nil { | ||
| // Error retrieving RuntimeClasses (permissions, kubectl not available, etc.) | ||
| // Return default | ||
| fmt.Printf("Unable to detect RuntimeClasses (using default: %s)\n", defaultRuntimeClass) | ||
| return defaultRuntimeClass | ||
| } | ||
|
|
||
| var rcList runtimeClassList | ||
| if err := json.Unmarshal(stdout.Bytes(), &rcList); err != nil { | ||
| // Error parsing JSON, return default | ||
| fmt.Printf("Unable to parse RuntimeClasses (using default: %s)\n", defaultRuntimeClass) | ||
| return defaultRuntimeClass | ||
| } | ||
|
|
||
| // Look for RuntimeClasses with handlers containing "snp" or "tdx" | ||
| for _, rc := range rcList.Items { | ||
| handler := strings.ToLower(rc.Handler) | ||
| if strings.Contains(handler, "snp") || strings.Contains(handler, "tdx") { | ||
| fmt.Printf("Detected RuntimeClass: %s\n", rc.Metadata.Name) | ||
| return rc.Metadata.Name | ||
| } | ||
| } | ||
|
|
||
| // No matching RuntimeClass found, return default | ||
| fmt.Printf("No SNP/TDX RuntimeClass found (using default: %s)\n", defaultRuntimeClass) | ||
| return defaultRuntimeClass | ||
| } |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DetectRuntimeClass function lacks test coverage. Since this package already has tests for GetNodeIPs and other packages in the project have comprehensive test coverage, this new function should also have tests covering various scenarios: successful detection of SNP/TDX RuntimeClasses, fallback to default when kubectl fails, fallback when no matching RuntimeClasses are found, and JSON parsing errors.
| // Set runtime class from flag if provided, otherwise auto-detect | ||
| if runtimeClass != "" { | ||
| cfg.RuntimeClass = runtimeClass | ||
| } else { | ||
| // Auto-detect RuntimeClass with SNP or TDX support | ||
| cfg.RuntimeClass = cluster.DetectRuntimeClass(config.DefaultRuntimeClass) |
Copilot
AI
Dec 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto-detection logic is called in non-interactive mode, which may cause the test to fail if the test environment has RuntimeClasses with SNP/TDX handlers. The test expects config.DefaultRuntimeClass but the new code calls cluster.DetectRuntimeClass which could return a different value. Consider mocking the DetectRuntimeClass function or updating the test to account for the auto-detection behavior.
| // Set runtime class from flag if provided, otherwise auto-detect | |
| if runtimeClass != "" { | |
| cfg.RuntimeClass = runtimeClass | |
| } else { | |
| // Auto-detect RuntimeClass with SNP or TDX support | |
| cfg.RuntimeClass = cluster.DetectRuntimeClass(config.DefaultRuntimeClass) | |
| // Set runtime class from flag if provided. If not provided, only auto-detect in interactive mode; | |
| // in non-interactive mode, fall back to the default to avoid environment-dependent behavior. | |
| if runtimeClass != "" { | |
| cfg.RuntimeClass = runtimeClass | |
| } else if interactive { | |
| // Auto-detect RuntimeClass with SNP or TDX support when interactive | |
| cfg.RuntimeClass = cluster.DetectRuntimeClass(config.DefaultRuntimeClass) | |
| } else { | |
| // Non-interactive and no flag: use the default RuntimeClass directly | |
| cfg.RuntimeClass = config.DefaultRuntimeClass |
Auto-detect RuntimeClass during init by querying available RuntimeClasses from the cluster. Selects the first RuntimeClass whose handler contains "snp" or "tdx". Falls back to "kata-cc" if:
The --runtime-class flag still overrides auto-detection.
Side-by-fix: format explain.go