Skip to content

Commit 28cae6c

Browse files
abhay-nutanixabhay-nutanix
authored
feat: Change detachVolumeGroupsFromVM helper to work with V4 API (converged client) (#616)
1 parent ca31a0f commit 28cae6c

File tree

8 files changed

+395
-44
lines changed

8 files changed

+395
-44
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ mocks: ## Generate mocks for the project
378378
mockgen -destination=mocks/converged/storage_containers.go -package=mockconverged github.com/nutanix-cloud-native/prism-go-client/converged StorageContainers
379379
mockgen -destination=mocks/converged/subnets.go -package=mockconverged github.com/nutanix-cloud-native/prism-go-client/converged Subnets
380380
mockgen -destination=mocks/converged/tasks.go -package=mockconverged github.com/nutanix-cloud-native/prism-go-client/converged Tasks
381+
mockgen -destination=mocks/converged/volume_groups.go -package=mockconverged github.com/nutanix-cloud-native/prism-go-client/converged VolumeGroups
381382

382383
GOTESTPKGS = $(shell go list ./... | grep -v /mocks | grep -v /templates)
383384

‎controllers/helpers.go‎

Lines changed: 3 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,11 @@ import (
3232
"github.com/nutanix-cloud-native/prism-go-client/converged"
3333
v4Converged "github.com/nutanix-cloud-native/prism-go-client/converged/v4"
3434
prismclientv3 "github.com/nutanix-cloud-native/prism-go-client/v3"
35-
prismclientv4 "github.com/nutanix-cloud-native/prism-go-client/v4"
3635
clusterModels "github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4/models/clustermgmt/v4/config"
3736
subnetModels "github.com/nutanix/ntnx-api-golang-clients/networking-go-client/v4/models/networking/v4/config"
3837
prismModels "github.com/nutanix/ntnx-api-golang-clients/prism-go-client/v4/models/prism/v4/config"
3938
vmmconfig "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/ahv/config"
4039
imageModels "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
41-
prismconfig "github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4/models/prism/v4/config"
4240
volumesconfig "github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4/models/volumes/v4/config"
4341
"k8s.io/apimachinery/pkg/api/resource"
4442
v1 "k8s.io/client-go/informers/core/v1"
@@ -1046,31 +1044,6 @@ func getPrismCentralClientForCluster(ctx context.Context, cluster *infrav1.Nutan
10461044
return v3Client, nil
10471045
}
10481046

1049-
func getPrismCentralV4ClientForCluster(ctx context.Context, cluster *infrav1.NutanixCluster, secretInformer v1.SecretInformer, mapInformer v1.ConfigMapInformer) (*prismclientv4.Client, error) {
1050-
log := ctrl.LoggerFrom(ctx)
1051-
1052-
clientHelper := nutanixclient.NewHelper(secretInformer, mapInformer)
1053-
managementEndpoint, err := clientHelper.BuildManagementEndpoint(ctx, cluster)
1054-
if err != nil {
1055-
log.Error(err, fmt.Sprintf("error occurred while getting management endpoint for cluster %q", cluster.GetNamespacedName()))
1056-
conditions.MarkFalse(cluster, infrav1.PrismCentralV4ClientCondition, infrav1.PrismCentralV4ClientInitializationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
1057-
return nil, err
1058-
}
1059-
1060-
client, err := nutanixclient.NutanixClientCacheV4.GetOrCreate(&nutanixclient.CacheParams{
1061-
NutanixCluster: cluster,
1062-
PrismManagementEndpoint: managementEndpoint,
1063-
})
1064-
if err != nil {
1065-
log.Error(err, "error occurred while getting nutanix prism v4 client from cache")
1066-
conditions.MarkFalse(cluster, infrav1.PrismCentralV4ClientCondition, infrav1.PrismCentralV4ClientInitializationFailed, capiv1.ConditionSeverityError, "%s", err.Error())
1067-
return nil, fmt.Errorf("nutanix prism v4 client error: %w", err)
1068-
}
1069-
1070-
conditions.MarkTrue(cluster, infrav1.PrismCentralV4ClientCondition)
1071-
return client, nil
1072-
}
1073-
10741047
func getPrismCentralConvergedV4ClientForCluster(ctx context.Context, cluster *infrav1.NutanixCluster, secretInformer v1.SecretInformer, mapInformer v1.ConfigMapInformer) (*v4Converged.Client, error) {
10751048
log := ctrl.LoggerFrom(ctx)
10761049

@@ -1109,7 +1082,7 @@ func isBackedByVolumeGroupReference(disk *vmmconfig.Disk) bool {
11091082
return ok
11101083
}
11111084

1112-
func detachVolumeGroupsFromVM(ctx context.Context, v4Client *prismclientv4.Client, vmName string, vmUUID string, vmDiskList []vmmconfig.Disk) error {
1085+
func detachVolumeGroupsFromVM(ctx context.Context, client *v4Converged.Client, vmName string, vmUUID string, vmDiskList []vmmconfig.Disk) error {
11131086
log := ctrl.LoggerFrom(ctx)
11141087
// Detach the volume groups from the virtual machine
11151088
for _, disk := range vmDiskList {
@@ -1125,15 +1098,11 @@ func detachVolumeGroupsFromVM(ctx context.Context, v4Client *prismclientv4.Clien
11251098
ExtId: ptr.To(vmUUID),
11261099
}
11271100

1128-
resp, err := v4Client.VolumeGroupsApiInstance.DetachVm(&volumeGroupExtId, body)
1101+
_, err := client.VolumeGroups.DetachFromVM(ctx, volumeGroupExtId, *body)
11291102
if err != nil {
11301103
return fmt.Errorf("failed to detach volume group %s from virtual machine %s: %w", volumeGroupExtId, vmUUID, err)
11311104
}
1132-
1133-
data := resp.GetData()
1134-
if _, ok := data.(prismconfig.TaskReference); !ok {
1135-
return fmt.Errorf("failed to cast response to TaskReference")
1136-
}
1105+
return nil
11371106
}
11381107

11391108
return nil

‎controllers/helpers_test.go‎

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
vmmModels "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/ahv/config"
4141
policyModels "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/ahv/policies"
4242
imageModels "github.com/nutanix/ntnx-api-golang-clients/vmm-go-client/v4/models/vmm/v4/content"
43+
volumesconfig "github.com/nutanix/ntnx-api-golang-clients/volumes-go-client/v4/models/volumes/v4/config"
4344

4445
. "github.com/onsi/ginkgo/v2"
4546
. "github.com/onsi/gomega"
@@ -2299,6 +2300,215 @@ func TestGpuVendorStringToGpuVendor(t *testing.T) {
22992300
}
23002301
}
23012302

2303+
func TestDetachVolumeGroupsFromVM(t *testing.T) {
2304+
ctrl := gomock.NewController(t)
2305+
defer ctrl.Finish()
2306+
2307+
ctx := context.Background()
2308+
2309+
t.Run("should skip detachment when no disks provided", func(t *testing.T) {
2310+
mockClientWrapper := NewMockConvergedClient(ctrl)
2311+
2312+
vmName := "test-vm"
2313+
vmUUID := "00000000-0000-0000-0000-000000000001"
2314+
vmDisks := []vmmModels.Disk{}
2315+
2316+
// No expectations: DetachFromVM should NOT be called
2317+
err := detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2318+
2319+
assert.NoError(t, err)
2320+
})
2321+
2322+
t.Run("should skip detachment when disk has no backing info", func(t *testing.T) {
2323+
mockClientWrapper := NewMockConvergedClient(ctrl)
2324+
2325+
vmName := "test-vm"
2326+
vmUUID := "00000000-0000-0000-0000-000000000001"
2327+
2328+
// Disk without VG backing (leave BackingInfo nil)
2329+
disk := vmmModels.NewDisk()
2330+
vmDisks := []vmmModels.Disk{*disk}
2331+
2332+
// No expectations: DetachFromVM should NOT be called
2333+
err := detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2334+
2335+
assert.NoError(t, err)
2336+
})
2337+
2338+
t.Run("should skip detachment when disk is not backed by volume group", func(t *testing.T) {
2339+
mockClientWrapper := NewMockConvergedClient(ctrl)
2340+
2341+
vmName := "test-vm"
2342+
vmUUID := "00000000-0000-0000-0000-000000000001"
2343+
2344+
// Disk backed by VmDisk (not volume group)
2345+
disk := vmmModels.NewDisk()
2346+
vmDisk := vmmModels.NewVmDisk()
2347+
vmDisk.DiskSizeBytes = ptr.To(int64(21474836480))
2348+
err := disk.SetBackingInfo(*vmDisk)
2349+
require.NoError(t, err)
2350+
2351+
vmDisks := []vmmModels.Disk{*disk}
2352+
2353+
// No expectations: DetachFromVM should NOT be called
2354+
err = detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2355+
2356+
assert.NoError(t, err)
2357+
})
2358+
2359+
t.Run("should successfully detach single volume group", func(t *testing.T) {
2360+
mockClientWrapper := NewMockConvergedClient(ctrl)
2361+
2362+
vmName := "test-vm"
2363+
vmUUID := "00000000-0000-0000-0000-000000000001"
2364+
vgID := "11111111-1111-1111-1111-111111111111"
2365+
2366+
// Build a Disk backed by an ADSFVolumeGroupReference
2367+
disk := vmmModels.NewDisk()
2368+
ref := vmmModels.NewADSFVolumeGroupReference()
2369+
ref.VolumeGroupExtId = &vgID
2370+
err := disk.SetBackingInfo(*ref)
2371+
require.NoError(t, err)
2372+
2373+
vmDisks := []vmmModels.Disk{*disk}
2374+
2375+
expectedVG := &volumesconfig.VolumeGroup{
2376+
ExtId: ptr.To(vgID),
2377+
}
2378+
2379+
// Expect DetachFromVM to be called once with the correct VG ID
2380+
mockClientWrapper.MockVolumeGroups.EXPECT().
2381+
DetachFromVM(ctx, vgID, gomock.Any()).
2382+
DoAndReturn(func(_ context.Context, _ string, attachment volumesconfig.VmAttachment) (*volumesconfig.VolumeGroup, error) {
2383+
assert.Equal(t, vmUUID, *attachment.ExtId)
2384+
return expectedVG, nil
2385+
})
2386+
2387+
err = detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2388+
2389+
assert.NoError(t, err)
2390+
})
2391+
2392+
t.Run("should return error when DetachFromVM fails", func(t *testing.T) {
2393+
mockClientWrapper := NewMockConvergedClient(ctrl)
2394+
2395+
vmName := "test-vm"
2396+
vmUUID := "00000000-0000-0000-0000-000000000001"
2397+
vgID := "22222222-2222-2222-2222-222222222222"
2398+
2399+
disk := vmmModels.NewDisk()
2400+
ref := vmmModels.NewADSFVolumeGroupReference()
2401+
ref.VolumeGroupExtId = &vgID
2402+
err := disk.SetBackingInfo(*ref)
2403+
require.NoError(t, err)
2404+
2405+
vmDisks := []vmmModels.Disk{*disk}
2406+
2407+
expectedError := errors.New("failed to detach volume group")
2408+
2409+
// Return error from DetachFromVM
2410+
mockClientWrapper.MockVolumeGroups.EXPECT().
2411+
DetachFromVM(ctx, vgID, gomock.Any()).
2412+
Return(nil, expectedError)
2413+
2414+
err = detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2415+
2416+
assert.Error(t, err)
2417+
assert.Contains(t, err.Error(), "failed to detach volume group")
2418+
assert.Contains(t, err.Error(), vgID)
2419+
assert.Contains(t, err.Error(), vmUUID)
2420+
})
2421+
2422+
t.Run("should return after detaching first volume group", func(t *testing.T) {
2423+
mockClientWrapper := NewMockConvergedClient(ctrl)
2424+
2425+
vmName := "test-vm"
2426+
vmUUID := "00000000-0000-0000-0000-000000000001"
2427+
vgID1 := "11111111-1111-1111-1111-111111111111"
2428+
vgID2 := "22222222-2222-2222-2222-222222222222"
2429+
2430+
// Build two disks backed by volume groups
2431+
disk1 := vmmModels.NewDisk()
2432+
ref1 := vmmModels.NewADSFVolumeGroupReference()
2433+
ref1.VolumeGroupExtId = &vgID1
2434+
err := disk1.SetBackingInfo(*ref1)
2435+
require.NoError(t, err)
2436+
2437+
disk2 := vmmModels.NewDisk()
2438+
ref2 := vmmModels.NewADSFVolumeGroupReference()
2439+
ref2.VolumeGroupExtId = &vgID2
2440+
err = disk2.SetBackingInfo(*ref2)
2441+
require.NoError(t, err)
2442+
2443+
vmDisks := []vmmModels.Disk{*disk1, *disk2}
2444+
2445+
// Only expect DetachFromVM to be called once (for the first VG)
2446+
// The function returns after the first detachment
2447+
mockClientWrapper.MockVolumeGroups.EXPECT().
2448+
DetachFromVM(ctx, vgID1, gomock.Any()).
2449+
Return(&volumesconfig.VolumeGroup{}, nil).
2450+
Times(1)
2451+
2452+
err = detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2453+
2454+
assert.NoError(t, err)
2455+
})
2456+
2457+
t.Run("should handle mixed disk types and only detach volume groups", func(t *testing.T) {
2458+
mockClientWrapper := NewMockConvergedClient(ctrl)
2459+
2460+
vmName := "test-vm"
2461+
vmUUID := "00000000-0000-0000-0000-000000000001"
2462+
vgID := "11111111-1111-1111-1111-111111111111"
2463+
2464+
// First disk: regular VmDisk (should be skipped)
2465+
disk1 := vmmModels.NewDisk()
2466+
vmDisk := vmmModels.NewVmDisk()
2467+
vmDisk.DiskSizeBytes = ptr.To(int64(21474836480))
2468+
err := disk1.SetBackingInfo(*vmDisk)
2469+
require.NoError(t, err)
2470+
2471+
// Second disk: backed by volume group (should be detached)
2472+
disk2 := vmmModels.NewDisk()
2473+
ref := vmmModels.NewADSFVolumeGroupReference()
2474+
ref.VolumeGroupExtId = &vgID
2475+
err = disk2.SetBackingInfo(*ref)
2476+
require.NoError(t, err)
2477+
2478+
vmDisks := []vmmModels.Disk{*disk1, *disk2}
2479+
2480+
// Only expect DetachFromVM to be called for the volume group disk
2481+
mockClientWrapper.MockVolumeGroups.EXPECT().
2482+
DetachFromVM(ctx, vgID, gomock.Any()).
2483+
Return(&volumesconfig.VolumeGroup{}, nil)
2484+
2485+
err = detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2486+
2487+
assert.NoError(t, err)
2488+
})
2489+
2490+
t.Run("should handle nil volume group ExtId gracefully", func(t *testing.T) {
2491+
mockClientWrapper := NewMockConvergedClient(ctrl)
2492+
2493+
vmName := "test-vm"
2494+
vmUUID := "00000000-0000-0000-0000-000000000001"
2495+
2496+
// Build a disk with volume group reference but nil ExtId
2497+
disk := vmmModels.NewDisk()
2498+
ref := vmmModels.NewADSFVolumeGroupReference()
2499+
ref.VolumeGroupExtId = nil // Nil ExtId should cause panic or error
2500+
err := disk.SetBackingInfo(*ref)
2501+
require.NoError(t, err)
2502+
2503+
vmDisks := []vmmModels.Disk{*disk}
2504+
2505+
// This should panic when trying to dereference nil VolumeGroupExtId
2506+
assert.Panics(t, func() {
2507+
_ = detachVolumeGroupsFromVM(ctx, mockClientWrapper.Client, vmName, vmUUID, vmDisks)
2508+
})
2509+
})
2510+
}
2511+
23022512
type MockConvergedClientWrapper struct {
23032513
Client *v4Converged.Client
23042514

@@ -2310,6 +2520,7 @@ type MockConvergedClientWrapper struct {
23102520
MockSubnets *mockconverged.MockSubnets[subnetModels.Subnet]
23112521
MockVMs *mockconverged.MockVMs[vmmModels.Vm]
23122522
MockTasks *mockconverged.MockTasks[prismModels.Task, prismErrors.AppMessage]
2523+
MockVolumeGroups *mockconverged.MockVolumeGroups[volumesconfig.VolumeGroup, volumesconfig.VmAttachment]
23132524
}
23142525

23152526
// NewMockConvergedClient creates a new mock converged client
@@ -2323,6 +2534,7 @@ func NewMockConvergedClient(ctrl *gomock.Controller) *MockConvergedClientWrapper
23232534
mockTasks := mockconverged.NewMockTasks[prismModels.Task, prismErrors.AppMessage](ctrl)
23242535
// Create mock VMs service with the correct type
23252536
mockVMs := mockconverged.NewMockVMs[vmmModels.Vm](ctrl)
2537+
mockVolumeGroups := mockconverged.NewMockVolumeGroups[volumesconfig.VolumeGroup, volumesconfig.VmAttachment](ctrl)
23262538

23272539
realClient := &v4Converged.Client{
23282540
Client: converged.Client[
@@ -2337,6 +2549,8 @@ func NewMockConvergedClient(ctrl *gomock.Controller) *MockConvergedClientWrapper
23372549
vmmModels.Vm,
23382550
prismModels.Task,
23392551
prismErrors.AppMessage,
2552+
volumesconfig.VolumeGroup,
2553+
volumesconfig.VmAttachment,
23402554
]{
23412555
AntiAffinityPolicies: mockAntiAffinityPolicies,
23422556
Clusters: mockClusters,
@@ -2346,6 +2560,7 @@ func NewMockConvergedClient(ctrl *gomock.Controller) *MockConvergedClientWrapper
23462560
Subnets: mockSubnets,
23472561
VMs: mockVMs,
23482562
Tasks: mockTasks,
2563+
VolumeGroups: mockVolumeGroups,
23492564
},
23502565
}
23512566

@@ -2359,5 +2574,6 @@ func NewMockConvergedClient(ctrl *gomock.Controller) *MockConvergedClientWrapper
23592574
MockSubnets: mockSubnets,
23602575
MockVMs: mockVMs,
23612576
MockTasks: mockTasks,
2577+
MockVolumeGroups: mockVolumeGroups,
23622578
}
23632579
}

‎controllers/nutanixmachine_controller.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -399,15 +399,9 @@ func (r *NutanixMachineReconciler) reconcileDelete(rctx *nctx.MachineContext) (r
399399
}
400400

401401
func (r *NutanixMachineReconciler) detachVolumeGroups(rctx *nctx.MachineContext, vmName string, vmUUID string, vmDiskList []vmmconfig.Disk) error {
402-
v4Client, err := getPrismCentralV4ClientForCluster(rctx.Context, rctx.NutanixCluster, r.SecretInformer, r.ConfigMapInformer)
403-
if err != nil {
404-
return fmt.Errorf("error occurred while fetching Prism Central v4 client: %w", err)
405-
}
406-
407-
if err := detachVolumeGroupsFromVM(rctx.Context, v4Client, vmName, vmUUID, vmDiskList); err != nil {
402+
if err := detachVolumeGroupsFromVM(rctx.Context, rctx.ConvergedClient, vmName, vmUUID, vmDiskList); err != nil {
408403
return fmt.Errorf("failed to detach volume groups from VM %s with UUID %s: %w", vmName, vmUUID, err)
409404
}
410-
411405
return nil
412406
}
413407

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ require (
187187
)
188188

189189
replace (
190-
github.com/nutanix-cloud-native/prism-go-client => github.com/nutanix-cloud-native/prism-go-client v0.5.2-0.20251007035527-2603309773ae
190+
github.com/nutanix-cloud-native/prism-go-client => github.com/nutanix-cloud-native/prism-go-client v0.5.2-0.20251023040331-ea5df46774dd
191191
// CVE fixes for https://avd.aquasec.com/nvd/2024/cve-2024-45338
192192
golang.org/x/net => golang.org/x/net v0.33.0
193193
sigs.k8s.io/kind v0.20.0 => sigs.k8s.io/kind v0.22.0

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,8 @@ github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A=
268268
github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc=
269269
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
270270
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
271-
github.com/nutanix-cloud-native/prism-go-client v0.5.2-0.20251007035527-2603309773ae h1:lvRDs0dPOx0lLY811IeFcmHYb6qKKdK51hEoqLd4108=
272-
github.com/nutanix-cloud-native/prism-go-client v0.5.2-0.20251007035527-2603309773ae/go.mod h1:MGMzqONi2f9aZHeWwQYwL5j/r6Kxv56g6VIvkVFIlbU=
271+
github.com/nutanix-cloud-native/prism-go-client v0.5.2-0.20251023040331-ea5df46774dd h1:UhfmnlEC/l7Ll+bzXkPF9J+IA0mJPbMWgPpzzZnJYGM=
272+
github.com/nutanix-cloud-native/prism-go-client v0.5.2-0.20251023040331-ea5df46774dd/go.mod h1:MGMzqONi2f9aZHeWwQYwL5j/r6Kxv56g6VIvkVFIlbU=
273273
github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4 v4.1.1 h1:4ZkiY4yavcy3Suwi5ntofjh3mzL3muTCVs9OPkZ940k=
274274
github.com/nutanix/ntnx-api-golang-clients/clustermgmt-go-client/v4 v4.1.1/go.mod h1:sd4Fnk6MVfEDVY+8WyRoQTmLhi2SgZ3riySWErVHf8E=
275275
github.com/nutanix/ntnx-api-golang-clients/networking-go-client/v4 v4.1.1 h1:XqAQX+sT8EZBRoJbhBm2jpjYGnHd//mjx1PjQI+zIGQ=

0 commit comments

Comments
 (0)