From da0975a4c8dfb4c68cb5f34972ef8f6fafbd6389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Prokopi=C4=87?= Date: Thu, 12 Aug 2021 16:25:44 +0200 Subject: [PATCH] Refactor managed machine pool --- azure/interfaces.go | 8 -- azure/mocks/service_mock.go | 51 ------- azure/scope/managedcontrolplane.go | 59 ++++++++ azure/services/agentpools/agentpools.go | 50 ++++--- azure/services/agentpools/agentpools_test.go | 136 ++++++++++++------ azure/services/agentpools/service.go | 33 ----- azure/types.go | 31 +++- .../azuremanagedmachinepool_controller.go | 4 +- .../azuremanagedmachinepool_reconciler.go | 84 ++++------- 9 files changed, 231 insertions(+), 225 deletions(-) delete mode 100644 azure/services/agentpools/service.go diff --git a/azure/interfaces.go b/azure/interfaces.go index 27b3f0a091d..6001f0c9c85 100644 --- a/azure/interfaces.go +++ b/azure/interfaces.go @@ -31,14 +31,6 @@ type Reconciler interface { Delete(ctx context.Context) error } -// OldService is a generic interface for services that have not yet been refactored. -// Once all services have been converted to use Service, this should be removed. -// Example: virtualnetworks service would offer Reconcile/Delete methods. -type OldService interface { - Reconcile(ctx context.Context, spec interface{}) error - Delete(ctx context.Context, spec interface{}) error -} - // CredentialGetter is a Service which knows how to retrieve credentials for an Azure // resource in a resource group. type CredentialGetter interface { diff --git a/azure/mocks/service_mock.go b/azure/mocks/service_mock.go index f5bd6ffe867..fec8422ec57 100644 --- a/azure/mocks/service_mock.go +++ b/azure/mocks/service_mock.go @@ -80,57 +80,6 @@ func (mr *MockReconcilerMockRecorder) Reconcile(ctx interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reconcile", reflect.TypeOf((*MockReconciler)(nil).Reconcile), ctx) } -// MockOldService is a mock of OldService interface. -type MockOldService struct { - ctrl *gomock.Controller - recorder *MockOldServiceMockRecorder -} - -// MockOldServiceMockRecorder is the mock recorder for MockOldService. -type MockOldServiceMockRecorder struct { - mock *MockOldService -} - -// NewMockOldService creates a new mock instance. -func NewMockOldService(ctrl *gomock.Controller) *MockOldService { - mock := &MockOldService{ctrl: ctrl} - mock.recorder = &MockOldServiceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockOldService) EXPECT() *MockOldServiceMockRecorder { - return m.recorder -} - -// Delete mocks base method. -func (m *MockOldService) Delete(ctx context.Context, spec interface{}) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", ctx, spec) - ret0, _ := ret[0].(error) - return ret0 -} - -// Delete indicates an expected call of Delete. -func (mr *MockOldServiceMockRecorder) Delete(ctx, spec interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockOldService)(nil).Delete), ctx, spec) -} - -// Reconcile mocks base method. -func (m *MockOldService) Reconcile(ctx context.Context, spec interface{}) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Reconcile", ctx, spec) - ret0, _ := ret[0].(error) - return ret0 -} - -// Reconcile indicates an expected call of Reconcile. -func (mr *MockOldServiceMockRecorder) Reconcile(ctx, spec interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Reconcile", reflect.TypeOf((*MockOldService)(nil).Reconcile), ctx, spec) -} - // MockCredentialGetter is a mock of CredentialGetter interface. type MockCredentialGetter struct { ctrl *gomock.Controller diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 5337901fd0a..da36caa09df 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -127,6 +127,14 @@ func (s *ManagedControlPlaneScope) ResourceGroup() string { return s.ControlPlane.Spec.ResourceGroupName } +// NodeResourceGroup returns the managed control plane's node resource group. +func (s *ManagedControlPlaneScope) NodeResourceGroup() string { + if s.ControlPlane == nil { + return "" + } + return s.ControlPlane.Spec.NodeResourceGroupName +} + // ClusterName returns the managed control plane's name. func (s *ManagedControlPlaneScope) ClusterName() string { return s.Cluster.Name @@ -450,6 +458,57 @@ func (s *ManagedControlPlaneScope) GetSystemAgentPoolSpecs(ctx context.Context) return ammps, nil } +// AgentPoolSpec returns an azure.AgentPoolSpec for currently reconciled AzureManagedMachinePool. +func (s *ManagedControlPlaneScope) AgentPoolSpec() azure.AgentPoolSpec { + var normalizedVersion *string + if s.MachinePool.Spec.Template.Spec.Version != nil { + v := strings.TrimPrefix(*s.MachinePool.Spec.Template.Spec.Version, "v") + normalizedVersion = &v + } + + replicas := int32(1) + if s.MachinePool.Spec.Replicas != nil { + replicas = *s.MachinePool.Spec.Replicas + } + + agentPoolSpec := azure.AgentPoolSpec{ + Name: s.InfraMachinePool.Name, + ResourceGroup: s.ControlPlane.Spec.ResourceGroupName, + Cluster: s.ControlPlane.Name, + SKU: s.InfraMachinePool.Spec.SKU, + Replicas: replicas, + Version: normalizedVersion, + VnetSubnetID: azure.SubnetID( + s.ControlPlane.Spec.SubscriptionID, + s.ControlPlane.Spec.ResourceGroupName, + s.ControlPlane.Spec.VirtualNetwork.Name, + s.ControlPlane.Spec.VirtualNetwork.Subnet.Name, + ), + Mode: s.InfraMachinePool.Spec.Mode, + } + + if s.InfraMachinePool.Spec.OSDiskSizeGB != nil { + agentPoolSpec.OSDiskSizeGB = *s.InfraMachinePool.Spec.OSDiskSizeGB + } + + return agentPoolSpec +} + +// SetAgentPoolProviderIDList sets a list of agent pool's Azure VM IDs. +func (s *ManagedControlPlaneScope) SetAgentPoolProviderIDList(providerIDs []string) { + s.InfraMachinePool.Spec.ProviderIDList = providerIDs +} + +// SetAgentPoolReplicas sets the number of agent pool replicas. +func (s *ManagedControlPlaneScope) SetAgentPoolReplicas(replicas int32) { + s.InfraMachinePool.Status.Replicas = replicas +} + +// SetAgentPoolReady sets the flag that indicates if the agent pool is ready or not. +func (s *ManagedControlPlaneScope) SetAgentPoolReady(ready bool) { + s.InfraMachinePool.Status.Ready = ready +} + // SetControlPlaneEndpoint sets a control plane endpoint. func (s *ManagedControlPlaneScope) SetControlPlaneEndpoint(endpoint clusterv1.APIEndpoint) { s.ControlPlane.Spec.ControlPlaneEndpoint = endpoint diff --git a/azure/services/agentpools/agentpools.go b/azure/services/agentpools/agentpools.go index 986ba8b3b60..d5eecd13583 100644 --- a/azure/services/agentpools/agentpools.go +++ b/azure/services/agentpools/agentpools.go @@ -21,6 +21,7 @@ import ( "fmt" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-05-01/containerservice" + "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" "k8s.io/klog/v2" @@ -30,28 +31,38 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -// Spec contains properties to create a agent pool. -type Spec struct { - Name string - ResourceGroup string - Cluster string - Version *string - SKU string - Replicas int32 - OSDiskSizeGB int32 - VnetSubnetID string - Mode string +// ManagedMachinePoolScope defines the scope interface for a managed machine pool. +type ManagedMachinePoolScope interface { + logr.Logger + azure.ClusterDescriber + + NodeResourceGroup() string + AgentPoolSpec() azure.AgentPoolSpec + SetAgentPoolProviderIDList([]string) + SetAgentPoolReplicas(int32) + SetAgentPoolReady(bool) +} + +// Service provides operations on Azure resources. +type Service struct { + scope ManagedMachinePoolScope + Client +} + +// New creates a new service. +func New(scope ManagedMachinePoolScope) *Service { + return &Service{ + scope: scope, + Client: NewClient(scope), + } } // Reconcile idempotently creates or updates a agent pool, if possible. -func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { +func (s *Service) Reconcile(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "agentpools.Service.Reconcile") defer span.End() - agentPoolSpec, ok := spec.(*Spec) - if !ok { - return errors.New("invalid agent pool specification") - } + agentPoolSpec := s.scope.AgentPoolSpec() profile := containerservice.AgentPool{ ManagedClusterAgentPoolProfileProperties: &containerservice.ManagedClusterAgentPoolProfileProperties{ @@ -123,14 +134,11 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { } // Delete deletes the virtual network with the provided name. -func (s *Service) Delete(ctx context.Context, spec interface{}) error { +func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "agentpools.Service.Delete") defer span.End() - agentPoolSpec, ok := spec.(*Spec) - if !ok { - return errors.New("invalid agent pool specification") - } + agentPoolSpec := s.scope.AgentPoolSpec() klog.V(2).Infof("deleting agent pool %s ", agentPoolSpec.Name) err := s.Client.Delete(ctx, agentPoolSpec.ResourceGroup, agentPoolSpec.Cluster, agentPoolSpec.Name) diff --git a/azure/services/agentpools/agentpools_test.go b/azure/services/agentpools/agentpools_test.go index 47803ed9968..525dbce9cf5 100644 --- a/azure/services/agentpools/agentpools_test.go +++ b/azure/services/agentpools/agentpools_test.go @@ -22,55 +22,32 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2021-05-01/containerservice" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + capi "sigs.k8s.io/cluster-api/api/v1alpha4" + capiexp "sigs.k8s.io/cluster-api/exp/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/scope" "sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools/mock_agentpools" + infraexpv1 "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha4" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) -const ( - expectedInvalidSpec = "invalid agent pool specification" -) - -func TestInvalidAgentPoolsSpec(t *testing.T) { - g := NewWithT(t) - - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - agentpoolsMock := mock_agentpools.NewMockClient(mockCtrl) - - s := &Service{ - Client: agentpoolsMock, - } - - // Wrong Spec - wrongSpec := &network.LoadBalancer{} - - err := s.Reconcile(context.TODO(), &wrongSpec) - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(expectedInvalidSpec)) - - err = s.Delete(context.TODO(), &wrongSpec) - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(expectedInvalidSpec)) -} - func TestReconcile(t *testing.T) { provisioningstatetestcases := []struct { name string - agentpoolSpec Spec + agentpoolSpec azure.AgentPoolSpec provisioningStatesToTest []string expectedError string expect func(m *mock_agentpools.MockClientMockRecorder, provisioningstate string) }{ { name: "agentpool in terminal provisioning state", - agentpoolSpec: Spec{ + agentpoolSpec: azure.AgentPoolSpec{ ResourceGroup: "my-rg", Cluster: "my-cluster", Name: "my-agentpool", @@ -86,7 +63,7 @@ func TestReconcile(t *testing.T) { }, { name: "agentpool in nonterminal provisioning state", - agentpoolSpec: Spec{ + agentpoolSpec: azure.AgentPoolSpec{ ResourceGroup: "my-rg", Cluster: "my-cluster", Name: "my-agentpool", @@ -113,14 +90,31 @@ func TestReconcile(t *testing.T) { defer mockCtrl.Finish() agentpoolsMock := mock_agentpools.NewMockClient(mockCtrl) + machinePoolScope := &scope.ManagedControlPlaneScope{ + ControlPlane: &infraexpv1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.agentpoolSpec.Cluster, + }, + Spec: infraexpv1.AzureManagedControlPlaneSpec{ + ResourceGroupName: tc.agentpoolSpec.ResourceGroup, + }, + }, + MachinePool: &capiexp.MachinePool{}, + InfraMachinePool: &infraexpv1.AzureManagedMachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.agentpoolSpec.Name, + }, + }, + } tc.expect(agentpoolsMock.EXPECT(), provisioningstate) s := &Service{ Client: agentpoolsMock, + scope: machinePoolScope, } - err := s.Reconcile(context.TODO(), &tc.agentpoolSpec) + err := s.Reconcile(context.TODO()) if tc.expectedError != "" { g.Expect(err.Error()).To(HavePrefix(tc.expectedError)) g.Expect(err.Error()).To(ContainSubstring(provisioningstate)) @@ -133,13 +127,13 @@ func TestReconcile(t *testing.T) { testcases := []struct { name string - agentPoolsSpec Spec + agentPoolsSpec azure.AgentPoolSpec expectedError string expect func(m *mock_agentpools.MockClientMockRecorder) }{ { name: "no agentpool exists", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ ResourceGroup: "my-rg", Cluster: "my-cluster", Name: "my-agentpool", @@ -152,7 +146,7 @@ func TestReconcile(t *testing.T) { }, { name: "fail to get existing agent pool", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -168,7 +162,7 @@ func TestReconcile(t *testing.T) { }, { name: "can create an Agent Pool", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -185,7 +179,7 @@ func TestReconcile(t *testing.T) { }, { name: "fail to create an Agent Pool", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -202,7 +196,7 @@ func TestReconcile(t *testing.T) { }, { name: "fail to update an Agent Pool", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -227,7 +221,7 @@ func TestReconcile(t *testing.T) { }, { name: "no update needed on Agent Pool", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -262,15 +256,48 @@ func TestReconcile(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + replicas := tc.agentPoolsSpec.Replicas + osDiskSizeGB := tc.agentPoolsSpec.OSDiskSizeGB + agentpoolsMock := mock_agentpools.NewMockClient(mockCtrl) + machinePoolScope := &scope.ManagedControlPlaneScope{ + ControlPlane: &infraexpv1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.agentPoolsSpec.Cluster, + }, + Spec: infraexpv1.AzureManagedControlPlaneSpec{ + ResourceGroupName: tc.agentPoolsSpec.ResourceGroup, + }, + }, + MachinePool: &capiexp.MachinePool{ + Spec: capiexp.MachinePoolSpec{ + Replicas: &replicas, + Template: capi.MachineTemplateSpec{ + Spec: capi.MachineSpec{ + Version: tc.agentPoolsSpec.Version, + }, + }, + }, + }, + InfraMachinePool: &infraexpv1.AzureManagedMachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.agentPoolsSpec.Name, + }, + Spec: infraexpv1.AzureManagedMachinePoolSpec{ + SKU: tc.agentPoolsSpec.SKU, + OSDiskSizeGB: &osDiskSizeGB, + }, + }, + } tc.expect(agentpoolsMock.EXPECT()) s := &Service{ Client: agentpoolsMock, + scope: machinePoolScope, } - err := s.Reconcile(context.TODO(), &tc.agentPoolsSpec) + err := s.Reconcile(context.TODO()) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(tc.expectedError)) @@ -284,13 +311,13 @@ func TestReconcile(t *testing.T) { func TestDeleteAgentPools(t *testing.T) { testcases := []struct { name string - agentPoolsSpec Spec + agentPoolsSpec azure.AgentPoolSpec expectedError string expect func(m *mock_agentpools.MockClientMockRecorder) }{ { name: "successfully delete an existing agent pool", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -302,7 +329,7 @@ func TestDeleteAgentPools(t *testing.T) { }, { name: "agent pool already deleted", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -315,7 +342,7 @@ func TestDeleteAgentPools(t *testing.T) { }, { name: "agent pool deletion fails", - agentPoolsSpec: Spec{ + agentPoolsSpec: azure.AgentPoolSpec{ Name: "my-agent-pool", ResourceGroup: "my-rg", Cluster: "my-cluster", @@ -337,14 +364,31 @@ func TestDeleteAgentPools(t *testing.T) { defer mockCtrl.Finish() agentPoolsMock := mock_agentpools.NewMockClient(mockCtrl) + machinePoolScope := &scope.ManagedControlPlaneScope{ + ControlPlane: &infraexpv1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.agentPoolsSpec.Cluster, + }, + Spec: infraexpv1.AzureManagedControlPlaneSpec{ + ResourceGroupName: tc.agentPoolsSpec.ResourceGroup, + }, + }, + MachinePool: &capiexp.MachinePool{}, + InfraMachinePool: &infraexpv1.AzureManagedMachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.agentPoolsSpec.Name, + }, + }, + } tc.expect(agentPoolsMock.EXPECT()) s := &Service{ Client: agentPoolsMock, + scope: machinePoolScope, } - err := s.Delete(context.TODO(), &tc.agentPoolsSpec) + err := s.Delete(context.TODO()) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err).To(MatchError(tc.expectedError)) diff --git a/azure/services/agentpools/service.go b/azure/services/agentpools/service.go deleted file mode 100644 index 6f84e3b16f9..00000000000 --- a/azure/services/agentpools/service.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2020 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 agentpools - -import ( - "sigs.k8s.io/cluster-api-provider-azure/azure" -) - -// Service provides operations on Azure resources. -type Service struct { - Client -} - -// NewService creates a new service. -func NewService(auth azure.Authorizer) *Service { - return &Service{ - Client: NewClient(auth), - } -} diff --git a/azure/types.go b/azure/types.go index 4920bb21c2c..03ef22c5913 100644 --- a/azure/types.go +++ b/azure/types.go @@ -20,6 +20,7 @@ import ( "reflect" "github.com/google/go-cmp/cmp" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha4" ) @@ -156,7 +157,7 @@ type BastionSpec struct { } // AzureBastionSpec defines the specification for azure bastion feature. -type AzureBastionSpec struct { //nolint +type AzureBastionSpec struct { // nolint Name string SubnetSpec infrav1.SubnetSpec PublicIPName string @@ -358,8 +359,30 @@ type ManagedClusterSpec struct { // AgentPoolSpec contains agent pool specification details. type AgentPoolSpec struct { - Name string - SKU string - Replicas int32 + // Name is the name of agent pool. + Name string + + // ResourceGroup is the name of the Azure resource group for the AKS Cluster. + ResourceGroup string + + // Cluster is the name of the AKS cluster. + Cluster string + + // Version defines the desired Kubernetes version. + Version *string + + // SKU defines the Azure VM size for the agent pool VMs. + SKU string + + // Replicas is the number of desired machines. + Replicas int32 + + // OSDiskSizeGB is the OS disk size in GB for every machine in this agent pool. OSDiskSizeGB int32 + + // VnetSubnetID is the Azure Resource ID for the subnet which should contain nodes. + VnetSubnetID string + + // Mode represents mode of an agent pool. Possible values include: 'System', 'User'. + Mode string } diff --git a/exp/controllers/azuremanagedmachinepool_controller.go b/exp/controllers/azuremanagedmachinepool_controller.go index 0089bc3e9d5..131af62ad14 100644 --- a/exp/controllers/azuremanagedmachinepool_controller.go +++ b/exp/controllers/azuremanagedmachinepool_controller.go @@ -233,7 +233,7 @@ func (r *AzureManagedMachinePoolReconciler) reconcileNormal(ctx context.Context, return reconcile.Result{}, err } - if err := r.createAzureManagedMachinePoolService(scope).Reconcile(ctx, scope); err != nil { + if err := r.createAzureManagedMachinePoolService(scope).Reconcile(ctx); err != nil { if IsAgentPoolVMSSNotFoundError(err) { // if the underlying VMSS is not yet created, requeue for 30s in the future return reconcile.Result{ @@ -260,7 +260,7 @@ func (r *AzureManagedMachinePoolReconciler) reconcileDelete(ctx context.Context, // So, remove the finalizer. controllerutil.RemoveFinalizer(scope.InfraMachinePool, infrav1.ClusterFinalizer) } else { - if err := r.createAzureManagedMachinePoolService(scope).Delete(ctx, scope); err != nil { + if err := r.createAzureManagedMachinePoolService(scope).Delete(ctx); err != nil { return reconcile.Result{}, errors.Wrapf(err, "error deleting AzureManagedMachinePool %s/%s", scope.InfraMachinePool.Namespace, scope.InfraMachinePool.Name) } // Machine pool successfully deleted, remove the finalizer. diff --git a/exp/controllers/azuremanagedmachinepool_reconciler.go b/exp/controllers/azuremanagedmachinepool_reconciler.go index 52c992178ce..51cf2d7a43f 100644 --- a/exp/controllers/azuremanagedmachinepool_reconciler.go +++ b/exp/controllers/azuremanagedmachinepool_reconciler.go @@ -23,20 +23,19 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-06-30/compute" "github.com/pkg/errors" + "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/scope" "sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools" "sigs.k8s.io/cluster-api-provider-azure/azure/services/scalesets" "sigs.k8s.io/cluster-api-provider-azure/util/tele" - - "sigs.k8s.io/controller-runtime/pkg/client" ) type ( // azureManagedMachinePoolService contains the services required by the cluster controller. azureManagedMachinePoolService struct { - kubeclient client.Client - agentPoolsSvc azure.OldService + scope agentpools.ManagedMachinePoolScope + agentPoolsSvc azure.Reconciler scaleSetsSvc NodeLister } @@ -80,75 +79,46 @@ func (a *AgentPoolVMSSNotFoundError) Is(target error) bool { // newAzureManagedMachinePoolService populates all the services based on input scope. func newAzureManagedMachinePoolService(scope *scope.ManagedControlPlaneScope) *azureManagedMachinePoolService { return &azureManagedMachinePoolService{ - kubeclient: scope.Client, - agentPoolsSvc: agentpools.NewService(scope), + scope: scope, + agentPoolsSvc: agentpools.New(scope), scaleSetsSvc: scalesets.NewClient(scope), } } // Reconcile reconciles all the services in a predetermined order. -func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context, scope *scope.ManagedControlPlaneScope) error { +func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "controllers.azureManagedMachinePoolService.Reconcile") defer span.End() - scope.Logger.Info("reconciling machine pool") - - var normalizedVersion *string - if scope.MachinePool.Spec.Template.Spec.Version != nil { - v := strings.TrimPrefix(*scope.MachinePool.Spec.Template.Spec.Version, "v") - normalizedVersion = &v - } - - replicas := int32(1) - if scope.MachinePool.Spec.Replicas != nil { - replicas = *scope.MachinePool.Spec.Replicas - } - - agentPoolSpec := &agentpools.Spec{ - Name: scope.InfraMachinePool.Name, - ResourceGroup: scope.ControlPlane.Spec.ResourceGroupName, - Cluster: scope.ControlPlane.Name, - SKU: scope.InfraMachinePool.Spec.SKU, - Replicas: replicas, - Version: normalizedVersion, - VnetSubnetID: azure.SubnetID( - scope.ControlPlane.Spec.SubscriptionID, - scope.ControlPlane.Spec.ResourceGroupName, - scope.ControlPlane.Spec.VirtualNetwork.Name, - scope.ControlPlane.Spec.VirtualNetwork.Subnet.Name, - ), - Mode: scope.InfraMachinePool.Spec.Mode, - } - - if scope.InfraMachinePool.Spec.OSDiskSizeGB != nil { - agentPoolSpec.OSDiskSizeGB = *scope.InfraMachinePool.Spec.OSDiskSizeGB - } + s.scope.Info("reconciling machine pool") + agentPoolName := s.scope.AgentPoolSpec().Name - if err := s.agentPoolsSvc.Reconcile(ctx, agentPoolSpec); err != nil { - return errors.Wrapf(err, "failed to reconcile machine pool %s", scope.InfraMachinePool.Name) + if err := s.agentPoolsSvc.Reconcile(ctx); err != nil { + return errors.Wrapf(err, "failed to reconcile machine pool %s", agentPoolName) } - vmss, err := s.scaleSetsSvc.List(ctx, scope.ControlPlane.Spec.NodeResourceGroupName) + nodeResourceGroup := s.scope.NodeResourceGroup() + vmss, err := s.scaleSetsSvc.List(ctx, nodeResourceGroup) if err != nil { - return errors.Wrapf(err, "failed to list vmss in resource group %s", scope.ControlPlane.Spec.NodeResourceGroupName) + return errors.Wrapf(err, "failed to list vmss in resource group %s", nodeResourceGroup) } var match *compute.VirtualMachineScaleSet for _, ss := range vmss { ss := ss - if ss.Tags["poolName"] != nil && *ss.Tags["poolName"] == scope.InfraMachinePool.Name { + if ss.Tags["poolName"] != nil && *ss.Tags["poolName"] == agentPoolName { match = &ss break } } if match == nil { - return NewAgentPoolVMSSNotFoundError(scope.ControlPlane.Spec.NodeResourceGroupName, scope.InfraMachinePool.Name) + return NewAgentPoolVMSSNotFoundError(nodeResourceGroup, agentPoolName) } - instances, err := s.scaleSetsSvc.ListInstances(ctx, scope.ControlPlane.Spec.NodeResourceGroupName, *match.Name) + instances, err := s.scaleSetsSvc.ListInstances(ctx, nodeResourceGroup, *match.Name) if err != nil { - return errors.Wrapf(err, "failed to reconcile machine pool %s", scope.InfraMachinePool.Name) + return errors.Wrapf(err, "failed to reconcile machine pool %s", agentPoolName) } var providerIDs = make([]string, len(instances)) @@ -156,27 +126,21 @@ func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context, scope *s providerIDs[i] = strings.ToLower(azure.ProviderIDPrefix + *instances[i].ID) } - scope.InfraMachinePool.Spec.ProviderIDList = providerIDs - scope.InfraMachinePool.Status.Replicas = int32(len(providerIDs)) - scope.InfraMachinePool.Status.Ready = true + s.scope.SetAgentPoolProviderIDList(providerIDs) + s.scope.SetAgentPoolReplicas(int32(len(providerIDs))) + s.scope.SetAgentPoolReady(true) - scope.Logger.Info("reconciled machine pool successfully") + s.scope.Info("reconciled machine pool successfully") return nil } // Delete reconciles all the services in a predetermined order. -func (s *azureManagedMachinePoolService) Delete(ctx context.Context, scope *scope.ManagedControlPlaneScope) error { +func (s *azureManagedMachinePoolService) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "controllers.azureManagedMachinePoolService.Delete") defer span.End() - agentPoolSpec := &agentpools.Spec{ - Name: scope.InfraMachinePool.Name, - ResourceGroup: scope.ControlPlane.Spec.ResourceGroupName, - Cluster: scope.ControlPlane.Name, - } - - if err := s.agentPoolsSvc.Delete(ctx, agentPoolSpec); err != nil { - return errors.Wrapf(err, "failed to delete machine pool %s", scope.InfraMachinePool.Name) + if err := s.agentPoolsSvc.Delete(ctx); err != nil { + return errors.Wrapf(err, "failed to delete machine pool %s", s.scope.AgentPoolSpec().Name) } return nil