Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: defork cloud-provider-azure #6947

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 34 additions & 27 deletions cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ func (as *AgentPool) initialize() error {
ctx, cancel := getContextWithCancel()
defer cancel()

template, err := as.manager.azClient.deploymentsClient.ExportTemplate(ctx, as.manager.config.ResourceGroup, as.manager.config.Deployment)
template, err := as.manager.azClient.deploymentClient.ExportTemplate(ctx, as.manager.config.ResourceGroup, as.manager.config.Deployment)
if err != nil {
klog.Errorf("deploymentsClient.ExportTemplate(%s, %s) failed: %v", as.manager.config.ResourceGroup, as.manager.config.Deployment, err)
return err
klog.Errorf("deploymentClient.ExportTemplate(%s, %s) failed: %v", as.manager.config.ResourceGroup, as.manager.config.Deployment, err)
comtalyst marked this conversation as resolved.
Show resolved Hide resolved
return err.Error()
}

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

func (as *AgentPool) getAllSucceededAndFailedDeployments() (succeededAndFailedDeployments []resources.DeploymentExtended, err error) {
func (as *AgentPool) getAllSucceededAndFailedDeployments() ([]resources.DeploymentExtended, error) {
ctx, cancel := getContextWithCancel()
defer cancel()

deploymentsFilter := "provisioningState eq 'Succeeded' or provisioningState eq 'Failed'"
succeededAndFailedDeployments, err = as.manager.azClient.deploymentsClient.List(ctx, as.manager.config.ResourceGroup, deploymentsFilter, nil)
if err != nil {
klog.Errorf("getAllSucceededAndFailedDeployments: failed to list succeeded or failed deployments with error: %v", err)
return nil, err
allDeployments, rerr := as.manager.azClient.deploymentClient.List(ctx, as.manager.config.ResourceGroup)
if rerr != nil {
klog.Errorf("getAllSucceededAndFailedDeployments: failed to list deployments with error: %v", rerr.Error())
return nil, rerr.Error()
}

result := make([]resources.DeploymentExtended, 0)
for _, deployment := range allDeployments {
if deployment.Properties == nil || deployment.Properties.ProvisioningState == nil {
continue
}
if *deployment.Properties.ProvisioningState == "Succeeded" || *deployment.Properties.ProvisioningState == "Failed" {
result = append(result, deployment)
}
}

return succeededAndFailedDeployments, err
return result, rerr.Error()
}

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

Expand Down Expand Up @@ -317,22 +326,20 @@ func (as *AgentPool) IncreaseSize(delta int) error {
}
ctx, cancel := getContextWithCancel()
defer cancel()
klog.V(3).Infof("Waiting for deploymentsClient.CreateOrUpdate(%s, %s, %v)", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
resp, err := as.manager.azClient.deploymentsClient.CreateOrUpdate(ctx, as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
isSuccess, realError := isSuccessHTTPResponse(resp, err)
if isSuccess {
klog.V(3).Infof("deploymentsClient.CreateOrUpdate(%s, %s, %v) success", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)

// Update cache after scale success.
as.curSize = int64(expectedSize)
as.lastRefresh = time.Now()
klog.V(6).Info("IncreaseSize: invalidating cache")
as.manager.invalidateCache()
return nil
klog.V(3).Infof("Waiting for deploymentClient.CreateOrUpdate(%s, %s, %v)", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)
rerr := as.manager.azClient.deploymentClient.CreateOrUpdate(ctx, as.manager.config.ResourceGroup, newDeploymentName, newDeployment, "")
if rerr != nil {
klog.Errorf("deploymentClient.CreateOrUpdate for deployment %q failed: %v", newDeploymentName, rerr.Error())
return rerr.Error()
}
klog.V(3).Infof("deploymentClient.CreateOrUpdate(%s, %s, %v) success", as.manager.config.ResourceGroup, newDeploymentName, newDeployment)

klog.Errorf("deploymentsClient.CreateOrUpdate for deployment %q failed: %v", newDeploymentName, realError)
return realError
// Update cache after scale success.
as.curSize = int64(expectedSize)
as.lastRefresh = time.Now()
klog.V(6).Info("IncreaseSize: invalidating cache")
as.manager.invalidateCache()
return nil
}

// AtomicIncreaseSize is not implemented.
Expand Down
23 changes: 12 additions & 11 deletions cluster-autoscaler/cloudprovider/azure/azure_agent_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/storageaccountclient/mockstorageaccountclient"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmclient/mockvmclient"
providerazureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"

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

for _, test := range testCases {
testAS := newTestAgentPool(newTestAzureManager(t), "testAS")
testAS.manager.azClient.deploymentsClient = &DeploymentsClientMock{
testAS.manager.azClient.deploymentClient = &DeploymentClientMock{
FakeStore: test.deployments,
}

err := testAS.deleteOutdatedDeployments()
assert.Equal(t, test.expectedErr, err, test.desc)
existedDeployments, err := testAS.manager.azClient.deploymentsClient.List(context.Background(), "", "", to.Int32Ptr(0))
existedDeployments, _ := testAS.manager.azClient.deploymentClient.List(context.Background(), "")
existedDeploymentsNames := make(map[string]bool)
for _, deployment := range existedDeployments {
existedDeploymentsNames[*deployment.Name] = true
Expand All @@ -185,7 +186,7 @@ func TestGetVMsFromCache(t *testing.T) {
mockVMClient := mockvmclient.NewMockInterface(ctrl)
testAS.manager.azClient.virtualMachinesClient = mockVMClient
mockVMClient.EXPECT().List(gomock.Any(), testAS.manager.config.ResourceGroup).Return(expectedVMs, nil)
testAS.manager.config.VMType = vmTypeStandard
testAS.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(testAS.manager.azClient, refreshInterval, *testAS.manager.config)
assert.NoError(t, err)
testAS.manager.azureCache = ac
Expand All @@ -204,7 +205,7 @@ func TestGetVMIndexes(t *testing.T) {
mockVMClient := mockvmclient.NewMockInterface(ctrl)
as.manager.azClient.virtualMachinesClient = mockVMClient
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
assert.NoError(t, err)
as.manager.azureCache = ac
Expand Down Expand Up @@ -244,7 +245,7 @@ func TestGetCurSize(t *testing.T) {
mockVMClient := mockvmclient.NewMockInterface(ctrl)
as.manager.azClient.virtualMachinesClient = mockVMClient
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
assert.NoError(t, err)
as.manager.azureCache = ac
Expand All @@ -269,7 +270,7 @@ func TestAgentPoolTargetSize(t *testing.T) {
as.manager.azClient.virtualMachinesClient = mockVMClient
expectedVMs := getExpectedVMs()
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
assert.NoError(t, err)
as.manager.azureCache = ac
Expand All @@ -289,7 +290,7 @@ func TestAgentPoolIncreaseSize(t *testing.T) {
as.manager.azClient.virtualMachinesClient = mockVMClient
expectedVMs := getExpectedVMs()
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil).MaxTimes(2)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
assert.NoError(t, err)
as.manager.azureCache = ac
Expand Down Expand Up @@ -318,7 +319,7 @@ func TestDecreaseTargetSize(t *testing.T) {
as.manager.azClient.virtualMachinesClient = mockVMClient
expectedVMs := getExpectedVMs()
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil).MaxTimes(3)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
assert.NoError(t, err)
as.manager.azureCache = ac
Expand Down Expand Up @@ -437,9 +438,9 @@ func TestAgentPoolDeleteNodes(t *testing.T) {
mockSAClient := mockstorageaccountclient.NewMockInterface(ctrl)
as.manager.azClient.storageAccountsClient = mockSAClient
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
as.manager.config.VMType = vmTypeVMSS
as.manager.config.VMType = providerazureconsts.VMTypeVMSS
assert.NoError(t, err)
as.manager.azureCache = ac

Expand Down Expand Up @@ -505,7 +506,7 @@ func TestAgentPoolNodes(t *testing.T) {
mockVMClient := mockvmclient.NewMockInterface(ctrl)
as.manager.azClient.virtualMachinesClient = mockVMClient
mockVMClient.EXPECT().List(gomock.Any(), as.manager.config.ResourceGroup).Return(expectedVMs, nil)
as.manager.config.VMType = vmTypeStandard
as.manager.config.VMType = providerazureconsts.VMTypeStandard
ac, err := newAzureCache(as.manager.azClient, refreshInterval, *as.manager.config)
assert.NoError(t, err)
as.manager.azureCache = ac
Expand Down
5 changes: 3 additions & 2 deletions cluster-autoscaler/cloudprovider/azure/azure_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/Azure/go-autorest/autorest/to"
"github.com/Azure/skewer"
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
providerazureconsts "sigs.k8s.io/cloud-provider-azure/pkg/consts"

"k8s.io/klog/v2"
)
Expand Down Expand Up @@ -436,7 +437,7 @@ func (m *azureCache) FindForInstance(instance *azureRef, vmType string) (cloudpr
}

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

if vmType == vmTypeStandard {
if vmType == providerazureconsts.VMTypeStandard {
// Omit virtual machines with providerID not in Azure resource ID format.
if ok := virtualMachineRE.Match([]byte(inst.Name)); !ok {
klog.V(3).Infof("Instance %q is not in Azure resource ID format, omit it in autoscaler", instance.Name)
Expand Down
5 changes: 3 additions & 2 deletions cluster-autoscaler/cloudprovider/azure/azure_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

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

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

ac.unownedInstances[inst] = false
nodeGroup, err = ac.FindForInstance(&inst, vmTypeStandard)
nodeGroup, err = ac.FindForInstance(&inst, providerazureconsts.VMTypeStandard)
assert.Nil(t, nodeGroup)
assert.NoError(t, err)
assert.True(t, ac.unownedInstances[inst])
Expand Down
Loading
Loading