Skip to content

Commit

Permalink
api: Support DC for ResolveLibItemStor
Browse files Browse the repository at this point in the history 
This patch adds a datacenter parameter to the
`ResolveLibraryItemStorage` function for when the
datacenter is known in advance. This is more
efficient as it obviates the need to discover the
datacenter each time through a loop.

BREAKING: This change requires updating the use of the
          ResolveLibraryItemStorage function to account for
          its signature change.

Signed-off-by: akutz <akutz@vmware.com>
  • Loading branch information
@akutz
akutz committed Dec 20, 2024
1 parent ff82b3a commit 64ca62f
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 92 deletions.
2 changes: 1 addition & 1 deletion cli/library/info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ func (r infoResultsWriter) writeFile(
}
if r.cmd.Stor {
label = "Resolved URI"
err = r.cmd.pathFinder.ResolveLibraryItemStorage(r.ctx, s)
err = r.cmd.pathFinder.ResolveLibraryItemStorage(r.ctx, nil, s)
if err != nil {
return err
}
Expand Down
121 changes: 84 additions & 37 deletions vapi/library/finder/path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ package finder

import (
"context"
"errors"
"fmt"
"net/url"
"path"
"strings"

"github.com/vmware/govmomi/internal"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/property"
"github.com/vmware/govmomi/vapi/library"
Expand Down Expand Up @@ -135,79 +135,126 @@ func (f *PathFinder) datastoreName(ctx context.Context, id string) (string, erro
return name, nil
}

func (f *PathFinder) convertPath(ctx context.Context, b *mo.Datastore, path string) (string, error) {
if !internal.IsDatastoreVSAN(*b) {
func (f *PathFinder) convertPath(
ctx context.Context,
dc *object.Datacenter,
ds mo.Datastore,
path string) (string, error) {

if v := ds.Capability.TopLevelDirectoryCreateSupported; v != nil && *v {
return path, nil
}

var dc *object.Datacenter

entities, err := mo.Ancestors(ctx, f.c, f.c.ServiceContent.PropertyCollector, b.Self)
if err != nil {
return "", err
if dc == nil {
entities, err := mo.Ancestors(
ctx,
f.c,
f.c.ServiceContent.PropertyCollector,
ds.Self)
if err != nil {
return "", fmt.Errorf("failed to find ancestors: %w", err)
}
for _, entity := range entities {
if entity.Self.Type == "Datacenter" {
dc = object.NewDatacenter(f.c, entity.Self)
break
}
}
}

for _, entity := range entities {
if entity.Self.Type == "Datacenter" {
dc = object.NewDatacenter(f.c, entity.Self)
break
}
if dc == nil {
return "", errors.New("failed to find datacenter")
}

m := object.NewDatastoreNamespaceManager(f.c)
return m.ConvertNamespacePathToUuidPath(ctx, dc, path)
}

// ResolveLibraryItemStorage transforms StorageURIs Datastore url (uuid) to Datastore name.
func (f *PathFinder) ResolveLibraryItemStorage(ctx context.Context, storage []library.Storage) error {
// ResolveLibraryItemStorage transforms the StorageURIs field in the provided
// storage items from a datastore URL, ex.
// "ds:///vmfs/volumes/DATASTORE_UUID/contentlib-LIB_UUID/ITEM_UUID/file.vmdk",
// to the format that includes the datastore name, ex.
// "[DATASTORE_NAME] contentlib-LIB_UUID/ITEM_UUID/file.vmdk".
//
// If a storage item resides on a datastore that does not support the creation
// of top-level directories, then this means the datastore is vSAN and the
// storage item path needs to be further converted. If this occurs, then the
// datacenter to which the datastore belongs is required. If the datacenter
// parameter is non-nil, it is used, otherwise the datacenter for each datastore
// is resolved as needed. It is much more efficient to send in the datacenter if
// it is known ahead of time that the content library is stored on a vSAN
// datastore.
func (f *PathFinder) ResolveLibraryItemStorage(
ctx context.Context,
datacenter *object.Datacenter,
storage []library.Storage) error {

// TODO:
// - reuse PathFinder.cache
// - the transform here isn't Content Library specific, but is currently the only known use case
backing := map[string]*mo.Datastore{}
var ids []types.ManagedObjectReference

// don't think we can have more than 1 Datastore backing currently, future proof anyhow
// - the transform here isn't Content Library specific, but is currently
// the only known use case
var (
ids []types.ManagedObjectReference
datastoreMap = map[string]mo.Datastore{}
)

// Currently ContentLibrary only supports a single storage backing, but this
// future proofs things.
for _, item := range storage {
id := item.StorageBacking.DatastoreID
if _, ok := backing[id]; ok {
if _, ok := datastoreMap[id]; ok {
continue
}
backing[id] = nil
ids = append(ids, types.ManagedObjectReference{Type: "Datastore", Value: id})
datastoreMap[id] = mo.Datastore{}
ids = append(
ids,
types.ManagedObjectReference{Type: "Datastore", Value: id})
}

var ds []mo.Datastore
pc := property.DefaultCollector(f.c)
props := []string{"name", "summary.url", "summary.type"}
if err := pc.Retrieve(ctx, ids, props, &ds); err != nil {
var (
datastores []mo.Datastore
pc = property.DefaultCollector(f.c)
props = []string{
"name",
"summary.url",
"capability.topLevelDirectoryCreateSupported",
}
)

if err := pc.Retrieve(ctx, ids, props, &datastores); err != nil {
return err
}

for i := range ds {
backing[ds[i].Self.Value] = &ds[i]
for i := range datastores {
datastoreMap[datastores[i].Self.Value] = datastores[i]
}

for _, item := range storage {
b := backing[item.StorageBacking.DatastoreID]
dsurl := b.Summary.Url
ds := datastoreMap[item.StorageBacking.DatastoreID]
dsURL := ds.Summary.Url

for i := range item.StorageURIs {
uri, err := url.Parse(item.StorageURIs[i])
szURI := item.StorageURIs[i]
uri, err := url.Parse(szURI)
if err != nil {
return err
return fmt.Errorf(
"failed to parse storage URI %q: %w", szURI, err)
}

uri.OmitHost = false // `ds://` required for ConvertNamespacePathToUuidPath()
uri.Path = path.Clean(uri.Path) // required for ConvertNamespacePathToUuidPath()
uri.RawQuery = ""
u, err := f.convertPath(ctx, b, uri.String())

uriPath := uri.String()
u, err := f.convertPath(ctx, datacenter, ds, uriPath)
if err != nil {
return err
return fmt.Errorf("failed to convert path %q: %w", uriPath, err)
}
u = strings.TrimPrefix(u, dsurl)
u = strings.TrimPrefix(u, dsURL)
u = strings.TrimPrefix(u, "/")

item.StorageURIs[i] = (&object.DatastorePath{
Datastore: b.Name,
Datastore: ds.Name,
Path: u,
}).String()
}
Expand Down
180 changes: 126 additions & 54 deletions vapi/library/finder/path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ package finder_test
import (
"context"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"

"github.com/vmware/govmomi/find"
"github.com/vmware/govmomi/object"
"github.com/vmware/govmomi/simulator"
Expand All @@ -35,60 +36,131 @@ import (
)

func TestResolveLibraryItemStorage(t *testing.T) {
simulator.Test(func(ctx context.Context, vc *vim25.Client) {
rc := rest.NewClient(vc)

ds, err := find.NewFinder(vc).Datastore(ctx, "*")
if err != nil {
t.Fatal(err)
}

var props mo.Datastore
err = ds.Properties(ctx, ds.Reference(), []string{"name", "summary"}, &props)
if err != nil {
t.Fatal(err)
}

fsTypes := []string{
string(types.HostFileSystemVolumeFileSystemTypeOTHER),
string(types.HostFileSystemVolumeFileSystemTypeVsan),
}

for _, fs := range fsTypes {
// client uses DatastoreNamespaceManager only when datastore fs is vsan/vvol
simulator.Map.Get(ds.Reference()).(*simulator.Datastore).Summary.Type = fs

u := props.Summary.Url

storage := []library.Storage{
{
StorageBacking: library.StorageBacking{DatastoreID: ds.Reference().Value, Type: "DATASTORE"},
StorageURIs: []string{
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_name}_${file_id}.iso", u),
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_name}_${file_id}.iso?serverId=${server_id}", u),

testCases := []struct {
name string
nilDatacenter bool
topLevelDirectoryCreateSupported *bool
}{
{
name: "Nil datacenter and nil topLevelCreate",
nilDatacenter: true,
topLevelDirectoryCreateSupported: nil,
},
{
name: "Nil datacenter and false topLevelCreate",
nilDatacenter: true,
topLevelDirectoryCreateSupported: types.New(false),
},
{
name: "Nil datacenter and true topLevelCreate",
nilDatacenter: true,
topLevelDirectoryCreateSupported: types.New(true),
},
{
name: "Non-nil datacenter and nil topLevelCreate",
nilDatacenter: false,
topLevelDirectoryCreateSupported: nil,
},
{
name: "Non-Nil datacenter and false topLevelCreate",
nilDatacenter: false,
topLevelDirectoryCreateSupported: types.New(false),
},
{
name: "Non-Nil datacenter and true topLevelCreate",
nilDatacenter: false,
topLevelDirectoryCreateSupported: types.New(true),
},
}

for i := range testCases {
tc := testCases[i]

t.Run(tc.name, func(t *testing.T) {

simulator.Test(func(ctx context.Context, vc *vim25.Client) {

vf := find.NewFinder(vc)
rc := rest.NewClient(vc)
lf := finder.NewPathFinder(library.NewManager(rc), vc)

dc, err := vf.Datacenter(ctx, "*")
if !assert.NoError(t, err) || !assert.NotNil(t, dc) {
t.FailNow()
}

ds, err := vf.Datastore(ctx, "*")
if !assert.NoError(t, err) || !assert.NotNil(t, ds) {
t.FailNow()
}

var (
dsName string
dsURL string
moDS mo.Datastore
)
if !assert.NoError(
t,
ds.Properties(
ctx,
ds.Reference(),
[]string{"name", "summary"},
&moDS)) {
t.FailNow()
}

dsName = moDS.Name
dsURL = moDS.Summary.Url

storage := []library.Storage{
{
StorageBacking: library.StorageBacking{
DatastoreID: ds.Reference().Value,
Type: "DATASTORE",
},
StorageURIs: []string{
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_1_name}_${file_1_id}.iso", dsURL),
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_2_name}_${file_2_id}.iso?serverId=${server_id}", dsURL),
},
},
},
}

f := finder.NewPathFinder(library.NewManager(rc), vc)

err = f.ResolveLibraryItemStorage(ctx, storage)
if err != nil {
t.Fatal(err)
}

var path object.DatastorePath
for _, s := range storage {
for _, u := range s.StorageURIs {
path.FromString(u)
if path.Datastore != props.Name {
t.Errorf("failed to parse %s", u)
}
if strings.Contains(u, "?") {
t.Errorf("includes query: %s", u)
}

var fsType string
if v := tc.topLevelDirectoryCreateSupported; v != nil && *v {
fsType = string(types.HostFileSystemVolumeFileSystemTypeOTHER)
} else {
fsType = string(types.HostFileSystemVolumeFileSystemTypeVsan)
}

simulator.Map.WithLock(
simulator.SpoofContext(),
ds.Reference(),
func() {
ds := simulator.Map.Get(ds.Reference()).(*simulator.Datastore)
ds.Summary.Type = fsType
ds.Capability.TopLevelDirectoryCreateSupported = tc.topLevelDirectoryCreateSupported
})

if !assert.NoError(
t,
lf.ResolveLibraryItemStorage(ctx, dc, storage)) {

t.FailNow()
}

assert.Len(t, storage, 1)
assert.Len(t, storage[0].StorageURIs, 2)

for _, s := range storage {
for _, u := range s.StorageURIs {
var path object.DatastorePath
path.FromString(u)
assert.Equal(t, path.Datastore, dsName)
assert.NotContains(t, u, "?")
}
}
}
}
})
})
})
}
}

0 comments on commit 64ca62f

Please sign in to comment.