Skip to content

Commit

Permalink
Fix comments on spbm review
Browse files Browse the repository at this point in the history
  • Loading branch information
rikatz committed Oct 30, 2023
1 parent 1172a62 commit 67dd3d7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
20 changes: 10 additions & 10 deletions pkg/services/govmomi/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,40 +401,40 @@ func (vms *VMService) reconcileStoragePolicy(ctx context.Context, virtualMachine
return err
}

arrayOfEntities := make([]pbmTypes.PbmServerObjectRef, 0)
disksRefs := make([]pbmTypes.PbmServerObjectRef, 0)
// diskMap is just an auxiliar map so we don't need to iterate over and over disks to get their configs
// if we realize they are not on the right storage policy
diskMap := make(map[string]*types.VirtualDisk)

disks := devices.SelectByType((*types.VirtualDisk)(nil))

// We iterate over disks and create an array of entities, so we just need to make a single call
// We iterate over disks and create an array of disks refs, so we just need to make a single call
// against vCenter, instead of one call per disk
// the diskMap is an auxiliar way of, besides the arrayOfEntities, we have a "searchable" disk configuration
// the diskMap is an auxiliar way of, besides the disksRefs, we have a "searchable" disk configuration
// in case we need to reconfigure a disk, to get its config
for _, d := range disks {
disk := d.(*types.VirtualDisk)
// entities associated with storage policy has key in the form <vm-ID>:<disk>
diskID := fmt.Sprintf("%s:%d", virtualMachineCtx.Obj.Reference().Value, disk.Key)
diskMap[diskID] = disk

arrayOfEntities = append(arrayOfEntities, pbmTypes.PbmServerObjectRef{
disksRefs = append(disksRefs, pbmTypes.PbmServerObjectRef{
ObjectType: string(pbmTypes.PbmObjectTypeVirtualDiskId),
Key: diskID,
})
}

entities, err := QueryAssociatedProfiles(ctx, pbmClient, arrayOfEntities)
diskObjects, err := QueryAssociatedProfiles(ctx, pbmClient, disksRefs)
if err != nil {
return err
return errors.Wrap(err, "unable to query disks associated profiles")
}

for k := range entities {
if !isStoragePolicyIDPresent(storageProfileID, entities[k]) {
virtualMachineCtx.Logger.V(5).Info("storage policy not found on disk, adding for reconciliation", "disk", entities[k].Object.Key)
for k := range diskObjects {
if !isStoragePolicyIDPresent(storageProfileID, diskObjects[k]) {
virtualMachineCtx.Logger.V(5).Info("storage policy not found on disk, adding for reconciliation", "disk", diskObjects[k].Object.Key)
config := &types.VirtualDeviceConfigSpec{
Operation: types.VirtualDeviceConfigSpecOperationEdit,
Device: diskMap[entities[k].Object.Key],
Device: diskMap[diskObjects[k].Object.Key],
Profile: []types.BaseVirtualMachineProfileSpec{
&types.VirtualMachineDefinedProfileSpec{ProfileId: storageProfileID},
},
Expand Down
1 change: 1 addition & 0 deletions pkg/services/govmomi/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ func Test_ReconcileStoragePolicy(t *testing.T) {
})

t.Run("when the requested storage policy exists should pass", func(t *testing.T) {
// This Method should be implemented on Govmomi vcsim and then we can unskip this test
t.Skip("PbmQueryAssociatedProfiles is not yet implemented on PBM simulator")
g = NewWithT(t)
before()
Expand Down

0 comments on commit 67dd3d7

Please sign in to comment.