Skip to content

Commit

Permalink
move scalesets serviceName to scope
Browse files Browse the repository at this point in the history
  • Loading branch information
Cecile Robert-Michon committed Aug 31, 2021
1 parent 34f3399 commit 38dab78
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 21 deletions.
6 changes: 4 additions & 2 deletions azure/scope/machinepool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/azure"
machinepool "sigs.k8s.io/cluster-api-provider-azure/azure/scope/strategies/machinepool_deployments"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
Expand All @@ -48,6 +47,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

// TODO: move this to scalesets.go once we remove the usage in here
const ScalesetsServiceName = "scalesets"

type (
// MachinePoolScopeParams defines the input parameters used to create a new MachinePoolScope.
MachinePoolScopeParams struct {
Expand Down Expand Up @@ -300,7 +302,7 @@ func (m *MachinePoolScope) applyAzureMachinePoolMachines(ctx context.Context) er
return nil
}

if futures.Has(m.AzureMachinePool, m.Name(), scalesets.ServiceName) {
if futures.Has(m.AzureMachinePool, m.Name(), ScalesetsServiceName) {
m.V(4).Info("exiting early due an in-progress long running operation on the ScaleSet")
// exit early to be less greedy about delete
return nil
Expand Down
19 changes: 9 additions & 10 deletions azure/services/scalesets/scalesets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,11 @@ import (
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
"sigs.k8s.io/cluster-api-provider-azure/azure/scope"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
)

const ServiceName = "scalesets"

type (
// ScaleSetScope defines the scope interface for a scale sets service.
ScaleSetScope interface {
Expand Down Expand Up @@ -89,7 +88,7 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {

// check if there is an ongoing long running operation
var (
future = s.Scope.GetLongRunningOperationState(s.Scope.ScaleSetSpec().Name, ServiceName)
future = s.Scope.GetLongRunningOperationState(s.Scope.ScaleSetSpec().Name, scope.ScalesetsServiceName)
fetchedVMSS *azure.VMSS
)

Expand Down Expand Up @@ -144,7 +143,7 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {
}

// if we get to here, we have completed any long running VMSS operations (creates / updates)
s.Scope.DeleteLongRunningOperationState(s.Scope.ScaleSetSpec().Name, ServiceName)
s.Scope.DeleteLongRunningOperationState(s.Scope.ScaleSetSpec().Name, scope.ScalesetsServiceName)
return nil
}

Expand All @@ -171,7 +170,7 @@ func (s *Service) Delete(ctx context.Context) error {
}()

// check if there is an ongoing long running operation
future := s.Scope.GetLongRunningOperationState(vmssSpec.Name, ServiceName)
future := s.Scope.GetLongRunningOperationState(vmssSpec.Name, scope.ScalesetsServiceName)
if future != nil {
// if the operation is not complete this will return an error
_, err := s.GetResultIfDone(ctx, future)
Expand All @@ -180,7 +179,7 @@ func (s *Service) Delete(ctx context.Context) error {
}

// ScaleSet has been deleted
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, ServiceName)
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, scope.ScalesetsServiceName)
return nil
}

Expand All @@ -195,7 +194,7 @@ func (s *Service) Delete(ctx context.Context) error {
return errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", vmssSpec.Name, s.Scope.ResourceGroup())
}

future, err = converters.SDKToFuture(sdkFuture, infrav1.DeleteFuture, ServiceName, vmssSpec.Name, s.Scope.ResourceGroup())
future, err = converters.SDKToFuture(sdkFuture, infrav1.DeleteFuture, scope.ScalesetsServiceName, vmssSpec.Name, s.Scope.ResourceGroup())
if err != nil {
return errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", vmssSpec.Name, s.Scope.ResourceGroup())
}
Expand All @@ -209,7 +208,7 @@ func (s *Service) Delete(ctx context.Context) error {
}

// future is either nil, or the result of the future is complete
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, ServiceName)
s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, scope.ScalesetsServiceName)
return nil
}

Expand All @@ -229,7 +228,7 @@ func (s *Service) createVMSS(ctx context.Context) (*infrav1.Future, error) {
return nil, errors.Wrap(err, "cannot create VMSS")
}

future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, ServiceName, spec.Name, s.Scope.ResourceGroup())
future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, scope.ScalesetsServiceName, spec.Name, s.Scope.ResourceGroup())
if err != nil {
return future, errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", spec.Name, s.Scope.ResourceGroup())
}
Expand Down Expand Up @@ -284,7 +283,7 @@ func (s *Service) patchVMSSIfNeeded(ctx context.Context, infraVMSS *azure.VMSS)
return nil, errors.Wrap(err, "failed updating VMSS")
}

future, err := converters.SDKToFuture(sdkFuture, infrav1.PatchFuture, ServiceName, spec.Name, s.Scope.ResourceGroup())
future, err := converters.SDKToFuture(sdkFuture, infrav1.PatchFuture, scope.ScalesetsServiceName, spec.Name, s.Scope.ResourceGroup())
if err != nil {
return nil, errors.Wrapf(err, "failed to delete VMSS %s in resource group %s", spec.Name, s.Scope.ResourceGroup())
}
Expand Down
18 changes: 9 additions & 9 deletions azure/services/scalesets/scalesets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func TestReconcileVMSS(t *testing.T) {
createdVMSS := newDefaultVMSS("VM_SIZE")
instances := newDefaultInstances()
_ = setupDefaultVMSSInProgressOperationDoneExpectations(s, m, createdVMSS, instances)
s.DeleteLongRunningOperationState(defaultSpec.Name, serviceName)
s.DeleteLongRunningOperationState(defaultSpec.Name, scope.ScalesetsServiceName)
},
},
{
Expand All @@ -285,7 +285,7 @@ func TestReconcileVMSS(t *testing.T) {
createdVMSS := newDefaultWindowsVMSS()
instances := newDefaultInstances()
_ = setupDefaultVMSSInProgressOperationDoneExpectations(s, m, createdVMSS, instances)
s.DeleteLongRunningOperationState(defaultSpec.Name, serviceName)
s.DeleteLongRunningOperationState(defaultSpec.Name, scope.ScalesetsServiceName)
},
},
{
Expand Down Expand Up @@ -624,11 +624,11 @@ func TestDeleteVMSS(t *testing.T) {
s.ResourceGroup().AnyTimes().Return("my-existing-rg")
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
future := &infrav1.Future{}
s.GetLongRunningOperationState("my-existing-vmss", serviceName).Return(future)
s.GetLongRunningOperationState("my-existing-vmss", scope.ScalesetsServiceName).Return(future)
m.GetResultIfDone(gomockinternal.AContext(), future).Return(compute.VirtualMachineScaleSet{}, nil)
m.Get(gomockinternal.AContext(), "my-existing-rg", "my-existing-vmss").
Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
s.DeleteLongRunningOperationState("my-existing-vmss", serviceName)
s.DeleteLongRunningOperationState("my-existing-vmss", scope.ScalesetsServiceName)
},
},
{
Expand All @@ -642,7 +642,7 @@ func TestDeleteVMSS(t *testing.T) {
}).AnyTimes()
s.ResourceGroup().AnyTimes().Return(resourceGroup)
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
s.GetLongRunningOperationState(name, serviceName).Return(nil)
s.GetLongRunningOperationState(name, scope.ScalesetsServiceName).Return(nil)
m.DeleteAsync(gomockinternal.AContext(), resourceGroup, name).
Return(nil, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
m.Get(gomockinternal.AContext(), resourceGroup, name).
Expand All @@ -660,7 +660,7 @@ func TestDeleteVMSS(t *testing.T) {
}).AnyTimes()
s.ResourceGroup().AnyTimes().Return(resourceGroup)
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
s.GetLongRunningOperationState(name, serviceName).Return(nil)
s.GetLongRunningOperationState(name, scope.ScalesetsServiceName).Return(nil)
m.DeleteAsync(gomockinternal.AContext(), resourceGroup, name).
Return(nil, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error"))
m.Get(gomockinternal.AContext(), resourceGroup, name).
Expand Down Expand Up @@ -1208,7 +1208,7 @@ func setupDefaultVMSSInProgressOperationDoneExpectations(s *mock_scalesets.MockS
Name: defaultVMSSName,
Data: "",
}
s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(future)
s.GetLongRunningOperationState(defaultVMSSName, scope.ScalesetsServiceName).Return(future)
m.GetResultIfDone(gomockinternal.AContext(), future).Return(createdVMSS, nil).AnyTimes()
m.ListInstances(gomockinternal.AContext(), defaultResourceGroup, defaultVMSSName).Return(instances, nil).AnyTimes()
s.MaxSurge().Return(1, nil)
Expand All @@ -1219,7 +1219,7 @@ func setupDefaultVMSSInProgressOperationDoneExpectations(s *mock_scalesets.MockS

func setupDefaultVMSSStartCreatingExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder, m *mock_scalesets.MockClientMockRecorder) {
setupDefaultVMSSExpectations(s)
s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(nil)
s.GetLongRunningOperationState(defaultVMSSName, scope.ScalesetsServiceName).Return(nil)
m.Get(gomockinternal.AContext(), defaultResourceGroup, defaultVMSSName).
Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
}
Expand Down Expand Up @@ -1285,7 +1285,7 @@ func setupVMSSExpectationsWithoutVMImage(s *mock_scalesets.MockScaleSetScopeMock
func setupDefaultVMSSUpdateExpectations(s *mock_scalesets.MockScaleSetScopeMockRecorder) {
setupUpdateVMSSExpectations(s)
s.SetProviderID(azure.ProviderIDPrefix + "vmss-id")
s.GetLongRunningOperationState(defaultVMSSName, serviceName).Return(nil)
s.GetLongRunningOperationState(defaultVMSSName, scope.ScalesetsServiceName).Return(nil)
s.MaxSurge().Return(1, nil)
s.SetVMSSState(gomock.Any())
}

0 comments on commit 38dab78

Please sign in to comment.