Skip to content

Commit 41163a5

Browse files
dlipovetskyadiantum
dlipovetsky
and
adiantum
authored
refactor: Make available complete image info, instead of UUID only (#489)
* refactor: Make available complete image info, instead of UUID only We plan to make decisions based on some of the information. * fixup! refactor: Make available complete image info, instead of UUID only Refactor further; add unit tests with mocks * fixup! refactor: Make available complete image info, instead of UUID only Use longer name for receivers * fixup! refactor: Make available complete image info, instead of UUID only Add unit test for new String method * fixup! refactor: Make available complete image info, instead of UUID only Add envtest-based test for getDiskList. I adapted this from Ilya's work in #493. Co-authored-by: Ilya Alekseyev <[email protected]> * fixup! refactor: Make available complete image info, instead of UUID only Rename function from GetImageByNameOrUUID to GetImage * fixup! refactor: Make available complete image info, instead of UUID only Add and use IsName and IsUUID helpers * fixup! refactor: Make available complete image info, instead of UUID only Rename input parameter from image to id, for clarity * fixup! refactor: Make available complete image info, instead of UUID only As requested, do not use nested switch --------- Co-authored-by: Ilya Alekseyev <[email protected]>
1 parent dff89f0 commit 41163a5

File tree

7 files changed

+490
-31
lines changed

7 files changed

+490
-31
lines changed

‎api/v1beta1/nutanix_types.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,24 @@ type NutanixResourceIdentifier struct {
7474
Name *string `json:"name,omitempty"`
7575
}
7676

77+
func (nri NutanixResourceIdentifier) String() string {
78+
if nri.Type == NutanixIdentifierUUID && nri.UUID != nil {
79+
return *nri.UUID
80+
}
81+
if nri.Type == NutanixIdentifierName && nri.Name != nil {
82+
return *nri.Name
83+
}
84+
return ""
85+
}
86+
87+
func (nri NutanixResourceIdentifier) IsUUID() bool {
88+
return nri.Type == NutanixIdentifierUUID && nri.UUID != nil
89+
}
90+
91+
func (nri NutanixResourceIdentifier) IsName() bool {
92+
return nri.Type == NutanixIdentifierName && nri.Name != nil
93+
}
94+
7795
type NutanixCategoryIdentifier struct {
7896
// key is the Key of category in PC.
7997
// +optional

‎api/v1beta1/nutanix_types_test.go‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
Copyright 2022 Nutanix
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1beta1
18+
19+
import (
20+
"testing"
21+
22+
"k8s.io/utils/ptr"
23+
)
24+
25+
func TestNutanixResourceIdentifier_String(t *testing.T) {
26+
type fields struct {
27+
Type NutanixIdentifierType
28+
UUID *string
29+
Name *string
30+
}
31+
tests := []struct {
32+
name string
33+
fields fields
34+
want string
35+
}{
36+
{
37+
name: "returns name, because type is Name",
38+
fields: fields{
39+
Type: NutanixIdentifierName,
40+
Name: ptr.To("name"),
41+
UUID: ptr.To("uuid"),
42+
},
43+
want: "name",
44+
},
45+
{
46+
name: "returns UUID, because type is UUID",
47+
fields: fields{
48+
Type: NutanixIdentifierUUID,
49+
Name: ptr.To("name"),
50+
UUID: ptr.To("uuid"),
51+
},
52+
want: "uuid",
53+
},
54+
{
55+
name: "returns empty string, because type is undefined",
56+
fields: fields{
57+
Type: "",
58+
Name: ptr.To("name"),
59+
UUID: ptr.To("uuid"),
60+
},
61+
want: "",
62+
},
63+
}
64+
for _, tt := range tests {
65+
t.Run(tt.name, func(t *testing.T) {
66+
nri := NutanixResourceIdentifier{
67+
Type: tt.fields.Type,
68+
UUID: tt.fields.UUID,
69+
Name: tt.fields.Name,
70+
}
71+
if got := nri.String(); got != tt.want {
72+
t.Errorf("NutanixResourceIdentifier.String() = %v, want %v", got, tt.want)
73+
}
74+
})
75+
}
76+
}

‎controllers/helpers.go‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -298,46 +298,41 @@ func GetSubnetUUID(ctx context.Context, client *prismclientv3.Client, peUUID str
298298
return foundSubnetUUID, nil
299299
}
300300

301-
// GetImageUUID returns the UUID of the image with the given name
302-
func GetImageUUID(ctx context.Context, client *prismclientv3.Client, imageName, imageUUID *string) (string, error) {
303-
var foundImageUUID string
304-
305-
if imageUUID == nil && imageName == nil {
306-
return "", fmt.Errorf("image name or image uuid must be passed in order to retrieve the image")
307-
}
308-
if imageUUID != nil {
309-
imageIntentResponse, err := client.V3.GetImage(ctx, *imageUUID)
301+
// GetImage returns an image. If no UUID is provided, returns the unique image with the name.
302+
// Returns an error if no image has the UUID, if no image has the name, or more than one image has the name.
303+
func GetImage(ctx context.Context, client *prismclientv3.Client, id infrav1.NutanixResourceIdentifier) (*prismclientv3.ImageIntentResponse, error) {
304+
switch {
305+
case id.IsUUID():
306+
resp, err := client.V3.GetImage(ctx, *id.UUID)
310307
if err != nil {
311308
if strings.Contains(fmt.Sprint(err), "ENTITY_NOT_FOUND") {
312-
return "", fmt.Errorf("failed to find image with UUID %s: %v", *imageUUID, err)
309+
return nil, fmt.Errorf("failed to find image with UUID %s: %v", *id.UUID, err)
313310
}
314311
}
315-
foundImageUUID = *imageIntentResponse.Metadata.UUID
316-
} else { // else search by name
312+
return resp, nil
313+
case id.IsName():
317314
responseImages, err := client.V3.ListAllImage(ctx, "")
318315
if err != nil {
319-
return "", err
316+
return nil, err
320317
}
321318
// Validate filtered Images
322319
foundImages := make([]*prismclientv3.ImageIntentResponse, 0)
323320
for _, s := range responseImages.Entities {
324321
imageSpec := s.Spec
325-
if strings.EqualFold(*imageSpec.Name, *imageName) {
322+
if strings.EqualFold(*imageSpec.Name, *id.Name) {
326323
foundImages = append(foundImages, s)
327324
}
328325
}
329326
if len(foundImages) == 0 {
330-
return "", fmt.Errorf("failed to retrieve image by name %s", *imageName)
327+
return nil, fmt.Errorf("found no image with name %s", *id.Name)
331328
} else if len(foundImages) > 1 {
332-
return "", fmt.Errorf("more than one image found with name %s", *imageName)
329+
return nil, fmt.Errorf("more than one image found with name %s", *id.Name)
333330
} else {
334-
foundImageUUID = *foundImages[0].Metadata.UUID
335-
}
336-
if foundImageUUID == "" {
337-
return "", fmt.Errorf("failed to retrieve image by name or uuid. Verify input parameters")
331+
return foundImages[0], nil
338332
}
333+
default:
334+
return nil, fmt.Errorf("image identifier is missing both name and uuid")
339335
}
340-
return foundImageUUID, nil
341336
}
342337

343338
// HasTaskInProgress returns true if the given task is in progress

‎controllers/helpers_test.go‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"encoding/json"
2222
"errors"
23+
"reflect"
2324
"testing"
2425

2526
credentialtypes "github.com/nutanix-cloud-native/prism-go-client/environment/credentials"
@@ -31,10 +32,12 @@ import (
3132
"go.uber.org/mock/gomock"
3233
corev1 "k8s.io/api/core/v1"
3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
35+
"k8s.io/utils/ptr"
3436
"sigs.k8s.io/cluster-api/util"
3537

3638
infrav1 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/api/v1beta1"
3739
mockk8sclient "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/mocks/k8sclient"
40+
mocknutanixv3 "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/mocks/nutanix"
3841
nutanixclient "github.com/nutanix-cloud-native/cluster-api-provider-nutanix/pkg/client"
3942
)
4043

@@ -229,3 +232,200 @@ func TestGetPrismCentralClientForCluster(t *testing.T) {
229232
assert.NoError(t, err)
230233
})
231234
}
235+
236+
func TestGetImageByNameOrUUID(t *testing.T) {
237+
tests := []struct {
238+
name string
239+
clientBuilder func() *prismclientv3.Client
240+
id infrav1.NutanixResourceIdentifier
241+
want *prismclientv3.ImageIntentResponse
242+
wantErr bool
243+
}{
244+
{
245+
name: "missing name and UUID in the input",
246+
clientBuilder: func() *prismclientv3.Client {
247+
mockctrl := gomock.NewController(t)
248+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
249+
250+
return &prismclientv3.Client{V3: mockv3Service}
251+
},
252+
id: infrav1.NutanixResourceIdentifier{},
253+
wantErr: true,
254+
},
255+
{
256+
name: "image UUID not found",
257+
clientBuilder: func() *prismclientv3.Client {
258+
mockctrl := gomock.NewController(t)
259+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
260+
mockv3Service.EXPECT().GetImage(gomock.Any(), gomock.Any()).Return(nil, errors.New("ENTITY_NOT_FOUND"))
261+
262+
return &prismclientv3.Client{V3: mockv3Service}
263+
},
264+
id: infrav1.NutanixResourceIdentifier{
265+
Type: infrav1.NutanixIdentifierUUID,
266+
UUID: ptr.To("32432daf-fb0e-4202-b444-2439f43a24c5"),
267+
},
268+
wantErr: true,
269+
},
270+
{
271+
name: "image name query fails",
272+
clientBuilder: func() *prismclientv3.Client {
273+
mockctrl := gomock.NewController(t)
274+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
275+
mockv3Service.EXPECT().ListAllImage(gomock.Any(), gomock.Any()).Return(nil, errors.New("fake error"))
276+
277+
return &prismclientv3.Client{V3: mockv3Service}
278+
},
279+
id: infrav1.NutanixResourceIdentifier{
280+
Type: infrav1.NutanixIdentifierName,
281+
Name: ptr.To("example"),
282+
},
283+
wantErr: true,
284+
},
285+
{
286+
name: "image UUID found",
287+
clientBuilder: func() *prismclientv3.Client {
288+
mockctrl := gomock.NewController(t)
289+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
290+
mockv3Service.EXPECT().GetImage(gomock.Any(), gomock.Any()).Return(
291+
&prismclientv3.ImageIntentResponse{
292+
Spec: &prismclientv3.Image{
293+
Name: ptr.To("example"),
294+
},
295+
}, nil,
296+
)
297+
298+
return &prismclientv3.Client{V3: mockv3Service}
299+
},
300+
id: infrav1.NutanixResourceIdentifier{
301+
Type: infrav1.NutanixIdentifierUUID,
302+
UUID: ptr.To("32432daf-fb0e-4202-b444-2439f43a24c5"),
303+
},
304+
want: &prismclientv3.ImageIntentResponse{
305+
Spec: &prismclientv3.Image{
306+
Name: ptr.To("example"),
307+
},
308+
},
309+
},
310+
{
311+
name: "image name found",
312+
clientBuilder: func() *prismclientv3.Client {
313+
mockctrl := gomock.NewController(t)
314+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
315+
mockv3Service.EXPECT().ListAllImage(gomock.Any(), gomock.Any()).Return(
316+
&prismclientv3.ImageListIntentResponse{
317+
Entities: []*prismclientv3.ImageIntentResponse{
318+
{
319+
Spec: &prismclientv3.Image{
320+
Name: ptr.To("example"),
321+
},
322+
},
323+
},
324+
}, nil,
325+
)
326+
327+
return &prismclientv3.Client{V3: mockv3Service}
328+
},
329+
id: infrav1.NutanixResourceIdentifier{
330+
Type: infrav1.NutanixIdentifierName,
331+
Name: ptr.To("example"),
332+
},
333+
want: &prismclientv3.ImageIntentResponse{
334+
Spec: &prismclientv3.Image{
335+
Name: ptr.To("example"),
336+
},
337+
},
338+
},
339+
{
340+
name: "image name matches multiple images",
341+
clientBuilder: func() *prismclientv3.Client {
342+
mockctrl := gomock.NewController(t)
343+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
344+
mockv3Service.EXPECT().ListAllImage(gomock.Any(), gomock.Any()).Return(
345+
&prismclientv3.ImageListIntentResponse{
346+
Entities: []*prismclientv3.ImageIntentResponse{
347+
{
348+
Spec: &prismclientv3.Image{
349+
Name: ptr.To("example"),
350+
},
351+
},
352+
{
353+
Spec: &prismclientv3.Image{
354+
Name: ptr.To("example"),
355+
},
356+
},
357+
},
358+
}, nil,
359+
)
360+
361+
return &prismclientv3.Client{V3: mockv3Service}
362+
},
363+
id: infrav1.NutanixResourceIdentifier{
364+
Type: infrav1.NutanixIdentifierName,
365+
Name: ptr.To("example"),
366+
},
367+
wantErr: true,
368+
},
369+
{
370+
name: "image name matches zero images",
371+
clientBuilder: func() *prismclientv3.Client {
372+
mockctrl := gomock.NewController(t)
373+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
374+
mockv3Service.EXPECT().ListAllImage(gomock.Any(), gomock.Any()).Return(
375+
&prismclientv3.ImageListIntentResponse{
376+
Entities: []*prismclientv3.ImageIntentResponse{},
377+
}, nil,
378+
)
379+
380+
return &prismclientv3.Client{V3: mockv3Service}
381+
},
382+
id: infrav1.NutanixResourceIdentifier{
383+
Type: infrav1.NutanixIdentifierName,
384+
Name: ptr.To("example"),
385+
},
386+
wantErr: true,
387+
},
388+
{
389+
name: "image name matches one image",
390+
clientBuilder: func() *prismclientv3.Client {
391+
mockctrl := gomock.NewController(t)
392+
mockv3Service := mocknutanixv3.NewMockService(mockctrl)
393+
mockv3Service.EXPECT().ListAllImage(gomock.Any(), gomock.Any()).Return(
394+
&prismclientv3.ImageListIntentResponse{
395+
Entities: []*prismclientv3.ImageIntentResponse{
396+
{
397+
Spec: &prismclientv3.Image{
398+
Name: ptr.To("example"),
399+
},
400+
},
401+
},
402+
}, nil,
403+
)
404+
405+
return &prismclientv3.Client{V3: mockv3Service}
406+
},
407+
id: infrav1.NutanixResourceIdentifier{
408+
Type: infrav1.NutanixIdentifierName,
409+
Name: ptr.To("example"),
410+
},
411+
want: &prismclientv3.ImageIntentResponse{
412+
Spec: &prismclientv3.Image{
413+
Name: ptr.To("example"),
414+
},
415+
},
416+
},
417+
}
418+
for _, tt := range tests {
419+
t.Run(tt.name, func(t *testing.T) {
420+
ctx := context.Background()
421+
got, err := GetImage(ctx, tt.clientBuilder(), tt.id)
422+
if (err != nil) != tt.wantErr {
423+
t.Errorf("GetImageByNameOrUUID() error = %v, wantErr %v", err, tt.wantErr)
424+
return
425+
}
426+
if !reflect.DeepEqual(got, tt.want) {
427+
t.Errorf("GetImageByNameOrUUID() = %v, want %v", got, tt.want)
428+
}
429+
})
430+
}
431+
}

0 commit comments

Comments
 (0)