diff --git a/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation.go b/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation.go index 56e1c5b043f..559adc9e29f 100644 --- a/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation.go +++ b/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation.go @@ -10,6 +10,7 @@ import ( "fmt" "net/http" "path/filepath" + "runtime/debug" "strings" "sync" @@ -104,12 +105,28 @@ func (f *frontend) _getPreResizeControlPlaneVMsValidation( } } + // safeGo wraps a validation function with panic recovery. The + // dynamicRESTMapper in controller-runtime v0.11.2 can nil-pointer panic + // when the API server is unreachable (lazy init leaves staticMapper nil). + // Since these run in child goroutines, the HTTP Panic middleware cannot + // catch them — an unrecovered panic here would crash the entire RP process. + safeGo := func(fn func() error) func() { + return func() { + defer func() { + if r := recover(); r != nil { + collect(fmt.Errorf("panic: %v\n%s", r, debug.Stack())) + } + }() + collect(fn()) + } + } + var wg sync.WaitGroup - wg.Go(func() { collect(f.validateVMSKU(ctx, doc, subscriptionDoc, desiredVMSize, log)) }) - wg.Go(func() { collect(validateAPIServerHealth(ctx, k)) }) - wg.Go(func() { collect(validateEtcdHealth(ctx, k)) }) - wg.Go(func() { collect(validateClusterSP(ctx, k)) }) + wg.Go(safeGo(func() error { return f.validateVMSKU(ctx, doc, subscriptionDoc, desiredVMSize, log) })) + wg.Go(safeGo(func() error { return validateAPIServerHealth(ctx, k) })) + wg.Go(safeGo(func() error { return validateEtcdHealth(ctx, k) })) + wg.Go(safeGo(func() error { return validateClusterSP(ctx, k) })) wg.Wait() @@ -130,7 +147,7 @@ func (f *frontend) _getPreResizeControlPlaneVMsValidation( // defaultValidateResizeQuota creates an FP-authorized compute usage client and // delegates to checkResizeComputeQuota. Injected via f.validateResizeQuota so // tests can swap it with quotaCheckDisabled. -func defaultValidateResizeQuota(ctx context.Context, environment env.Interface, subscriptionDoc *api.SubscriptionDocument, location, currentVMSize, desiredVMSize string) error { +func defaultValidateResizeQuota(ctx context.Context, environment env.Interface, subscriptionDoc *api.SubscriptionDocument, location string, currentVMSizes []string, desiredVMSize string) error { tenantID := subscriptionDoc.Subscription.Properties.TenantID fpAuthorizer, err := environment.FPAuthorizer(tenantID, nil, environment.Environment().ResourceManagerScope) @@ -139,7 +156,7 @@ func defaultValidateResizeQuota(ctx context.Context, environment env.Interface, } spComputeUsage := compute.NewUsageClient(environment.Environment(), subscriptionDoc.ID, fpAuthorizer) - return checkResizeComputeQuota(ctx, spComputeUsage, location, currentVMSize, desiredVMSize) + return checkResizeComputeQuota(ctx, spComputeUsage, location, currentVMSizes, desiredVMSize) } // checkResizeComputeQuota verifies that the subscription has enough remaining @@ -147,42 +164,57 @@ func defaultValidateResizeQuota(ctx context.Context, environment env.Interface, // master nodes. // // Unlike validateQuota in quota_validation.go (which checks absolute totals for -// cluster creation), this computes the incremental delta: same-family resizes -// only need (newCores − currentCores) × nodeCount; cross-family resizes need -// the full new cores for the target family but only the net delta for "cores". +// cluster creation), this computes the incremental delta per VM. Each master VM +// may have a different current size (e.g. after a partial resize), so we +// calculate the delta individually and sum across all VMs that need resizing. +// +// Same-family resizes only need (newCores − currentCores) per VM; cross-family +// resizes need the full new cores for the target family but only the net delta +// for regional "cores". // // This checks subscription-level quota only, not Azure regional datacenter // capacity — without a capacity reservation, AllocationFailed errors can only // be detected at ARM PUT time. -func checkResizeComputeQuota(ctx context.Context, spComputeUsage compute.UsageClient, location, currentVMSize, desiredVMSize string) error { +func checkResizeComputeQuota(ctx context.Context, spComputeUsage compute.UsageClient, location string, currentVMSizes []string, desiredVMSize string) error { newSizeStruct, ok := validate.VMSizeFromName(api.VMSize(desiredVMSize)) if !ok { return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "vmSize", fmt.Sprintf("The provided VM SKU '%s' is not supported.", desiredVMSize)) } - currentSizeStruct, ok := validate.VMSizeFromName(api.VMSize(currentVMSize)) - if !ok { - return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "vmSize", - fmt.Sprintf("The current VM SKU '%s' could not be resolved.", currentVMSize)) - } + requiredByQuota := map[string]int{} - // Same family: only the delta matters. Cross-family: full new cores needed. - additionalCoresPerNode := newSizeStruct.CoreCount - if newSizeStruct.Family == currentSizeStruct.Family { - additionalCoresPerNode = newSizeStruct.CoreCount - currentSizeStruct.CoreCount - if additionalCoresPerNode <= 0 { - return nil + for _, currentVMSize := range currentVMSizes { + if strings.EqualFold(currentVMSize, desiredVMSize) { + continue // VM already at desired size, no quota needed } - } - totalAdditionalCores := additionalCoresPerNode * api.ControlPlaneNodeCount - // Regional "cores" delta accounts for freed cores from the old VM. - totalAdditionalRegionalCores := max((newSizeStruct.CoreCount-currentSizeStruct.CoreCount)*api.ControlPlaneNodeCount, 0) + currentSizeStruct, ok := validate.VMSizeFromName(api.VMSize(currentVMSize)) + if !ok { + return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "vmSize", + fmt.Sprintf("The current VM SKU '%s' could not be resolved.", currentVMSize)) + } + + // Same family: only the delta matters. Cross-family: full new cores needed. + additionalFamilyCores := newSizeStruct.CoreCount + if newSizeStruct.Family == currentSizeStruct.Family { + additionalFamilyCores = newSizeStruct.CoreCount - currentSizeStruct.CoreCount + } - requiredByQuota := map[string]int{ - newSizeStruct.Family: totalAdditionalCores, - "cores": totalAdditionalRegionalCores, + if additionalFamilyCores > 0 { + requiredByQuota[newSizeStruct.Family] += additionalFamilyCores + } + + // Regional "cores" delta accounts for freed cores from the old VM. + regionalDelta := newSizeStruct.CoreCount - currentSizeStruct.CoreCount + if regionalDelta > 0 { + requiredByQuota["cores"] += regionalDelta + } + } + + // All VMs already at desired size or downsizing — no quota check needed. + if len(requiredByQuota) == 0 { + return nil } usages, err := spComputeUsage.List(ctx, location) @@ -214,7 +246,7 @@ func checkResizeComputeQuota(ctx context.Context, spComputeUsage compute.UsageCl } // quotaCheckDisabled is a no-op replacement for f.validateResizeQuota in tests. -func quotaCheckDisabled(_ context.Context, _ env.Interface, _ *api.SubscriptionDocument, _, _, _ string) error { +func quotaCheckDisabled(_ context.Context, _ env.Interface, _ *api.SubscriptionDocument, _ string, _ []string, _ string) error { return nil } @@ -352,8 +384,18 @@ func (f *frontend) validateVMSKU( return err } - currentVMSize := string(doc.OpenShiftCluster.Properties.MasterProfile.VMSize) - err = f.validateResizeQuota(ctx, f.env, subscriptionDoc, location, currentVMSize, desiredVMSize) + currentVMSizes, err := a.MasterVMSizes(ctx) + if err != nil { + return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInternalServerError, "", + fmt.Sprintf("Failed to retrieve current master VM sizes from Azure: %v", err)) + } + + if len(currentVMSizes) == 0 { + return api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInternalServerError, "", + "No master VMs found in the cluster resource group.") + } + + err = f.validateResizeQuota(ctx, f.env, subscriptionDoc, location, currentVMSizes, desiredVMSize) if err != nil { return err } diff --git a/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation_test.go b/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation_test.go index f18175ceba7..db96e58aa8c 100644 --- a/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation_test.go +++ b/pkg/frontend/admin_openshiftcluster_vmresize_pre_validation_test.go @@ -157,6 +157,9 @@ func TestPreResizeControlPlaneVMsValidation(t *testing.T) { Capabilities: []*armcompute.ResourceSKUCapabilities{}, }, }, nil) + a.EXPECT(). + MasterVMSizes(gomock.Any()). + Return([]string{"Standard_D8s_v3", "Standard_D8s_v3", "Standard_D8s_v3"}, nil) }, kubeMocks: allKubeChecksHealthyMock, wantStatusCode: http.StatusOK, @@ -374,6 +377,116 @@ func TestPreResizeControlPlaneVMsValidation(t *testing.T) { wantStatusCode: http.StatusBadRequest, wantError: `400: InvalidParameter: : Pre-flight validation failed. Details: InvalidParameter: vmSize: The selected SKU 'Standard_D8s_v3' is restricted in region 'eastus' for selected subscription`, }, + { + name: "MasterVMSizes returns error", + resourceID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + vmSize: "Standard_D8s_v3", + fixture: func(f *testdatabase.Fixture) { + f.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{ + Key: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")), + OpenShiftCluster: &api.OpenShiftCluster{ + ID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + Location: "eastus", + Properties: api.OpenShiftClusterProperties{ + MasterProfile: api.MasterProfile{ + VMSize: api.VMSizeStandardD8sV3, + }, + ClusterProfile: api.ClusterProfile{ + ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", mockSubID), + }, + }, + }, + }) + f.AddSubscriptionDocuments(&api.SubscriptionDocument{ + ID: mockSubID, + Subscription: &api.Subscription{ + State: api.SubscriptionStateRegistered, + Properties: &api.SubscriptionProperties{ + TenantID: mockTenantID, + }, + }, + }) + }, + mocks: func(tt *test, a *mock_adminactions.MockAzureActions) { + a.EXPECT(). + VMGetSKUs(gomock.Any(), []string{"Standard_D8s_v3"}). + Return(map[string]*armcompute.ResourceSKU{ + "Standard_D8s_v3": { + Name: pointerutils.ToPtr("Standard_D8s_v3"), + ResourceType: pointerutils.ToPtr("virtualMachines"), + Locations: pointerutils.ToSlicePtr([]string{"eastus"}), + LocationInfo: []*armcompute.ResourceSKULocationInfo{ + { + Location: pointerutils.ToPtr("eastus"), + }, + }, + Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}), + Capabilities: []*armcompute.ResourceSKUCapabilities{}, + }, + }, nil) + a.EXPECT(). + MasterVMSizes(gomock.Any()). + Return(nil, fmt.Errorf("authorization denied")) + }, + kubeMocks: allKubeChecksHealthyMock, + wantStatusCode: http.StatusBadRequest, + wantError: `400: InvalidParameter: : Pre-flight validation failed. Details: InternalServerError: : Failed to retrieve current master VM sizes from Azure: authorization denied`, + }, + { + name: "MasterVMSizes returns empty slice", + resourceID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + vmSize: "Standard_D8s_v3", + fixture: func(f *testdatabase.Fixture) { + f.AddOpenShiftClusterDocuments(&api.OpenShiftClusterDocument{ + Key: strings.ToLower(testdatabase.GetResourcePath(mockSubID, "resourceName")), + OpenShiftCluster: &api.OpenShiftCluster{ + ID: testdatabase.GetResourcePath(mockSubID, "resourceName"), + Location: "eastus", + Properties: api.OpenShiftClusterProperties{ + MasterProfile: api.MasterProfile{ + VMSize: api.VMSizeStandardD8sV3, + }, + ClusterProfile: api.ClusterProfile{ + ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", mockSubID), + }, + }, + }, + }) + f.AddSubscriptionDocuments(&api.SubscriptionDocument{ + ID: mockSubID, + Subscription: &api.Subscription{ + State: api.SubscriptionStateRegistered, + Properties: &api.SubscriptionProperties{ + TenantID: mockTenantID, + }, + }, + }) + }, + mocks: func(tt *test, a *mock_adminactions.MockAzureActions) { + a.EXPECT(). + VMGetSKUs(gomock.Any(), []string{"Standard_D8s_v3"}). + Return(map[string]*armcompute.ResourceSKU{ + "Standard_D8s_v3": { + Name: pointerutils.ToPtr("Standard_D8s_v3"), + ResourceType: pointerutils.ToPtr("virtualMachines"), + Locations: pointerutils.ToSlicePtr([]string{"eastus"}), + LocationInfo: []*armcompute.ResourceSKULocationInfo{ + { + Location: pointerutils.ToPtr("eastus"), + }, + }, + Restrictions: pointerutils.ToSlicePtr([]armcompute.ResourceSKURestrictions{}), + Capabilities: []*armcompute.ResourceSKUCapabilities{}, + }, + }, nil) + a.EXPECT(). + MasterVMSizes(gomock.Any()). + Return([]string{}, nil) + }, + kubeMocks: allKubeChecksHealthyMock, + wantStatusCode: http.StatusBadRequest, + wantError: `400: InvalidParameter: : Pre-flight validation failed. Details: InternalServerError: : No master VMs found in the cluster resource group.`, + }, } { t.Run(tt.name, func(t *testing.T) { ti := newTestInfra(t).WithSubscriptions().WithOpenShiftClusters() @@ -428,20 +541,23 @@ func TestCheckResizeComputeQuota(t *testing.T) { ctx := context.Background() type test struct { - name string - currentVMSize string - vmSize string - mocks func(*mock_compute.MockUsageClient) - wantErr string + name string + currentVMSizes []string + vmSize string + mocks func(*mock_compute.MockUsageClient) + wantErr string } + // Helper to create a slice of 3 identical sizes (all masters same size). + threeMasters := func(size string) []string { return []string{size, size, size} } + for _, tt := range []*test{ { // D8s_v3 (8 cores) → D16s_v3 (16 cores), same family. // Delta per node = 8, total = 8 × 3 = 24. 76 in use, limit 100 → 24 remaining = exact fit. - name: "same family upsize - enough quota for delta across all masters", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_D16s_v3", + name: "same family upsize - enough quota for delta across all masters", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_D16s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -466,9 +582,9 @@ func TestCheckResizeComputeQuota(t *testing.T) { { // D8s_v3 (8 cores) → D16s_v3 (16 cores), same family. // Delta per node = 8, total = 8 × 3 = 24. 77 in use, limit 100 → 23 remaining < 24. - name: "same family upsize - not enough quota for all masters", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_D16s_v3", + name: "same family upsize - not enough quota for all masters", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_D16s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -492,24 +608,24 @@ func TestCheckResizeComputeQuota(t *testing.T) { wantErr: "400: ResourceQuotaExceeded: vmSize: Resource quota of standardDSv3Family exceeded. Maximum allowed: 100, Current in use: 77, Additional requested: 24.", }, { - name: "same family downsize - no quota check needed", - currentVMSize: "Standard_D16s_v3", - vmSize: "Standard_D8s_v3", - mocks: func(cuc *mock_compute.MockUsageClient) {}, + name: "same family downsize - no quota check needed", + currentVMSizes: threeMasters("Standard_D16s_v3"), + vmSize: "Standard_D8s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) {}, }, { - name: "same family same size - no quota check needed", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_D8s_v3", - mocks: func(cuc *mock_compute.MockUsageClient) {}, + name: "same family same size - no quota check needed", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_D8s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) {}, }, { // D8s_v3 → E8s_v3, cross family. Full new cores: 8 × 3 = 24. // Family: 76 in use, limit 100 → 24 remaining = exact fit. // Regional cores delta = (8 - 8) × 3 = 0, no check needed. - name: "cross family - full new cores checked for all masters", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_E8s_v3", + name: "cross family - full new cores checked for all masters", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_E8s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -534,9 +650,9 @@ func TestCheckResizeComputeQuota(t *testing.T) { { // D8s_v3 → E8s_v3, cross family. Full new cores: 8 × 3 = 24. // 77 in use, limit 100 → 23 remaining < 24. - name: "cross family - not enough quota for all masters", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_E8s_v3", + name: "cross family - not enough quota for all masters", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_E8s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -563,9 +679,9 @@ func TestCheckResizeComputeQuota(t *testing.T) { // D8s_v3 (8 cores) → E16s_v3 (16 cores), cross family upsize. // Family quota: plenty of room (50 in use, limit 200). // Regional cores delta = (16 - 8) × 3 = 24. 177 in use, limit 200 → 23 remaining < 24. - name: "cross family - regional cores quota exceeded", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_E16s_v3", + name: "cross family - regional cores quota exceeded", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_E16s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -592,9 +708,9 @@ func TestCheckResizeComputeQuota(t *testing.T) { // D8s_v3 → E4s_v3, cross family downsize. // Family: full new cores = 4 × 3 = 12. // Regional cores delta = (4 - 8) × 3 = -12 → clamped to 0, no regional check. - name: "cross family downsize - regional cores not checked", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_E4s_v3", + name: "cross family downsize - regional cores not checked", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_E4s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -617,9 +733,9 @@ func TestCheckResizeComputeQuota(t *testing.T) { }, }, { - name: "family not in usage list - no quota limit", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_D16s_v3", + name: "family not in usage list - no quota limit", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_D16s_v3", mocks: func(cuc *mock_compute.MockUsageClient) { cuc.EXPECT(). List(ctx, "eastus"). @@ -635,11 +751,112 @@ func TestCheckResizeComputeQuota(t *testing.T) { }, }, { - name: "unsupported new VM size", - currentVMSize: "Standard_D8s_v3", - vmSize: "Standard_Nonexistent_v99", - mocks: func(cuc *mock_compute.MockUsageClient) {}, - wantErr: "400: InvalidParameter: vmSize: The provided VM SKU 'Standard_Nonexistent_v99' is not supported.", + name: "unsupported new VM size", + currentVMSizes: threeMasters("Standard_D8s_v3"), + vmSize: "Standard_Nonexistent_v99", + mocks: func(cuc *mock_compute.MockUsageClient) {}, + wantErr: "400: InvalidParameter: vmSize: The provided VM SKU 'Standard_Nonexistent_v99' is not supported.", + }, + { + // Mixed sizes: partial resize scenario. + // master-0: D16s_v3 (already resized), master-1: D8s_v3, master-2: D8s_v3. + // Target: D16s_v3. Only 2 VMs need resizing, delta = 8 × 2 = 16. + name: "mixed sizes - partial resize only needs quota for remaining VMs", + currentVMSizes: []string{"Standard_D16s_v3", "Standard_D8s_v3", "Standard_D8s_v3"}, + vmSize: "Standard_D16s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) { + cuc.EXPECT(). + List(ctx, "eastus"). + Return([]mgmtcompute.Usage{ + { + Name: &mgmtcompute.UsageName{ + Value: pointerutils.ToPtr("standardDSv3Family"), + }, + CurrentValue: pointerutils.ToPtr(int32(84)), + Limit: pointerutils.ToPtr(int64(100)), + }, + { + Name: &mgmtcompute.UsageName{ + Value: pointerutils.ToPtr("cores"), + }, + CurrentValue: pointerutils.ToPtr(int32(84)), + Limit: pointerutils.ToPtr(int64(200)), + }, + }, nil) + }, + }, + { + // Mixed sizes: partial resize, not enough quota. + // master-0: D16s_v3, master-1: D8s_v3, master-2: D8s_v3. + // Target: D16s_v3. Need 8 × 2 = 16. 85 in use, limit 100 → 15 remaining < 16. + name: "mixed sizes - partial resize not enough quota", + currentVMSizes: []string{"Standard_D16s_v3", "Standard_D8s_v3", "Standard_D8s_v3"}, + vmSize: "Standard_D16s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) { + cuc.EXPECT(). + List(ctx, "eastus"). + Return([]mgmtcompute.Usage{ + { + Name: &mgmtcompute.UsageName{ + Value: pointerutils.ToPtr("standardDSv3Family"), + }, + CurrentValue: pointerutils.ToPtr(int32(85)), + Limit: pointerutils.ToPtr(int64(100)), + }, + { + Name: &mgmtcompute.UsageName{ + Value: pointerutils.ToPtr("cores"), + }, + CurrentValue: pointerutils.ToPtr(int32(85)), + Limit: pointerutils.ToPtr(int64(200)), + }, + }, nil) + }, + wantErr: "400: ResourceQuotaExceeded: vmSize: Resource quota of standardDSv3Family exceeded. Maximum allowed: 100, Current in use: 85, Additional requested: 16.", + }, + { + // All VMs already at desired size — no quota check needed (idempotent). + name: "all VMs already at desired size - no quota check needed", + currentVMSizes: threeMasters("Standard_D16s_v3"), + vmSize: "Standard_D16s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) {}, + }, + { + // Cross-family accumulation: 2 VMs in DSv3, 1 VM in ESv3, resize to D16s_v3. + // D8s_v3 (x2): same family, delta = 8 each = 16 for DSv3. + // E8s_v3 (x1): cross family, full 16 cores for DSv3. + // Total DSv3 = 16 + 16 = 32. 69 in use, limit 100 → 31 remaining < 32. + name: "cross-family accumulation - same and cross family contribute to same target", + currentVMSizes: []string{"Standard_D8s_v3", "Standard_D8s_v3", "Standard_E8s_v3"}, + vmSize: "Standard_D16s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) { + cuc.EXPECT(). + List(ctx, "eastus"). + Return([]mgmtcompute.Usage{ + { + Name: &mgmtcompute.UsageName{ + Value: pointerutils.ToPtr("standardDSv3Family"), + }, + CurrentValue: pointerutils.ToPtr(int32(69)), + Limit: pointerutils.ToPtr(int64(100)), + }, + { + Name: &mgmtcompute.UsageName{ + Value: pointerutils.ToPtr("cores"), + }, + CurrentValue: pointerutils.ToPtr(int32(50)), + Limit: pointerutils.ToPtr(int64(200)), + }, + }, nil) + }, + wantErr: "400: ResourceQuotaExceeded: vmSize: Resource quota of standardDSv3Family exceeded. Maximum allowed: 100, Current in use: 69, Additional requested: 32.", + }, + { + name: "unresolvable current VM size", + currentVMSizes: []string{"Standard_D8s_v3", "Standard_Unknown_v99", "Standard_D8s_v3"}, + vmSize: "Standard_D16s_v3", + mocks: func(cuc *mock_compute.MockUsageClient) {}, + wantErr: "400: InvalidParameter: vmSize: The current VM SKU 'Standard_Unknown_v99' could not be resolved.", }, } { t.Run(tt.name, func(t *testing.T) { @@ -649,7 +866,7 @@ func TestCheckResizeComputeQuota(t *testing.T) { computeUsageClient := mock_compute.NewMockUsageClient(controller) tt.mocks(computeUsageClient) - err := checkResizeComputeQuota(ctx, computeUsageClient, "eastus", tt.currentVMSize, tt.vmSize) + err := checkResizeComputeQuota(ctx, computeUsageClient, "eastus", tt.currentVMSizes, tt.vmSize) utilerror.AssertErrorMessage(t, err, tt.wantErr) }) } diff --git a/pkg/frontend/adminactions/azureactions.go b/pkg/frontend/adminactions/azureactions.go index 131eb635e3d..aaba50b78ee 100644 --- a/pkg/frontend/adminactions/azureactions.go +++ b/pkg/frontend/adminactions/azureactions.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net/http" + "strings" "github.com/sirupsen/logrus" @@ -38,6 +39,8 @@ type AzureActions interface { VMSizeList(ctx context.Context) ([]string, error) VMGetSKUs(ctx context.Context, vmSizes []string) (map[string]*sdkcompute.ResourceSKU, error) VMResize(ctx context.Context, vmName string, vmSize string) error + GetMasterVMs(ctx context.Context) ([]mgmtcompute.VirtualMachine, error) + MasterVMSizes(ctx context.Context) ([]string, error) ResourceGroupHasVM(ctx context.Context, vmName string) (bool, error) VMSerialConsole(ctx context.Context, log *logrus.Entry, vmName string, target io.Writer) error ResourceDeleteAndWait(ctx context.Context, resourceID string) error @@ -169,6 +172,39 @@ func (a *azureActions) VMResize(ctx context.Context, vmName string, size string) return a.virtualMachines.CreateOrUpdateAndWait(ctx, clusterRGName, vmName, vm) } +func (a *azureActions) GetMasterVMs(ctx context.Context) ([]mgmtcompute.VirtualMachine, error) { + clusterRGName := stringutils.LastTokenByte(a.oc.Properties.ClusterProfile.ResourceGroupID, '/') + vmList, err := a.virtualMachines.List(ctx, clusterRGName) + if err != nil { + return nil, err + } + + var masterVMs []mgmtcompute.VirtualMachine + for _, vm := range vmList { + if vm.Name != nil && strings.Contains(*vm.Name, "-master-") { + masterVMs = append(masterVMs, vm) + } + } + + return masterVMs, nil +} + +func (a *azureActions) MasterVMSizes(ctx context.Context) ([]string, error) { + masterVMs, err := a.GetMasterVMs(ctx) + if err != nil { + return nil, err + } + + var sizes []string + for _, vm := range masterVMs { + if vm.HardwareProfile != nil { + sizes = append(sizes, string(vm.HardwareProfile.VMSize)) + } + } + + return sizes, nil +} + func (a *azureActions) ResourceGroupHasVM(ctx context.Context, vmName string) (bool, error) { clusterRGName := stringutils.LastTokenByte(a.oc.Properties.ClusterProfile.ResourceGroupID, '/') vmList, err := a.virtualMachines.List(ctx, clusterRGName) diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index 7e2adfcf0bf..2b654c74a7f 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -115,7 +115,7 @@ type frontend struct { // these helps us to test and mock easier now func() time.Time systemDataClusterDocEnricher func(*api.OpenShiftClusterDocument, *api.SystemData) - validateResizeQuota func(ctx context.Context, environment env.Interface, subscriptionDoc *api.SubscriptionDocument, location, currentVMSize, desiredVMSize string) error + validateResizeQuota func(ctx context.Context, environment env.Interface, subscriptionDoc *api.SubscriptionDocument, location string, currentVMSizes []string, desiredVMSize string) error streamResponder StreamResponder } diff --git a/pkg/util/mocks/adminactions/azureactions.go b/pkg/util/mocks/adminactions/azureactions.go index f97c7d5ca02..155b157f1a6 100644 --- a/pkg/util/mocks/adminactions/azureactions.go +++ b/pkg/util/mocks/adminactions/azureactions.go @@ -61,6 +61,21 @@ func (mr *MockAzureActionsMockRecorder) GetEffectiveRouteTable(ctx, nicName any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEffectiveRouteTable", reflect.TypeOf((*MockAzureActions)(nil).GetEffectiveRouteTable), ctx, nicName) } +// GetMasterVMs mocks base method. +func (m *MockAzureActions) GetMasterVMs(ctx context.Context) ([]compute.VirtualMachine, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetMasterVMs", ctx) + ret0, _ := ret[0].([]compute.VirtualMachine) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetMasterVMs indicates an expected call of GetMasterVMs. +func (mr *MockAzureActionsMockRecorder) GetMasterVMs(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMasterVMs", reflect.TypeOf((*MockAzureActions)(nil).GetMasterVMs), ctx) +} + // GetVirtualMachine mocks base method. func (m *MockAzureActions) GetVirtualMachine(ctx context.Context, resourceGroupName, VMName string, expand compute.InstanceViewTypes) (compute.VirtualMachine, error) { m.ctrl.T.Helper() @@ -91,6 +106,21 @@ func (mr *MockAzureActionsMockRecorder) GroupResourceList(ctx any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GroupResourceList", reflect.TypeOf((*MockAzureActions)(nil).GroupResourceList), ctx) } +// MasterVMSizes mocks base method. +func (m *MockAzureActions) MasterVMSizes(ctx context.Context) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MasterVMSizes", ctx) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// MasterVMSizes indicates an expected call of MasterVMSizes. +func (mr *MockAzureActionsMockRecorder) MasterVMSizes(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MasterVMSizes", reflect.TypeOf((*MockAzureActions)(nil).MasterVMSizes), ctx) +} + // NICReconcileFailedState mocks base method. func (m *MockAzureActions) NICReconcileFailedState(ctx context.Context, nicName string) error { m.ctrl.T.Helper()