Skip to content

Commit

Permalink
Clean up tests, still need to fix race bug
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jont828
Jont828 committed Jun 9, 2023
1 parent db8afc4 commit 029ac75
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 388 deletions.
12 changes: 5 additions & 7 deletions azure/scope/machinepool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,9 @@ type (

// 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
VMImage *infrav1.Image
VMSKU resourceskus.SKU
availabilitySetSKU resourceskus.SKU
BootstrapData string
VMImage *infrav1.Image
VMSKU resourceskus.SKU
}
)

Expand Down Expand Up @@ -128,8 +127,9 @@ func NewMachinePoolScope(params MachinePoolScopeParams) (*MachinePoolScope, erro
}, nil
}

// InitMachinePoolCache sets cached information about the machine pool to be used in the scope.
func (m *MachinePoolScope) InitMachinePoolCache(ctx context.Context) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "azure.MachineScope.InitMachineCache")
ctx, _, done := tele.StartSpanWithLogger(ctx, "scope.MachinePoolScope.InitMachinePoolCache")
defer done()

if m.cache == nil {
Expand All @@ -155,7 +155,6 @@ func (m *MachinePoolScope) InitMachinePoolCache(ctx context.Context) error {
if err != nil {
return errors.Wrapf(err, "failed to get VM SKU %s in compute api", m.AzureMachinePool.Spec.Template.VMSize)
}

}

return nil
Expand Down Expand Up @@ -189,7 +188,6 @@ func (m *MachinePoolScope) ScaleSetSpec(ctx context.Context) azure.ResourceSpecG
shouldPatchCustomData, err := m.HasBootstrapDataChanges(ctx)
if err != nil {
log.Error(err, "failed to check for bootstrap data changes")
// return nil, errors.Wrap(err, "unable to calculate custom data hash")
}
if shouldPatchCustomData {
log.V(4).Info("custom data changed")
Expand Down
5 changes: 0 additions & 5 deletions azure/services/resourceskus/sku.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,7 @@ const (
func (s SKU) HasCapability(name string) bool {
if s.Capabilities != nil {
for _, capability := range *s.Capabilities {
// TODO: remove
// fmt.Printf("Loop: name is %s, values is %s, comp name is %s\n", *capability.Name, *capability.Value, name)
if capability.Name != nil && *capability.Name == name {
// fmt.Printf("Names match")
// fmt.Printf("Capability value is %s\n", *capability.Value)
// fmt.Printf("CapabilitySupported is %s\n", CapabilitySupported)
if capability.Value != nil && strings.EqualFold(*capability.Value, string(CapabilitySupported)) {
return true
}
Expand Down
132 changes: 6 additions & 126 deletions azure/services/scalesets/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,27 +45,11 @@ type Client interface {
Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error)
}

type (
// AzureClient contains the Azure go-sdk Client.
AzureClient struct {
scalesetvms compute.VirtualMachineScaleSetVMsClient
scalesets compute.VirtualMachineScaleSetsClient
}

genericScaleSetFuture interface {
DoneWithContext(ctx context.Context, sender autorest.Sender) (done bool, err error)
Result(client compute.VirtualMachineScaleSetsClient) (vmss compute.VirtualMachineScaleSet, err error)
}

genericScaleSetFutureImpl struct {
azureautorest.FutureAPI
result func(client compute.VirtualMachineScaleSetsClient) (vmss compute.VirtualMachineScaleSet, err error)
}

deleteResultAdapter struct {
compute.VirtualMachineScaleSetsDeleteFuture
}
)
// AzureClient contains the Azure go-sdk Client.
type AzureClient struct {
scalesetvms compute.VirtualMachineScaleSetVMsClient
scalesets compute.VirtualMachineScaleSetsClient
}

var _ Client = &AzureClient{}

Expand Down Expand Up @@ -173,98 +157,6 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou
return result, nil, err
}

// // UpdateAsync update a VM scale set without waiting for the result of the operation. UpdateAsync sends a PATCH
// // request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing
// // progress of the operation.
// //
// // Parameters:
// //
// // resourceGroupName - the name of the resource group.
// // vmssName - the name of the VM scale set to create or update. parameters - the scale set object.
// func (ac *AzureClient) UpdateAsync(ctx context.Context, resourceGroupName, vmssName string, parameters compute.VirtualMachineScaleSetUpdate) (*infrav1.Future, error) {
// ctx, _, done := tele.StartSpanWithLogger(ctx, "scalesets.AzureClient.UpdateAsync")
// defer done()

// future, err := ac.scalesets.Update(ctx, resourceGroupName, vmssName, parameters)
// if err != nil {
// return nil, errors.Wrapf(err, "failed updating vmss named %q", vmssName)
// }

// ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout)
// defer cancel()

// err = future.WaitForCompletionRef(ctx, ac.scalesets.Client)
// if err != nil {
// // if an error occurs, return the future.
// // this means the long-running operation didn't finish in the specified timeout.
// return converters.SDKToFuture(&future, infrav1.PatchFuture, serviceName, vmssName, resourceGroupName)
// }
// // todo: this returns the result VMSS, we should use it
// _, err = future.Result(ac.scalesets)

// // if the operation completed, return a nil future.
// return nil, err
// }

// // GetResultIfDone fetches the result of a long-running operation future if it is done.
// func (ac *AzureClient) GetResultIfDone(ctx context.Context, future *infrav1.Future) (compute.VirtualMachineScaleSet, error) {
// var genericFuture genericScaleSetFuture
// futureData, err := base64.URLEncoding.DecodeString(future.Data)
// if err != nil {
// return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to base64 decode future data")
// }

// switch future.Type {
// // case infrav1.PatchFuture:
// // var future compute.VirtualMachineScaleSetsUpdateFuture
// // if err := json.Unmarshal(futureData, &future); err != nil {
// // return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data")
// // }

// // genericFuture = &genericScaleSetFutureImpl{
// // FutureAPI: &future,
// // result: future.Result,
// // }
// // case infrav1.PutFuture:
// // var future compute.VirtualMachineScaleSetsCreateOrUpdateFuture
// // if err := json.Unmarshal(futureData, &future); err != nil {
// // return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data")
// // }

// // genericFuture = &genericScaleSetFutureImpl{
// // FutureAPI: &future,
// // result: future.Result,
// // }
// case infrav1.DeleteFuture:
// var future compute.VirtualMachineScaleSetsDeleteFuture
// if err := json.Unmarshal(futureData, &future); err != nil {
// return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data")
// }

// genericFuture = &deleteResultAdapter{
// VirtualMachineScaleSetsDeleteFuture: future,
// }
// default:
// return compute.VirtualMachineScaleSet{}, errors.Errorf("unknown future type %q", future.Type)
// }

// done, err := genericFuture.DoneWithContext(ctx, ac.scalesets)
// if err != nil {
// return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed checking if the operation was complete")
// }

// if !done {
// return compute.VirtualMachineScaleSet{}, azure.WithTransientError(azure.NewOperationNotDoneError(future), 15*time.Second)
// }

// vmss, err := genericFuture.Result(ac.scalesets)
// if err != nil {
// return vmss, errors.Wrap(err, "failed fetching the result of operation for vmss")
// }

// return vmss, nil
// }

// UpdateInstances update instances of a VM scale set.
func (ac *AzureClient) UpdateInstances(ctx context.Context, resourceGroupName, vmssName string, instanceIDs []string) error {
ctx, _, done := tele.StartSpanWithLogger(ctx, "scalesets.AzureClient.UpdateInstances")
Expand All @@ -291,7 +183,7 @@ func (ac *AzureClient) UpdateInstances(ctx context.Context, resourceGroupName, v
//
// Parameters:
//
// spec - The ResourceSpecGetter containing used for name and resource group of the virutal machine scale set.
// spec - The ResourceSpecGetter containing used for name and resource group of the virtual machine scale set.
func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) {
ctx, _, done := tele.StartSpanWithLogger(ctx, "scalesets.AzureClient.DeleteAsync")
defer done()
Expand Down Expand Up @@ -368,15 +260,3 @@ func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAP
return nil, errors.Errorf("unknown future type %q", futureType)
}
}

// // Result wraps the delete result so that we can treat it generically. The only thing we care about is if the delete
// // was successful. If it wasn't, an error will be returned.
// func (da *deleteResultAdapter) Result(client compute.VirtualMachineScaleSetsClient) (compute.VirtualMachineScaleSet, error) {
// _, err := da.VirtualMachineScaleSetsDeleteFuture.Result(client)
// return compute.VirtualMachineScaleSet{}, err
// }

// // Result returns the Result so that we can treat it generically.
// func (g *genericScaleSetFutureImpl) Result(client compute.VirtualMachineScaleSetsClient) (compute.VirtualMachineScaleSet, error) {
// return g.result(client)
// }
54 changes: 0 additions & 54 deletions azure/services/scalesets/mock_scalesets/client_mock.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 0 additions & 23 deletions azure/services/scalesets/scalesets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,6 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {
if !ok {
return errors.Errorf("%T is not a compute.VirtualMachineScaleSet", result)
}
// vmssInstances, err := s.Client.ListInstances(ctx, spec)
// if err != nil {
// return errors.Wrap(err, "failed to list instances")
// }

fetchedVMSS := converters.SDKToVMSS(vmss, vmssInstances)
// Note: converters.SDKToVMSS() will never return nil but we should check anyway
Expand Down Expand Up @@ -195,7 +191,6 @@ func (s *Service) Delete(ctx context.Context) error {
s.Scope.UpdateDeleteStatus(infrav1.BootstrapSucceededCondition, serviceName, err)

return err

}

func (s *Service) validateSpec(ctx context.Context) error {
Expand Down Expand Up @@ -328,24 +323,6 @@ func (s *Service) getVirtualMachineScaleSet(ctx context.Context, spec azure.Reso
return converters.SDKToVMSS(vmss, vmssInstances), nil
}

// getVirtualMachineScaleSetIfDone gets a Virtual Machine Scale Set and its instances from Azure if the future is completed.
// func (s *Service) getVirtualMachineScaleSetIfDone(ctx context.Context, future *infrav1.Future) (*azure.VMSS, error) {
// ctx, _, done := tele.StartSpanWithLogger(ctx, "scalesets.Service.getVirtualMachineScaleSetIfDone")
// defer done()

// vmss, err := s.GetResultIfDone(ctx, future)
// if err != nil {
// return nil, errors.Wrap(err, "failed to get result from future")
// }

// vmssInstances, err := s.Client.ListInstances(ctx, future.ResourceGroup, future.Name)
// if err != nil {
// return nil, errors.Wrap(err, "failed to list instances")
// }

// return converters.SDKToVMSS(vmss, vmssInstances), nil
// }

// IsManaged returns always returns true as CAPZ does not support BYO scale set.
func (s *Service) IsManaged(ctx context.Context) (bool, error) {
return true, nil
Expand Down
Loading

0 comments on commit 029ac75

Please sign in to comment.