From 3bb54ef2d88447a1edafa7c2425ec64d732071bd Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Fri, 9 Jul 2021 16:39:26 -0700 Subject: [PATCH] Add LongRunningOperationStates types and conditions --- api/v1alpha3/azurecluster_conversion.go | 12 ++ api/v1alpha3/zz_generated.conversion.go | 17 +- api/v1alpha4/azurecluster_types.go | 15 ++ api/v1alpha4/azuremachine_types.go | 15 ++ api/v1alpha4/conditions_consts.go | 41 +++++ api/v1alpha4/types.go | 20 ++- api/v1alpha4/zz_generated.deepcopy.go | 29 +++ azure/converters/futures.go | 62 +++++++ azure/defaults.go | 7 + azure/interfaces.go | 9 + azure/mocks/service_mock.go | 86 +++++++++ azure/scope/cluster.go | 28 +++ azure/scope/machine.go | 27 +++ azure/scope/machinepool.go | 29 ++- azure/scope/machinepoolmachine.go | 29 ++- azure/scope/managedcontrolplane.go | 27 +++ azure/services/futures/futures.go | 41 +++++ azure/services/scalesets/client.go | 170 +++++------------- .../scalesets/mock_scalesets/client_mock.go | 70 +------- .../mock_scalesets/scalesets_mock.go | 53 +++++- azure/services/scalesets/scalesets.go | 53 +++--- azure/services/scalesets/scalesets_test.go | 2 +- azure/services/scalesetvms/client.go | 16 +- .../mock_scalesetvms/scalesetvms_mock.go | 53 +++++- azure/services/scalesetvms/scalesetvms.go | 14 +- .../virtualmachines/virtualmachines.go | 9 +- ...ucture.cluster.x-k8s.io_azureclusters.yaml | 28 +++ ...ter.x-k8s.io_azuremachinepoolmachines.yaml | 1 + ...re.cluster.x-k8s.io_azuremachinepools.yaml | 49 ++--- ...ucture.cluster.x-k8s.io_azuremachines.yaml | 28 +++ ...er.x-k8s.io_azuremanagedcontrolplanes.yaml | 28 +++ .../v1alpha3/azuremachinepool_conversion.go | 4 + .../azuremanagedcontrolplane_conversion.go | 5 + exp/api/v1alpha3/zz_generated.conversion.go | 35 ++-- exp/api/v1alpha4/azuremachinepool_types.go | 14 +- .../v1alpha4/azuremachinepoolmachine_types.go | 14 +- .../azuremanagedcontrolplane_types.go | 15 ++ exp/api/v1alpha4/zz_generated.deepcopy.go | 23 ++- test/e2e/aks.go | 2 + test/e2e/azure_machinepool_drain.go | 8 +- test/e2e/retry.go | 3 +- util/futures/getter.go | 52 ++++++ util/futures/getter_test.go | 75 ++++++++ util/futures/setter.go | 70 ++++++++ util/futures/setter_test.go | 116 ++++++++++++ util/reconciler/defaults.go | 8 +- 46 files changed, 1163 insertions(+), 349 deletions(-) create mode 100644 azure/converters/futures.go create mode 100644 azure/services/futures/futures.go create mode 100644 util/futures/getter.go create mode 100644 util/futures/getter_test.go create mode 100644 util/futures/setter.go create mode 100644 util/futures/setter_test.go diff --git a/api/v1alpha3/azurecluster_conversion.go b/api/v1alpha3/azurecluster_conversion.go index 5ee6d2507b6..60103e5c3f9 100644 --- a/api/v1alpha3/azurecluster_conversion.go +++ b/api/v1alpha3/azurecluster_conversion.go @@ -304,3 +304,15 @@ func Convert_v1alpha3_VnetSpec_To_v1alpha4_VnetSpec(in *VnetSpec, out *infrav1al func Convert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in *infrav1alpha4.LoadBalancerSpec, out *LoadBalancerSpec, s apiconversion.Scope) error { return autoConvert_v1alpha4_LoadBalancerSpec_To_v1alpha3_LoadBalancerSpec(in, out, s) } + +// Convert_v1alpha3_Future_To_v1alpha4_Future is an autogenerated conversion function. +func Convert_v1alpha3_Future_To_v1alpha4_Future(in *Future, out *infrav1alpha4.Future, s apiconversion.Scope) error { + out.Data = in.FutureData + return autoConvert_v1alpha3_Future_To_v1alpha4_Future(in, out, s) +} + +// Convert_v1alpha4_Future_To_v1alpha3_Future is an autogenerated conversion function. +func Convert_v1alpha4_Future_To_v1alpha3_Future(in *infrav1alpha4.Future, out *Future, s apiconversion.Scope) error { + out.FutureData = in.Data + return autoConvert_v1alpha4_Future_To_v1alpha3_Future(in, out, s) +} diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index 0fdef669975..650c437ba9c 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -707,6 +707,7 @@ func autoConvert_v1alpha4_AzureClusterStatus_To_v1alpha3_AzureClusterStatus(in * out.FailureDomains = *(*apiv1alpha3.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) out.Ready = in.Ready out.Conditions = *(*apiv1alpha3.Conditions)(unsafe.Pointer(&in.Conditions)) + // WARNING: in.LongRunningOperationStates requires manual conversion: does not exist in peer-type return nil } @@ -884,6 +885,7 @@ func autoConvert_v1alpha4_AzureMachineStatus_To_v1alpha3_AzureMachineStatus(in * out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) out.Conditions = *(*apiv1alpha3.Conditions)(unsafe.Pointer(&in.Conditions)) + // WARNING: in.LongRunningOperationStates requires manual conversion: does not exist in peer-type return nil } @@ -1199,28 +1201,19 @@ func autoConvert_v1alpha3_Future_To_v1alpha4_Future(in *Future, out *v1alpha4.Fu out.Type = in.Type out.ResourceGroup = in.ResourceGroup out.Name = in.Name - out.FutureData = in.FutureData + // WARNING: in.FutureData requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha3_Future_To_v1alpha4_Future is an autogenerated conversion function. -func Convert_v1alpha3_Future_To_v1alpha4_Future(in *Future, out *v1alpha4.Future, s conversion.Scope) error { - return autoConvert_v1alpha3_Future_To_v1alpha4_Future(in, out, s) -} - func autoConvert_v1alpha4_Future_To_v1alpha3_Future(in *v1alpha4.Future, out *Future, s conversion.Scope) error { out.Type = in.Type out.ResourceGroup = in.ResourceGroup + // WARNING: in.ServiceName requires manual conversion: does not exist in peer-type out.Name = in.Name - out.FutureData = in.FutureData + // WARNING: in.Data requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha4_Future_To_v1alpha3_Future is an autogenerated conversion function. -func Convert_v1alpha4_Future_To_v1alpha3_Future(in *v1alpha4.Future, out *Future, s conversion.Scope) error { - return autoConvert_v1alpha4_Future_To_v1alpha3_Future(in, out, s) -} - func autoConvert_v1alpha3_Image_To_v1alpha4_Image(in *Image, out *v1alpha4.Image, s conversion.Scope) error { out.ID = (*string)(unsafe.Pointer(in.ID)) if in.SharedGallery != nil { diff --git a/api/v1alpha4/azurecluster_types.go b/api/v1alpha4/azurecluster_types.go index f821699ce52..1f11aacf031 100644 --- a/api/v1alpha4/azurecluster_types.go +++ b/api/v1alpha4/azurecluster_types.go @@ -96,6 +96,11 @@ type AzureClusterStatus struct { // Conditions defines current service state of the AzureCluster. // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` + + // LongRunningOperationStates saves the states for Azure long-running operations so they can be continued on the + // next reconciliation loop. + // +optional + LongRunningOperationStates Futures `json:"longRunningOperationStates,omitempty"` } // +kubebuilder:object:root=true @@ -137,6 +142,16 @@ func (c *AzureCluster) SetConditions(conditions clusterv1.Conditions) { c.Status.Conditions = conditions } +// GetFutures returns the list of long running operation states for an AzureCluster API object. +func (c *AzureCluster) GetFutures() Futures { + return c.Status.LongRunningOperationStates +} + +// SetFutures will set the given long running operation states on an AzureCluster object. +func (c *AzureCluster) SetFutures(futures Futures) { + c.Status.LongRunningOperationStates = futures +} + func init() { SchemeBuilder.Register(&AzureCluster{}, &AzureClusterList{}) } diff --git a/api/v1alpha4/azuremachine_types.go b/api/v1alpha4/azuremachine_types.go index 5a093044c36..3b2dd02186b 100644 --- a/api/v1alpha4/azuremachine_types.go +++ b/api/v1alpha4/azuremachine_types.go @@ -175,6 +175,11 @@ type AzureMachineStatus struct { // Conditions defines current service state of the AzureMachine. // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` + + // LongRunningOperationStates saves the states for Azure long-running operations so they can be continued on the + // next reconciliation loop. + // +optional + LongRunningOperationStates Futures `json:"longRunningOperationStates,omitempty"` } // +kubebuilder:object:root=true @@ -216,6 +221,16 @@ func (m *AzureMachine) SetConditions(conditions clusterv1.Conditions) { m.Status.Conditions = conditions } +// GetFutures returns the list of long running operation states for an AzureMachine API object. +func (m *AzureMachine) GetFutures() Futures { + return m.Status.LongRunningOperationStates +} + +// SetFutures will set the given long running operation states on an AzureMachine object. +func (m *AzureMachine) SetFutures(futures Futures) { + m.Status.LongRunningOperationStates = futures +} + func init() { SchemeBuilder.Register(&AzureMachine{}, &AzureMachineList{}) } diff --git a/api/v1alpha4/conditions_consts.go b/api/v1alpha4/conditions_consts.go index fa888dbeac2..90045df2a41 100644 --- a/api/v1alpha4/conditions_consts.go +++ b/api/v1alpha4/conditions_consts.go @@ -75,3 +75,44 @@ const ( // ScaleSetModelOutOfDateReason describes the machine pool model being out of date. ScaleSetModelOutOfDateReason = "ScaleSetModelOutOfDate" ) + +// Azure Services Conditions and Reasons. +const ( + // ResourceGroupReadyCondition means the resource group exists and is ready to be used. + ResourceGroupReadyCondition clusterv1.ConditionType = "ResourceGroupReady" + // VNetReadyCondition means the virtual network exists and is ready to be used. + VNetReadyCondition clusterv1.ConditionType = "VNetReady" + // SecurityGroupsReadyCondition means the security groups exist and are ready to be used. + SecurityGroupsReadyCondition clusterv1.ConditionType = "SecurityGroupsReady" + // RouteTablesReadyCondition means the route tables exist and are ready to be used. + RouteTablesReadyCondition clusterv1.ConditionType = "RouteTablesReady" + // PublicIPsReadyCondition means the public IPs exist and are ready to be used. + PublicIPsReadyCondition clusterv1.ConditionType = "PublicIPsReady" + // NATGatewaysReadyCondition means the NAT gateways exist and are ready to be used. + NATGatewaysReadyCondition clusterv1.ConditionType = "NATGatewaysReady" + // SubnetsReadyCondition means the subnets exist and are ready to be used. + SubnetsReadyCondition clusterv1.ConditionType = "SubnetsReady" + // LoadBalancersReadyCondition means the load balancers exist and are ready to be used. + LoadBalancersReadyCondition clusterv1.ConditionType = "LoadBalancersReady" + // PrivateDNSReadyCondition means the private DNS exists and is ready to be used. + PrivateDNSReadyCondition clusterv1.ConditionType = "PrivateDNSReady" + // BastionHostReadyCondition means the bastion host exists and is ready to be used. + BastionHostReadyCondition clusterv1.ConditionType = "BastionHostReady" + // InboundNATRulesReadyCondition means the inbound NAT rules exist and are ready to be used. + InboundNATRulesReadyCondition clusterv1.ConditionType = "InboundNATRulesReady" + // AvailabilitySetReadyCondition means the availability set exists and is ready to be used. + AvailabilitySetReadyCondition clusterv1.ConditionType = "AvailabilitySetReady" + // RoleAssignmentReadyCondition means the role assignment exists and is ready to be used. + RoleAssignmentReadyCondition clusterv1.ConditionType = "RoleAssignmentReady" + + // CreatingReason means the resource is being created. + CreatingReason = "Creating" + // FailedReason means the resource failed to be created. + FailedReason = "Failed" + // DeletingReason means the resource is being deleted. + DeletingReason = "Deleting" + // DeletedReason means the resource was deleted. + DeletedReason = "Deleted" + // DeletionFailedReason means the resource failed to be deleted. + DeletionFailedReason = "DeletionFailed" +) diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index 4475a9169b6..58c607a721d 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -29,21 +29,27 @@ const ( Node string = "node" ) +type Futures []Future + // Future contains the data needed for an Azure long-running operation to continue across reconcile loops. type Future struct { - // Type describes the type of future, update, create, delete, etc + // Type describes the type of future, such update, create, delete, etc. Type string `json:"type"` - // ResourceGroup is the Azure resource group for the resource + // ResourceGroup is the Azure resource group for the resource. // +optional ResourceGroup string `json:"resourceGroup,omitempty"` - // Name is the name of the Azure resource - // +optional - Name string `json:"name,omitempty"` + // ServiceName is the name of the Azure service. + // Together with the name of the resource, this forms the unique identifier for the future. + ServiceName string `json:"serviceName"` + + // Name is the name of the Azure resource. + // Together with the service name, this forms the unique identifier for the future. + Name string `json:"name"` - // FutureData is the base64 url encoded json Azure AutoRest Future - FutureData string `json:"futureData,omitempty"` + // Data is the base64 url encoded json Azure AutoRest Future. + Data string `json:"data,omitempty"` } // NetworkSpec specifies what the Azure networking resources should look like. diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index d7899ada440..030a972ad74 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -298,6 +298,11 @@ func (in *AzureClusterStatus) DeepCopyInto(out *AzureClusterStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.LongRunningOperationStates != nil { + in, out := &in.LongRunningOperationStates, &out.LongRunningOperationStates + *out = make(Futures, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureClusterStatus. @@ -464,6 +469,11 @@ func (in *AzureMachineStatus) DeepCopyInto(out *AzureMachineStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.LongRunningOperationStates != nil { + in, out := &in.LongRunningOperationStates, &out.LongRunningOperationStates + *out = make(Futures, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureMachineStatus. @@ -801,6 +811,25 @@ func (in *Future) DeepCopy() *Future { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in Futures) DeepCopyInto(out *Futures) { + { + in := &in + *out = make(Futures, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Futures. +func (in Futures) DeepCopy() Futures { + if in == nil { + return nil + } + out := new(Futures) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Image) DeepCopyInto(out *Image) { *out = *in diff --git a/azure/converters/futures.go b/azure/converters/futures.go new file mode 100644 index 00000000000..29e87cbe84b --- /dev/null +++ b/azure/converters/futures.go @@ -0,0 +1,62 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package converters + +import ( + "encoding/base64" + "encoding/json" + + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" +) + +const ( + // PatchFuture is a future that was derived from a PATCH request. + PatchFuture string = "PATCH" + // PutFuture is a future that was derived from a PUT request. + PutFuture string = "PUT" + // DeleteFuture is a future that was derived from a DELETE request. + DeleteFuture string = "DELETE" +) + +func SDKToFuture(future azureautorest.FutureAPI, futureType, service, resourceName, rgName string) (*infrav1.Future, error) { + jsonData, err := future.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal async future") + } + + return &infrav1.Future{ + Type: futureType, + ResourceGroup: rgName, + ServiceName: service, + Name: resourceName, + Data: base64.URLEncoding.EncodeToString(jsonData), + }, nil +} + +func FutureToSDK(future *infrav1.Future) (azureautorest.FutureAPI, error) { + futureData, err := base64.URLEncoding.DecodeString(future.Data) + if err != nil { + return nil, errors.Wrap(err, "failed to base64 decode future data") + } + var genericFuture azureautorest.FutureAPI + if err := json.Unmarshal(futureData, &genericFuture); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + return genericFuture, nil +} diff --git a/azure/defaults.go b/azure/defaults.go index fb2d439e8c7..8643212cd7e 100644 --- a/azure/defaults.go +++ b/azure/defaults.go @@ -328,6 +328,13 @@ func SetAutoRestClientDefaults(c *autorest.Client, auth autorest.Authorizer) { // The wrapped Sender should set the x-ms-correlation-request-id on the given // request, then pass the new request to the underlying Sender. c.Sender = autorest.DecorateSender(c.Sender, msCorrelationIDSendDecorator) + // The default number of retries is 3. This means the client will attempt to retry operation results like resource + // conflicts (HTTP 409). For a reconciling controller, this is undesirable behavior since if the controller runs + // into an error reconciling, the controller would be better off to end with an error and try again later. + // + // Unfortunately, the naming of this field is a bit misleading. This is not actually "retry attempts", it actually + // is attempts. Setting this to a value of 0 will cause a panic in Go AutoRest. + c.RetryAttempts = 1 AutoRestClientAppendUserAgent(c, UserAgent()) } diff --git a/azure/interfaces.go b/azure/interfaces.go index 6001f0c9c85..12b03e25e6a 100644 --- a/azure/interfaces.go +++ b/azure/interfaces.go @@ -22,6 +22,7 @@ import ( "github.com/Azure/go-autorest/autorest" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // Reconciler is a generic interface used by components offering a type of service. @@ -80,6 +81,14 @@ type ClusterDescriber interface { CloudProviderConfigOverrides() *infrav1.CloudProviderConfigOverrides } +type AsyncStatusUpdater interface { + SetLongRunningOperationState(*infrav1.Future) + GetLongRunningOperationState(string, string) *infrav1.Future + DeleteLongRunningOperationState(string, string) + SetConditionTrue(clusterv1.ConditionType) + SetConditionFalse(clusterv1.ConditionType, string, clusterv1.ConditionSeverity) +} + // ClusterScoper combines the ClusterDescriber and NetworkDescriber interfaces. type ClusterScoper interface { ClusterDescriber diff --git a/azure/mocks/service_mock.go b/azure/mocks/service_mock.go index fec8422ec57..1e5e90abf8d 100644 --- a/azure/mocks/service_mock.go +++ b/azure/mocks/service_mock.go @@ -27,6 +27,7 @@ import ( autorest "github.com/Azure/go-autorest/autorest" gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // MockReconciler is a mock of Reconciler interface. @@ -731,6 +732,91 @@ func (mr *MockClusterDescriberMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockClusterDescriber)(nil).TenantID)) } +// MockAsyncStatusUpdater is a mock of AsyncStatusUpdater interface. +type MockAsyncStatusUpdater struct { + ctrl *gomock.Controller + recorder *MockAsyncStatusUpdaterMockRecorder +} + +// MockAsyncStatusUpdaterMockRecorder is the mock recorder for MockAsyncStatusUpdater. +type MockAsyncStatusUpdaterMockRecorder struct { + mock *MockAsyncStatusUpdater +} + +// NewMockAsyncStatusUpdater creates a new mock instance. +func NewMockAsyncStatusUpdater(ctrl *gomock.Controller) *MockAsyncStatusUpdater { + mock := &MockAsyncStatusUpdater{ctrl: ctrl} + mock.recorder = &MockAsyncStatusUpdaterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAsyncStatusUpdater) EXPECT() *MockAsyncStatusUpdaterMockRecorder { + return m.recorder +} + +// DeleteLongRunningOperationState mocks base method. +func (m *MockAsyncStatusUpdater) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockAsyncStatusUpdaterMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + +// GetLongRunningOperationState mocks base method. +func (m *MockAsyncStatusUpdater) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1alpha4.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockAsyncStatusUpdaterMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).GetLongRunningOperationState), arg0, arg1) +} + +// SetConditionFalse mocks base method. +func (m *MockAsyncStatusUpdater) SetConditionFalse(arg0 v1alpha40.ConditionType, arg1 string, arg2 v1alpha40.ConditionSeverity) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetConditionFalse", arg0, arg1, arg2) +} + +// SetConditionFalse indicates an expected call of SetConditionFalse. +func (mr *MockAsyncStatusUpdaterMockRecorder) SetConditionFalse(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionFalse", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetConditionFalse), arg0, arg1, arg2) +} + +// SetConditionTrue mocks base method. +func (m *MockAsyncStatusUpdater) SetConditionTrue(arg0 v1alpha40.ConditionType) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetConditionTrue", arg0) +} + +// SetConditionTrue indicates an expected call of SetConditionTrue. +func (mr *MockAsyncStatusUpdaterMockRecorder) SetConditionTrue(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionTrue", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetConditionTrue), arg0) +} + +// SetLongRunningOperationState mocks base method. +func (m *MockAsyncStatusUpdater) SetLongRunningOperationState(arg0 *v1alpha4.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockAsyncStatusUpdaterMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockAsyncStatusUpdater)(nil).SetLongRunningOperationState), arg0) +} + // MockClusterScoper is a mock of ClusterScoper interface. type MockClusterScoper struct { ctrl *gomock.Controller diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index ed833741987..75e5cb6e720 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -36,6 +36,7 @@ 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/util/futures" ) // ClusterScopeParams defines the input parameters used to create a new Scope. @@ -547,6 +548,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { ), ) + // TODO (cecile): add new conditions to owned conditions return s.patchHelper.Patch( ctx, s.AzureCluster, @@ -678,3 +680,29 @@ func (s *ClusterScope) getOutboundLBPublicIPSpecs(outboundLB *infrav1.LoadBalanc return outboundIPSpecs } + +// SetLongRunningOperationState will set the future on the AzureCluster status to allow the resource to continue +// in the next reconciliation. +func (s *ClusterScope) SetLongRunningOperationState(future *infrav1.Future) { + futures.Set(s.AzureCluster, future) +} + +// GetLongRunningOperationState will get the future on the AzureCluster status. +func (s *ClusterScope) GetLongRunningOperationState(name, service string) *infrav1.Future { + return futures.Get(s.AzureCluster, name, service) +} + +// DeleteLongRunningOperationState will delete the future from the AzureCluster status. +func (s *ClusterScope) DeleteLongRunningOperationState(name, service string) { + futures.Delete(s.AzureCluster, name, service) +} + +// SetConditionTrue will set a condition to true on the AzureCluster status. +func (s *ClusterScope) SetConditionTrue(condition clusterv1.ConditionType) { + conditions.MarkTrue(s.AzureCluster, condition) +} + +// SetConditionFalse will set a condition to false on the AzureCluster status. +func (s *ClusterScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { + conditions.MarkFalse(s.AzureCluster, condition, reason, severity, "") // TODO: add message +} diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 8f71d522df1..97037df1a18 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -39,6 +39,7 @@ 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/util/futures" ) // MachineScopeParams defines the input parameters used to create a new MachineScope. @@ -561,3 +562,29 @@ func (m *MachineScope) SetSubnetName() error { return nil } + +// SetLongRunningOperationState will set the future on the AzureMachine status to allow the resource to continue +// in the next reconciliation. +func (m *MachineScope) SetLongRunningOperationState(future *infrav1.Future) { + futures.Set(m.AzureMachine, future) +} + +// GetLongRunningOperationState will get the future on the AzureMachine status. +func (m *MachineScope) GetLongRunningOperationState(name, service string) *infrav1.Future { + return futures.Get(m.AzureMachine, name, service) +} + +// DeleteLongRunningOperationState will delete the future from the AzureMachine status. +func (m *MachineScope) DeleteLongRunningOperationState(name, service string) { + futures.Delete(m.AzureMachine, name, service) +} + +// SetConditionTrue will set a condition to true on the AzureMachine status. +func (m *MachineScope) SetConditionTrue(condition clusterv1.ConditionType) { + conditions.MarkTrue(m.AzureMachine, condition) +} + +// SetConditionFalse will set a condition to false on the AzureMachine status. +func (m *MachineScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { + conditions.MarkFalse(m.AzureMachine, condition, reason, severity, "") // TODO: add message +} diff --git a/azure/scope/machinepool.go b/azure/scope/machinepool.go index 71208b99822..1192c53fe00 100644 --- a/azure/scope/machinepool.go +++ b/azure/scope/machinepool.go @@ -22,6 +22,8 @@ import ( "strings" "time" + "sigs.k8s.io/cluster-api-provider-azure/util/futures" + "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -297,7 +299,8 @@ func (m *MachinePoolScope) applyAzureMachinePoolMachines(ctx context.Context) er return nil } - if m.GetLongRunningOperationState() != nil { + // TODO: use a const for service name + if futures.Has(m.AzureMachinePool, m.Name(), "scalesets") { 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 @@ -373,13 +376,17 @@ func (m *MachinePoolScope) createMachine(ctx context.Context, machine azure.VMSS // SetLongRunningOperationState will set the future on the AzureMachinePool status to allow the resource to continue // in the next reconciliation. func (m *MachinePoolScope) SetLongRunningOperationState(future *infrav1.Future) { - m.AzureMachinePool.Status.LongRunningOperationState = future + futures.Set(m.AzureMachinePool, future) } -// GetLongRunningOperationState will get the future on the AzureMachinePool status to allow the resource to continue -// in the next reconciliation. -func (m *MachinePoolScope) GetLongRunningOperationState() *infrav1.Future { - return m.AzureMachinePool.Status.LongRunningOperationState +// GetLongRunningOperationState will get the future on the AzureMachinePool status. +func (m *MachinePoolScope) GetLongRunningOperationState(name, service string) *infrav1.Future { + return futures.Get(m.AzureMachinePool, name, service) +} + +// DeleteLongRunningOperationState will delete the future from the AzureMachinePool status. +func (m *MachinePoolScope) DeleteLongRunningOperationState(name, service string) { + futures.Delete(m.AzureMachinePool, name, service) } // setProvisioningStateAndConditions sets the AzureMachinePool provisioning state and conditions. @@ -618,3 +625,13 @@ func (m *MachinePoolScope) SetSubnetName() error { return nil } + +// SetConditionTrue will set a condition to true on the AzureMachinePool status. +func (m *MachinePoolScope) SetConditionTrue(condition clusterv1.ConditionType) { + conditions.MarkTrue(m.AzureMachinePool, condition) +} + +// SetConditionFalse will set a condition to false on the AzureMachinePool status. +func (m *MachinePoolScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { + conditions.MarkFalse(m.AzureMachinePool, condition, reason, severity, "") // TODO: add message +} diff --git a/azure/scope/machinepoolmachine.go b/azure/scope/machinepoolmachine.go index f29ab7373e1..f40b68fa499 100644 --- a/azure/scope/machinepoolmachine.go +++ b/azure/scope/machinepoolmachine.go @@ -43,6 +43,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/util/futures" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -173,14 +174,30 @@ func (s *MachinePoolMachineScope) ScaleSetName() string { return s.MachinePoolScope.Name() } -// GetLongRunningOperationState gets a future representing the current state of a long-running operation if one exists. -func (s *MachinePoolMachineScope) GetLongRunningOperationState() *infrav1.Future { - return s.AzureMachinePoolMachine.Status.LongRunningOperationState +// SetLongRunningOperationState will set the future on the AzureMachinePoolMachine status to allow the resource to continue +// in the next reconciliation. +func (s *MachinePoolMachineScope) SetLongRunningOperationState(future *infrav1.Future) { + futures.Set(s.AzureMachinePoolMachine, future) } -// SetLongRunningOperationState sets a future representing the current state of a long-running operation. -func (s *MachinePoolMachineScope) SetLongRunningOperationState(future *infrav1.Future) { - s.AzureMachinePoolMachine.Status.LongRunningOperationState = future +// GetLongRunningOperationState will get the future on the AzureMachinePoolMachine status. +func (s *MachinePoolMachineScope) GetLongRunningOperationState(name, service string) *infrav1.Future { + return futures.Get(s.AzureMachinePoolMachine, name, service) +} + +// DeleteLongRunningOperationState will delete the future from the AzureMachinePoolMachine status. +func (s *MachinePoolMachineScope) DeleteLongRunningOperationState(name, service string) { + futures.Delete(s.AzureMachinePoolMachine, name, service) +} + +// SetConditionTrue will set a condition to true on the AzureMachinePoolMachine status. +func (s *MachinePoolMachineScope) SetConditionTrue(condition clusterv1.ConditionType) { + conditions.MarkTrue(s.AzureMachinePoolMachine, condition) +} + +// SetConditionFalse will set a condition to false on the AzureMachinePoolMachine status. +func (s *MachinePoolMachineScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { + conditions.MarkFalse(s.AzureMachinePoolMachine, condition, reason, severity, "") // TODO: add message } // SetVMSSVM update the scope with the current state of the VMSS VM. diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index ebd4b90e1dc..78f9945b005 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -39,6 +39,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-azure/azure" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/util/futures" ) // ManagedControlPlaneScopeParams defines the input parameters used to create a new managed @@ -544,3 +545,29 @@ func (s *ManagedControlPlaneScope) GetKubeConfigData() []byte { func (s *ManagedControlPlaneScope) SetKubeConfigData(kubeConfigData []byte) { s.kubeConfigData = kubeConfigData } + +// SetLongRunningOperationState will set the future on the AzureManagedControlPlane status to allow the resource to continue +// in the next reconciliation. +func (s *ManagedControlPlaneScope) SetLongRunningOperationState(future *infrav1.Future) { + futures.Set(s.ControlPlane, future) +} + +// GetLongRunningOperationState will get the future on the AzureManagedControlPlane status. +func (s *ManagedControlPlaneScope) GetLongRunningOperationState(name, service string) *infrav1.Future { + return futures.Get(s.ControlPlane, name, service) +} + +// DeleteLongRunningOperationState will delete the future from the AzureManagedControlPlane status. +func (s *ManagedControlPlaneScope) DeleteLongRunningOperationState(name, service string) { + futures.Delete(s.ControlPlane, name, service) +} + +// SetConditionTrue is a no-op for managed control planes. +func (s *ManagedControlPlaneScope) SetConditionTrue(condition clusterv1.ConditionType) { + // TODO: add condition to AzureManagedControlPlane status +} + +// SetConditionFalse is a no-op for managed control planes. +func (s *ManagedControlPlaneScope) SetConditionFalse(condition clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity) { + // TODO: add condition to AzureManagedControlPlane status +} diff --git a/azure/services/futures/futures.go b/azure/services/futures/futures.go new file mode 100644 index 00000000000..41dab338ce0 --- /dev/null +++ b/azure/services/futures/futures.go @@ -0,0 +1,41 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package futures + +import ( + "context" + "encoding/base64" + "encoding/json" + + "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" +) + +func Done(ctx context.Context, future *infrav1.Future, client autorest.Sender) (bool, error) { + futureData, err := base64.URLEncoding.DecodeString(future.Data) + if err != nil { + return false, errors.Wrap(err, "failed to base64 decode future data") + } + var genericFuture azureautorest.FutureAPI + if err := json.Unmarshal(futureData, &genericFuture); err != nil { + return false, errors.Wrap(err, "failed to unmarshal future data") + } + + return genericFuture.DoneWithContext(ctx, client) +} diff --git a/azure/services/scalesets/client.go b/azure/services/scalesets/client.go index 1d525e85c9c..5526813276c 100644 --- a/azure/services/scalesets/client.go +++ b/azure/services/scalesets/client.go @@ -24,7 +24,6 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-11-01/network" "github.com/Azure/go-autorest/autorest" azureautorest "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" @@ -32,6 +31,8 @@ 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/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -40,15 +41,11 @@ type Client interface { List(context.Context, string) ([]compute.VirtualMachineScaleSet, error) ListInstances(context.Context, string, string) ([]compute.VirtualMachineScaleSetVM, error) Get(context.Context, string, string) (compute.VirtualMachineScaleSet, error) - CreateOrUpdate(context.Context, string, string, compute.VirtualMachineScaleSet) error - CreateOrUpdateAsync(context.Context, string, string, compute.VirtualMachineScaleSet) (*infrav1.Future, error) - Update(context.Context, string, string, compute.VirtualMachineScaleSetUpdate) (compute.VirtualMachineScaleSet, error) - UpdateAsync(context.Context, string, string, compute.VirtualMachineScaleSetUpdate) (*infrav1.Future, error) + CreateOrUpdateAsync(context.Context, string, string, compute.VirtualMachineScaleSet) (*compute.VirtualMachineScaleSetsCreateOrUpdateFuture, error) + UpdateAsync(context.Context, string, string, compute.VirtualMachineScaleSetUpdate) (*compute.VirtualMachineScaleSetsUpdateFuture, error) GetResultIfDone(ctx context.Context, future *infrav1.Future) (compute.VirtualMachineScaleSet, error) UpdateInstances(context.Context, string, string, []string) error - Delete(context.Context, string, string) error - DeleteAsync(context.Context, string, string) (*infrav1.Future, error) - GetPublicIPAddress(context.Context, string, string) (network.PublicIPAddress, error) + DeleteAsync(context.Context, string, string) (*compute.VirtualMachineScaleSetsDeleteFuture, error) } type ( @@ -56,7 +53,6 @@ type ( AzureClient struct { scalesetvms compute.VirtualMachineScaleSetVMsClient scalesets compute.VirtualMachineScaleSetsClient - publicIPs network.PublicIPAddressesClient } genericScaleSetFuture interface { @@ -74,15 +70,6 @@ type ( } ) -const ( - // PatchFuture is a future that was derived from a PATCH request to VMSS. - PatchFuture string = "PATCH" - // PutFuture is a future that was derived from a PUT request to VMSS. - PutFuture string = "PUT" - // DeleteFuture is a future that was derived from a DELETE request to VMSS. - DeleteFuture string = "DELETE" -) - var _ Client = &AzureClient{} // NewClient creates a new VMSS client from subscription ID. @@ -90,7 +77,6 @@ func NewClient(auth azure.Authorizer) *AzureClient { return &AzureClient{ scalesetvms: newVirtualMachineScaleSetVMsClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()), scalesets: newVirtualMachineScaleSetsClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()), - publicIPs: newPublicIPsClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()), } } @@ -105,21 +91,6 @@ func newVirtualMachineScaleSetVMsClient(subscriptionID string, baseURI string, a func newVirtualMachineScaleSetsClient(subscriptionID string, baseURI string, authorizer autorest.Authorizer) compute.VirtualMachineScaleSetsClient { c := compute.NewVirtualMachineScaleSetsClientWithBaseURI(baseURI, subscriptionID) azure.SetAutoRestClientDefaults(&c.Client, authorizer) - - // The default number of retries is 3. This means the client will attempt to retry operation results like resource - // conflicts (HTTP 409). For a reconciling controller, this is undesirable behavior since if the controller runs - // into an error reconciling, the controller would be better off to end with an error and try again later. - // - // Unfortunately, the naming of this field is a bit misleading. This is not actually "retry attempts", it actually - // is attempts. Setting this to a value of 0 will cause a panic in Go AutoRest. - c.RetryAttempts = 1 - return c -} - -// newPublicIPsClient creates a new publicIPs client from subscription ID. -func newPublicIPsClient(subscriptionID string, baseURI string, authorizer autorest.Authorizer) network.PublicIPAddressesClient { - c := network.NewPublicIPAddressesClientWithBaseURI(baseURI, subscriptionID) - azure.SetAutoRestClientDefaults(&c.Client, authorizer) return c } @@ -173,26 +144,9 @@ func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vmssName stri return ac.scalesets.Get(ctx, resourceGroupName, vmssName, "") } -// CreateOrUpdate the operation to create or update a virtual machine scale set. -func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vmssName string, vmss compute.VirtualMachineScaleSet) error { - ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.CreateOrUpdate") - defer span.End() - - future, err := ac.scalesets.CreateOrUpdate(ctx, resourceGroupName, vmssName, vmss) - if err != nil { - return err - } - err = future.WaitForCompletionRef(ctx, ac.scalesets.Client) - if err != nil { - return err - } - _, err = future.Result(ac.scalesets) - return err -} - // CreateOrUpdateAsync the operation to create or update a virtual machine scale set without waiting for the operation // to complete. -func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, resourceGroupName, vmssName string, vmss compute.VirtualMachineScaleSet) (*infrav1.Future, error) { +func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, resourceGroupName, vmssName string, vmss compute.VirtualMachineScaleSet) (*compute.VirtualMachineScaleSetsCreateOrUpdateFuture, error) { ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.CreateOrUpdateAsync") defer span.End() @@ -201,41 +155,21 @@ func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, resourceGroupNam return nil, err } - jsonData, err := future.MarshalJSON() - if err != nil { - return nil, errors.Wrap(err, "failed to marshal async future") - } - - return &infrav1.Future{ - Type: PutFuture, - ResourceGroup: resourceGroupName, - Name: vmssName, - FutureData: base64.URLEncoding.EncodeToString(jsonData), - }, nil -} - -// Update update a VM scale set. -// Parameters: resourceGroupName - the name of the resource group. VMScaleSetName - the name of the VM scale set to create or update. parameters - the scale set object. -func (ac *AzureClient) Update(ctx context.Context, resourceGroupName, vmssName string, parameters compute.VirtualMachineScaleSetUpdate) (compute.VirtualMachineScaleSet, error) { - ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.Update") - defer span.End() - - future, err := ac.scalesets.Update(ctx, resourceGroupName, vmssName, parameters) - if err != nil { - return compute.VirtualMachineScaleSet{}, 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 { - return compute.VirtualMachineScaleSet{}, errors.Wrapf(err, "failed waiting for completion of operation for vmss named %q", vmssName) + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &future, nil } - vmss, err := future.Result(ac.scalesets) - if err != nil { - return vmss, errors.Wrapf(err, "failed fetching the result of operation for vmss named %q", vmssName) - } + // todo: this returns the result VMSS, we should use it + _, err = future.Result(ac.scalesets) - return vmss, nil + // if the operation completed, return a nil future. + return nil, err } // UpdateAsync update a VM scale set without waiting for the result of the operation. UpdateAsync sends a PATCH @@ -245,7 +179,7 @@ func (ac *AzureClient) Update(ctx context.Context, resourceGroupName, vmssName s // 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) { +func (ac *AzureClient) UpdateAsync(ctx context.Context, resourceGroupName, vmssName string, parameters compute.VirtualMachineScaleSetUpdate) (*compute.VirtualMachineScaleSetsUpdateFuture, error) { ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.UpdateAsync") defer span.End() @@ -254,29 +188,32 @@ func (ac *AzureClient) UpdateAsync(ctx context.Context, resourceGroupName, vmssN return nil, errors.Wrapf(err, "failed updating vmss named %q", vmssName) } - jsonData, err := future.MarshalJSON() + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = future.WaitForCompletionRef(ctx, ac.scalesets.Client) if err != nil { - return nil, errors.Wrap(err, "failed to marshal async future") + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &future, nil } + // todo: this returns the result VMSS, we should use it + _, err = future.Result(ac.scalesets) - return &infrav1.Future{ - Type: PatchFuture, - ResourceGroup: resourceGroupName, - Name: vmssName, - FutureData: base64.URLEncoding.EncodeToString(jsonData), - }, nil + // 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.FutureData) + 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 PatchFuture: + case converters.PatchFuture: var future compute.VirtualMachineScaleSetsUpdateFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data") @@ -286,7 +223,7 @@ func (ac *AzureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu FutureAPI: &future, result: future.Result, } - case PutFuture: + case converters.PutFuture: var future compute.VirtualMachineScaleSetsCreateOrUpdateFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data") @@ -296,7 +233,7 @@ func (ac *AzureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu FutureAPI: &future, result: future.Result, } - case DeleteFuture: + case converters.DeleteFuture: var future compute.VirtualMachineScaleSetsDeleteFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSet{}, errors.Wrap(err, "failed to unmarshal future data") @@ -346,23 +283,6 @@ func (ac *AzureClient) UpdateInstances(ctx context.Context, resourceGroupName, v return err } -// Delete the operation to delete a virtual machine scale set. -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vmssName string) error { - ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.Delete") - defer span.End() - - future, err := ac.scalesets.Delete(ctx, resourceGroupName, vmssName, to.BoolPtr(false)) - if err != nil { - return err - } - err = future.WaitForCompletionRef(ctx, ac.scalesets.Client) - if err != nil { - return err - } - _, err = future.Result(ac.scalesets) - return err -} - // DeleteAsync is the operation to delete a virtual machine scale set asynchronously. DeleteAsync sends a DELETE // 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. @@ -370,7 +290,7 @@ func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vmssName s // 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) DeleteAsync(ctx context.Context, resourceGroupName, vmssName string) (*infrav1.Future, error) { +func (ac *AzureClient) DeleteAsync(ctx context.Context, resourceGroupName, vmssName string) (*compute.VirtualMachineScaleSetsDeleteFuture, error) { ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.DeleteAsync") defer span.End() @@ -379,25 +299,19 @@ func (ac *AzureClient) DeleteAsync(ctx context.Context, resourceGroupName, vmssN return nil, errors.Wrapf(err, "failed deleting vmss named %q", vmssName) } - jsonData, err := future.MarshalJSON() + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = future.WaitForCompletionRef(ctx, ac.scalesets.Client) if err != nil { - return nil, errors.Wrap(err, "failed to marshal async future") + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &future, nil } + _, err = future.Result(ac.scalesets) - return &infrav1.Future{ - Type: DeleteFuture, - ResourceGroup: resourceGroupName, - Name: vmssName, - FutureData: base64.URLEncoding.EncodeToString(jsonData), - }, nil -} - -// GetPublicIPAddress gets the public IP address for the given public IP name. -func (ac *AzureClient) GetPublicIPAddress(ctx context.Context, resourceGroupName, publicIPName string) (network.PublicIPAddress, error) { - ctx, span := tele.Tracer().Start(ctx, "scalesets.AzureClient.GetPublicIPAddress") - defer span.End() - - return ac.publicIPs.Get(ctx, resourceGroupName, publicIPName, "true") + // if the operation completed, return a nil future. + return nil, err } // Result wraps the delete result so that we can treat it generically. The only thing we care about is if the delete diff --git a/azure/services/scalesets/mock_scalesets/client_mock.go b/azure/services/scalesets/mock_scalesets/client_mock.go index bc14837bf8d..41efba51bb4 100644 --- a/azure/services/scalesets/mock_scalesets/client_mock.go +++ b/azure/services/scalesets/mock_scalesets/client_mock.go @@ -54,25 +54,11 @@ func (m *MockClient) EXPECT() *MockClientMockRecorder { return m.recorder } -// CreateOrUpdate mocks base method. -func (m *MockClient) CreateOrUpdate(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachineScaleSet) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 -} - -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockClient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3) -} - // CreateOrUpdateAsync mocks base method. -func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachineScaleSet) (*v1alpha4.Future, error) { +func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachineScaleSet) (*compute.VirtualMachineScaleSetsCreateOrUpdateFuture, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(*v1alpha4.Future) + ret0, _ := ret[0].(*compute.VirtualMachineScaleSetsCreateOrUpdateFuture) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -83,25 +69,11 @@ func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1, arg2, arg3 int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1, arg2, arg3) } -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 -} - -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2) -} - // DeleteAsync mocks base method. -func (m *MockClient) DeleteAsync(arg0 context.Context, arg1, arg2 string) (*v1alpha4.Future, error) { +func (m *MockClient) DeleteAsync(arg0 context.Context, arg1, arg2 string) (*compute.VirtualMachineScaleSetsDeleteFuture, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1, arg2) - ret0, _ := ret[0].(*v1alpha4.Future) + ret0, _ := ret[0].(*compute.VirtualMachineScaleSetsDeleteFuture) ret1, _ := ret[1].(error) return ret0, ret1 } @@ -127,21 +99,6 @@ func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2) } -// GetPublicIPAddress mocks base method. -func (m *MockClient) GetPublicIPAddress(arg0 context.Context, arg1, arg2 string) (network.PublicIPAddress, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetPublicIPAddress", arg0, arg1, arg2) - ret0, _ := ret[0].(network.PublicIPAddress) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetPublicIPAddress indicates an expected call of GetPublicIPAddress. -func (mr *MockClientMockRecorder) GetPublicIPAddress(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPublicIPAddress", reflect.TypeOf((*MockClient)(nil).GetPublicIPAddress), arg0, arg1, arg2) -} - // GetResultIfDone mocks base method. func (m *MockClient) GetResultIfDone(ctx context.Context, future *v1alpha4.Future) (compute.VirtualMachineScaleSet, error) { m.ctrl.T.Helper() @@ -187,26 +144,11 @@ func (mr *MockClientMockRecorder) ListInstances(arg0, arg1, arg2 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListInstances", reflect.TypeOf((*MockClient)(nil).ListInstances), arg0, arg1, arg2) } -// Update mocks base method. -func (m *MockClient) Update(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachineScaleSetUpdate) (compute.VirtualMachineScaleSet, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Update", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(compute.VirtualMachineScaleSet) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Update indicates an expected call of Update. -func (mr *MockClientMockRecorder) Update(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Update", reflect.TypeOf((*MockClient)(nil).Update), arg0, arg1, arg2, arg3) -} - // UpdateAsync mocks base method. -func (m *MockClient) UpdateAsync(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachineScaleSetUpdate) (*v1alpha4.Future, error) { +func (m *MockClient) UpdateAsync(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachineScaleSetUpdate) (*compute.VirtualMachineScaleSetsUpdateFuture, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdateAsync", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(*v1alpha4.Future) + ret0, _ := ret[0].(*compute.VirtualMachineScaleSetsUpdateFuture) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/azure/services/scalesets/mock_scalesets/scalesets_mock.go b/azure/services/scalesets/mock_scalesets/scalesets_mock.go index 0e2af5edfbd..eddc8adc9ba 100644 --- a/azure/services/scalesets/mock_scalesets/scalesets_mock.go +++ b/azure/services/scalesets/mock_scalesets/scalesets_mock.go @@ -29,6 +29,7 @@ import ( gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // MockScaleSetScope is a mock of ScaleSetScope interface. @@ -180,6 +181,18 @@ func (mr *MockScaleSetScopeMockRecorder) ClusterName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockScaleSetScope)(nil).ClusterName)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockScaleSetScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockScaleSetScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockScaleSetScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // Enabled mocks base method. func (m *MockScaleSetScope) Enabled() bool { m.ctrl.T.Helper() @@ -212,32 +225,32 @@ func (mr *MockScaleSetScopeMockRecorder) Error(err, msg interface{}, keysAndValu } // GetBootstrapData mocks base method. -func (m *MockScaleSetScope) GetBootstrapData(ctx context.Context) (string, error) { +func (m *MockScaleSetScope) GetBootstrapData(arg0 context.Context) (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetBootstrapData", ctx) + ret := m.ctrl.Call(m, "GetBootstrapData", arg0) ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) return ret0, ret1 } // GetBootstrapData indicates an expected call of GetBootstrapData. -func (mr *MockScaleSetScopeMockRecorder) GetBootstrapData(ctx interface{}) *gomock.Call { +func (mr *MockScaleSetScopeMockRecorder) GetBootstrapData(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBootstrapData", reflect.TypeOf((*MockScaleSetScope)(nil).GetBootstrapData), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBootstrapData", reflect.TypeOf((*MockScaleSetScope)(nil).GetBootstrapData), arg0) } // GetLongRunningOperationState mocks base method. -func (m *MockScaleSetScope) GetLongRunningOperationState() *v1alpha4.Future { +func (m *MockScaleSetScope) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetLongRunningOperationState") + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) ret0, _ := ret[0].(*v1alpha4.Future) return ret0 } // GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. -func (mr *MockScaleSetScopeMockRecorder) GetLongRunningOperationState() *gomock.Call { +func (mr *MockScaleSetScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockScaleSetScope)(nil).GetLongRunningOperationState)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockScaleSetScope)(nil).GetLongRunningOperationState), arg0, arg1) } // GetVMImage mocks base method. @@ -367,6 +380,30 @@ func (mr *MockScaleSetScopeMockRecorder) SetAnnotation(arg0, arg1 interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAnnotation", reflect.TypeOf((*MockScaleSetScope)(nil).SetAnnotation), arg0, arg1) } +// SetConditionFalse mocks base method. +func (m *MockScaleSetScope) SetConditionFalse(arg0 v1alpha40.ConditionType, arg1 string, arg2 v1alpha40.ConditionSeverity) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetConditionFalse", arg0, arg1, arg2) +} + +// SetConditionFalse indicates an expected call of SetConditionFalse. +func (mr *MockScaleSetScopeMockRecorder) SetConditionFalse(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionFalse", reflect.TypeOf((*MockScaleSetScope)(nil).SetConditionFalse), arg0, arg1, arg2) +} + +// SetConditionTrue mocks base method. +func (m *MockScaleSetScope) SetConditionTrue(arg0 v1alpha40.ConditionType) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetConditionTrue", arg0) +} + +// SetConditionTrue indicates an expected call of SetConditionTrue. +func (mr *MockScaleSetScopeMockRecorder) SetConditionTrue(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionTrue", reflect.TypeOf((*MockScaleSetScope)(nil).SetConditionTrue), arg0) +} + // SetLongRunningOperationState mocks base method. func (m *MockScaleSetScope) SetLongRunningOperationState(arg0 *v1alpha4.Future) { m.ctrl.T.Helper() diff --git a/azure/services/scalesets/scalesets.go b/azure/services/scalesets/scalesets.go index 8c65aeca110..39447a7ee8a 100644 --- a/azure/services/scalesets/scalesets.go +++ b/azure/services/scalesets/scalesets.go @@ -37,25 +37,21 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -const ( - // UltraSSDStorageAccountType identifies the Ultra disk storage account type. - UltraSSDStorageAccountType = "UltraSSD_LRS" -) +const serviceName = "scalesets" type ( // ScaleSetScope defines the scope interface for a scale sets service. ScaleSetScope interface { logr.Logger azure.ClusterDescriber - GetBootstrapData(ctx context.Context) (string, error) - GetLongRunningOperationState() *infrav1.Future + azure.AsyncStatusUpdater + GetBootstrapData(context.Context) (string, error) GetVMImage() (*infrav1.Image, error) SaveVMImageToStatus(*infrav1.Image) MaxSurge() (int, error) ScaleSetSpec() azure.ScaleSetSpec VMSSExtensionSpecs() []azure.VMSSExtensionSpec SetAnnotation(string, string) - SetLongRunningOperationState(*infrav1.Future) SetProviderID(string) SetVMSSState(*azure.VMSS) } @@ -93,7 +89,7 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) { // check if there is an ongoing long running operation var ( - future = s.Scope.GetLongRunningOperationState() + future = s.Scope.GetLongRunningOperationState(s.Scope.ScaleSetSpec().Name, serviceName) fetchedVMSS *azure.VMSS ) @@ -147,8 +143,8 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) { } } - // if we get to hear, we have completed any long running VMSS operations (creates / updates) - s.Scope.SetLongRunningOperationState(nil) + // if we get to here, we have completed any long running VMSS operations (creates / updates) + s.Scope.DeleteLongRunningOperationState(s.Scope.ScaleSetSpec().Name, serviceName) return nil } @@ -175,7 +171,7 @@ func (s *Service) Delete(ctx context.Context) error { }() // check if there is an ongoing long running operation - future := s.Scope.GetLongRunningOperationState() + future := s.Scope.GetLongRunningOperationState(vmssSpec.Name, serviceName) if future != nil { // if the operation is not complete this will return an error _, err := s.GetResultIfDone(ctx, future) @@ -184,13 +180,13 @@ func (s *Service) Delete(ctx context.Context) error { } // ScaleSet has been deleted - s.Scope.SetLongRunningOperationState(nil) + s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, serviceName) return nil } // no long running delete operation is active, so delete the ScaleSet s.Scope.V(2).Info("deleting VMSS", "scale set", vmssSpec.Name) - future, err = s.Client.DeleteAsync(ctx, s.Scope.ResourceGroup(), vmssSpec.Name) + sdkFuture, err := s.Client.DeleteAsync(ctx, s.Scope.ResourceGroup(), vmssSpec.Name) if err != nil { if azure.ResourceNotFound(err) { // already deleted @@ -199,6 +195,11 @@ 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, converters.DeleteFuture, serviceName, 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()) + } + s.Scope.SetLongRunningOperationState(future) if future != nil { // if future exists, check state of the future @@ -208,7 +209,7 @@ func (s *Service) Delete(ctx context.Context) error { } // future is either nil, or the result of the future is complete - s.Scope.SetLongRunningOperationState(nil) + s.Scope.DeleteLongRunningOperationState(vmssSpec.Name, serviceName) return nil } @@ -223,9 +224,14 @@ func (s *Service) createVMSS(ctx context.Context) (*infrav1.Future, error) { return nil, errors.Wrap(err, "failed building VMSS from spec") } - future, err := s.Client.CreateOrUpdateAsync(ctx, s.Scope.ResourceGroup(), spec.Name, vmss) + sdkFuture, err := s.Client.CreateOrUpdateAsync(ctx, s.Scope.ResourceGroup(), spec.Name, vmss) if err != nil { - return future, errors.Wrap(err, "cannot create VMSS") + return nil, errors.Wrap(err, "cannot create VMSS") + } + + future, err := converters.SDKToFuture(sdkFuture, converters.PutFuture, serviceName, 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()) } s.Scope.V(2).Info("starting to create VMSS", "scale set", spec.Name) @@ -270,12 +276,17 @@ func (s *Service) patchVMSSIfNeeded(ctx context.Context, infraVMSS *azure.VMSS) } s.Scope.V(4).Info("patching vmss", "scale set", spec.Name, "patch", patch) - future, err := s.UpdateAsync(ctx, s.Scope.ResourceGroup(), spec.Name, patch) + sdkFuture, err := s.UpdateAsync(ctx, s.Scope.ResourceGroup(), spec.Name, patch) if err != nil { if azure.ResourceConflict(err) { - return future, azure.WithTransientError(err, 30*time.Second) + return nil, azure.WithTransientError(err, 30*time.Second) } - return future, errors.Wrap(err, "failed updating VMSS") + return nil, errors.Wrap(err, "failed updating VMSS") + } + + future, err := converters.SDKToFuture(sdkFuture, converters.PatchFuture, serviceName, 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()) } s.Scope.SetLongRunningOperationState(future) @@ -337,7 +348,7 @@ func (s *Service) validateSpec(ctx context.Context) error { } for _, zone := range zones { - if disks.ManagedDisk != nil && disks.ManagedDisk.StorageAccountType == UltraSSDStorageAccountType && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, zone) { + if disks.ManagedDisk != nil && disks.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, zone) { return azure.WithTerminalError(fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", spec.Size, location)) } } @@ -482,7 +493,7 @@ func (s *Service) buildVMSSFromSpec(ctx context.Context, vmssSpec azure.ScaleSet } for _, dataDisk := range vmssSpec.DataDisks { - if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == UltraSSDStorageAccountType { + if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) { vmss.VirtualMachineScaleSetProperties.AdditionalCapabilities = &compute.AdditionalCapabilities{ UltraSSDEnabled: to.BoolPtr(true), } diff --git a/azure/services/scalesets/scalesets_test.go b/azure/services/scalesets/scalesets_test.go index 032c68651e0..098d734412b 100644 --- a/azure/services/scalesets/scalesets_test.go +++ b/azure/services/scalesets/scalesets_test.go @@ -1206,7 +1206,7 @@ func setupDefaultVMSSInProgressOperationDoneExpectations(s *mock_scalesets.MockS Type: PutFuture, ResourceGroup: defaultResourceGroup, Name: defaultVMSSName, - FutureData: "", + Data: "", } s.GetLongRunningOperationState().Return(future) m.GetResultIfDone(gomockinternal.AContext(), future).Return(createdVMSS, nil).AnyTimes() diff --git a/azure/services/scalesetvms/client.go b/azure/services/scalesetvms/client.go index 928207162be..870d3173e08 100644 --- a/azure/services/scalesetvms/client.go +++ b/azure/services/scalesetvms/client.go @@ -29,6 +29,7 @@ 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/util/tele" ) @@ -55,11 +56,6 @@ type ( } ) -const ( - // DeleteFuture is a future that was derived from a DELETE request to VMSS. - DeleteFuture string = "DELETE" -) - var _ client = &azureClient{} // newClient creates a new VMSS client from subscription ID. @@ -92,13 +88,13 @@ func (ac *azureClient) GetResultIfDone(ctx context.Context, future *infrav1.Futu defer span.End() var genericFuture genericScaleSetVMFuture - futureData, err := base64.URLEncoding.DecodeString(future.FutureData) + futureData, err := base64.URLEncoding.DecodeString(future.Data) if err != nil { return compute.VirtualMachineScaleSetVM{}, errors.Wrapf(err, "failed to base64 decode future data") } switch future.Type { - case DeleteFuture: + case converters.DeleteFuture: var future compute.VirtualMachineScaleSetVMsDeleteFuture if err := json.Unmarshal(futureData, &future); err != nil { return compute.VirtualMachineScaleSetVM{}, errors.Wrap(err, "failed to unmarshal future data") @@ -151,10 +147,10 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, resourceGroupName, vmssN } return &infrav1.Future{ - Type: DeleteFuture, + Type: converters.DeleteFuture, ResourceGroup: resourceGroupName, - Name: vmssName, - FutureData: base64.URLEncoding.EncodeToString(jsonData), + Name: instanceID, + Data: base64.URLEncoding.EncodeToString(jsonData), }, nil } diff --git a/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go b/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go index 593a95eccf7..aa26ab9c5b7 100644 --- a/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go +++ b/azure/services/scalesetvms/mock_scalesetvms/scalesetvms_mock.go @@ -28,6 +28,7 @@ import ( gomock "github.com/golang/mock/gomock" v1alpha4 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1alpha40 "sigs.k8s.io/cluster-api/api/v1alpha4" ) // MockScaleSetVMScope is a mock of ScaleSetVMScope interface. @@ -179,6 +180,18 @@ func (mr *MockScaleSetVMScopeMockRecorder) ClusterName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockScaleSetVMScope)(nil).ClusterName)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockScaleSetVMScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockScaleSetVMScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockScaleSetVMScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // Enabled mocks base method. func (m *MockScaleSetVMScope) Enabled() bool { m.ctrl.T.Helper() @@ -211,17 +224,17 @@ func (mr *MockScaleSetVMScopeMockRecorder) Error(err, msg interface{}, keysAndVa } // GetLongRunningOperationState mocks base method. -func (m *MockScaleSetVMScope) GetLongRunningOperationState() *v1alpha4.Future { +func (m *MockScaleSetVMScope) GetLongRunningOperationState(arg0, arg1 string) *v1alpha4.Future { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetLongRunningOperationState") + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) ret0, _ := ret[0].(*v1alpha4.Future) return ret0 } // GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. -func (mr *MockScaleSetVMScopeMockRecorder) GetLongRunningOperationState() *gomock.Call { +func (mr *MockScaleSetVMScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockScaleSetVMScope)(nil).GetLongRunningOperationState)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockScaleSetVMScope)(nil).GetLongRunningOperationState), arg0, arg1) } // HashKey mocks base method. @@ -311,16 +324,40 @@ func (mr *MockScaleSetVMScopeMockRecorder) ScaleSetName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ScaleSetName", reflect.TypeOf((*MockScaleSetVMScope)(nil).ScaleSetName)) } +// SetConditionFalse mocks base method. +func (m *MockScaleSetVMScope) SetConditionFalse(arg0 v1alpha40.ConditionType, arg1 string, arg2 v1alpha40.ConditionSeverity) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetConditionFalse", arg0, arg1, arg2) +} + +// SetConditionFalse indicates an expected call of SetConditionFalse. +func (mr *MockScaleSetVMScopeMockRecorder) SetConditionFalse(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionFalse", reflect.TypeOf((*MockScaleSetVMScope)(nil).SetConditionFalse), arg0, arg1, arg2) +} + +// SetConditionTrue mocks base method. +func (m *MockScaleSetVMScope) SetConditionTrue(arg0 v1alpha40.ConditionType) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetConditionTrue", arg0) +} + +// SetConditionTrue indicates an expected call of SetConditionTrue. +func (mr *MockScaleSetVMScopeMockRecorder) SetConditionTrue(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetConditionTrue", reflect.TypeOf((*MockScaleSetVMScope)(nil).SetConditionTrue), arg0) +} + // SetLongRunningOperationState mocks base method. -func (m *MockScaleSetVMScope) SetLongRunningOperationState(future *v1alpha4.Future) { +func (m *MockScaleSetVMScope) SetLongRunningOperationState(arg0 *v1alpha4.Future) { m.ctrl.T.Helper() - m.ctrl.Call(m, "SetLongRunningOperationState", future) + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) } // SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. -func (mr *MockScaleSetVMScopeMockRecorder) SetLongRunningOperationState(future interface{}) *gomock.Call { +func (mr *MockScaleSetVMScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockScaleSetVMScope)(nil).SetLongRunningOperationState), future) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockScaleSetVMScope)(nil).SetLongRunningOperationState), arg0) } // SetVMSSVM mocks base method. diff --git a/azure/services/scalesetvms/scalesetvms.go b/azure/services/scalesetvms/scalesetvms.go index bda99d8d905..fffd95c3137 100644 --- a/azure/services/scalesetvms/scalesetvms.go +++ b/azure/services/scalesetvms/scalesetvms.go @@ -23,22 +23,22 @@ import ( "github.com/go-logr/logr" "github.com/pkg/errors" - 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/util/tele" ) +const serviceName = "scalesetvms" + type ( // ScaleSetVMScope defines the scope interface for a scale sets service. ScaleSetVMScope interface { logr.Logger azure.ClusterDescriber + azure.AsyncStatusUpdater InstanceID() string ScaleSetName() string SetVMSSVM(vmssvm *azure.VMSSVM) - GetLongRunningOperationState() *infrav1.Future - SetLongRunningOperationState(future *infrav1.Future) } // Service provides operations on Azure resources. @@ -101,9 +101,9 @@ func (s *Service) Delete(ctx context.Context) error { }() log.V(4).Info("entering delete") - future := s.Scope.GetLongRunningOperationState() + future := s.Scope.GetLongRunningOperationState(instanceID, serviceName) if future != nil { - if future.Type != DeleteFuture { + if future.Type != converters.DeleteFuture { return azure.WithTransientError(errors.New("attempting to delete, non-delete operation in progress"), 30*time.Second) } @@ -115,7 +115,7 @@ func (s *Service) Delete(ctx context.Context) error { // there was no error in fetching the result, the future has been completed log.V(4).Info("successfully deleted the instance") - s.Scope.SetLongRunningOperationState(nil) + s.Scope.DeleteLongRunningOperationState(instanceID, serviceName) return nil } @@ -137,6 +137,6 @@ func (s *Service) Delete(ctx context.Context) error { return errors.Wrap(err, "failed to get result of long running operation") } - s.Scope.SetLongRunningOperationState(nil) + s.Scope.DeleteLongRunningOperationState(instanceID, serviceName) return nil } diff --git a/azure/services/virtualmachines/virtualmachines.go b/azure/services/virtualmachines/virtualmachines.go index 43ae5dba139..560e1d850a6 100644 --- a/azure/services/virtualmachines/virtualmachines.go +++ b/azure/services/virtualmachines/virtualmachines.go @@ -39,11 +39,6 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -const ( - // UltraSSDStorageAccountType identifies the Ultra disk storage account type. - UltraSSDStorageAccountType = "UltraSSD_LRS" -) - // VMScope defines the scope interface for a virtual machines service. type VMScope interface { logr.Logger @@ -199,7 +194,7 @@ func (s *Service) Reconcile(ctx context.Context) error { } for _, dataDisk := range vmSpec.DataDisks { - if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == UltraSSDStorageAccountType { + if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) { virtualMachine.VirtualMachineProperties.AdditionalCapabilities = &compute.AdditionalCapabilities{ UltraSSDEnabled: to.BoolPtr(true), } @@ -436,7 +431,7 @@ func (s *Service) generateStorageProfile(ctx context.Context, vmSpec azure.VMSpe // check the support for ultra disks based on location and vm size location := s.Scope.Location() - if disk.ManagedDisk.StorageAccountType == UltraSSDStorageAccountType && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, vmSpec.Zone) { + if disk.ManagedDisk.StorageAccountType == string(compute.UltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, vmSpec.Zone) { return nil, azure.WithTerminalError(fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", vmSpec.Size, location)) } } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml index 8ff27b9ec12..43155fdfed2 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -1187,6 +1187,34 @@ spec: This list will be used by Cluster API to try and spread the machines across the failure domains.' type: object + longRunningOperationStates: + description: LongRunningOperationStates saves the states for Azure + long-running operations so they can be continued on the next reconciliation + loop. + items: + description: Future contains the data needed for an Azure long-running + operation to continue across reconcile loops. + properties: + futureData: + description: FutureData is the base64 url encoded json Azure + AutoRest Future + type: string + name: + description: Name is the name of the Azure resource + type: string + resourceGroup: + description: ResourceGroup is the Azure resource group for the + resource + type: string + type: + description: Type describes the type of future, update, create, + delete, etc + type: string + required: + - name + - type + type: object + type: array ready: description: Ready is true when the provider resource is ready. type: boolean diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml index 563346bcb39..454c62aa44f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepoolmachines.yaml @@ -173,6 +173,7 @@ spec: delete, etc type: string required: + - name - type type: object nodeRef: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index 485f7d28946..529fea6385f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -1237,29 +1237,34 @@ spec: - latestModelApplied type: object type: array - longRunningOperationState: - description: LongRunningOperationState saves the state for an Azure - long-running operations so it can be continued on the next reconciliation + longRunningOperationStates: + description: LongRunningOperationStates saves the state for Azure + long-running operations so they can be continued on the next reconciliation loop. - properties: - futureData: - description: FutureData is the base64 url encoded json Azure AutoRest - Future - type: string - name: - description: Name is the name of the Azure resource - type: string - resourceGroup: - description: ResourceGroup is the Azure resource group for the - resource - type: string - type: - description: Type describes the type of future, update, create, - delete, etc - type: string - required: - - type - type: object + items: + description: Future contains the data needed for an Azure long-running + operation to continue across reconcile loops. + properties: + futureData: + description: FutureData is the base64 url encoded json Azure + AutoRest Future + type: string + name: + description: Name is the name of the Azure resource + type: string + resourceGroup: + description: ResourceGroup is the Azure resource group for the + resource + type: string + type: + description: Type describes the type of future, update, create, + delete, etc + type: string + required: + - name + - type + type: object + type: array provisioningState: description: ProvisioningState is the provisioning state of the Azure virtual machine. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index 644701f84f0..11adc20ff5b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -940,6 +940,34 @@ spec: during the reconciliation of Machines can be added as events to the Machine object and/or logged in the controller's output." type: string + longRunningOperationStates: + description: LongRunningOperationStates saves the states for Azure + long-running operations so they can be continued on the next reconciliation + loop. + items: + description: Future contains the data needed for an Azure long-running + operation to continue across reconcile loops. + properties: + futureData: + description: FutureData is the base64 url encoded json Azure + AutoRest Future + type: string + name: + description: Name is the name of the Azure resource + type: string + resourceGroup: + description: ResourceGroup is the Azure resource group for the + resource + type: string + type: + description: Type describes the type of future, update, create, + delete, etc + type: string + required: + - name + - type + type: object + type: array ready: description: Ready is true when the provider resource is ready. type: boolean diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml index d3db0c8dbc6..70bf2c67501 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedcontrolplanes.yaml @@ -367,6 +367,34 @@ spec: fully ready. In the AzureManagedControlPlane implementation, these are identical. type: boolean + longRunningOperationStates: + description: LongRunningOperationStates saves the states for Azure + long-running operations so they can be continued on the next reconciliation + loop. + items: + description: Future contains the data needed for an Azure long-running + operation to continue across reconcile loops. + properties: + futureData: + description: FutureData is the base64 url encoded json Azure + AutoRest Future + type: string + name: + description: Name is the name of the Azure resource + type: string + resourceGroup: + description: ResourceGroup is the Azure resource group for the + resource + type: string + type: + description: Type describes the type of future, update, create, + delete, etc + type: string + required: + - name + - type + type: object + type: array ready: description: Ready is true when the provider resource is ready. type: boolean diff --git a/exp/api/v1alpha3/azuremachinepool_conversion.go b/exp/api/v1alpha3/azuremachinepool_conversion.go index 46061553b74..15ebc4f8029 100644 --- a/exp/api/v1alpha3/azuremachinepool_conversion.go +++ b/exp/api/v1alpha3/azuremachinepool_conversion.go @@ -106,3 +106,7 @@ func Convert_v1alpha4_AzureMachinePoolSpec_To_v1alpha3_AzureMachinePoolSpec(in * func Convert_v1alpha4_AzureMachinePoolStatus_To_v1alpha3_AzureMachinePoolStatus(in *expv1alpha4.AzureMachinePoolStatus, out *AzureMachinePoolStatus, s convert.Scope) error { return autoConvert_v1alpha4_AzureMachinePoolStatus_To_v1alpha3_AzureMachinePoolStatus(in, out, s) } + +func Convert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(in *AzureMachinePoolStatus, out *expv1alpha4.AzureMachinePoolStatus, s convert.Scope) error { + return autoConvert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(in, out, s) +} diff --git a/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go b/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go index 7e86dc15fb9..c49178686d7 100644 --- a/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go +++ b/exp/api/v1alpha3/azuremanagedcontrolplane_conversion.go @@ -62,3 +62,8 @@ func (dst *AzureManagedControlPlane) ConvertFrom(srcRaw conversion.Hub) error { func Convert_v1alpha4_AzureManagedControlPlaneSpec_To_v1alpha3_AzureManagedControlPlaneSpec(in *expv1alpha4.AzureManagedControlPlaneSpec, out *AzureManagedControlPlaneSpec, s apiconversion.Scope) error { return autoConvert_v1alpha4_AzureManagedControlPlaneSpec_To_v1alpha3_AzureManagedControlPlaneSpec(in, out, s) } + +// Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus is an autogenerated conversion function. +func Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(in *expv1alpha4.AzureManagedControlPlaneStatus, out *AzureManagedControlPlaneStatus, s apiconversion.Scope) error { + return autoConvert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(in, out, s) +} diff --git a/exp/api/v1alpha3/zz_generated.conversion.go b/exp/api/v1alpha3/zz_generated.conversion.go index cb0d982489e..881cf7ba233 100644 --- a/exp/api/v1alpha3/zz_generated.conversion.go +++ b/exp/api/v1alpha3/zz_generated.conversion.go @@ -90,11 +90,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*AzureMachinePoolStatus)(nil), (*v1alpha4.AzureMachinePoolStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(a.(*AzureMachinePoolStatus), b.(*v1alpha4.AzureMachinePoolStatus), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*AzureManagedCluster)(nil), (*v1alpha4.AzureManagedCluster)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_AzureManagedCluster_To_v1alpha4_AzureManagedCluster(a.(*AzureManagedCluster), b.(*v1alpha4.AzureManagedCluster), scope) }); err != nil { @@ -165,11 +160,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha4.AzureManagedControlPlaneStatus)(nil), (*AzureManagedControlPlaneStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(a.(*v1alpha4.AzureManagedControlPlaneStatus), b.(*AzureManagedControlPlaneStatus), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*AzureManagedMachinePool)(nil), (*v1alpha4.AzureManagedMachinePool)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_AzureManagedMachinePool_To_v1alpha4_AzureManagedMachinePool(a.(*AzureManagedMachinePool), b.(*v1alpha4.AzureManagedMachinePool), scope) }); err != nil { @@ -235,6 +225,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*AzureMachinePoolStatus)(nil), (*v1alpha4.AzureMachinePoolStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(a.(*AzureMachinePoolStatus), b.(*v1alpha4.AzureMachinePoolStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*clusterapiproviderazureapiv1alpha3.Image)(nil), (*clusterapiproviderazureapiv1alpha4.Image)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_Image_To_v1alpha4_Image(a.(*clusterapiproviderazureapiv1alpha3.Image), b.(*clusterapiproviderazureapiv1alpha4.Image), scope) }); err != nil { @@ -270,6 +265,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha4.AzureManagedControlPlaneStatus)(nil), (*AzureManagedControlPlaneStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(a.(*v1alpha4.AzureManagedControlPlaneStatus), b.(*AzureManagedControlPlaneStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*clusterapiproviderazureapiv1alpha4.Image)(nil), (*clusterapiproviderazureapiv1alpha3.Image)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_Image_To_v1alpha3_Image(a.(*clusterapiproviderazureapiv1alpha4.Image), b.(*clusterapiproviderazureapiv1alpha3.Image), scope) }); err != nil { @@ -505,15 +505,10 @@ func autoConvert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolSta out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) out.Conditions = *(*apiv1alpha4.Conditions)(unsafe.Pointer(&in.Conditions)) - out.LongRunningOperationState = (*clusterapiproviderazureapiv1alpha4.Future)(unsafe.Pointer(in.LongRunningOperationState)) + // WARNING: in.LongRunningOperationState requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus is an autogenerated conversion function. -func Convert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(in *AzureMachinePoolStatus, out *v1alpha4.AzureMachinePoolStatus, s conversion.Scope) error { - return autoConvert_v1alpha3_AzureMachinePoolStatus_To_v1alpha4_AzureMachinePoolStatus(in, out, s) -} - func autoConvert_v1alpha4_AzureMachinePoolStatus_To_v1alpha3_AzureMachinePoolStatus(in *v1alpha4.AzureMachinePoolStatus, out *AzureMachinePoolStatus, s conversion.Scope) error { out.Ready = in.Ready out.Replicas = in.Replicas @@ -524,7 +519,7 @@ func autoConvert_v1alpha4_AzureMachinePoolStatus_To_v1alpha3_AzureMachinePoolSta out.FailureReason = (*errors.MachineStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) out.Conditions = *(*apiv1alpha3.Conditions)(unsafe.Pointer(&in.Conditions)) - out.LongRunningOperationState = (*clusterapiproviderazureapiv1alpha3.Future)(unsafe.Pointer(in.LongRunningOperationState)) + // WARNING: in.LongRunningOperationStates requires manual conversion: does not exist in peer-type return nil } @@ -784,14 +779,10 @@ func Convert_v1alpha3_AzureManagedControlPlaneStatus_To_v1alpha4_AzureManagedCon func autoConvert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(in *v1alpha4.AzureManagedControlPlaneStatus, out *AzureManagedControlPlaneStatus, s conversion.Scope) error { out.Ready = in.Ready out.Initialized = in.Initialized + // WARNING: in.LongRunningOperationStates requires manual conversion: does not exist in peer-type return nil } -// Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus is an autogenerated conversion function. -func Convert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(in *v1alpha4.AzureManagedControlPlaneStatus, out *AzureManagedControlPlaneStatus, s conversion.Scope) error { - return autoConvert_v1alpha4_AzureManagedControlPlaneStatus_To_v1alpha3_AzureManagedControlPlaneStatus(in, out, s) -} - func autoConvert_v1alpha3_AzureManagedMachinePool_To_v1alpha4_AzureManagedMachinePool(in *AzureManagedMachinePool, out *v1alpha4.AzureManagedMachinePool, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1alpha3_AzureManagedMachinePoolSpec_To_v1alpha4_AzureManagedMachinePoolSpec(&in.Spec, &out.Spec, s); err != nil { diff --git a/exp/api/v1alpha4/azuremachinepool_types.go b/exp/api/v1alpha4/azuremachinepool_types.go index 9ada3a2becf..d5d8aa48922 100644 --- a/exp/api/v1alpha4/azuremachinepool_types.go +++ b/exp/api/v1alpha4/azuremachinepool_types.go @@ -282,10 +282,10 @@ type ( // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` - // LongRunningOperationState saves the state for an Azure long-running operations so it can be continued on the + // LongRunningOperationStates saves the state for Azure long-running operations so they can be continued on the // next reconciliation loop. // +optional - LongRunningOperationState *infrav1.Future `json:"longRunningOperationState,omitempty"` + LongRunningOperationStates infrav1.Futures `json:"longRunningOperationStates,omitempty"` } // AzureMachinePoolInstanceStatus provides status information for each instance in the VMSS. @@ -357,6 +357,16 @@ func (amp *AzureMachinePool) SetConditions(conditions clusterv1.Conditions) { amp.Status.Conditions = conditions } +// GetFutures returns the list of long running operation states for an AzureMachinePool API object. +func (amp *AzureMachinePool) GetFutures() infrav1.Futures { + return amp.Status.LongRunningOperationStates +} + +// SetFutures will set the given long running operation states on an AzureMachinePool object. +func (amp *AzureMachinePool) SetFutures(futures infrav1.Futures) { + amp.Status.LongRunningOperationStates = futures +} + func init() { SchemeBuilder.Register(&AzureMachinePool{}, &AzureMachinePoolList{}) } diff --git a/exp/api/v1alpha4/azuremachinepoolmachine_types.go b/exp/api/v1alpha4/azuremachinepoolmachine_types.go index 870e540b5a1..ee42713a9c3 100644 --- a/exp/api/v1alpha4/azuremachinepoolmachine_types.go +++ b/exp/api/v1alpha4/azuremachinepoolmachine_types.go @@ -83,10 +83,10 @@ type ( // +optional Conditions clusterv1.Conditions `json:"conditions,omitempty"` - // LongRunningOperationState saves the state for an Azure long running operations so it can be continued on the + // LongRunningOperationStates saves the state for Azure long running operations so they can be continued on the // next reconciliation loop. // +optional - LongRunningOperationState *infrav1.Future `json:"longRunningOperationState,omitempty"` + LongRunningOperationStates infrav1.Futures `json:"longRunningOperationStates,omitempty"` // LatestModelApplied indicates the instance is running the most up-to-date VMSS model. A VMSS model describes // the image version the VM is running. If the instance is not running the latest model, it means the instance @@ -136,6 +136,16 @@ func (ampm *AzureMachinePoolMachine) SetConditions(conditions clusterv1.Conditio ampm.Status.Conditions = conditions } +// GetFutures returns the list of long running operation states for an AzureCluster API object. +func (ampm *AzureMachinePoolMachine) GetFutures() infrav1.Futures { + return ampm.Status.LongRunningOperationStates +} + +// SetFutures will set the given long running operation states on an AzureCluster object. +func (ampm *AzureMachinePoolMachine) SetFutures(futures infrav1.Futures) { + ampm.Status.LongRunningOperationStates = futures +} + func init() { SchemeBuilder.Register(&AzureMachinePoolMachine{}, &AzureMachinePoolMachineList{}) } diff --git a/exp/api/v1alpha4/azuremanagedcontrolplane_types.go b/exp/api/v1alpha4/azuremanagedcontrolplane_types.go index 785ed874065..05fafec880e 100644 --- a/exp/api/v1alpha4/azuremanagedcontrolplane_types.go +++ b/exp/api/v1alpha4/azuremanagedcontrolplane_types.go @@ -124,6 +124,11 @@ type AzureManagedControlPlaneStatus struct { // In the AzureManagedControlPlane implementation, these are identical. // +optional Initialized bool `json:"initialized,omitempty"` + + // LongRunningOperationStates saves the states for Azure long-running operations so they can be continued on the + // next reconciliation loop. + // +optional + LongRunningOperationStates infrav1.Futures `json:"longRunningOperationStates,omitempty"` } // +kubebuilder:object:root=true @@ -149,6 +154,16 @@ type AzureManagedControlPlaneList struct { Items []AzureManagedControlPlane `json:"items"` } +// GetFutures returns the list of long running operation states for an AzureMachine API object. +func (m *AzureManagedControlPlane) GetFutures() infrav1.Futures { + return m.Status.LongRunningOperationStates +} + +// SetFutures will set the given long running operation states on an AzureMachine object. +func (m *AzureManagedControlPlane) SetFutures(futures infrav1.Futures) { + m.Status.LongRunningOperationStates = futures +} + func init() { SchemeBuilder.Register(&AzureManagedControlPlane{}, &AzureManagedControlPlaneList{}) } diff --git a/exp/api/v1alpha4/zz_generated.deepcopy.go b/exp/api/v1alpha4/zz_generated.deepcopy.go index a58fa5c93a3..54dbabef6b6 100644 --- a/exp/api/v1alpha4/zz_generated.deepcopy.go +++ b/exp/api/v1alpha4/zz_generated.deepcopy.go @@ -253,10 +253,10 @@ func (in *AzureMachinePoolMachineStatus) DeepCopyInto(out *AzureMachinePoolMachi (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.LongRunningOperationState != nil { - in, out := &in.LongRunningOperationState, &out.LongRunningOperationState - *out = new(apiv1alpha4.Future) - **out = **in + if in.LongRunningOperationStates != nil { + in, out := &in.LongRunningOperationStates, &out.LongRunningOperationStates + *out = make(apiv1alpha4.Futures, len(*in)) + copy(*out, *in) } } @@ -398,10 +398,10 @@ func (in *AzureMachinePoolStatus) DeepCopyInto(out *AzureMachinePoolStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.LongRunningOperationState != nil { - in, out := &in.LongRunningOperationState, &out.LongRunningOperationState - *out = new(apiv1alpha4.Future) - **out = **in + if in.LongRunningOperationStates != nil { + in, out := &in.LongRunningOperationStates, &out.LongRunningOperationStates + *out = make(apiv1alpha4.Futures, len(*in)) + copy(*out, *in) } } @@ -511,7 +511,7 @@ func (in *AzureManagedControlPlane) DeepCopyInto(out *AzureManagedControlPlane) out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureManagedControlPlane. @@ -621,6 +621,11 @@ func (in *AzureManagedControlPlaneSpec) DeepCopy() *AzureManagedControlPlaneSpec // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AzureManagedControlPlaneStatus) DeepCopyInto(out *AzureManagedControlPlaneStatus) { *out = *in + if in.LongRunningOperationStates != nil { + in, out := &in.LongRunningOperationStates, &out.LongRunningOperationStates + *out = make(apiv1alpha4.Futures, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureManagedControlPlaneStatus. diff --git a/test/e2e/aks.go b/test/e2e/aks.go index dee953ddf74..6e535650b7a 100644 --- a/test/e2e/aks.go +++ b/test/e2e/aks.go @@ -22,6 +22,8 @@ import ( "context" "errors" "fmt" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-30/compute" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2020-02-01/containerservice" "github.com/Azure/go-autorest/autorest/azure/auth" . "github.com/onsi/ginkgo" diff --git a/test/e2e/azure_machinepool_drain.go b/test/e2e/azure_machinepool_drain.go index 0371eeaf6ea..5a5ec3d4016 100644 --- a/test/e2e/azure_machinepool_drain.go +++ b/test/e2e/azure_machinepool_drain.go @@ -93,14 +93,14 @@ func AzureMachinePoolDrainSpec(ctx context.Context, inputGetter func() AzureMach func testMachinePoolCordonAndDrain(ctx context.Context, mgmtClusterProxy, workloadClusterProxy framework.ClusterProxy, amp v1alpha4.AzureMachinePool) { var ( - isWindows = amp.Spec.Template.OSDisk.OSType == azure.WindowsOS - clientset = workloadClusterProxy.GetClientSet() - owningMachinePool = func() *clusterv1exp.MachinePool { + isWindows = amp.Spec.Template.OSDisk.OSType == azure.WindowsOS + clientset = workloadClusterProxy.GetClientSet() + owningMachinePool = func() *clusterv1exp.MachinePool { mp, err := getOwnerMachinePool(ctx, mgmtClusterProxy.GetClient(), amp.ObjectMeta) Expect(err).ToNot(HaveOccurred()) return mp }() - + machinePoolReplicas = func() int32 { Expect(owningMachinePool.Spec.Replicas).ToNot(BeNil(), "owning machine pool replicas must not be nil") Expect(*owningMachinePool.Spec.Replicas).To(BeNumerically(">=", 2), "owning machine pool replicas must be greater than or equal to 2") diff --git a/test/e2e/retry.go b/test/e2e/retry.go index 989d372df46..7760c460121 100644 --- a/test/e2e/retry.go +++ b/test/e2e/retry.go @@ -19,8 +19,9 @@ limitations under the License. package e2e import ( - "k8s.io/apimachinery/pkg/util/wait" "time" + + "k8s.io/apimachinery/pkg/util/wait" ) const ( diff --git a/util/futures/getter.go b/util/futures/getter.go new file mode 100644 index 00000000000..c8ab3225890 --- /dev/null +++ b/util/futures/getter.go @@ -0,0 +1,52 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package futures + +import ( + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// Getter interface defines methods that an object should implement in order to +// use the futures package for getting long running operation states. +type Getter interface { + client.Object + + // GetFutures returns the list of long running operation states for an object. + GetFutures() infrav1.Futures +} + +// Get returns the future with the given name, if the future does not exists, +// it returns nil. +func Get(from Getter, name, service string) *infrav1.Future { + futures := from.GetFutures() + if futures == nil { + return nil + } + + for _, f := range futures { + if f.Name == name && f.ServiceName == service { + return &f + } + } + return nil +} + +// Has returns true if a future with the given name exists. +func Has(from Getter, name, service string) bool { + return Get(from, name, service) != nil +} diff --git a/util/futures/getter_test.go b/util/futures/getter_test.go new file mode 100644 index 00000000000..274d8ed6cd1 --- /dev/null +++ b/util/futures/getter_test.go @@ -0,0 +1,75 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package futures + +import ( + "testing" + + . "github.com/onsi/gomega" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" +) + +func TestGet(t *testing.T) { + g := NewWithT(t) + + azurecluster := &infrav1.AzureCluster{} + + vmName := "my-vm" + vnetName := "my-vnet" + vm := "virtualmachines" + vnet := "virtualnetworks" + vmFuture := fakeFuture(vmName) + vnetFuture := fakeFuture(vnetName) + + g.Expect(Get(azurecluster, vmName, vm)).To(BeNil()) + g.Expect(Get(azurecluster, vnetName, vnet)).To(BeNil()) + + azurecluster.SetFutures(infrav1.Futures{vmFuture, vnetFuture}) + + g.Expect(Get(azurecluster, vmName, vm)).To(Equal(vmFuture)) + g.Expect(Get(azurecluster, vmName, vnet)).To(BeNil()) + g.Expect(Get(azurecluster, vnetName, vnet)).To(Equal(vnetFuture)) +} + +func TestHas(t *testing.T) { + g := NewWithT(t) + + azurecluster := &infrav1.AzureCluster{} + + vmName := "my-vm" + vm := "virtualmachines" + vnet := "virtualnetworks" + vmFuture := fakeFuture(vmName) + + g.Expect(Has(azurecluster, vmName, vm)).To(BeFalse()) + g.Expect(Has(azurecluster, "foo", vm)).To(BeFalse()) + + azurecluster.SetFutures(infrav1.Futures{vmFuture}) + + g.Expect(Has(azurecluster, vmName, vm)).To(BeTrue()) + g.Expect(Has(azurecluster, "foo", vm)).To(BeFalse()) + g.Expect(Has(azurecluster, vmName, vnet)).To(BeFalse()) +} + +func fakeFuture(name string) infrav1.Future { + return infrav1.Future{ + Type: "PUT", + Name: name, + ResourceGroup: "test-rg", + Data: "", + } +} diff --git a/util/futures/setter.go b/util/futures/setter.go new file mode 100644 index 00000000000..5ee0a84e4c3 --- /dev/null +++ b/util/futures/setter.go @@ -0,0 +1,70 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package futures + +import ( + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" +) + +// Setter interface defines methods that an object should implement in order to +// use the futures package for setting futures. +type Setter interface { + Getter + SetFutures(infrav1.Futures) +} + +// Set sets the given future. +// +// NOTE: If a future already exists, we update it. +func Set(to Setter, future *infrav1.Future) { + if to == nil || future == nil { + return + } + + // Check if the new future already exists, and update it if it does. + futures := to.GetFutures() + exists := false + for i, f := range futures { + if f.Name == future.Name && f.ServiceName == future.ServiceName { + exists = true + futures[i] = *future + } + } + + // If the future does not exist, add it. + if !exists { + futures = append(futures, *future) + } + + to.SetFutures(futures) +} + +// Delete deletes the specified future. +func Delete(to Setter, name, service string) { + if to == nil || name == "" || service == "" { + return + } + + futures := to.GetFutures() + for i := len(futures) - 1; i >= 0; i-- { + if futures[i].Name == name && futures[i].ServiceName == service { + futures = append(futures[:i], futures[i+1:]...) + } + } + + to.SetFutures(futures) +} diff --git a/util/futures/setter_test.go b/util/futures/setter_test.go new file mode 100644 index 00000000000..716667d89c1 --- /dev/null +++ b/util/futures/setter_test.go @@ -0,0 +1,116 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package futures + +import ( + "testing" + + . "github.com/onsi/gomega" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" +) + +func TestSet(t *testing.T) { + a := fakeFuture("a") + b := fakeFuture("b") + newA := a + newA.Data = "new" + + tests := []struct { + name string + to Setter + future *infrav1.Future + want infrav1.Futures + }{ + { + name: "Set adds a future", + to: setterWithFutures(infrav1.Futures{}), + future: &a, + want: infrav1.Futures{a}, + }, + { + name: "Set adds more futures", + to: setterWithFutures(infrav1.Futures{a}), + future: &b, + want: infrav1.Futures{a, b}, + }, + { + name: "Set does not duplicate existing future", + to: setterWithFutures(infrav1.Futures{a, b}), + future: &a, + want: infrav1.Futures{a, b}, + }, + { + name: "Set updates an existing future", + to: setterWithFutures(infrav1.Futures{a, b}), + future: &newA, + want: infrav1.Futures{newA, b}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + Set(tt.to, tt.future) + + g.Expect(tt.to.GetFutures()).To(Equal(tt.want)) + }) + } +} + +func TestDelete(t *testing.T) { + a := fakeFuture("a") + b := fakeFuture("b") + c := fakeFuture("c") + d := fakeFuture("d") + + tests := []struct { + name string + to Setter + future *infrav1.Future + want infrav1.Futures + }{ + { + name: "Delete removes a future", + to: setterWithFutures(infrav1.Futures{a, b, c, d}), + future: &b, + want: infrav1.Futures{a, c, d}, + }, + { + name: "Delete does nothing if the future does not exist", + to: setterWithFutures(infrav1.Futures{a}), + future: &b, + want: infrav1.Futures{a}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + Set(tt.to, tt.future) + + g.Expect(tt.to.GetFutures()).To(Equal(tt.want)) + }) + } +} + +func setterWithFutures(futures infrav1.Futures) Setter { + obj := &infrav1.AzureCluster{} + obj.SetFutures(futures) + return obj +} diff --git a/util/reconciler/defaults.go b/util/reconciler/defaults.go index e6a8b6e59a5..adabdb7c5bd 100644 --- a/util/reconciler/defaults.go +++ b/util/reconciler/defaults.go @@ -22,9 +22,13 @@ import ( const ( // DefaultLoopTimeout is the default timeout for a reconcile loop (defaulted to the max ARM template duration). - DefaultLoopTimeout = 90 * time.Minute + DefaultLoopTimeout = 1 * time.Minute // DefaultMappingTimeout is the default timeout for a controller request mapping func. - DefaultMappingTimeout = 60 * time.Second + DefaultMappingTimeout = 30 * time.Second // TODO: can this really take 30 seconds? + // DefaultAzureServiceReconcileTimeout is the default timeout for an Azure service reconcile. + DefaultAzureServiceReconcileTimeout = 12 * time.Second + // DefaultAzureCallTimeout is the default timeout for an Azure request after which an Azure operation is considered long running. + DefaultAzureCallTimeout = 2 * time.Second ) // DefaultedLoopTimeout will default the timeout if it is zero-valued.