Skip to content

Commit

Permalink
Make scaleset service async
Browse files Browse the repository at this point in the history
  • Loading branch information
Jont828 committed Aug 14, 2023
1 parent 8893daa commit 236428f
Show file tree
Hide file tree
Showing 14 changed files with 1,750 additions and 1,945 deletions.
4 changes: 2 additions & 2 deletions azure/converters/vmss.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ const (
)

// SDKToVMSS converts an Azure SDK VirtualMachineScaleSet to the AzureMachinePool type.
func SDKToVMSS(sdkvmss compute.VirtualMachineScaleSet, sdkinstances []compute.VirtualMachineScaleSetVM) *azure.VMSS {
vmss := &azure.VMSS{
func SDKToVMSS(sdkvmss compute.VirtualMachineScaleSet, sdkinstances []compute.VirtualMachineScaleSetVM) azure.VMSS {
vmss := azure.VMSS{
ID: ptr.Deref(sdkvmss.ID, ""),
Name: ptr.Deref(sdkvmss.Name, ""),
State: infrav1.ProvisioningState(ptr.Deref(sdkvmss.ProvisioningState, "")),
Expand Down
6 changes: 3 additions & 3 deletions azure/converters/vmss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func Test_SDKToVMSS(t *testing.T) {
cases := []struct {
Name string
SubjectFactory func(*gomega.GomegaWithT) (compute.VirtualMachineScaleSet, []compute.VirtualMachineScaleSetVM)
Expect func(*gomega.GomegaWithT, *azure.VMSS)
Expect func(*gomega.GomegaWithT, azure.VMSS)
}{
{
Name: "ShouldPopulateWithData",
Expand Down Expand Up @@ -84,7 +84,7 @@ func Test_SDKToVMSS(t *testing.T) {
},
}
},
Expect: func(g *gomega.GomegaWithT, actual *azure.VMSS) {
Expect: func(g *gomega.GomegaWithT, actual azure.VMSS) {
expected := azure.VMSS{
ID: "vmssID",
Name: "vmssName",
Expand All @@ -107,7 +107,7 @@ func Test_SDKToVMSS(t *testing.T) {
State: "Succeeded",
}
}
g.Expect(actual).To(gomega.Equal(&expected))
g.Expect(actual).To(gomega.Equal(expected))
},
},
}
Expand Down
68 changes: 55 additions & 13 deletions azure/scope/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type (
MachinePool *expv1.MachinePool
AzureMachinePool *infrav1exp.AzureMachinePool
ClusterScope azure.ClusterScoper
Caches *MachinePoolCache
Cache *MachinePoolCache
}

// MachinePoolScope defines a scope defined around a machine pool and its cluster.
Expand All @@ -77,16 +77,20 @@ type (
cache *MachinePoolCache
}

// MachinePoolCache stores common machine pool information so we don't have to hit the API multiple times within the same reconcile loop.
MachinePoolCache struct {
VMSKU resourceskus.SKU
}

// NodeStatus represents the status of a Kubernetes node.
NodeStatus struct {
Ready bool
Version string
}

// MachinePoolCache stores common machine pool information so we don't have to hit the API multiple times within the same reconcile loop.
MachinePoolCache struct {
BootstrapData string
HasBootstrapDataChanges bool
VMImage *infrav1.Image
VMSKU resourceskus.SKU
MaxSurge int
}
)

// NewMachinePoolScope creates a new MachinePoolScope from the supplied parameters.
Expand Down Expand Up @@ -133,6 +137,27 @@ func (m *MachinePoolScope) InitMachinePoolCache(ctx context.Context) error {
var err error
m.cache = &MachinePoolCache{}

m.cache.BootstrapData, err = m.GetBootstrapData(ctx)
if err != nil {
return err
}

m.cache.HasBootstrapDataChanges, err = m.HasBootstrapDataChanges(ctx)
if err != nil {
return err
}

m.cache.VMImage, err = m.GetVMImage(ctx)
if err != nil {
return err
}
m.SaveVMImageToStatus(m.cache.VMImage)

m.cache.MaxSurge, err = m.MaxSurge()
if err != nil {
return err
}

skuCache, err := resourceskus.GetCache(m, m.Location())
if err != nil {
return err
Expand All @@ -148,9 +173,19 @@ func (m *MachinePoolScope) InitMachinePoolCache(ctx context.Context) error {
}

// ScaleSetSpec returns the scale set spec.
func (m *MachinePoolScope) ScaleSetSpec() azure.ScaleSetSpec {
return azure.ScaleSetSpec{
func (m *MachinePoolScope) ScaleSetSpec(ctx context.Context) azure.ResourceSpecGetter {
ctx, log, done := tele.StartSpanWithLogger(ctx, "scope.MachinePoolScope.ScaleSetSpec")
defer done()

shouldPatchCustomData := false
if m.HasReplicasExternallyManaged(ctx) {
shouldPatchCustomData = m.cache.HasBootstrapDataChanges
log.V(4).Info("has bootstrap data changed?", "shouldPatchCustomData", "shouldPatchCustomData")
}

return &scalesets.ScaleSetSpec{
Name: m.Name(),
ResourceGroup: m.ResourceGroup(),
Size: m.AzureMachinePool.Spec.Template.VMSize,
Capacity: int64(ptr.Deref[int32](m.MachinePool.Spec.Replicas, 0)),
SSHKeyData: m.AzureMachinePool.Spec.Template.SSHPublicKey,
Expand All @@ -172,6 +207,16 @@ func (m *MachinePoolScope) ScaleSetSpec() azure.ScaleSetSpec {
NetworkInterfaces: m.AzureMachinePool.Spec.Template.NetworkInterfaces,
IPv6Enabled: m.IsIPv6Enabled(),
OrchestrationMode: m.AzureMachinePool.Spec.OrchestrationMode,
Location: m.AzureMachinePool.Spec.Location,
SubscriptionID: m.SubscriptionID(),
VMSSExtensionSpecs: m.VMSSExtensionSpecs(),
HasReplicasExternallyManaged: m.HasReplicasExternallyManaged(ctx),
ClusterName: m.ClusterName(),
AdditionalTags: m.AzureMachinePool.Spec.AdditionalTags,
SKU: m.cache.VMSKU,
VMImage: m.cache.VMImage,
BootstrapData: m.cache.BootstrapData,
ShouldPatchCustomData: shouldPatchCustomData,
}
}

Expand Down Expand Up @@ -614,11 +659,8 @@ func (m *MachinePoolScope) GetBootstrapData(ctx context.Context) (string, error)
}

// calculateBootstrapDataHash calculates the sha256 hash of the bootstrap data.
func (m *MachinePoolScope) calculateBootstrapDataHash(ctx context.Context) (string, error) {
bootstrapData, err := m.GetBootstrapData(ctx)
if err != nil {
return "", err
}
func (m *MachinePoolScope) calculateBootstrapDataHash(_ context.Context) (string, error) {
bootstrapData := m.cache.BootstrapData
h := sha256.New()
n, err := io.WriteString(h, bootstrapData)
if err != nil || n == 0 {
Expand Down
18 changes: 14 additions & 4 deletions azure/services/roleassignments/roleassignments.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type Service struct {
Scope RoleAssignmentScope
virtualMachinesGetter async.Getter
async.Reconciler
virtualMachineScaleSetClient scalesets.Client
virtualMachineScaleSetGetter async.Getter
}

// New creates a new service.
Expand All @@ -56,7 +56,7 @@ func New(scope RoleAssignmentScope) *Service {
return &Service{
Scope: scope,
virtualMachinesGetter: virtualmachines.NewClient(scope),
virtualMachineScaleSetClient: scalesets.NewClient(scope),
virtualMachineScaleSetGetter: scalesets.NewClient(scope),
Reconciler: async.New(scope, client, client),
}
}
Expand Down Expand Up @@ -141,10 +141,20 @@ func (s *Service) getVMSSPrincipalID(ctx context.Context) (*string, error) {
ctx, log, done := tele.StartSpanWithLogger(ctx, "roleassignments.Service.getVMPrincipalID")
defer done()
log.V(2).Info("fetching principal ID for VMSS")
resultVMSS, err := s.virtualMachineScaleSetClient.Get(ctx, s.Scope.ResourceGroup(), s.Scope.Name())
spec := &scalesets.ScaleSetSpec{
Name: s.Scope.Name(),
ResourceGroup: s.Scope.ResourceGroup(),
}

resultVMSSIface, err := s.virtualMachineScaleSetGetter.Get(ctx, spec)
if err != nil {
return nil, errors.Wrap(err, "failed to get principal ID for VMSS")
return nil, errors.Wrap(err, "failed to get VMSS")
}
resultVMSS, ok := resultVMSSIface.(compute.VirtualMachineScaleSet)
if !ok {
return nil, errors.Errorf("%T is not a compute.VirtualMachineScaleSet", resultVMSSIface)
}

return resultVMSS.Identity.PrincipalID, nil
}

Expand Down
13 changes: 9 additions & 4 deletions azure/services/roleassignments/roleassignments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments/mock_roleassignments"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets/mock_scalesets"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
Expand All @@ -55,6 +56,10 @@ var (

emptyRoleAssignmentSpec = RoleAssignmentSpec{}
fakeRoleAssignmentSpecs = []azure.ResourceSpecGetter{&fakeRoleAssignment1, &fakeRoleAssignment2, &emptyRoleAssignmentSpec}
fakeVMSSSpec = scalesets.ScaleSetSpec{
Name: "test-vmss",
ResourceGroup: "my-rg",
}
)

func TestReconcileRoleAssignmentsVM(t *testing.T) {
Expand Down Expand Up @@ -169,7 +174,7 @@ func TestReconcileRoleAssignmentsVMSS(t *testing.T) {
s.RoleAssignmentResourceType().Return(azure.VirtualMachineScaleSet)
s.ResourceGroup().Return("my-rg")
s.Name().Return("test-vmss")
mvmss.Get(gomockinternal.AContext(), "my-rg", "test-vmss").Return(compute.VirtualMachineScaleSet{
mvmss.Get(gomockinternal.AContext(), &fakeVMSSSpec).Return(compute.VirtualMachineScaleSet{
Identity: &compute.VirtualMachineScaleSetIdentity{
PrincipalID: &fakePrincipalID,
},
Expand All @@ -187,7 +192,7 @@ func TestReconcileRoleAssignmentsVMSS(t *testing.T) {
s.ResourceGroup().Return("my-rg")
s.Name().Return("test-vmss")
s.HasSystemAssignedIdentity().Return(true)
mvmss.Get(gomockinternal.AContext(), "my-rg", "test-vmss").Return(compute.VirtualMachineScaleSet{},
mvmss.Get(gomockinternal.AContext(), &fakeVMSSSpec).Return(compute.VirtualMachineScaleSet{},
autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: http.StatusInternalServerError}, "Internal Server Error"))
},
},
Expand All @@ -202,7 +207,7 @@ func TestReconcileRoleAssignmentsVMSS(t *testing.T) {
s.RoleAssignmentResourceType().Return(azure.VirtualMachineScaleSet)
s.ResourceGroup().Return("my-rg")
s.Name().Return("test-vmss")
mvmss.Get(gomockinternal.AContext(), "my-rg", "test-vmss").Return(compute.VirtualMachineScaleSet{
mvmss.Get(gomockinternal.AContext(), &fakeVMSSSpec).Return(compute.VirtualMachineScaleSet{
Identity: &compute.VirtualMachineScaleSetIdentity{
PrincipalID: &fakePrincipalID,
},
Expand All @@ -229,7 +234,7 @@ func TestReconcileRoleAssignmentsVMSS(t *testing.T) {
s := &Service{
Scope: scopeMock,
Reconciler: asyncMock,
virtualMachineScaleSetClient: vmMock,
virtualMachineScaleSetGetter: vmMock,
}

err := s.Reconcile(context.TODO())
Expand Down
Loading

0 comments on commit 236428f

Please sign in to comment.