Skip to content

Commit 527de12

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #6947 from comtalyst/comtalyst/cas-azure-defork-cloud-provider-azure
chore: defork cloud-provider-azure
2 parents 2caebb9 + 4e12429 commit 527de12

18 files changed

+1169
-1074
lines changed

‎cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,10 @@ func (as *AgentPool) initialize() error {
8282
ctx, cancel := getContextWithCancel()
8383
defer cancel()
8484

85-
template, err := as.manager.azClient.deploymentsClient.ExportTemplate(ctx, as.manager.config.ResourceGroup, as.manager.config.Deployment)
85+
template, err := as.manager.azClient.deploymentClient.ExportTemplate(ctx, as.manager.config.ResourceGroup, as.manager.config.Deployment)
8686
if err != nil {
87-
klog.Errorf("deploymentsClient.ExportTemplate(%s, %s) failed: %v", as.manager.config.ResourceGroup, as.manager.config.Deployment, err)
88-
return err
87+
klog.Errorf("deploymentClient.ExportTemplate(%s, %s) failed: %v", as.manager.config.ResourceGroup, as.manager.config.Deployment, err)
88+
return err.Error()
8989
}
9090

9191
as.template = template.Template.(map[string]interface{})
@@ -211,18 +211,27 @@ func (as *AgentPool) TargetSize() (int, error) {
211211
return int(size), nil
212212
}
213213

214-
func (as *AgentPool) getAllSucceededAndFailedDeployments() (succeededAndFailedDeployments []resources.DeploymentExtended, err error) {
214+
func (as *AgentPool) getAllSucceededAndFailedDeployments() ([]resources.DeploymentExtended, error) {
215215
ctx, cancel := getContextWithCancel()
216216
defer cancel()
217217

218-
deploymentsFilter := "provisioningState eq 'Succeeded' or provisioningState eq 'Failed'"
219-
succeededAndFailedDeployments, err = as.manager.azClient.deploymentsClient.List(ctx, as.manager.config.ResourceGroup, deploymentsFilter, nil)
220-
if err != nil {
221-
klog.Errorf("getAllSucceededAndFailedDeployments: failed to list succeeded or failed deployments with error: %v", err)
222-
return nil, err
218+
allDeployments, rerr := as.manager.azClient.deploymentClient.List(ctx, as.manager.config.ResourceGroup)
219+
if rerr != nil {
220+
klog.Errorf("getAllSucceededAndFailedDeployments: failed to list deployments with error: %v", rerr.Error())
221+
return nil, rerr.Error()
222+
}
223+
224+
result := make([]resources.DeploymentExtended, 0)
225+
for _, deployment := range allDeployments {
226+
if deployment.Properties == nil || deployment.Properties.ProvisioningState == nil {
227+
continue
228+
}
229+
if *deployment.Properties.ProvisioningState == "Succeeded" || *deployment.Properties.ProvisioningState == "Failed" {
230+
result = append(result, deployment)
231+
}
223232
}
224233

225-
return succeededAndFailedDeployments, err
234+
return result, rerr.Error()
226235
}
227236

228237
// deleteOutdatedDeployments keeps the newest deployments in the resource group and delete others,
@@ -258,9 +267,9 @@ func (as *AgentPool) deleteOutdatedDeployments() (err error) {
258267
errList := make([]error, 0)
259268
for _, deployment := range toBeDeleted {
260269
klog.V(4).Infof("deleteOutdatedDeployments: starts deleting outdated deployment (%s)", *deployment.Name)
261-
_, err := as.manager.azClient.deploymentsClient.Delete(ctx, as.manager.config.ResourceGroup, *deployment.Name)
262-
if err != nil {
263-
errList = append(errList, err)
270+
rerr := as.manager.azClient.deploymentClient.Delete(ctx, as.manager.config.ResourceGroup, *deployment.Name)
271+
if rerr != nil {
272+
errList = append(errList, rerr.Error())
264273
}
265274
}
266275

@@ -317,22 +326,20 @@ func (as *AgentPool) IncreaseSize(delta int) error {
317326
}
318327
ctx, cancel := getContextWithCancel()
319328
defer cancel()
320-
klog.V(3).Infof("Waiting for deploymentsClient.CreateOrUpdate(%s, %s, %v)", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
321-
resp, err := as.manager.azClient.deploymentsClient.CreateOrUpdate(ctx, as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
322-
isSuccess, realError := isSuccessHTTPResponse(resp, err)
323-
if isSuccess {
324-
klog.V(3).Infof("deploymentsClient.CreateOrUpdate(%s, %s, %v) success", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
325-
326-
// Update cache after scale success.
327-
as.curSize = int64(expectedSize)
328-
as.lastRefresh = time.Now()
329-
klog.V(6).Info("IncreaseSize: invalidating cache")
330-
as.manager.invalidateCache()
331-
return nil
329+
klog.V(3).Infof("Waiting for deploymentClient.CreateOrUpdate(%s, %s, %v)", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
330+
rerr := as.manager.azClient.deploymentClient.CreateOrUpdate(ctx, as.manager.config.ResourceGroup, newDeploymentName, newDeployment, "")
331+
if rerr != nil {
332+
klog.Errorf("deploymentClient.CreateOrUpdate for deployment %q failed: %v", newDeploymentName, rerr.Error())
333+
return rerr.Error()
332334
}
335+
klog.V(3).Infof("deploymentClient.CreateOrUpdate(%s, %s, %v) success", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
333336

334-
klog.Errorf("deploymentsClient.CreateOrUpdate for deployment %q failed: %v", newDeploymentName, realError)
335-
return realError
337+
// Update cache after scale success.
338+
as.curSize = int64(expectedSize)
339+
as.lastRefresh = time.Now()
340+
klog.V(6).Info("IncreaseSize: invalidating cache")
341+
as.manager.invalidateCache()
342+
return nil
336343
}
337344

338345
// AtomicIncreaseSize is not implemented.

‎cluster-autoscaler/cloudprovider/azure/azure_agent_pool_test.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/storageaccountclient/mockstorageaccountclient"
3030
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmclient/mockvmclient"
31+
providerazureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
3132
"sigs.k8s.io/cloud-provider-azure/pkg/retry"
3233

3334
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2022-08-01/compute"
@@ -156,13 +157,13 @@ func TestDeleteOutdatedDeployments(t *testing.T) {
156157

157158
for _, test := range testCases {
158159
testAS := newTestAgentPool(newTestAzureManager(t), "testAS")
159-
testAS.manager.azClient.deploymentsClient = &DeploymentsClientMock{
160+
testAS.manager.azClient.deploymentClient = &DeploymentClientMock{
160161
FakeStore: test.deployments,
161162
}
162163

163164
err := testAS.deleteOutdatedDeployments()
164165
assert.Equal(t, test.expectedErr, err, test.desc)
165-
existedDeployments, err := testAS.manager.azClient.deploymentsClient.List(context.Background(), "", "", to.Int32Ptr(0))
166+
existedDeployments, _ := testAS.manager.azClient.deploymentClient.List(context.Background(), "")
166167
existedDeploymentsNames := make(map[string]bool)
167168
for _, deployment := range existedDeployments {
168169
existedDeploymentsNames[*deployment.Name] = true
@@ -185,7 +186,7 @@ func TestGetVMsFromCache(t *testing.T) {
185186
mockVMClient := mockvmclient.NewMockInterface(ctrl)
186187
testAS.manager.azClient.virtualMachinesClient = mockVMClient
187188
mockVMClient.EXPECT().List(gomock.Any(), testAS.manager.config.ResourceGroup).Return(expectedVMs, nil)
188-
testAS.manager.config.VMType = vmTypeStandard
189+
testAS.manager.config.VMType = providerazureconsts.VMTypeStandard
189190
ac, err := newAzureCache(testAS.manager.azClient, refreshInterval, *testAS.manager.config)
190191
assert.NoError(t, err)
191192
testAS.manager.azureCache = ac
@@ -204,7 +205,7 @@ func TestGetVMIndexes(t *testing.T) {
204205
mockVMClient := mockvmclient.NewMockInterface(ctrl)
205206
as.manager.azClient.virtualMachinesClient = mockVMClient
206207
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
207-
as.manager.config.VMType = vmTypeStandard
208+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
208209
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
209210
assert.NoError(t, err)
210211
as.manager.azureCache = ac
@@ -244,7 +245,7 @@ func TestGetCurSize(t *testing.T) {
244245
mockVMClient := mockvmclient.NewMockInterface(ctrl)
245246
as.manager.azClient.virtualMachinesClient = mockVMClient
246247
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
247-
as.manager.config.VMType = vmTypeStandard
248+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
248249
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
249250
assert.NoError(t, err)
250251
as.manager.azureCache = ac
@@ -269,7 +270,7 @@ func TestAgentPoolTargetSize(t *testing.T) {
269270
as.manager.azClient.virtualMachinesClient = mockVMClient
270271
expectedVMs := getExpectedVMs()
271272
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
272-
as.manager.config.VMType = vmTypeStandard
273+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
273274
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
274275
assert.NoError(t, err)
275276
as.manager.azureCache = ac
@@ -289,7 +290,7 @@ func TestAgentPoolIncreaseSize(t *testing.T) {
289290
as.manager.azClient.virtualMachinesClient = mockVMClient
290291
expectedVMs := getExpectedVMs()
291292
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil).MaxTimes(2)
292-
as.manager.config.VMType = vmTypeStandard
293+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
293294
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
294295
assert.NoError(t, err)
295296
as.manager.azureCache = ac
@@ -318,7 +319,7 @@ func TestDecreaseTargetSize(t *testing.T) {
318319
as.manager.azClient.virtualMachinesClient = mockVMClient
319320
expectedVMs := getExpectedVMs()
320321
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil).MaxTimes(3)
321-
as.manager.config.VMType = vmTypeStandard
322+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
322323
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
323324
assert.NoError(t, err)
324325
as.manager.azureCache = ac
@@ -437,9 +438,9 @@ func TestAgentPoolDeleteNodes(t *testing.T) {
437438
mockSAClient := mockstorageaccountclient.NewMockInterface(ctrl)
438439
as.manager.azClient.storageAccountsClient = mockSAClient
439440
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
440-
as.manager.config.VMType = vmTypeStandard
441+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
441442
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
442-
as.manager.config.VMType = vmTypeVMSS
443+
as.manager.config.VMType = providerazureconsts.VMTypeVMSS
443444
assert.NoError(t, err)
444445
as.manager.azureCache = ac
445446

@@ -505,7 +506,7 @@ func TestAgentPoolNodes(t *testing.T) {
505506
mockVMClient := mockvmclient.NewMockInterface(ctrl)
506507
as.manager.azClient.virtualMachinesClient = mockVMClient
507508
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
508-
as.manager.config.VMType = vmTypeStandard
509+
as.manager.config.VMType = providerazureconsts.VMTypeStandard
509510
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
510511
assert.NoError(t, err)
511512
as.manager.azureCache = ac

‎cluster-autoscaler/cloudprovider/azure/azure_cache.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/Azure/go-autorest/autorest/to"
2929
"github.com/Azure/skewer"
3030
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
31+
providerazureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
3132

3233
"k8s.io/klog/v2"
3334
)
@@ -436,7 +437,7 @@ func (m *azureCache) FindForInstance(instance *azureRef, vmType string) (cloudpr
436437
}
437438

438439
// cluster with vmss pool only
439-
if vmType == vmTypeVMSS && len(vmsPoolSet) == 0 {
440+
if vmType == providerazureconsts.VMTypeVMSS && len(vmsPoolSet) == 0 {
440441
if m.areAllScaleSetsUniform() {
441442
// Omit virtual machines not managed by vmss only in case of uniform scale set.
442443
if ok := virtualMachineRE.Match([]byte(inst.Name)); ok {
@@ -447,7 +448,7 @@ func (m *azureCache) FindForInstance(instance *azureRef, vmType string) (cloudpr
447448
}
448449
}
449450

450-
if vmType == vmTypeStandard {
451+
if vmType == providerazureconsts.VMTypeStandard {
451452
// Omit virtual machines with providerID not in Azure resource ID format.
452453
if ok := virtualMachineRE.Match([]byte(inst.Name)); !ok {
453454
klog.V(3).Infof("Instance %q is not in Azure resource ID format, omit it in autoscaler", instance.Name)

‎cluster-autoscaler/cloudprovider/azure/azure_cache_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"testing"
2121

2222
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
23+
providerazureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
2324

2425
"github.com/stretchr/testify/assert"
2526
)
@@ -60,12 +61,12 @@ func TestFindForInstance(t *testing.T) {
6061
inst := azureRef{Name: "/subscriptions/sub/resourceGroups/rg/providers/foo"}
6162
ac.unownedInstances = make(map[azureRef]bool)
6263
ac.unownedInstances[inst] = true
63-
nodeGroup, err := ac.FindForInstance(&inst, vmTypeVMSS)
64+
nodeGroup, err := ac.FindForInstance(&inst, providerazureconsts.VMTypeVMSS)
6465
assert.Nil(t, nodeGroup)
6566
assert.NoError(t, err)
6667

6768
ac.unownedInstances[inst] = false
68-
nodeGroup, err = ac.FindForInstance(&inst, vmTypeStandard)
69+
nodeGroup, err = ac.FindForInstance(&inst, providerazureconsts.VMTypeStandard)
6970
assert.Nil(t, nodeGroup)
7071
assert.NoError(t, err)
7172
assert.True(t, ac.unownedInstances[inst])

0 commit comments

Comments
 (0)