Skip to content

Commit ffae527

Browse files
committed
refactor(kubevirt): Extract helper functions from create
Decompose the create function into smaller, focused helper functions: - parseCreateParameters: Parses and validates input parameters into a struct - matchDataSource: Matches workload input against available DataSources - resolvePreference: Resolves preference from DataSource defaults or cluster resources - resolveInstancetype: Resolves instancetype from DataSource defaults or size/performance hints - matchInstancetypeBySize: Matches instancetype based on size and performance criteria - filterInstancetypesBySize: Filters instancetypes by size hint - buildTemplateParams: Constructs vmParams from resolved resources - renderTemplate: Renders the VM creation plan template This refactoring improves code organization and readability by: - Following the single responsibility principle - Making the main create function a clear orchestrator - Reducing complexity and improving testability - Enhancing maintainability through focused, reusable functions Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
1 parent 2d5687a commit ffae527

File tree

1 file changed

+167
-105
lines changed
  • pkg/toolsets/kubevirt/vm/create

1 file changed

+167
-105
lines changed

pkg/toolsets/kubevirt/vm/create/tool.go

Lines changed: 167 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -108,173 +108,235 @@ type InstancetypeInfo struct {
108108
}
109109

110110
func create(params api.ToolHandlerParams) (*api.ToolCallResult, error) {
111-
// Parse required parameters
112-
namespace, err := getRequiredString(params, "namespace")
111+
// Parse and validate input parameters
112+
createParams, err := parseCreateParameters(params)
113113
if err != nil {
114114
return api.NewToolCallResult("", err), nil
115115
}
116116

117-
name, err := getRequiredString(params, "name")
117+
// Search for available DataSources
118+
dataSources, _ := searchDataSources(params, createParams.Workload)
119+
120+
// Match DataSource based on workload input
121+
matchedDataSource := matchDataSource(dataSources, createParams.Workload)
122+
123+
// Resolve preference from DataSource defaults or cluster resources
124+
preference := resolvePreference(params, createParams.Preference, matchedDataSource, createParams.Workload)
125+
126+
// Resolve instancetype from DataSource defaults or size/performance hints
127+
instancetype := resolveInstancetype(params, createParams, matchedDataSource)
128+
129+
// Build template parameters from resolved resources
130+
templateParams := buildTemplateParams(createParams, matchedDataSource, instancetype, preference)
131+
132+
// Render the VM creation plan template
133+
result, err := renderTemplate(templateParams)
118134
if err != nil {
119135
return api.NewToolCallResult("", err), nil
120136
}
121137

122-
// Parse optional parameters
123-
osInput := getOptionalString(params, "workload")
124-
if osInput == "" {
125-
osInput = "fedora" // Default to fedora if not specified
126-
}
127-
instancetype := getOptionalString(params, "instancetype")
128-
preference := getOptionalString(params, "preference")
129-
size := getOptionalString(params, "size")
130-
performance := getOptionalString(params, "performance")
138+
return api.NewToolCallResult(result, nil), nil
139+
}
140+
141+
// createParameters holds parsed input parameters for VM creation
142+
type createParameters struct {
143+
Namespace string
144+
Name string
145+
Workload string
146+
Instancetype string
147+
Preference string
148+
Size string
149+
Performance string
150+
}
131151

132-
// Normalize performance parameter to instance type prefix
133-
performance = normalizePerformance(performance)
152+
// parseCreateParameters parses and validates input parameters
153+
func parseCreateParameters(params api.ToolHandlerParams) (*createParameters, error) {
154+
namespace, err := getRequiredString(params, "namespace")
155+
if err != nil {
156+
return nil, err
157+
}
134158

135-
// Search for DataSources in the cluster
136-
dataSources, err := searchDataSources(params, osInput)
159+
name, err := getRequiredString(params, "name")
137160
if err != nil {
138-
// Don't fail completely, continue without DataSources
139-
dataSources = []DataSourceInfo{}
161+
return nil, err
140162
}
141163

142-
// Check if the operating_system input matches any DataSource
143-
normalizedInput := strings.ToLower(strings.TrimSpace(osInput))
144-
var matchedDataSource *DataSourceInfo
164+
workload := getOptionalString(params, "workload")
165+
if workload == "" {
166+
workload = "fedora" // Default to fedora if not specified
167+
}
168+
169+
performance := normalizePerformance(getOptionalString(params, "performance"))
170+
171+
return &createParameters{
172+
Namespace: namespace,
173+
Name: name,
174+
Workload: workload,
175+
Instancetype: getOptionalString(params, "instancetype"),
176+
Preference: getOptionalString(params, "preference"),
177+
Size: getOptionalString(params, "size"),
178+
Performance: performance,
179+
}, nil
180+
}
181+
182+
// matchDataSource finds a DataSource that matches the workload input
183+
func matchDataSource(dataSources []DataSourceInfo, workload string) *DataSourceInfo {
184+
normalizedInput := strings.ToLower(strings.TrimSpace(workload))
145185

146186
// First try exact match
147187
for i := range dataSources {
148188
ds := &dataSources[i]
149-
if strings.EqualFold(ds.Name, normalizedInput) || strings.EqualFold(ds.Name, osInput) {
150-
matchedDataSource = ds
151-
break
189+
if strings.EqualFold(ds.Name, normalizedInput) || strings.EqualFold(ds.Name, workload) {
190+
return ds
152191
}
153192
}
154193

155194
// If no exact match, try partial matching (e.g., "rhel" matches "rhel9")
156195
// Only match against real DataSources with namespaces, not built-in containerdisks
157-
if matchedDataSource == nil {
158-
for i := range dataSources {
159-
ds := &dataSources[i]
160-
// Only do partial matching for real DataSources (those with namespaces)
161-
if ds.Namespace != "" && strings.Contains(strings.ToLower(ds.Name), normalizedInput) {
162-
matchedDataSource = ds
163-
break
164-
}
196+
for i := range dataSources {
197+
ds := &dataSources[i]
198+
// Only do partial matching for real DataSources (those with namespaces)
199+
if ds.Namespace != "" && strings.Contains(strings.ToLower(ds.Name), normalizedInput) {
200+
return ds
165201
}
166202
}
167203

168-
// Use DataSource default preference if available and preference not specified
169-
if preference == "" && matchedDataSource != nil && matchedDataSource.DefaultPreference != "" {
170-
preference = matchedDataSource.DefaultPreference
204+
return nil
205+
}
206+
207+
// resolvePreference determines the preference to use from DataSource defaults or cluster resources
208+
func resolvePreference(params api.ToolHandlerParams, explicitPreference string, matchedDataSource *DataSourceInfo, workload string) string {
209+
// Use explicitly specified preference if provided
210+
if explicitPreference != "" {
211+
return explicitPreference
171212
}
172213

173-
// Check if the operating_system input matches any Preference when preference is not provided
174-
if preference == "" {
175-
preferences := searchPreferences(params)
176-
for i := range preferences {
177-
pref := &preferences[i]
178-
// Try to match preference name against the OS input
179-
// Common patterns: "fedora", "rhel.9", "ubuntu", etc.
180-
if strings.Contains(strings.ToLower(pref.Name), normalizedInput) {
181-
preference = pref.Name
182-
break
183-
}
214+
// Use DataSource default preference if available
215+
if matchedDataSource != nil && matchedDataSource.DefaultPreference != "" {
216+
return matchedDataSource.DefaultPreference
217+
}
218+
219+
// Try to match preference name against the workload input
220+
preferences := searchPreferences(params)
221+
normalizedInput := strings.ToLower(strings.TrimSpace(workload))
222+
223+
for i := range preferences {
224+
pref := &preferences[i]
225+
// Common patterns: "fedora", "rhel.9", "ubuntu", etc.
226+
if strings.Contains(strings.ToLower(pref.Name), normalizedInput) {
227+
return pref.Name
184228
}
185229
}
186230

187-
// Use DataSource default instancetype if available and instancetype not specified and size not specified
188-
if instancetype == "" && size == "" && matchedDataSource != nil && matchedDataSource.DefaultInstancetype != "" {
189-
instancetype = matchedDataSource.DefaultInstancetype
231+
return ""
232+
}
233+
234+
// resolveInstancetype determines the instancetype to use from DataSource defaults or size/performance hints
235+
func resolveInstancetype(params api.ToolHandlerParams, createParams *createParameters, matchedDataSource *DataSourceInfo) string {
236+
// Use explicitly specified instancetype if provided
237+
if createParams.Instancetype != "" {
238+
return createParams.Instancetype
239+
}
240+
241+
// Use DataSource default instancetype if available (when size not specified)
242+
if createParams.Size == "" && matchedDataSource != nil && matchedDataSource.DefaultInstancetype != "" {
243+
return matchedDataSource.DefaultInstancetype
190244
}
191245

192-
// Check if the size parameter matches any instance type when instancetype is not provided
193-
if instancetype == "" && size != "" {
194-
instancetypes := searchInstancetypes(params)
195-
normalizedSize := strings.ToLower(strings.TrimSpace(size))
196-
normalizedPerformance := strings.ToLower(strings.TrimSpace(performance))
246+
// Match instancetype based on size and performance hints
247+
if createParams.Size != "" {
248+
return matchInstancetypeBySize(params, createParams.Size, createParams.Performance)
249+
}
197250

198-
// First, filter instance types by size
199-
var candidatesBySize []InstancetypeInfo
200-
for i := range instancetypes {
201-
it := &instancetypes[i]
202-
// Match instance types that contain the size hint in their name
203-
if strings.Contains(strings.ToLower(it.Name), normalizedSize) {
204-
candidatesBySize = append(candidatesBySize, *it)
205-
}
206-
}
251+
return ""
252+
}
207253

208-
// Then, filter by performance family
209-
// Try exact match first (e.g., "u1.small")
210-
for i := range candidatesBySize {
211-
it := &candidatesBySize[i]
212-
// Check if instance type name starts with the performance family
213-
if strings.HasPrefix(strings.ToLower(it.Name), normalizedPerformance+".") {
214-
instancetype = it.Name
215-
break
216-
}
254+
// matchInstancetypeBySize finds an instancetype that matches the size and performance hints
255+
func matchInstancetypeBySize(params api.ToolHandlerParams, size, performance string) string {
256+
instancetypes := searchInstancetypes(params)
257+
normalizedSize := strings.ToLower(strings.TrimSpace(size))
258+
normalizedPerformance := strings.ToLower(strings.TrimSpace(performance))
259+
260+
// Filter instance types by size
261+
candidatesBySize := filterInstancetypesBySize(instancetypes, normalizedSize)
262+
if len(candidatesBySize) == 0 {
263+
return ""
264+
}
265+
266+
// Try to match by performance family prefix (e.g., "u1.small")
267+
for i := range candidatesBySize {
268+
it := &candidatesBySize[i]
269+
if strings.HasPrefix(strings.ToLower(it.Name), normalizedPerformance+".") {
270+
return it.Name
217271
}
272+
}
218273

219-
// If no exact match, check labels for performance characteristics
220-
if instancetype == "" {
221-
for i := range candidatesBySize {
222-
it := &candidatesBySize[i]
223-
// Check labels for performance family indicators
224-
if it.Labels != nil {
225-
// Check common label patterns
226-
if class, ok := it.Labels["instancetype.kubevirt.io/class"]; ok {
227-
if strings.EqualFold(class, normalizedPerformance) {
228-
instancetype = it.Name
229-
break
230-
}
231-
}
274+
// Try to match by performance family label
275+
for i := range candidatesBySize {
276+
it := &candidatesBySize[i]
277+
if it.Labels != nil {
278+
if class, ok := it.Labels["instancetype.kubevirt.io/class"]; ok {
279+
if strings.EqualFold(class, normalizedPerformance) {
280+
return it.Name
232281
}
233282
}
234283
}
284+
}
235285

236-
// If still no match, fall back to first candidate that matches size
237-
if instancetype == "" && len(candidatesBySize) > 0 {
238-
instancetype = candidatesBySize[0].Name
286+
// Fall back to first candidate that matches size
287+
return candidatesBySize[0].Name
288+
}
289+
290+
// filterInstancetypesBySize filters instancetypes that contain the size hint in their name
291+
func filterInstancetypesBySize(instancetypes []InstancetypeInfo, normalizedSize string) []InstancetypeInfo {
292+
var candidates []InstancetypeInfo
293+
for i := range instancetypes {
294+
it := &instancetypes[i]
295+
if strings.Contains(strings.ToLower(it.Name), normalizedSize) {
296+
candidates = append(candidates, *it)
239297
}
240298
}
299+
return candidates
300+
}
241301

242-
// Prepare template parameters
243-
templateParams := vmParams{
244-
Namespace: namespace,
245-
Name: name,
302+
// buildTemplateParams constructs the template parameters for VM creation
303+
func buildTemplateParams(createParams *createParameters, matchedDataSource *DataSourceInfo, instancetype, preference string) vmParams {
304+
params := vmParams{
305+
Namespace: createParams.Namespace,
306+
Name: createParams.Name,
246307
Instancetype: instancetype,
247308
Preference: preference,
248309
}
249310

250311
if matchedDataSource != nil && matchedDataSource.Namespace != "" {
251312
// Use the matched DataSource (real cluster DataSource with namespace)
252-
templateParams.UseDataSource = true
253-
templateParams.DataSourceName = matchedDataSource.Name
254-
templateParams.DataSourceNamespace = matchedDataSource.Namespace
255-
templateParams.ContainerDisk = "" // Not using container disk
313+
params.UseDataSource = true
314+
params.DataSourceName = matchedDataSource.Name
315+
params.DataSourceNamespace = matchedDataSource.Namespace
256316
} else if matchedDataSource != nil {
257317
// Matched a built-in containerdisk (no namespace)
258-
templateParams.UseDataSource = false
259-
templateParams.ContainerDisk = matchedDataSource.Source
318+
params.ContainerDisk = matchedDataSource.Source
260319
} else {
261-
// No match, resolve container disk image from OS name
262-
templateParams.UseDataSource = false
263-
templateParams.ContainerDisk = resolveContainerDisk(osInput)
320+
// No match, resolve container disk image from workload name
321+
params.ContainerDisk = resolveContainerDisk(createParams.Workload)
264322
}
265323

266-
// Render template
324+
return params
325+
}
326+
327+
// renderTemplate renders the VM creation plan template
328+
func renderTemplate(templateParams vmParams) (string, error) {
267329
tmpl, err := template.New("vm").Parse(planTemplate)
268330
if err != nil {
269-
return api.NewToolCallResult("", fmt.Errorf("failed to parse template: %w", err)), nil
331+
return "", fmt.Errorf("failed to parse template: %w", err)
270332
}
271333

272334
var result strings.Builder
273335
if err := tmpl.Execute(&result, templateParams); err != nil {
274-
return api.NewToolCallResult("", fmt.Errorf("failed to render template: %w", err)), nil
336+
return "", fmt.Errorf("failed to render template: %w", err)
275337
}
276338

277-
return api.NewToolCallResult(result.String(), nil), nil
339+
return result.String(), nil
278340
}
279341

280342
// Helper functions

0 commit comments

Comments
 (0)