Skip to content

Commit 64ca62f

Browse files
akutzakutz
committedDec 20, 2024·
api: Support DC for ResolveLibItemStor
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>
·
v0.50.0-alpha.0v0.47.0
1 parent ff82b3a commit 64ca62f

File tree

3 files changed

+211
-92
lines changed

3 files changed

+211
-92
lines changed
 

‎cli/library/info.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ func (r infoResultsWriter) writeFile(
277277
}
278278
if r.cmd.Stor {
279279
label = "Resolved URI"
280-
err = r.cmd.pathFinder.ResolveLibraryItemStorage(r.ctx, s)
280+
err = r.cmd.pathFinder.ResolveLibraryItemStorage(r.ctx, nil, s)
281281
if err != nil {
282282
return err
283283
}

‎vapi/library/finder/path.go

+84-37
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ package finder
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"net/url"
2324
"path"
2425
"strings"
2526

26-
"github.com/vmware/govmomi/internal"
2727
"github.com/vmware/govmomi/object"
2828
"github.com/vmware/govmomi/property"
2929
"github.com/vmware/govmomi/vapi/library"
@@ -135,79 +135,126 @@ func (f *PathFinder) datastoreName(ctx context.Context, id string) (string, erro
135135
return name, nil
136136
}
137137

138-
func (f *PathFinder) convertPath(ctx context.Context, b *mo.Datastore, path string) (string, error) {
139-
if !internal.IsDatastoreVSAN(*b) {
138+
func (f *PathFinder) convertPath(
139+
ctx context.Context,
140+
dc *object.Datacenter,
141+
ds mo.Datastore,
142+
path string) (string, error) {
143+
144+
if v := ds.Capability.TopLevelDirectoryCreateSupported; v != nil && *v {
140145
return path, nil
141146
}
142147

143-
var dc *object.Datacenter
144-
145-
entities, err := mo.Ancestors(ctx, f.c, f.c.ServiceContent.PropertyCollector, b.Self)
146-
if err != nil {
147-
return "", err
148+
if dc == nil {
149+
entities, err := mo.Ancestors(
150+
ctx,
151+
f.c,
152+
f.c.ServiceContent.PropertyCollector,
153+
ds.Self)
154+
if err != nil {
155+
return "", fmt.Errorf("failed to find ancestors: %w", err)
156+
}
157+
for _, entity := range entities {
158+
if entity.Self.Type == "Datacenter" {
159+
dc = object.NewDatacenter(f.c, entity.Self)
160+
break
161+
}
162+
}
148163
}
149164

150-
for _, entity := range entities {
151-
if entity.Self.Type == "Datacenter" {
152-
dc = object.NewDatacenter(f.c, entity.Self)
153-
break
154-
}
165+
if dc == nil {
166+
return "", errors.New("failed to find datacenter")
155167
}
156168

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

161-
// ResolveLibraryItemStorage transforms StorageURIs Datastore url (uuid) to Datastore name.
162-
func (f *PathFinder) ResolveLibraryItemStorage(ctx context.Context, storage []library.Storage) error {
173+
// ResolveLibraryItemStorage transforms the StorageURIs field in the provided
174+
// storage items from a datastore URL, ex.
175+
// "ds:///vmfs/volumes/DATASTORE_UUID/contentlib-LIB_UUID/ITEM_UUID/file.vmdk",
176+
// to the format that includes the datastore name, ex.
177+
// "[DATASTORE_NAME] contentlib-LIB_UUID/ITEM_UUID/file.vmdk".
178+
//
179+
// If a storage item resides on a datastore that does not support the creation
180+
// of top-level directories, then this means the datastore is vSAN and the
181+
// storage item path needs to be further converted. If this occurs, then the
182+
// datacenter to which the datastore belongs is required. If the datacenter
183+
// parameter is non-nil, it is used, otherwise the datacenter for each datastore
184+
// is resolved as needed. It is much more efficient to send in the datacenter if
185+
// it is known ahead of time that the content library is stored on a vSAN
186+
// datastore.
187+
func (f *PathFinder) ResolveLibraryItemStorage(
188+
ctx context.Context,
189+
datacenter *object.Datacenter,
190+
storage []library.Storage) error {
191+
163192
// TODO:
164193
// - reuse PathFinder.cache
165-
// - the transform here isn't Content Library specific, but is currently the only known use case
166-
backing := map[string]*mo.Datastore{}
167-
var ids []types.ManagedObjectReference
168-
169-
// don't think we can have more than 1 Datastore backing currently, future proof anyhow
194+
// - the transform here isn't Content Library specific, but is currently
195+
// the only known use case
196+
var (
197+
ids []types.ManagedObjectReference
198+
datastoreMap = map[string]mo.Datastore{}
199+
)
200+
201+
// Currently ContentLibrary only supports a single storage backing, but this
202+
// future proofs things.
170203
for _, item := range storage {
171204
id := item.StorageBacking.DatastoreID
172-
if _, ok := backing[id]; ok {
205+
if _, ok := datastoreMap[id]; ok {
173206
continue
174207
}
175-
backing[id] = nil
176-
ids = append(ids, types.ManagedObjectReference{Type: "Datastore", Value: id})
208+
datastoreMap[id] = mo.Datastore{}
209+
ids = append(
210+
ids,
211+
types.ManagedObjectReference{Type: "Datastore", Value: id})
177212
}
178213

179-
var ds []mo.Datastore
180-
pc := property.DefaultCollector(f.c)
181-
props := []string{"name", "summary.url", "summary.type"}
182-
if err := pc.Retrieve(ctx, ids, props, &ds); err != nil {
214+
var (
215+
datastores []mo.Datastore
216+
pc = property.DefaultCollector(f.c)
217+
props = []string{
218+
"name",
219+
"summary.url",
220+
"capability.topLevelDirectoryCreateSupported",
221+
}
222+
)
223+
224+
if err := pc.Retrieve(ctx, ids, props, &datastores); err != nil {
183225
return err
184226
}
185227

186-
for i := range ds {
187-
backing[ds[i].Self.Value] = &ds[i]
228+
for i := range datastores {
229+
datastoreMap[datastores[i].Self.Value] = datastores[i]
188230
}
189231

190232
for _, item := range storage {
191-
b := backing[item.StorageBacking.DatastoreID]
192-
dsurl := b.Summary.Url
233+
ds := datastoreMap[item.StorageBacking.DatastoreID]
234+
dsURL := ds.Summary.Url
193235

194236
for i := range item.StorageURIs {
195-
uri, err := url.Parse(item.StorageURIs[i])
237+
szURI := item.StorageURIs[i]
238+
uri, err := url.Parse(szURI)
196239
if err != nil {
197-
return err
240+
return fmt.Errorf(
241+
"failed to parse storage URI %q: %w", szURI, err)
198242
}
243+
199244
uri.OmitHost = false // `ds://` required for ConvertNamespacePathToUuidPath()
200245
uri.Path = path.Clean(uri.Path) // required for ConvertNamespacePathToUuidPath()
201246
uri.RawQuery = ""
202-
u, err := f.convertPath(ctx, b, uri.String())
247+
248+
uriPath := uri.String()
249+
u, err := f.convertPath(ctx, datacenter, ds, uriPath)
203250
if err != nil {
204-
return err
251+
return fmt.Errorf("failed to convert path %q: %w", uriPath, err)
205252
}
206-
u = strings.TrimPrefix(u, dsurl)
253+
u = strings.TrimPrefix(u, dsURL)
207254
u = strings.TrimPrefix(u, "/")
208255

209256
item.StorageURIs[i] = (&object.DatastorePath{
210-
Datastore: b.Name,
257+
Datastore: ds.Name,
211258
Path: u,
212259
}).String()
213260
}

‎vapi/library/finder/path_test.go

+126-54
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ package finder_test
1919
import (
2020
"context"
2121
"fmt"
22-
"strings"
2322
"testing"
2423

24+
"github.com/stretchr/testify/assert"
25+
2526
"github.com/vmware/govmomi/find"
2627
"github.com/vmware/govmomi/object"
2728
"github.com/vmware/govmomi/simulator"
@@ -35,60 +36,131 @@ import (
3536
)
3637

3738
func TestResolveLibraryItemStorage(t *testing.T) {
38-
simulator.Test(func(ctx context.Context, vc *vim25.Client) {
39-
rc := rest.NewClient(vc)
40-
41-
ds, err := find.NewFinder(vc).Datastore(ctx, "*")
42-
if err != nil {
43-
t.Fatal(err)
44-
}
45-
46-
var props mo.Datastore
47-
err = ds.Properties(ctx, ds.Reference(), []string{"name", "summary"}, &props)
48-
if err != nil {
49-
t.Fatal(err)
50-
}
51-
52-
fsTypes := []string{
53-
string(types.HostFileSystemVolumeFileSystemTypeOTHER),
54-
string(types.HostFileSystemVolumeFileSystemTypeVsan),
55-
}
56-
57-
for _, fs := range fsTypes {
58-
// client uses DatastoreNamespaceManager only when datastore fs is vsan/vvol
59-
simulator.Map.Get(ds.Reference()).(*simulator.Datastore).Summary.Type = fs
60-
61-
u := props.Summary.Url
62-
63-
storage := []library.Storage{
64-
{
65-
StorageBacking: library.StorageBacking{DatastoreID: ds.Reference().Value, Type: "DATASTORE"},
66-
StorageURIs: []string{
67-
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_name}_${file_id}.iso", u),
68-
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_name}_${file_id}.iso?serverId=${server_id}", u),
39+
40+
testCases := []struct {
41+
name string
42+
nilDatacenter bool
43+
topLevelDirectoryCreateSupported *bool
44+
}{
45+
{
46+
name: "Nil datacenter and nil topLevelCreate",
47+
nilDatacenter: true,
48+
topLevelDirectoryCreateSupported: nil,
49+
},
50+
{
51+
name: "Nil datacenter and false topLevelCreate",
52+
nilDatacenter: true,
53+
topLevelDirectoryCreateSupported: types.New(false),
54+
},
55+
{
56+
name: "Nil datacenter and true topLevelCreate",
57+
nilDatacenter: true,
58+
topLevelDirectoryCreateSupported: types.New(true),
59+
},
60+
{
61+
name: "Non-nil datacenter and nil topLevelCreate",
62+
nilDatacenter: false,
63+
topLevelDirectoryCreateSupported: nil,
64+
},
65+
{
66+
name: "Non-Nil datacenter and false topLevelCreate",
67+
nilDatacenter: false,
68+
topLevelDirectoryCreateSupported: types.New(false),
69+
},
70+
{
71+
name: "Non-Nil datacenter and true topLevelCreate",
72+
nilDatacenter: false,
73+
topLevelDirectoryCreateSupported: types.New(true),
74+
},
75+
}
76+
77+
for i := range testCases {
78+
tc := testCases[i]
79+
80+
t.Run(tc.name, func(t *testing.T) {
81+
82+
simulator.Test(func(ctx context.Context, vc *vim25.Client) {
83+
84+
vf := find.NewFinder(vc)
85+
rc := rest.NewClient(vc)
86+
lf := finder.NewPathFinder(library.NewManager(rc), vc)
87+
88+
dc, err := vf.Datacenter(ctx, "*")
89+
if !assert.NoError(t, err) || !assert.NotNil(t, dc) {
90+
t.FailNow()
91+
}
92+
93+
ds, err := vf.Datastore(ctx, "*")
94+
if !assert.NoError(t, err) || !assert.NotNil(t, ds) {
95+
t.FailNow()
96+
}
97+
98+
var (
99+
dsName string
100+
dsURL string
101+
moDS mo.Datastore
102+
)
103+
if !assert.NoError(
104+
t,
105+
ds.Properties(
106+
ctx,
107+
ds.Reference(),
108+
[]string{"name", "summary"},
109+
&moDS)) {
110+
t.FailNow()
111+
}
112+
113+
dsName = moDS.Name
114+
dsURL = moDS.Summary.Url
115+
116+
storage := []library.Storage{
117+
{
118+
StorageBacking: library.StorageBacking{
119+
DatastoreID: ds.Reference().Value,
120+
Type: "DATASTORE",
121+
},
122+
StorageURIs: []string{
123+
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_1_name}_${file_1_id}.iso", dsURL),
124+
fmt.Sprintf("%s/contentlib-${lib_id}/${item_id}/${file_2_name}_${file_2_id}.iso?serverId=${server_id}", dsURL),
125+
},
69126
},
70-
},
71-
}
72-
73-
f := finder.NewPathFinder(library.NewManager(rc), vc)
74-
75-
err = f.ResolveLibraryItemStorage(ctx, storage)
76-
if err != nil {
77-
t.Fatal(err)
78-
}
79-
80-
var path object.DatastorePath
81-
for _, s := range storage {
82-
for _, u := range s.StorageURIs {
83-
path.FromString(u)
84-
if path.Datastore != props.Name {
85-
t.Errorf("failed to parse %s", u)
86-
}
87-
if strings.Contains(u, "?") {
88-
t.Errorf("includes query: %s", u)
127+
}
128+
129+
var fsType string
130+
if v := tc.topLevelDirectoryCreateSupported; v != nil && *v {
131+
fsType = string(types.HostFileSystemVolumeFileSystemTypeOTHER)
132+
} else {
133+
fsType = string(types.HostFileSystemVolumeFileSystemTypeVsan)
134+
}
135+
136+
simulator.Map.WithLock(
137+
simulator.SpoofContext(),
138+
ds.Reference(),
139+
func() {
140+
ds := simulator.Map.Get(ds.Reference()).(*simulator.Datastore)
141+
ds.Summary.Type = fsType
142+
ds.Capability.TopLevelDirectoryCreateSupported = tc.topLevelDirectoryCreateSupported
143+
})
144+
145+
if !assert.NoError(
146+
t,
147+
lf.ResolveLibraryItemStorage(ctx, dc, storage)) {
148+
149+
t.FailNow()
150+
}
151+
152+
assert.Len(t, storage, 1)
153+
assert.Len(t, storage[0].StorageURIs, 2)
154+
155+
for _, s := range storage {
156+
for _, u := range s.StorageURIs {
157+
var path object.DatastorePath
158+
path.FromString(u)
159+
assert.Equal(t, path.Datastore, dsName)
160+
assert.NotContains(t, u, "?")
89161
}
90162
}
91-
}
92-
}
93-
})
163+
})
164+
})
165+
}
94166
}

0 commit comments

Comments
 (0)
Please sign in to comment.