Skip to content

Commit 95a4855

Browse files
hawkowlhawkowl
committed
update uses of the computeskus to use the new APIs
1 parent ff1071c commit 95a4855

13 files changed

+115
-117
lines changed

‎pkg/cluster/loadbalancerinternal.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func MigrateInternalLoadBalancerZones(
6060
}
6161
}
6262

63-
filteredSkus, err := computeskus.GetVMSkusForCurrentRegion(ctx, resourceSkusClient, location)
63+
filteredSkus, err := computeskus.SelectVMSkusInCurrentRegion(ctx, resourceSkusClient, location, []string{string(doc.OpenShiftCluster.Properties.MasterProfile.VMSize)})
6464
if err != nil {
6565
return doc, err
6666
}

‎pkg/cluster/loadbalancerinternal_test.go‎

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,20 @@ func TestUpdateLoadBalancerZonalNoopAndErrorPaths(t *testing.T) {
9494
}, nil,
9595
)
9696

97-
sku.EXPECT().List(gomock.Any(), "location eq eastus", false).Return([]*armcompute.ResourceSKU{
98-
{
99-
Name: pointerutils.ToPtr(string(api.VMSizeStandardD16asV4)),
100-
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
101-
LocationInfo: pointerutils.ToSlicePtr([]armcompute.ResourceSKULocationInfo{
102-
{
103-
Zones: []*string{},
104-
},
105-
}),
106-
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
107-
ResourceType: pointerutils.ToPtr("virtualMachines"),
108-
},
109-
}, nil)
97+
sku.EXPECT().List(gomock.Any(), "location eq eastus", false).
98+
Return(func(yield func(*armcompute.ResourceSKU, error) bool) {
99+
yield(&armcompute.ResourceSKU{
100+
Name: pointerutils.ToPtr(string(api.VMSizeStandardD16asV4)),
101+
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
102+
LocationInfo: pointerutils.ToSlicePtr([]armcompute.ResourceSKULocationInfo{
103+
{
104+
Zones: []*string{},
105+
},
106+
}),
107+
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
108+
ResourceType: pointerutils.ToPtr("virtualMachines"),
109+
}, nil)
110+
})
110111
},
111112
expectedLogs: []testlog.ExpectedLogEntry{
112113
{
@@ -136,15 +137,8 @@ func TestUpdateLoadBalancerZonalNoopAndErrorPaths(t *testing.T) {
136137
}, nil,
137138
)
138139

139-
sku.EXPECT().List(gomock.Any(), "location eq eastus", false).Return([]*armcompute.ResourceSKU{
140-
{
141-
Name: pointerutils.ToPtr(string(api.VMSizeStandardD16asV4)),
142-
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
143-
LocationInfo: pointerutils.ToSlicePtr([]armcompute.ResourceSKULocationInfo{}),
144-
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
145-
ResourceType: pointerutils.ToPtr("virtualMachines"),
146-
},
147-
}, nil)
140+
sku.EXPECT().List(gomock.Any(), "location eq eastus", false).
141+
Return(func(yield func(*armcompute.ResourceSKU, error) bool) {})
148142
},
149143
expectedLogs: []testlog.ExpectedLogEntry{},
150144
wantErrs: []error{errVMAvailability},
@@ -170,7 +164,10 @@ func TestUpdateLoadBalancerZonalNoopAndErrorPaths(t *testing.T) {
170164
}, nil,
171165
)
172166

173-
sku.EXPECT().List(gomock.Any(), "location eq eastus", false).Return([]*armcompute.ResourceSKU{}, errTestSKUFetchError)
167+
sku.EXPECT().List(gomock.Any(), "location eq eastus", false).
168+
Return(func(yield func(*armcompute.ResourceSKU, error) bool) {
169+
yield(nil, errTestSKUFetchError)
170+
})
174171
},
175172
expectedLogs: []testlog.ExpectedLogEntry{},
176173
wantErrs: []error{computeskus.ErrListVMResourceSKUs, errTestSKUFetchError},
@@ -380,17 +377,18 @@ func TestUpdateLoadBalancerZonalMigration(t *testing.T) {
380377
}, nil,
381378
)
382379

383-
skus.EXPECT().List(gomock.Any(), "location eq eastus", false).Return([]*armcompute.ResourceSKU{
384-
{
385-
Name: pointerutils.ToPtr(string(api.VMSizeStandardD16asV4)),
386-
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
387-
LocationInfo: pointerutils.ToSlicePtr([]armcompute.ResourceSKULocationInfo{
388-
{Zones: pointerutils.ToSlicePtr([]string{"1", "2", "3"})},
389-
}),
390-
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
391-
ResourceType: pointerutils.ToPtr("virtualMachines"),
392-
},
393-
}, nil)
380+
skus.EXPECT().List(gomock.Any(), "location eq eastus", false).
381+
Return(func(yield func(*armcompute.ResourceSKU, error) bool) {
382+
yield(&armcompute.ResourceSKU{
383+
Name: pointerutils.ToPtr(string(api.VMSizeStandardD16asV4)),
384+
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
385+
LocationInfo: pointerutils.ToSlicePtr([]armcompute.ResourceSKULocationInfo{
386+
{Zones: pointerutils.ToSlicePtr([]string{"1", "2", "3"})},
387+
}),
388+
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
389+
ResourceType: pointerutils.ToPtr("virtualMachines"),
390+
}, nil)
391+
})
394392

395393
plsFIPRemoval := plses.EXPECT().CreateOrUpdateAndWait(gomock.Any(), rgName, infraID+"-pls", armnetwork.PrivateLinkService{
396394
Properties: &armnetwork.PrivateLinkServiceProperties{

‎pkg/cluster/validate.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ func (m *manager) validateResources(ctx context.Context) error {
3232
// created. This function is only to be called during cluster bootstrap!
3333
func (m *manager) validateZones(ctx context.Context) error {
3434
location := m.doc.OpenShiftCluster.Location
35-
filteredSkus, err := computeskus.GetVMSkusForCurrentRegion(ctx, m.armResourceSKUs, location)
35+
36+
filteredSkus, err := computeskus.SelectVMSkusInCurrentRegion(ctx, m.armResourceSKUs, location, []string{
37+
string(m.doc.OpenShiftCluster.Properties.MasterProfile.VMSize),
38+
string(m.doc.OpenShiftCluster.Properties.WorkerProfiles[0].VMSize),
39+
})
3640
if err != nil {
3741
return err
3842
}

‎pkg/cluster/validate_test.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package cluster
66
import (
77
"context"
88
"errors"
9-
"fmt"
109
"strings"
1110
"testing"
1211

@@ -180,8 +179,18 @@ func TestValidateZones(t *testing.T) {
180179

181180
resourceSkusClient := mock_armcompute.NewMockResourceSKUsClient(controller)
182181
resourceSkusClient.EXPECT().
183-
List(gomock.Any(), fmt.Sprintf("location eq %v", "eastus"), false).
184-
Return(skus, tt.resourceSkusClientErr)
182+
List(gomock.Any(), "location eq eastus", false).
183+
Return(func(yield func(*armcompute.ResourceSKU, error) bool) {
184+
if tt.resourceSkusClientErr != nil {
185+
yield(nil, tt.resourceSkusClientErr)
186+
return
187+
}
188+
for _, v := range skus {
189+
if !yield(v, nil) {
190+
return
191+
}
192+
}
193+
})
185194

186195
m := &manager{
187196
env: mockEnv,

‎pkg/frontend/admin_openshiftcluster_vmresize_pre_validation.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
2929
"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute"
3030
"github.com/Azure/ARO-RP/pkg/util/clusteroperators"
31-
"github.com/Azure/ARO-RP/pkg/util/computeskus"
3231
)
3332

3433
// getPreResizeControlPlaneVMsValidation is the HTTP handler; the underscore
@@ -337,15 +336,12 @@ func (f *frontend) validateVMSKU(
337336
return err
338337
}
339338

340-
skus, err := a.VMSizeList(ctx)
339+
filteredSkus, err := a.VMGetSKUs(ctx, []string{desiredVMSize})
341340
if err != nil {
342341
return err
343342
}
344343

345344
location := doc.OpenShiftCluster.Location
346-
347-
filteredSkus := computeskus.FilterVMSizes(skus, location)
348-
349345
sku, err := checkSKUAvailability(filteredSkus, location, "vmSize", desiredVMSize)
350346
if err != nil {
351347
return err

‎pkg/frontend/admin_openshiftcluster_vmresize_pre_validation_test.go‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ func TestPreResizeControlPlaneVMsValidation(t *testing.T) {
142142
},
143143
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
144144
a.EXPECT().
145-
VMSizeList(gomock.Any()).
146-
Return([]*armcompute.ResourceSKU{
147-
{
145+
VMGetSKUs(gomock.Any(), []string{"Standard_D8s_v3"}).
146+
Return(map[string]*armcompute.ResourceSKU{
147+
"Standard_D8s_v3": {
148148
Name: pointerutils.ToPtr("Standard_D8s_v3"),
149149
ResourceType: pointerutils.ToPtr("virtualMachines"),
150150
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
@@ -308,21 +308,8 @@ func TestPreResizeControlPlaneVMsValidation(t *testing.T) {
308308
},
309309
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
310310
a.EXPECT().
311-
VMSizeList(gomock.Any()).
312-
Return([]*armcompute.ResourceSKU{
313-
{
314-
Name: pointerutils.ToPtr("Standard_D16s_v3"),
315-
ResourceType: pointerutils.ToPtr("virtualMachines"),
316-
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),
317-
LocationInfo: []*armcompute.ResourceSKULocationInfo{
318-
{
319-
Location: pointerutils.ToPtr("eastus"),
320-
},
321-
},
322-
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
323-
Capabilities: []*armcompute.ResourceSKUCapabilities{},
324-
},
325-
}, nil)
311+
VMGetSKUs(gomock.Any(), []string{"Standard_D8s_v3"}).
312+
Return(map[string]*armcompute.ResourceSKU{}, nil)
326313
},
327314
kubeMocks: allKubeChecksHealthyMock,
328315
wantStatusCode: http.StatusBadRequest,
@@ -360,9 +347,9 @@ func TestPreResizeControlPlaneVMsValidation(t *testing.T) {
360347
},
361348
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
362349
a.EXPECT().
363-
VMSizeList(gomock.Any()).
364-
Return([]*armcompute.ResourceSKU{
365-
{
350+
VMGetSKUs(gomock.Any(), []string{"Standard_D8s_v3"}).
351+
Return(map[string]*armcompute.ResourceSKU{
352+
"Standard_D8s_v3": {
366353
Name: pointerutils.ToPtr("Standard_D8s_v3"),
367354
ResourceType: pointerutils.ToPtr("virtualMachines"),
368355
Locations: pointerutils.ToSlicePtr([]string{"eastus"}),

‎pkg/frontend/admin_openshiftcluster_vmsizelist.go‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,12 @@ import (
99
"fmt"
1010
"net/http"
1111
"path/filepath"
12+
"slices"
1213
"strings"
1314

1415
"github.com/go-chi/chi/v5"
1516
"github.com/sirupsen/logrus"
1617

17-
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
18-
1918
"github.com/Azure/ARO-RP/pkg/api"
2019
"github.com/Azure/ARO-RP/pkg/database/cosmosdb"
2120
"github.com/Azure/ARO-RP/pkg/frontend/middleware"
@@ -66,19 +65,8 @@ func (f *frontend) _getAdminOpenShiftClusterVMResizeOptions(ctx context.Context,
6665
return nil, err
6766
}
6867

69-
return json.Marshal(f.filterVMSkus(skus))
70-
}
71-
72-
func (f *frontend) filterVMSkus(skus []*armcompute.ResourceSKU) []string {
73-
filteredSkus := []string{}
74-
75-
for _, sku := range skus {
76-
if sku.Restrictions != nil && len(sku.Restrictions) == 0 {
77-
if sku.Name != nil {
78-
filteredSkus = append(filteredSkus, *sku.Name)
79-
}
80-
}
81-
}
68+
// Sort for stability
69+
slices.Sort(skus)
8270

83-
return filteredSkus
71+
return json.Marshal(skus)
8472
}

‎pkg/frontend/admin_openshiftcluster_vmsizelist_test.go‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,11 @@ import (
1313
"github.com/sirupsen/logrus"
1414
"go.uber.org/mock/gomock"
1515

16-
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
17-
1816
"github.com/Azure/ARO-RP/pkg/api"
1917
"github.com/Azure/ARO-RP/pkg/env"
2018
"github.com/Azure/ARO-RP/pkg/frontend/adminactions"
2119
"github.com/Azure/ARO-RP/pkg/metrics/noop"
2220
mock_adminactions "github.com/Azure/ARO-RP/pkg/util/mocks/adminactions"
23-
"github.com/Azure/ARO-RP/pkg/util/pointerutils"
2421
testdatabase "github.com/Azure/ARO-RP/test/database"
2522
)
2623

@@ -68,23 +65,10 @@ func TestAdminListVMSizeList(t *testing.T) {
6865
mocks: func(tt *test, a *mock_adminactions.MockAzureActions) {
6966
a.EXPECT().
7067
VMSizeList(gomock.Any()).
71-
Return([]*armcompute.ResourceSKU{
72-
{
73-
Name: pointerutils.ToPtr("Standard_D8s_v90"),
74-
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{
75-
{
76-
Type: pointerutils.ToPtr(armcompute.ResourceSKURestrictionsTypeLocation),
77-
},
78-
}),
79-
},
80-
{
81-
Name: pointerutils.ToPtr("Standard_D8s_v9001"),
82-
Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}),
83-
},
84-
}, nil)
68+
Return([]string{"Standard_D8s_v9001", "Standard_D8s_v80"}, nil)
8569
},
8670
wantStatusCode: http.StatusOK,
87-
wantResponse: []byte(`["Standard_D8s_v9001"]` + "\n"),
71+
wantResponse: []byte(`["Standard_D8s_v80","Standard_D8s_v9001"]` + "\n"),
8872
},
8973
{
9074
name: "cluster not found",

‎pkg/frontend/adminactions/azureactions.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/compute"
2323
"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/features"
2424
"github.com/Azure/ARO-RP/pkg/util/azureclient/mgmt/storage"
25+
"github.com/Azure/ARO-RP/pkg/util/computeskus"
2526
"github.com/Azure/ARO-RP/pkg/util/stringutils"
2627
)
2728

@@ -34,7 +35,8 @@ type AzureActions interface {
3435
VMRedeployAndWait(ctx context.Context, vmName string) error
3536
VMStartAndWait(ctx context.Context, vmName string) error
3637
VMStopAndWait(ctx context.Context, vmName string, deallocateVM bool) error
37-
VMSizeList(ctx context.Context) ([]*sdkcompute.ResourceSKU, error)
38+
VMSizeList(ctx context.Context) ([]string, error)
39+
VMGetSKUs(ctx context.Context, vmSizes []string) (map[string]*sdkcompute.ResourceSKU, error)
3840
VMResize(ctx context.Context, vmName string, vmSize string) error
3941
ResourceGroupHasVM(ctx context.Context, vmName string) (bool, error)
4042
VMSerialConsole(ctx context.Context, log *logrus.Entry, vmName string, target io.Writer) error
@@ -148,9 +150,12 @@ func (a *azureActions) VMStopAndWait(ctx context.Context, vmName string, dealloc
148150
return a.virtualMachines.StopAndWait(ctx, clusterRGName, vmName, deallocateVM)
149151
}
150152

151-
func (a *azureActions) VMSizeList(ctx context.Context) ([]*sdkcompute.ResourceSKU, error) {
152-
filter := fmt.Sprintf("location eq '%s'", a.env.Location())
153-
return a.resourceSkus.List(ctx, filter, false)
153+
func (a *azureActions) VMGetSKUs(ctx context.Context, vmSizes []string) (map[string]*sdkcompute.ResourceSKU, error) {
154+
return computeskus.SelectVMSkusInCurrentRegion(ctx, a.resourceSkus, a.env.Location(), vmSizes)
155+
}
156+
157+
func (a *azureActions) VMSizeList(ctx context.Context) ([]string, error) {
158+
return computeskus.ListUnrestrictedVMSkusInCurrentRegion(ctx, a.resourceSkus, a.env.Location())
154159
}
155160

156161
func (a *azureActions) VMResize(ctx context.Context, vmName string, size string) error {

‎pkg/frontend/sku_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func TestValidateVMSku(t *testing.T) {
9595
workerProfile1Sku: "Standard_D4s_v2",
9696
workerProfile2Sku: "Standard_D4s_v2",
9797
resourceSkusClientErr: errors.New("unable to retrieve skus information"),
98-
wantErr: "unable to retrieve skus information",
98+
wantErr: "failure listing resource SKUs: unable to retrieve skus information",
9999
masterEncryptionAtHost: api.EncryptionAtHostDisabled,
100100
workerEncryptionAtHost: api.EncryptionAtHostDisabled,
101101
},
@@ -320,7 +320,17 @@ func TestValidateVMSku(t *testing.T) {
320320
resourceSkusClient := mock_armcompute.NewMockResourceSKUsClient(controller)
321321
resourceSkusClient.EXPECT().
322322
List(gomock.Any(), fmt.Sprintf("location eq %v", "eastus"), false).
323-
Return(skus, tt.resourceSkusClientErr)
323+
Return(func(yield func(*armcompute.ResourceSKU, error) bool) {
324+
if tt.resourceSkusClientErr != nil {
325+
yield(nil, tt.resourceSkusClientErr)
326+
return
327+
}
328+
for _, v := range skus {
329+
if !yield(v, nil) {
330+
return
331+
}
332+
}
333+
})
324334

325335
err := validateVMSku(context.Background(), oc, resourceSkusClient)
326336
utilerror.AssertErrorMessage(t, err, tt.wantErr)

0 commit comments

Comments
 (0)