Skip to content

Commit 46210f0

Browse files
adiantumadiantum
authored
refactor: Change GetTaskUUIDFromVM helper to work with V4 converged client (#614)
* Change GetTaskUUIDFromVM helper to work with V4 converged client * Fixed case for v3-v4 task id incompatibility and add more tests * Fix filter whitespaces and braces * Fix lint and unit tests errors after merge
1 parent 1335c5f commit 46210f0

File tree

4 files changed

+1302
-28
lines changed

4 files changed

+1302
-28
lines changed

‎controllers/helpers.go‎

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"bytes"
2121
"context"
2222
"fmt"
23-
"reflect"
2423
"regexp"
2524
"slices"
2625
"sort"
@@ -53,8 +52,6 @@ import (
5352
const (
5453
providerIdPrefix = "nutanix://"
5554

56-
taskSucceededMessage = "SUCCEEDED"
57-
5855
subnetTypeOverlay = "OVERLAY"
5956

6057
gpuUnused = "UNUSED"
@@ -538,42 +535,41 @@ func ImageMarkedForDeletion(ctx context.Context, client *v4Converged.Client, ima
538535
}
539536

540537
// HasTaskInProgress returns true if the given task is in progress
541-
func HasTaskInProgress(ctx context.Context, client *prismclientv3.Client, taskUUID string) (bool, error) {
538+
func HasTaskInProgress(ctx context.Context, client *v4Converged.Client, taskUUID string) (bool, error) {
542539
log := ctrl.LoggerFrom(ctx)
543-
taskStatus, err := nutanixclient.GetTaskStatus(ctx, client, taskUUID)
540+
task, err := client.Tasks.Get(ctx, taskUUID)
544541
if err != nil {
545542
return false, err
546543
}
547-
if taskStatus != taskSucceededMessage {
548-
log.V(1).Info(fmt.Sprintf("VM task with UUID %s still in progress: %s", taskUUID, taskStatus))
544+
if *task.Status == prismModels.TASKSTATUS_RUNNING || *task.Status == prismModels.TASKSTATUS_QUEUED {
545+
log.V(1).Info(fmt.Sprintf("VM task with UUID %s still in progress: %v", taskUUID, *task.Status))
549546
return true, nil
550547
}
551548
return false, nil
552549
}
553550

554551
// GetTaskUUIDFromVM returns the UUID of the task that created the VM with the given UUID
555-
func GetTaskUUIDFromVM(vm *prismclientv3.VMIntentResponse) (string, error) {
556-
if vm == nil {
557-
return "", fmt.Errorf("cannot extract task uuid from empty vm object")
552+
func GetTaskUUIDFromVM(ctx context.Context, convergedClient *v4Converged.Client, vmId string) (string, error) {
553+
log := ctrl.LoggerFrom(ctx)
554+
if vmId == "" {
555+
return "", fmt.Errorf("cannot extract task uuid for empty vm id")
558556
}
559-
if vm.Status.ExecutionContext == nil {
560-
return "", nil
557+
log.V(1).Info(fmt.Sprintf("Getting task uuid for vm %s", vmId))
558+
filterString := "entitiesAffected/any(a:a/extId eq '%s') " +
559+
" and (status eq Prism.Config.TaskStatus'RUNNING' or status eq Prism.Config.TaskStatus'QUEUED')"
560+
561+
tasks, err := convergedClient.Tasks.List(ctx, converged.WithFilter(fmt.Sprintf(filterString, vmId)))
562+
if err != nil {
563+
return "", err
561564
}
562-
taskInterface := vm.Status.ExecutionContext.TaskUUID
563-
vmName := *vm.Spec.Name
564565

565-
switch t := reflect.TypeOf(taskInterface).Kind(); t {
566-
case reflect.Slice:
567-
l := taskInterface.([]interface{})
568-
if len(l) != 1 {
569-
return "", fmt.Errorf("did not find expected amount of task UUIDs for VM %s", vmName)
570-
}
571-
return l[0].(string), nil
572-
case reflect.String:
573-
return taskInterface.(string), nil
574-
default:
575-
return "", fmt.Errorf("invalid type found for task uuid extracted from vm %s: %v", vmName, t)
566+
log.V(1).Info(fmt.Sprintf("Found %d running or queued tasks for vm %s", len(tasks), vmId))
567+
if len(tasks) == 0 {
568+
return "", nil // no running tasks found
576569
}
570+
571+
log.V(1).Info(fmt.Sprintf("Returning task uuid %s for vm %s", *tasks[0].ExtId, vmId))
572+
return *tasks[0].ExtId, nil
577573
}
578574

579575
// GetSubnetUUIDList returns a list of subnet UUIDs for the given list of subnet names

‎controllers/helpers_test.go‎

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,3 +2012,231 @@ func NewMockConvergedClient(ctrl *gomock.Controller) *MockConvergedClientWrapper
20122012
MockTasks: mockTasks,
20132013
}
20142014
}
2015+
2016+
func TestGetTaskUUIDFromVM(t *testing.T) {
2017+
tests := []struct {
2018+
name string
2019+
clientBuilder func() *v4Converged.Client
2020+
vmId string
2021+
want string
2022+
wantErr bool
2023+
errorMessage string
2024+
}{
2025+
{
2026+
name: "empty vmId returns error",
2027+
clientBuilder: func() *v4Converged.Client {
2028+
mockctrl := gomock.NewController(t)
2029+
convergedClient := NewMockConvergedClient(mockctrl)
2030+
return convergedClient.Client
2031+
},
2032+
vmId: "",
2033+
want: "",
2034+
wantErr: true,
2035+
errorMessage: "cannot extract task uuid for empty vm id",
2036+
},
2037+
{
2038+
name: "successful task UUID retrieval with running task",
2039+
clientBuilder: func() *v4Converged.Client {
2040+
mockctrl := gomock.NewController(t)
2041+
convergedClient := NewMockConvergedClient(mockctrl)
2042+
taskUUID := "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f"
2043+
tasks := []prismModels.Task{
2044+
{
2045+
ExtId: ptr.To(taskUUID),
2046+
},
2047+
}
2048+
convergedClient.MockTasks.EXPECT().List(gomock.Any(), gomock.Any()).Return(tasks, nil)
2049+
return convergedClient.Client
2050+
},
2051+
vmId: "vm-uuid-456",
2052+
want: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2053+
wantErr: false,
2054+
},
2055+
{
2056+
name: "no running tasks returns empty string",
2057+
clientBuilder: func() *v4Converged.Client {
2058+
mockctrl := gomock.NewController(t)
2059+
convergedClient := NewMockConvergedClient(mockctrl)
2060+
tasks := []prismModels.Task{}
2061+
convergedClient.MockTasks.EXPECT().List(gomock.Any(), gomock.Any()).Return(tasks, nil)
2062+
return convergedClient.Client
2063+
},
2064+
vmId: "vm-uuid-456",
2065+
want: "",
2066+
wantErr: false,
2067+
},
2068+
{
2069+
name: "multiple running tasks returns first task UUID",
2070+
clientBuilder: func() *v4Converged.Client {
2071+
mockctrl := gomock.NewController(t)
2072+
convergedClient := NewMockConvergedClient(mockctrl)
2073+
tasks := []prismModels.Task{
2074+
{
2075+
ExtId: ptr.To("ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f"),
2076+
},
2077+
{
2078+
ExtId: ptr.To("ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d590"),
2079+
},
2080+
}
2081+
convergedClient.MockTasks.EXPECT().List(gomock.Any(), gomock.Any()).Return(tasks, nil)
2082+
return convergedClient.Client
2083+
},
2084+
vmId: "vm-uuid-456",
2085+
want: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2086+
wantErr: false,
2087+
},
2088+
{
2089+
name: "API error returns error",
2090+
clientBuilder: func() *v4Converged.Client {
2091+
mockctrl := gomock.NewController(t)
2092+
convergedClient := NewMockConvergedClient(mockctrl)
2093+
convergedClient.MockTasks.EXPECT().List(gomock.Any(), gomock.Any()).Return(nil, errors.New("API connection failed"))
2094+
return convergedClient.Client
2095+
},
2096+
vmId: "vm-uuid-456",
2097+
want: "",
2098+
wantErr: true,
2099+
errorMessage: "API connection failed",
2100+
},
2101+
}
2102+
2103+
for _, tt := range tests {
2104+
t.Run(tt.name, func(t *testing.T) {
2105+
t.Log("Running test case ", tt.name)
2106+
ctx := context.Background()
2107+
got, err := GetTaskUUIDFromVM(ctx, tt.clientBuilder(), tt.vmId)
2108+
if (err != nil) != tt.wantErr {
2109+
t.Errorf("GetTaskUUIDFromVM() error = %v, wantErr %v", err, tt.wantErr)
2110+
return
2111+
}
2112+
if got != tt.want {
2113+
t.Errorf("GetTaskUUIDFromVM() = %v, want %v", got, tt.want)
2114+
}
2115+
if tt.errorMessage != "" && err != nil {
2116+
if !strings.Contains(err.Error(), tt.errorMessage) {
2117+
t.Errorf("GetTaskUUIDFromVM() error message = %v, want to contain %v", err.Error(), tt.errorMessage)
2118+
}
2119+
}
2120+
})
2121+
}
2122+
}
2123+
2124+
func TestHasTaskInProgress(t *testing.T) {
2125+
tests := []struct {
2126+
name string
2127+
clientBuilder func() *v4Converged.Client
2128+
taskUUID string
2129+
want bool
2130+
wantErr bool
2131+
errorMessage string
2132+
}{
2133+
{
2134+
name: "task succeeded returns false",
2135+
clientBuilder: func() *v4Converged.Client {
2136+
mockctrl := gomock.NewController(t)
2137+
convergedClient := NewMockConvergedClient(mockctrl)
2138+
taskStatus := prismModels.TASKSTATUS_SUCCEEDED
2139+
convergedClient.MockTasks.EXPECT().Get(gomock.Any(), "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f").Return(&prismModels.Task{
2140+
Status: &taskStatus,
2141+
}, nil)
2142+
return convergedClient.Client
2143+
},
2144+
taskUUID: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2145+
want: false,
2146+
wantErr: false,
2147+
},
2148+
{
2149+
name: "task running returns true",
2150+
clientBuilder: func() *v4Converged.Client {
2151+
mockctrl := gomock.NewController(t)
2152+
convergedClient := NewMockConvergedClient(mockctrl)
2153+
taskStatus := prismModels.TASKSTATUS_RUNNING
2154+
convergedClient.MockTasks.EXPECT().Get(gomock.Any(), "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f").Return(&prismModels.Task{
2155+
Status: &taskStatus,
2156+
}, nil)
2157+
return convergedClient.Client
2158+
},
2159+
taskUUID: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2160+
want: true,
2161+
wantErr: false,
2162+
},
2163+
{
2164+
name: "task queued returns true",
2165+
clientBuilder: func() *v4Converged.Client {
2166+
mockctrl := gomock.NewController(t)
2167+
convergedClient := NewMockConvergedClient(mockctrl)
2168+
taskStatus := prismModels.TASKSTATUS_QUEUED
2169+
convergedClient.MockTasks.EXPECT().Get(gomock.Any(), "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f").Return(&prismModels.Task{
2170+
Status: &taskStatus,
2171+
}, nil)
2172+
return convergedClient.Client
2173+
},
2174+
taskUUID: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2175+
want: true,
2176+
wantErr: false,
2177+
},
2178+
{
2179+
name: "task failed returns false (no error in v4 implementation)",
2180+
clientBuilder: func() *v4Converged.Client {
2181+
mockctrl := gomock.NewController(t)
2182+
convergedClient := NewMockConvergedClient(mockctrl)
2183+
taskStatus := prismModels.TASKSTATUS_FAILED
2184+
convergedClient.MockTasks.EXPECT().Get(gomock.Any(), "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f").Return(&prismModels.Task{
2185+
Status: &taskStatus,
2186+
}, nil)
2187+
return convergedClient.Client
2188+
},
2189+
taskUUID: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2190+
want: false,
2191+
wantErr: false,
2192+
},
2193+
{
2194+
name: "task suspended returns false",
2195+
clientBuilder: func() *v4Converged.Client {
2196+
mockctrl := gomock.NewController(t)
2197+
convergedClient := NewMockConvergedClient(mockctrl)
2198+
taskStatus := prismModels.TASKSTATUS_SUSPENDED
2199+
convergedClient.MockTasks.EXPECT().Get(gomock.Any(), "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f").Return(&prismModels.Task{
2200+
Status: &taskStatus,
2201+
}, nil)
2202+
return convergedClient.Client
2203+
},
2204+
taskUUID: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2205+
want: false,
2206+
wantErr: false,
2207+
},
2208+
{
2209+
name: "API error returns error",
2210+
clientBuilder: func() *v4Converged.Client {
2211+
mockctrl := gomock.NewController(t)
2212+
convergedClient := NewMockConvergedClient(mockctrl)
2213+
convergedClient.MockTasks.EXPECT().Get(gomock.Any(), "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f").Return(nil, errors.New("API error"))
2214+
return convergedClient.Client
2215+
},
2216+
taskUUID: "ZXJnb24=:b4b17e07-b81c-43f4-9bf5-62149975d58f",
2217+
want: false,
2218+
wantErr: true,
2219+
errorMessage: "API error",
2220+
},
2221+
}
2222+
2223+
for _, tt := range tests {
2224+
t.Run(tt.name, func(t *testing.T) {
2225+
t.Log("Running test case ", tt.name)
2226+
ctx := context.Background()
2227+
got, err := HasTaskInProgress(ctx, tt.clientBuilder(), tt.taskUUID)
2228+
if (err != nil) != tt.wantErr {
2229+
t.Errorf("HasTaskInProgress() error = %v, wantErr %v", err, tt.wantErr)
2230+
return
2231+
}
2232+
if got != tt.want {
2233+
t.Errorf("HasTaskInProgress() = %v, want %v", got, tt.want)
2234+
}
2235+
if tt.errorMessage != "" && err != nil {
2236+
if !strings.Contains(err.Error(), tt.errorMessage) {
2237+
t.Errorf("HasTaskInProgress() error message = %v, want to contain %v", err.Error(), tt.errorMessage)
2238+
}
2239+
}
2240+
})
2241+
}
2242+
}

‎controllers/nutanixmachine_controller.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r
357357
}
358358

359359
log.V(1).Info(fmt.Sprintf("VM %s with UUID %s was found.", *vm.Spec.Name, vmUUID))
360-
lastTaskUUID, err := GetTaskUUIDFromVM(vm)
360+
lastTaskUUID, err := GetTaskUUIDFromVM(ctx, convergedClient, vmUUID)
361361
if err != nil {
362362
errorMsg := fmt.Errorf("error occurred fetching task UUID from VM: %v", err)
363363
log.Error(errorMsg, "error fetching task UUID")
@@ -367,7 +367,7 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r
367367

368368
if lastTaskUUID != "" {
369369
log.Info(fmt.Sprintf("checking if VM %s with UUID %s has in progress tasks", vmName, vmUUID))
370-
taskInProgress, err := HasTaskInProgress(ctx, rctx.NutanixClient, lastTaskUUID)
370+
taskInProgress, err := HasTaskInProgress(ctx, rctx.ConvergedClient, lastTaskUUID)
371371
if err != nil {
372372
log.Error(err, fmt.Sprintf("error occurred while checking task %s for VM %s. Trying to delete VM", lastTaskUUID, vmName))
373373
}
@@ -793,6 +793,7 @@ func (r *NutanixMachineReconciler) getOrCreateVM(rctx *nctx.MachineContext) (*pr
793793
log := ctrl.LoggerFrom(ctx)
794794
vmName := rctx.Machine.Name
795795
v3Client := rctx.NutanixClient
796+
convergedClient := rctx.ConvergedClient
796797

797798
// Check if the VM already exists
798799
vm, err = FindVM(ctx, v3Client, rctx.NutanixMachine, vmName)
@@ -923,9 +924,10 @@ func (r *NutanixMachineReconciler) getOrCreateVM(rctx *nctx.MachineContext) (*pr
923924
rctx.NutanixMachine.Spec.ProviderID = GenerateProviderID(vmUuid)
924925
rctx.NutanixMachine.Status.VmUUID = vmUuid
925926

927+
// Refactor this when we migrate this helper function to v4ConvergedClient
926928
log.V(1).Info(fmt.Sprintf("Sent the post request to create VM %s. Got the vm UUID: %s, status.state: %s", vmName, vmUuid, *vmResponse.Status.State))
927929
log.V(1).Info(fmt.Sprintf("Getting task vmUUID for VM %s", vmName))
928-
lastTaskUUID, err := GetTaskUUIDFromVM(vmResponse)
930+
lastTaskUUID, err := GetTaskUUIDFromVM(ctx, convergedClient, vmUuid)
929931
if err != nil {
930932
errorMsg := fmt.Errorf("error occurred fetching task UUID from vm %s after creation: %v", rctx.Machine.Name, err)
931933
rctx.SetFailureStatus(createErrorFailureReason, errorMsg)
@@ -938,6 +940,12 @@ func (r *NutanixMachineReconciler) getOrCreateVM(rctx *nctx.MachineContext) (*pr
938940
return nil, errorMsg
939941
}
940942

943+
// Task ExtId is in the format of "<service base64>:<taskUUID>"
944+
taskUUIDsParts := strings.Split(lastTaskUUID, ":")
945+
if len(taskUUIDsParts) == 2 {
946+
lastTaskUUID = taskUUIDsParts[1]
947+
}
948+
941949
log.Info(fmt.Sprintf("Waiting for task %s to get completed for VM %s", lastTaskUUID, rctx.NutanixMachine.Name))
942950
if err := nutanixclient.WaitForTaskToSucceed(ctx, v3Client, lastTaskUUID); err != nil {
943951
errorMsg := fmt.Errorf("error occurred while waiting for task %s to start: %v", lastTaskUUID, err)

0 commit comments

Comments
 (0)