From 7d2df5d6919ac6d63bb15acd9dc604aaf82a40f8 Mon Sep 17 00:00:00 2001 From: Andreas Sommer Date: Mon, 30 Oct 2023 14:22:22 +0100 Subject: [PATCH] Trigger machine pool instance refresh (node rollout) if bootstrap config reference changes --- api/v1beta2/tags.go | 5 + exp/controllers/awsmachinepool_controller.go | 18 +- .../awsmachinepool_controller_test.go | 320 ++++++++++++++++-- .../awsmanagedmachinepool_controller.go | 11 +- pkg/cloud/scope/launchtemplate.go | 3 +- pkg/cloud/scope/machinepool.go | 24 +- pkg/cloud/scope/managednodegroup.go | 10 +- pkg/cloud/services/ec2/launchtemplate.go | 101 ++++-- pkg/cloud/services/ec2/launchtemplate_test.go | 128 +++++-- pkg/cloud/services/interfaces.go | 19 +- pkg/cloud/services/mock_services/doc.go | 2 + .../mock_services/ec2_interface_mock.go | 52 +-- .../mock_services/reconcile_interface_mock.go | 80 +++++ 13 files changed, 613 insertions(+), 160 deletions(-) create mode 100644 pkg/cloud/services/mock_services/reconcile_interface_mock.go diff --git a/api/v1beta2/tags.go b/api/v1beta2/tags.go index 829dd514c6..e6e0ea7e73 100644 --- a/api/v1beta2/tags.go +++ b/api/v1beta2/tags.go @@ -190,6 +190,11 @@ const ( // MachineNameTagKey is the key for machine name. MachineNameTagKey = "MachineName" + + // LaunchTemplateBootstrapDataSecret is the tag we use to store the `/` + // of the bootstrap secret that was used to create the user data for the latest launch + // template version. + LaunchTemplateBootstrapDataSecret = NameAWSProviderPrefix + "bootstrap-data-secret" ) // ClusterTagKey generates the key for resources associated with a cluster. diff --git a/exp/controllers/awsmachinepool_controller.go b/exp/controllers/awsmachinepool_controller.go index ab27e1bee5..1a30c90314 100644 --- a/exp/controllers/awsmachinepool_controller.go +++ b/exp/controllers/awsmachinepool_controller.go @@ -61,6 +61,7 @@ type AWSMachinePoolReconciler struct { WatchFilterValue string asgServiceFactory func(cloud.ClusterScoper) services.ASGInterface ec2ServiceFactory func(scope.EC2Scope) services.EC2Interface + reconcileServiceFactory func(scope.EC2Scope) services.MachinePoolReconcileInterface TagUnmanagedNetworkResources bool } @@ -79,6 +80,14 @@ func (r *AWSMachinePoolReconciler) getEC2Service(scope scope.EC2Scope) services. return ec2.NewService(scope) } +func (r *AWSMachinePoolReconciler) getReconcileService(scope scope.EC2Scope) services.MachinePoolReconcileInterface { + if r.reconcileServiceFactory != nil { + return r.reconcileServiceFactory(scope) + } + + return ec2.NewService(scope) +} + // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachinepools,verbs=get;list;watch;update;patch;delete // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=awsmachinepools/status,verbs=get;update;patch // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinepools;machinepools/status,verbs=get;list;watch;patch @@ -227,6 +236,7 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP ec2Svc := r.getEC2Service(ec2Scope) asgsvc := r.getASGService(clusterScope) + reconSvc := r.getReconcileService(ec2Scope) // Find existing ASG asg, err := r.findASG(machinePoolScope, asgsvc) @@ -269,7 +279,7 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP machinePoolScope.Info("starting instance refresh", "number of instances", machinePoolScope.MachinePool.Spec.Replicas) return asgsvc.StartASGInstanceRefresh(machinePoolScope) } - if err := ec2Svc.ReconcileLaunchTemplate(machinePoolScope, canUpdateLaunchTemplate, runPostLaunchTemplateUpdateOperation); err != nil { + if err := reconSvc.ReconcileLaunchTemplate(machinePoolScope, ec2Svc, canUpdateLaunchTemplate, runPostLaunchTemplateUpdateOperation); err != nil { r.Recorder.Eventf(machinePoolScope.AWSMachinePool, corev1.EventTypeWarning, "FailedLaunchTemplateReconcile", "Failed to reconcile launch template: %v", err) machinePoolScope.Error(err, "failed to reconcile launch template") return err @@ -317,7 +327,7 @@ func (r *AWSMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP ResourceService: asgsvc, }, } - err = ec2Svc.ReconcileTags(machinePoolScope, resourceServiceToUpdate) + err = reconSvc.ReconcileTags(machinePoolScope, resourceServiceToUpdate) if err != nil { return errors.Wrap(err, "error updating tags") } @@ -378,7 +388,7 @@ func (r *AWSMachinePoolReconciler) reconcileDelete(machinePoolScope *scope.Machi } launchTemplateID := machinePoolScope.AWSMachinePool.Status.LaunchTemplateID - launchTemplate, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName()) + launchTemplate, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName()) if err != nil { return err } @@ -422,7 +432,7 @@ func (r *AWSMachinePoolReconciler) updatePool(machinePoolScope *scope.MachinePoo asgDiff := diffASG(machinePoolScope, existingASG) if asgDiff != "" { - machinePoolScope.Debug("asg diff detected", "diff", subnetDiff) + machinePoolScope.Debug("asg diff detected", "asgDiff", asgDiff, "subnetDiff", subnetDiff) } if asgDiff != "" || subnetDiff != "" { machinePoolScope.Info("updating AutoScalingGroup") diff --git a/exp/controllers/awsmachinepool_controller_test.go b/exp/controllers/awsmachinepool_controller_test.go index 87779e8760..447ecd3fab 100644 --- a/exp/controllers/awsmachinepool_controller_test.go +++ b/exp/controllers/awsmachinepool_controller_test.go @@ -31,6 +31,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + apimachinerytypes "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" "k8s.io/utils/ptr" @@ -42,6 +43,7 @@ import ( "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/mock_services" + "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/userdata" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" capierrors "sigs.k8s.io/cluster-api/errors" @@ -52,15 +54,17 @@ import ( func TestAWSMachinePoolReconciler(t *testing.T) { var ( - reconciler AWSMachinePoolReconciler - cs *scope.ClusterScope - ms *scope.MachinePoolScope - mockCtrl *gomock.Controller - ec2Svc *mock_services.MockEC2Interface - asgSvc *mock_services.MockASGInterface - recorder *record.FakeRecorder - awsMachinePool *expinfrav1.AWSMachinePool - secret *corev1.Secret + reconciler AWSMachinePoolReconciler + cs *scope.ClusterScope + ms *scope.MachinePoolScope + mockCtrl *gomock.Controller + ec2Svc *mock_services.MockEC2Interface + asgSvc *mock_services.MockASGInterface + reconSvc *mock_services.MockMachinePoolReconcileInterface + recorder *record.FakeRecorder + awsMachinePool *expinfrav1.AWSMachinePool + secret *corev1.Secret + userDataSecretKey apimachinerytypes.NamespacedName ) setup := func(t *testing.T, g *WithT) { t.Helper() @@ -108,10 +112,17 @@ func TestAWSMachinePoolReconciler(t *testing.T) { "value": []byte("shell-script"), }, } + userDataSecretKey = apimachinerytypes.NamespacedName{ + Namespace: secret.Namespace, + Name: secret.Name, + } g.Expect(testEnv.Create(ctx, awsMachinePool)).To(Succeed()) g.Expect(testEnv.Create(ctx, secret)).To(Succeed()) + cs, err = setupCluster("test-cluster") + g.Expect(err).To(BeNil()) + ms, err = scope.NewMachinePoolScope( scope.MachinePoolScopeParams{ Client: testEnv.Client, @@ -143,12 +154,10 @@ func TestAWSMachinePoolReconciler(t *testing.T) { ) g.Expect(err).To(BeNil()) - cs, err = setupCluster("test-cluster") - g.Expect(err).To(BeNil()) - mockCtrl = gomock.NewController(t) ec2Svc = mock_services.NewMockEC2Interface(mockCtrl) asgSvc = mock_services.NewMockASGInterface(mockCtrl) + reconSvc = mock_services.NewMockMachinePoolReconcileInterface(mockCtrl) // If the test hangs for 9 minutes, increase the value here to the number of events during a reconciliation loop recorder = record.NewFakeRecorder(2) @@ -160,6 +169,9 @@ func TestAWSMachinePoolReconciler(t *testing.T) { asgServiceFactory: func(cloud.ClusterScoper) services.ASGInterface { return asgSvc }, + reconcileServiceFactory: func(scope.EC2Scope) services.MachinePoolReconcileInterface { + return reconSvc + }, Recorder: recorder, } } @@ -183,7 +195,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) { getASG := func(t *testing.T, g *WithT) { t.Helper() - ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", expectedErr).AnyTimes() + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil, expectedErr).AnyTimes() asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, expectedErr).AnyTimes() } t.Run("should exit immediately on an error state", func(t *testing.T) { @@ -254,7 +266,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) { getASG := func(t *testing.T, g *WithT) { t.Helper() - ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil).AnyTimes() + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil, nil).AnyTimes() asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil).AnyTimes() } t.Run("should look up by provider ID when one exists", func(t *testing.T) { @@ -265,7 +277,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) { getASG(t, g) expectedErr := errors.New("no connection available ") - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(expectedErr) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(expectedErr) err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) g.Expect(errors.Cause(err)).To(MatchError(expectedErr)) }) @@ -286,7 +298,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) { defer teardown(t, g) setSuspendedProcesses(t, g) - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil) asgSvc.EXPECT().CreateASG(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{ Name: "name", @@ -310,8 +322,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) { defer teardown(t, g) setSuspendedProcesses(t, g) ms.AWSMachinePool.Spec.SuspendProcesses.All = true - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{ Name: "name", }, nil) @@ -350,8 +362,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) { defer teardown(t, g) setSuspendedProcesses(t, g) - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&expinfrav1.AutoScalingGroup{ Name: "name", CurrentlySuspendProcesses: []string{"Launch", "process3"}, @@ -375,14 +387,11 @@ func TestAWSMachinePoolReconciler(t *testing.T) { Name: "an-asg", DesiredCapacity: ptr.To[int32](1), } - asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&asg, nil).AnyTimes() - asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{}, nil).Times(1) - asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).AnyTimes() - ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil).AnyTimes() - ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(nil, nil).AnyTimes() - ec2Svc.EXPECT().CreateLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return("", nil).AnyTimes() - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&asg, nil) + asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{}, nil) + asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) ms.MachinePool.Annotations = map[string]string{ clusterv1.ReplicasManagedByAnnotation: "somehow-externally-managed", @@ -416,8 +425,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) { }, }, Subnets: []string{"subnet1", "subnet2"}} - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&asg, nil).AnyTimes() asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{"subnet2", "subnet1"}, nil).Times(1) asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).Times(0) @@ -434,8 +443,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) { MinSize: int32(0), MaxSize: int32(100), Subnets: []string{"subnet1", "subnet2"}} - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&asg, nil).AnyTimes() asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{"subnet1"}, nil).Times(1) asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).Times(1) @@ -452,8 +461,8 @@ func TestAWSMachinePoolReconciler(t *testing.T) { MinSize: int32(0), MaxSize: int32(2), Subnets: []string{}} - ec2Svc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) - ec2Svc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileLaunchTemplate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + reconSvc.EXPECT().ReconcileTags(gomock.Any(), gomock.Any()).Return(nil) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&asg, nil).AnyTimes() asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{}, nil).Times(1) asgSvc.EXPECT().UpdateASG(gomock.Any()).Return(nil).Times(1) @@ -461,6 +470,245 @@ func TestAWSMachinePoolReconciler(t *testing.T) { err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) g.Expect(err).To(Succeed()) }) + + t.Run("ReconcileLaunchTemplate not mocked", func(t *testing.T) { + g := NewWithT(t) + setup(t, g) + reconciler.reconcileServiceFactory = nil // use real implementation, but keep EC2 calls mocked (`ec2ServiceFactory`) + reconSvc = nil // not used + defer teardown(t, g) + + launchTemplateIDExisting := "lt-existing" + + t.Run("nothing exists, so launch template and ASG must be created", func(t *testing.T) { + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Eq("test")).Return(nil, "", nil, nil) + ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(ptr.To[string]("ami-abcdef123"), nil) + ec2Svc.EXPECT().CreateLaunchTemplate(gomock.Any(), gomock.Eq(ptr.To[string]("ami-abcdef123")), gomock.Eq(userDataSecretKey), gomock.Eq([]byte("shell-script"))).Return("lt-ghijkl456", nil) + asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil) + asgSvc.EXPECT().CreateASG(gomock.Any()).DoAndReturn(func(scope *scope.MachinePoolScope) (*expinfrav1.AutoScalingGroup, error) { + g.Expect(scope.Name()).To(Equal("test")) + return &expinfrav1.AutoScalingGroup{ + Name: scope.Name(), + }, nil + }) + + err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) + g.Expect(err).To(Succeed()) + }) + + t.Run("launch template and ASG exist and need no update", func(t *testing.T) { + // Latest ID and version already stored, no need to retrieve it + ms.AWSMachinePool.Status.LaunchTemplateID = launchTemplateIDExisting + ms.AWSMachinePool.Status.LaunchTemplateVersion = ptr.To[string]("1") + + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Eq("test")).Return( + &expinfrav1.AWSLaunchTemplate{ + Name: "test", + AMI: infrav1.AMIReference{ + ID: ptr.To[string]("ami-existing"), + }, + }, + // No change to user data + userdata.ComputeHash([]byte("shell-script")), + &userDataSecretKey, + nil) + ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(ptr.To[string]("ami-existing"), nil) // no change + ec2Svc.EXPECT().LaunchTemplateNeedsUpdate(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil) + + asgSvc.EXPECT().GetASGByName(gomock.Any()).DoAndReturn(func(scope *scope.MachinePoolScope) (*expinfrav1.AutoScalingGroup, error) { + g.Expect(scope.Name()).To(Equal("test")) + + // No difference to `AWSMachinePool.spec` + return &expinfrav1.AutoScalingGroup{ + Name: scope.Name(), + Subnets: []string{ + "subnet-1", + }, + MinSize: awsMachinePool.Spec.MinSize, + MaxSize: awsMachinePool.Spec.MaxSize, + MixedInstancesPolicy: awsMachinePool.Spec.MixedInstancesPolicy.DeepCopy(), + }, nil + }) + asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{"subnet-1"}, nil) // no change + // No changes, so there must not be an ASG update! + asgSvc.EXPECT().UpdateASG(gomock.Any()).Times(0) + + err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) + g.Expect(err).To(Succeed()) + }) + + t.Run("launch template and ASG exist and only AMI ID changed", func(t *testing.T) { + // Latest ID and version already stored, no need to retrieve it + ms.AWSMachinePool.Status.LaunchTemplateID = launchTemplateIDExisting + ms.AWSMachinePool.Status.LaunchTemplateVersion = ptr.To[string]("1") + + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Eq("test")).Return( + &expinfrav1.AWSLaunchTemplate{ + Name: "test", + AMI: infrav1.AMIReference{ + ID: ptr.To[string]("ami-existing"), + }, + }, + // No change to user data + userdata.ComputeHash([]byte("shell-script")), + &userDataSecretKey, + nil) + ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(ptr.To[string]("ami-different"), nil) + ec2Svc.EXPECT().LaunchTemplateNeedsUpdate(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil) + asgSvc.EXPECT().CanStartASGInstanceRefresh(gomock.Any()).Return(true, nil) + ec2Svc.EXPECT().PruneLaunchTemplateVersions(gomock.Any()).Return(nil) + ec2Svc.EXPECT().CreateLaunchTemplateVersion(gomock.Any(), gomock.Any(), gomock.Eq(ptr.To[string]("ami-different")), gomock.Eq(apimachinerytypes.NamespacedName{Namespace: "default", Name: "bootstrap-data"}), gomock.Any()).Return(nil) + ec2Svc.EXPECT().GetLaunchTemplateLatestVersion(gomock.Any()).Return("2", nil) + // AMI change should trigger rolling out new nodes + asgSvc.EXPECT().StartASGInstanceRefresh(gomock.Any()) + + asgSvc.EXPECT().GetASGByName(gomock.Any()).DoAndReturn(func(scope *scope.MachinePoolScope) (*expinfrav1.AutoScalingGroup, error) { + g.Expect(scope.Name()).To(Equal("test")) + + // No difference to `AWSMachinePool.spec` + return &expinfrav1.AutoScalingGroup{ + Name: scope.Name(), + Subnets: []string{ + "subnet-1", + }, + MinSize: awsMachinePool.Spec.MinSize, + MaxSize: awsMachinePool.Spec.MaxSize, + MixedInstancesPolicy: awsMachinePool.Spec.MixedInstancesPolicy.DeepCopy(), + }, nil + }) + asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{"subnet-1"}, nil) // no change + // No changes, so there must not be an ASG update! + asgSvc.EXPECT().UpdateASG(gomock.Any()).Times(0) + + err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) + g.Expect(err).To(Succeed()) + }) + + t.Run("launch template and ASG exist and only bootstrap data secret name changed", func(t *testing.T) { + // Latest ID and version already stored, no need to retrieve it + ms.AWSMachinePool.Status.LaunchTemplateID = launchTemplateIDExisting + ms.AWSMachinePool.Status.LaunchTemplateVersion = ptr.To[string]("1") + + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Eq("test")).Return( + &expinfrav1.AWSLaunchTemplate{ + Name: "test", + AMI: infrav1.AMIReference{ + ID: ptr.To[string]("ami-existing"), + }, + }, + // No change to user data + userdata.ComputeHash([]byte("shell-script")), + // But the name of the secret changes from `previous-secret-name` to `bootstrap-data` + &apimachinerytypes.NamespacedName{Namespace: "default", Name: "previous-secret-name"}, + nil) + ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(ptr.To[string]("ami-existing"), nil) + ec2Svc.EXPECT().LaunchTemplateNeedsUpdate(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil) + asgSvc.EXPECT().CanStartASGInstanceRefresh(gomock.Any()).Return(true, nil) + ec2Svc.EXPECT().PruneLaunchTemplateVersions(gomock.Any()).Return(nil) + ec2Svc.EXPECT().CreateLaunchTemplateVersion(gomock.Any(), gomock.Any(), gomock.Eq(ptr.To[string]("ami-existing")), gomock.Eq(apimachinerytypes.NamespacedName{Namespace: "default", Name: "bootstrap-data"}), gomock.Any()).Return(nil) + ec2Svc.EXPECT().GetLaunchTemplateLatestVersion(gomock.Any()).Return("2", nil) + // Changing the bootstrap data secret name should trigger rolling out new nodes, no matter what the + // content (user data) is. This way, users can enforce a rollout by changing the bootstrap config + // reference (`MachinePool.spec.template.spec.bootstrap`). + asgSvc.EXPECT().StartASGInstanceRefresh(gomock.Any()) + + asgSvc.EXPECT().GetASGByName(gomock.Any()).DoAndReturn(func(scope *scope.MachinePoolScope) (*expinfrav1.AutoScalingGroup, error) { + g.Expect(scope.Name()).To(Equal("test")) + + // No difference to `AWSMachinePool.spec` + return &expinfrav1.AutoScalingGroup{ + Name: scope.Name(), + Subnets: []string{ + "subnet-1", + }, + MinSize: awsMachinePool.Spec.MinSize, + MaxSize: awsMachinePool.Spec.MaxSize, + MixedInstancesPolicy: awsMachinePool.Spec.MixedInstancesPolicy.DeepCopy(), + }, nil + }) + asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{"subnet-1"}, nil) // no change + // No changes, so there must not be an ASG update! + asgSvc.EXPECT().UpdateASG(gomock.Any()).Times(0) + + err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) + g.Expect(err).To(Succeed()) + }) + + t.Run("launch template and ASG created from zero, then bootstrap config reference changes", func(t *testing.T) { + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Eq("test")).Return(nil, "", nil, nil) + ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(ptr.To[string]("ami-abcdef123"), nil) + ec2Svc.EXPECT().CreateLaunchTemplate(gomock.Any(), gomock.Eq(ptr.To[string]("ami-abcdef123")), gomock.Eq(userDataSecretKey), gomock.Eq([]byte("shell-script"))).Return("lt-ghijkl456", nil) + asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil) + asgSvc.EXPECT().CreateASG(gomock.Any()).DoAndReturn(func(scope *scope.MachinePoolScope) (*expinfrav1.AutoScalingGroup, error) { + g.Expect(scope.Name()).To(Equal("test")) + return &expinfrav1.AutoScalingGroup{ + Name: scope.Name(), + }, nil + }) + + err := reconciler.reconcileNormal(context.Background(), ms, cs, cs) + g.Expect(err).To(Succeed()) + + g.Expect(ms.AWSMachinePool.Status.LaunchTemplateID).ToNot(BeEmpty()) + g.Expect(ptr.Deref[string](ms.AWSMachinePool.Status.LaunchTemplateVersion, "")).ToNot(BeEmpty()) + + // Data secret name changes + newBootstrapSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bootstrap-data-new", // changed + Namespace: "default", + }, + Data: map[string][]byte{ + "value": secret.Data["value"], // not changed + }, + } + g.Expect(testEnv.Create(ctx, newBootstrapSecret)).To(Succeed()) + ms.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName = ptr.To[string](newBootstrapSecret.Name) + + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Eq("test")).Return( + &expinfrav1.AWSLaunchTemplate{ + Name: "test", + AMI: infrav1.AMIReference{ + ID: ptr.To[string]("ami-existing"), + }, + }, + // No change to user data content + userdata.ComputeHash([]byte("shell-script")), + &apimachinerytypes.NamespacedName{Namespace: "default", Name: "bootstrap-data"}, + nil) + ec2Svc.EXPECT().DiscoverLaunchTemplateAMI(gomock.Any()).Return(ptr.To[string]("ami-existing"), nil) + ec2Svc.EXPECT().LaunchTemplateNeedsUpdate(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil) + asgSvc.EXPECT().CanStartASGInstanceRefresh(gomock.Any()).Return(true, nil) + ec2Svc.EXPECT().PruneLaunchTemplateVersions(gomock.Any()).Return(nil) + ec2Svc.EXPECT().CreateLaunchTemplateVersion(gomock.Any(), gomock.Any(), gomock.Eq(ptr.To[string]("ami-existing")), gomock.Eq(apimachinerytypes.NamespacedName{Namespace: "default", Name: "bootstrap-data-new"}), gomock.Any()).Return(nil) + ec2Svc.EXPECT().GetLaunchTemplateLatestVersion(gomock.Any()).Return("2", nil) + // Changing the bootstrap data secret name should trigger rolling out new nodes, no matter what the + // content (user data) is. This way, users can enforce a rollout by changing the bootstrap config + // reference (`MachinePool.spec.template.spec.bootstrap.configRef`). + asgSvc.EXPECT().StartASGInstanceRefresh(gomock.Any()) + + asgSvc.EXPECT().GetASGByName(gomock.Any()).DoAndReturn(func(scope *scope.MachinePoolScope) (*expinfrav1.AutoScalingGroup, error) { + g.Expect(scope.Name()).To(Equal("test")) + + // No difference to `AWSMachinePool.spec` + return &expinfrav1.AutoScalingGroup{ + Name: scope.Name(), + Subnets: []string{ + "subnet-1", + }, + MinSize: awsMachinePool.Spec.MinSize, + MaxSize: awsMachinePool.Spec.MaxSize, + MixedInstancesPolicy: awsMachinePool.Spec.MixedInstancesPolicy.DeepCopy(), + }, nil + }) + asgSvc.EXPECT().SubnetIDs(gomock.Any()).Return([]string{"subnet-1"}, nil) // no change + // No changes, so there must not be an ASG update! + asgSvc.EXPECT().UpdateASG(gomock.Any()).Times(0) + + err = reconciler.reconcileNormal(context.Background(), ms, cs, cs) + g.Expect(err).To(Succeed()) + }) + }) }) t.Run("Deleting an AWSMachinePool", func(t *testing.T) { @@ -491,7 +739,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) { finalizer(t, g) asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(nil, nil) - ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil).AnyTimes() + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil, nil).AnyTimes() buf := new(bytes.Buffer) klog.SetOutput(buf) @@ -513,7 +761,7 @@ func TestAWSMachinePoolReconciler(t *testing.T) { Status: expinfrav1.ASGStatusDeleteInProgress, } asgSvc.EXPECT().GetASGByName(gomock.Any()).Return(&inProgressASG, nil) - ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil).AnyTimes() + ec2Svc.EXPECT().GetLaunchTemplate(gomock.Any()).Return(nil, "", nil, nil).AnyTimes() buf := new(bytes.Buffer) klog.SetOutput(buf) diff --git a/exp/controllers/awsmanagedmachinepool_controller.go b/exp/controllers/awsmanagedmachinepool_controller.go index 259fc66f34..8c0d75c2ec 100644 --- a/exp/controllers/awsmanagedmachinepool_controller.go +++ b/exp/controllers/awsmanagedmachinepool_controller.go @@ -207,6 +207,7 @@ func (r *AWSManagedMachinePoolReconciler) reconcileNormal( ekssvc := eks.NewNodegroupService(machinePoolScope) ec2svc := r.getEC2Service(ec2Scope) + reconSvc := r.getReconcileService(ec2Scope) if machinePoolScope.ManagedMachinePool.Spec.AWSLaunchTemplate != nil { canUpdateLaunchTemplate := func() (bool, error) { @@ -215,7 +216,7 @@ func (r *AWSManagedMachinePoolReconciler) reconcileNormal( runPostLaunchTemplateUpdateOperation := func() error { return nil } - if err := ec2svc.ReconcileLaunchTemplate(machinePoolScope, canUpdateLaunchTemplate, runPostLaunchTemplateUpdateOperation); err != nil { + if err := reconSvc.ReconcileLaunchTemplate(machinePoolScope, ec2svc, canUpdateLaunchTemplate, runPostLaunchTemplateUpdateOperation); err != nil { r.Recorder.Eventf(machinePoolScope.ManagedMachinePool, corev1.EventTypeWarning, "FailedLaunchTemplateReconcile", "Failed to reconcile launch template: %v", err) machinePoolScope.Error(err, "failed to reconcile launch template") conditions.MarkFalse(machinePoolScope.ManagedMachinePool, expinfrav1.LaunchTemplateReadyCondition, expinfrav1.LaunchTemplateReconcileFailedReason, clusterv1.ConditionSeverityError, "") @@ -227,7 +228,7 @@ func (r *AWSManagedMachinePoolReconciler) reconcileNormal( ResourceID: &launchTemplateID, ResourceService: ec2svc, }} - if err := ec2svc.ReconcileTags(machinePoolScope, resourceServiceToUpdate); err != nil { + if err := reconSvc.ReconcileTags(machinePoolScope, resourceServiceToUpdate); err != nil { return errors.Wrap(err, "error updating tags") } @@ -258,7 +259,7 @@ func (r *AWSManagedMachinePoolReconciler) reconcileDelete( if machinePoolScope.ManagedMachinePool.Spec.AWSLaunchTemplate != nil { launchTemplateID := machinePoolScope.ManagedMachinePool.Status.LaunchTemplateID - launchTemplate, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName()) + launchTemplate, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName()) if err != nil { return err } @@ -347,3 +348,7 @@ func managedControlPlaneToManagedMachinePoolMapFunc(c client.Client, gvk schema. func (r *AWSManagedMachinePoolReconciler) getEC2Service(scope scope.EC2Scope) services.EC2Interface { return ec2.NewService(scope) } + +func (r *AWSManagedMachinePoolReconciler) getReconcileService(scope scope.EC2Scope) services.MachinePoolReconcileInterface { + return ec2.NewService(scope) +} diff --git a/pkg/cloud/scope/launchtemplate.go b/pkg/cloud/scope/launchtemplate.go index 2281c660d9..676e255365 100644 --- a/pkg/cloud/scope/launchtemplate.go +++ b/pkg/cloud/scope/launchtemplate.go @@ -18,6 +18,7 @@ package scope import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" @@ -36,7 +37,7 @@ type LaunchTemplateScope interface { SetLaunchTemplateIDStatus(id string) GetLaunchTemplateLatestVersionStatus() string SetLaunchTemplateLatestVersionStatus(version string) - GetRawBootstrapData() ([]byte, error) + GetRawBootstrapData() ([]byte, *types.NamespacedName, error) IsEKSManaged() bool AdditionalTags() infrav1.Tags diff --git a/pkg/cloud/scope/machinepool.go b/pkg/cloud/scope/machinepool.go index 8288a18860..069c76a41b 100644 --- a/pkg/cloud/scope/machinepool.go +++ b/pkg/cloud/scope/machinepool.go @@ -131,36 +131,32 @@ func (m *MachinePoolScope) Namespace() string { return m.AWSMachinePool.Namespace } -// GetRawBootstrapData returns the bootstrap data from the secret in the Machine's bootstrap.dataSecretName. -// todo(rudoi): stolen from MachinePool - any way to reuse? -func (m *MachinePoolScope) GetRawBootstrapData() ([]byte, error) { - data, _, err := m.getBootstrapData() +// GetRawBootstrapData returns the bootstrap data from the secret in the Machine's bootstrap.dataSecretName, +// including the secret's namespaced name. +func (m *MachinePoolScope) GetRawBootstrapData() ([]byte, *types.NamespacedName, error) { + data, _, bootstrapDataSecretKey, err := m.getBootstrapData() - return data, err + return data, bootstrapDataSecretKey, err } -func (m *MachinePoolScope) GetRawBootstrapDataWithFormat() ([]byte, string, error) { - return m.getBootstrapData() -} - -func (m *MachinePoolScope) getBootstrapData() ([]byte, string, error) { +func (m *MachinePoolScope) getBootstrapData() ([]byte, string, *types.NamespacedName, error) { if m.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName == nil { - return nil, "", errors.New("error retrieving bootstrap data: linked Machine's bootstrap.dataSecretName is nil") + return nil, "", nil, errors.New("error retrieving bootstrap data: linked Machine's bootstrap.dataSecretName is nil") } secret := &corev1.Secret{} key := types.NamespacedName{Namespace: m.Namespace(), Name: *m.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName} if err := m.Client.Get(context.TODO(), key, secret); err != nil { - return nil, "", errors.Wrapf(err, "failed to retrieve bootstrap data secret %s for AWSMachine %s/%s", key.Name, m.Namespace(), m.Name()) + return nil, "", nil, errors.Wrapf(err, "failed to retrieve bootstrap data secret %s for AWSMachinePool %s/%s", key.Name, m.Namespace(), m.Name()) } value, ok := secret.Data["value"] if !ok { - return nil, "", errors.New("error retrieving bootstrap data: secret value key is missing") + return nil, "", nil, errors.New("error retrieving bootstrap data: secret value key is missing") } - return value, string(secret.Data["format"]), nil + return value, string(secret.Data["format"]), &key, nil } // AdditionalTags merges AdditionalTags from the scope's AWSCluster and AWSMachinePool. If the same key is present in both, diff --git a/pkg/cloud/scope/managednodegroup.go b/pkg/cloud/scope/managednodegroup.go index 448a0eee9d..1950ea0221 100644 --- a/pkg/cloud/scope/managednodegroup.go +++ b/pkg/cloud/scope/managednodegroup.go @@ -323,24 +323,24 @@ func (s *ManagedMachinePoolScope) Namespace() string { return s.ManagedMachinePool.Namespace } -func (s *ManagedMachinePoolScope) GetRawBootstrapData() ([]byte, error) { +func (s *ManagedMachinePoolScope) GetRawBootstrapData() ([]byte, *types.NamespacedName, error) { if s.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName == nil { - return nil, errors.New("error retrieving bootstrap data: linked Machine's bootstrap.dataSecretName is nil") + return nil, nil, errors.New("error retrieving bootstrap data: linked Machine's bootstrap.dataSecretName is nil") } secret := &corev1.Secret{} key := types.NamespacedName{Namespace: s.Namespace(), Name: *s.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName} if err := s.Client.Get(context.TODO(), key, secret); err != nil { - return nil, errors.Wrapf(err, "failed to retrieve bootstrap data secret for AWSManagedMachinePool %s/%s", s.Namespace(), s.Name()) + return nil, nil, errors.Wrapf(err, "failed to retrieve bootstrap data secret for AWSManagedMachinePool %s/%s", s.Namespace(), s.Name()) } value, ok := secret.Data["value"] if !ok { - return nil, errors.New("error retrieving bootstrap data: secret value key is missing") + return nil, nil, errors.New("error retrieving bootstrap data: secret value key is missing") } - return value, nil + return value, &key, nil } func (s *ManagedMachinePoolScope) GetObjectMeta() *metav1.ObjectMeta { diff --git a/pkg/cloud/services/ec2/launchtemplate.go b/pkg/cloud/services/ec2/launchtemplate.go index 0708fadf43..57c4048682 100644 --- a/pkg/cloud/services/ec2/launchtemplate.go +++ b/pkg/cloud/services/ec2/launchtemplate.go @@ -29,12 +29,14 @@ import ( "github.com/google/go-cmp/cmp" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + apimachinerytypes "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" + "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/userdata" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -51,22 +53,25 @@ const ( TagsLastAppliedAnnotation = "sigs.k8s.io/cluster-api-provider-aws-last-applied-tags" ) +// ReconcileLaunchTemplate reconciles a launch template and triggers instance refresh conditionally, depending on +// changes. +// +//nolint:gocyclo func (s *Service) ReconcileLaunchTemplate( scope scope.LaunchTemplateScope, + ec2svc services.EC2Interface, canUpdateLaunchTemplate func() (bool, error), runPostLaunchTemplateUpdateOperation func() error, ) error { - bootstrapData, err := scope.GetRawBootstrapData() + bootstrapData, bootstrapDataSecretKey, err := scope.GetRawBootstrapData() if err != nil { record.Eventf(scope.GetMachinePool(), corev1.EventTypeWarning, "FailedGetBootstrapData", err.Error()) return err } bootstrapDataHash := userdata.ComputeHash(bootstrapData) - ec2svc := NewService(scope.GetEC2Scope()) - scope.Info("checking for existing launch template") - launchTemplate, launchTemplateUserDataHash, err := ec2svc.GetLaunchTemplate(scope.LaunchTemplateName()) + launchTemplate, launchTemplateUserDataHash, launchTemplateUserDataSecretKey, err := ec2svc.GetLaunchTemplate(scope.LaunchTemplateName()) if err != nil { conditions.MarkUnknown(scope.GetSetter(), expinfrav1.LaunchTemplateReadyCondition, expinfrav1.LaunchTemplateNotFoundReason, err.Error()) return err @@ -80,7 +85,7 @@ func (s *Service) ReconcileLaunchTemplate( if launchTemplate == nil { scope.Info("no existing launch template found, creating") - launchTemplateID, err := ec2svc.CreateLaunchTemplate(scope, imageID, bootstrapData) + launchTemplateID, err := ec2svc.CreateLaunchTemplate(scope, imageID, *bootstrapDataSecretKey, bootstrapData) if err != nil { conditions.MarkFalse(scope.GetSetter(), expinfrav1.LaunchTemplateReadyCondition, expinfrav1.LaunchTemplateCreateFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err @@ -125,7 +130,17 @@ func (s *Service) ReconcileLaunchTemplate( return err } - if needsUpdate || tagsChanged || *imageID != *launchTemplate.AMI.ID { + amiChanged := *imageID != *launchTemplate.AMI.ID + + // `launchTemplateUserDataSecretKey` can be nil since it comes from a tag on the launch template + // which may not exist in older launch templates created by older CAPA versions. + // On change, we trigger instance refresh (rollout of new nodes). Therefore, do not consider it a change if the + // launch template does not have the respective tag yet, as it could be surprising to users. Instead, ensure the + // tag is stored on the newly-generated launch template version, without rolling out nodes. + userDataSecretKeyChanged := launchTemplateUserDataSecretKey != nil && bootstrapDataSecretKey.String() != launchTemplateUserDataSecretKey.String() + launchTemplateNeedsUserDataSecretKeyTag := launchTemplateUserDataSecretKey == nil + + if needsUpdate || tagsChanged || amiChanged || userDataSecretKeyChanged { canUpdate, err := canUpdateLaunchTemplate() if err != nil { return err @@ -136,16 +151,18 @@ func (s *Service) ReconcileLaunchTemplate( } } + userDataHashChanged := launchTemplateUserDataHash != bootstrapDataHash + // Create a new launch template version if there's a difference in configuration, tags, // userdata, OR we've discovered a new AMI ID. - if needsUpdate || tagsChanged || *imageID != *launchTemplate.AMI.ID || launchTemplateUserDataHash != bootstrapDataHash { - scope.Info("creating new version for launch template", "existing", launchTemplate, "incoming", scope.GetLaunchTemplate()) + if needsUpdate || tagsChanged || amiChanged || userDataHashChanged || userDataSecretKeyChanged || launchTemplateNeedsUserDataSecretKeyTag { + scope.Info("creating new version for launch template", "existing", launchTemplate, "incoming", scope.GetLaunchTemplate(), "needsUpdate", needsUpdate, "tagsChanged", tagsChanged, "amiChanged", amiChanged, "userDataHashChanged", userDataHashChanged, "userDataSecretKeyChanged", userDataSecretKeyChanged) // There is a limit to the number of Launch Template Versions. // We ensure that the number of versions does not grow without bound by following a simple rule: Before we create a new version, we delete one old version, if there is at least one old version that is not in use. if err := ec2svc.PruneLaunchTemplateVersions(scope.GetLaunchTemplateIDStatus()); err != nil { return err } - if err := ec2svc.CreateLaunchTemplateVersion(scope.GetLaunchTemplateIDStatus(), scope, imageID, bootstrapData); err != nil { + if err := ec2svc.CreateLaunchTemplateVersion(scope.GetLaunchTemplateIDStatus(), scope, imageID, *bootstrapDataSecretKey, bootstrapData); err != nil { return err } version, err := ec2svc.GetLaunchTemplateLatestVersion(scope.GetLaunchTemplateIDStatus()) @@ -159,7 +176,7 @@ func (s *Service) ReconcileLaunchTemplate( } } - if needsUpdate || tagsChanged || *imageID != *launchTemplate.AMI.ID { + if needsUpdate || tagsChanged || amiChanged || userDataSecretKeyChanged { if err := runPostLaunchTemplateUpdateOperation(); err != nil { conditions.MarkFalse(scope.GetSetter(), expinfrav1.PostLaunchTemplateUpdateOperationCondition, expinfrav1.PostLaunchTemplateUpdateOperationFailedReason, clusterv1.ConditionSeverityError, err.Error()) return err @@ -323,9 +340,9 @@ func tagsChanged(annotation map[string]interface{}, src map[string]string) (bool // GetLaunchTemplate returns the existing LaunchTemplate or nothing if it doesn't exist. // For now by name until we need the input to be something different. -func (s *Service) GetLaunchTemplate(launchTemplateName string) (*expinfrav1.AWSLaunchTemplate, string, error) { +func (s *Service) GetLaunchTemplate(launchTemplateName string) (*expinfrav1.AWSLaunchTemplate, string, *apimachinerytypes.NamespacedName, error) { if launchTemplateName == "" { - return nil, "", nil + return nil, "", nil, nil } s.scope.Debug("Looking for existing LaunchTemplates") @@ -338,13 +355,13 @@ func (s *Service) GetLaunchTemplate(launchTemplateName string) (*expinfrav1.AWSL out, err := s.EC2Client.DescribeLaunchTemplateVersionsWithContext(context.TODO(), input) switch { case awserrors.IsNotFound(err): - return nil, "", nil + return nil, "", nil, nil case err != nil: - return nil, "", err + return nil, "", nil, err } if out == nil || out.LaunchTemplateVersions == nil || len(out.LaunchTemplateVersions) == 0 { - return nil, "", nil + return nil, "", nil, nil } return s.SDKToLaunchTemplate(out.LaunchTemplateVersions[0]) @@ -378,10 +395,10 @@ func (s *Service) GetLaunchTemplateID(launchTemplateName string) (string, error) } // CreateLaunchTemplate generates a launch template to be used with the autoscaling group. -func (s *Service) CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userData []byte) (string, error) { +func (s *Service) CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) (string, error) { s.scope.Info("Create a new launch template") - launchTemplateData, err := s.createLaunchTemplateData(scope, imageID, userData) + launchTemplateData, err := s.createLaunchTemplateData(scope, imageID, userDataSecretKey, userData) if err != nil { return "", errors.Wrapf(err, "unable to form launch template data") } @@ -422,10 +439,10 @@ func (s *Service) CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID } // CreateLaunchTemplateVersion will create a launch template. -func (s *Service) CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userData []byte) error { +func (s *Service) CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) error { s.scope.Debug("creating new launch template version", "machine-pool", scope.LaunchTemplateName()) - launchTemplateData, err := s.createLaunchTemplateData(scope, imageID, userData) + launchTemplateData, err := s.createLaunchTemplateData(scope, imageID, userDataSecretKey, userData) if err != nil { return errors.Wrapf(err, "unable to form launch template data") } @@ -443,7 +460,7 @@ func (s *Service) CreateLaunchTemplateVersion(id string, scope scope.LaunchTempl return nil } -func (s *Service) createLaunchTemplateData(scope scope.LaunchTemplateScope, imageID *string, userData []byte) (*ec2.RequestLaunchTemplateData, error) { +func (s *Service) createLaunchTemplateData(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) (*ec2.RequestLaunchTemplateData, error) { lt := scope.GetLaunchTemplate() // An explicit empty string for SSHKeyName means do not specify a key in the ASG launch @@ -514,7 +531,7 @@ func (s *Service) createLaunchTemplateData(scope scope.LaunchTemplateScope, imag } } - data.TagSpecifications = s.buildLaunchTemplateTagSpecificationRequest(scope) + data.TagSpecifications = s.buildLaunchTemplateTagSpecificationRequest(scope, userDataSecretKey) return data, nil } @@ -642,7 +659,7 @@ func (s *Service) deleteLaunchTemplateVersion(id string, version *int64) error { } // SDKToLaunchTemplate converts an AWS EC2 SDK instance to the CAPA instance type. -func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1.AWSLaunchTemplate, string, error) { +func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1.AWSLaunchTemplate, string, *apimachinerytypes.NamespacedName, error) { v := d.LaunchTemplateData i := &expinfrav1.AWSLaunchTemplate{ Name: aws.StringValue(d.LaunchTemplateName), @@ -689,14 +706,30 @@ func (s *Service) SDKToLaunchTemplate(d *ec2.LaunchTemplateVersion) (*expinfrav1 } if v.UserData == nil { - return i, userdata.ComputeHash(nil), nil + return i, userdata.ComputeHash(nil), nil, nil } decodedUserData, err := base64.StdEncoding.DecodeString(*v.UserData) if err != nil { - return nil, "", errors.Wrap(err, "unable to decode UserData") + return nil, "", nil, errors.Wrap(err, "unable to decode UserData") + } + decodedUserDataHash := userdata.ComputeHash(decodedUserData) + + for _, tagSpecification := range v.TagSpecifications { + if tagSpecification.ResourceType != nil && *tagSpecification.ResourceType == ec2.ResourceTypeInstance { + for _, tag := range tagSpecification.Tags { + if tag.Key != nil && *tag.Key == infrav1.LaunchTemplateBootstrapDataSecret && tag.Value != nil && strings.Contains(*tag.Value, "/") { + parts := strings.SplitN(*tag.Value, "/", 2) + launchTemplateUserDataSecretKey := &apimachinerytypes.NamespacedName{ + Namespace: parts[0], + Name: parts[1], + } + return i, decodedUserDataHash, launchTemplateUserDataSecretKey, nil + } + } + } } - return i, userdata.ComputeHash(decodedUserData), nil + return i, decodedUserDataHash, nil, nil } // LaunchTemplateNeedsUpdate checks if a new launch template version is needed. @@ -831,7 +864,7 @@ func (s *Service) GetAdditionalSecurityGroupsIDs(securityGroups []infrav1.AWSRes return additionalSecurityGroupsIDs, nil } -func (s *Service) buildLaunchTemplateTagSpecificationRequest(scope scope.LaunchTemplateScope) []*ec2.LaunchTemplateTagSpecificationRequest { +func (s *Service) buildLaunchTemplateTagSpecificationRequest(scope scope.LaunchTemplateScope, userDataSecretKey apimachinerytypes.NamespacedName) []*ec2.LaunchTemplateTagSpecificationRequest { tagSpecifications := make([]*ec2.LaunchTemplateTagSpecificationRequest, 0) additionalTags := scope.AdditionalTags() // Set the cloud provider tag @@ -845,19 +878,24 @@ func (s *Service) buildLaunchTemplateTagSpecificationRequest(scope scope.LaunchT Additional: additionalTags, }) - if len(tags) > 0 { - // tag instances + // tag instances + { + instanceTags := tags.DeepCopy() + instanceTags[infrav1.LaunchTemplateBootstrapDataSecret] = userDataSecretKey.String() + spec := &ec2.LaunchTemplateTagSpecificationRequest{ResourceType: aws.String(ec2.ResourceTypeInstance)} - for key, value := range tags { + for key, value := range instanceTags { spec.Tags = append(spec.Tags, &ec2.Tag{ Key: aws.String(key), Value: aws.String(value), }) } tagSpecifications = append(tagSpecifications, spec) + } - // tag EBS volumes - spec = &ec2.LaunchTemplateTagSpecificationRequest{ResourceType: aws.String(ec2.ResourceTypeVolume)} + // tag EBS volumes + if len(tags) > 0 { + spec := &ec2.LaunchTemplateTagSpecificationRequest{ResourceType: aws.String(ec2.ResourceTypeVolume)} for key, value := range tags { spec.Tags = append(spec.Tags, &ec2.Tag{ Key: aws.String(key), @@ -866,6 +904,7 @@ func (s *Service) buildLaunchTemplateTagSpecificationRequest(scope scope.LaunchT } tagSpecifications = append(tagSpecifications, spec) } + return tagSpecifications } diff --git a/pkg/cloud/services/ec2/launchtemplate_test.go b/pkg/cloud/services/ec2/launchtemplate_test.go index b65d992a30..4553ad4546 100644 --- a/pkg/cloud/services/ec2/launchtemplate_test.go +++ b/pkg/cloud/services/ec2/launchtemplate_test.go @@ -30,6 +30,7 @@ import ( "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -79,6 +80,16 @@ users: var testUserDataHash = userdata.ComputeHash([]byte(testUserData)) +func defaultEC2AndUserDataSecretKeyTags(name string, clusterName string, userDataSecretKey types.NamespacedName) []*ec2.Tag { + tags := defaultEC2Tags(name, clusterName) + tags = append(tags, &ec2.Tag{ + Key: aws.String(infrav1.LaunchTemplateBootstrapDataSecret), + Value: aws.String(userDataSecretKey.String()), + }) + sortTags(tags) + return tags +} + func TestGetLaunchTemplate(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -284,7 +295,7 @@ func TestGetLaunchTemplate(t *testing.T) { tc.expect(mockEC2Client.EXPECT()) } - launchTemplate, userData, err := s.GetLaunchTemplate(tc.launchTemplateName) + launchTemplate, userData, _, err := s.GetLaunchTemplate(tc.launchTemplateName) tc.check(g, launchTemplate, userData, err) }) } @@ -292,11 +303,12 @@ func TestGetLaunchTemplate(t *testing.T) { func TestServiceSDKToLaunchTemplate(t *testing.T) { tests := []struct { - name string - input *ec2.LaunchTemplateVersion - wantLT *expinfrav1.AWSLaunchTemplate - wantHash string - wantErr bool + name string + input *ec2.LaunchTemplateVersion + wantLT *expinfrav1.AWSLaunchTemplate + wantHash string + wantDataSecretKey *types.NamespacedName + wantErr bool }{ { name: "lots of input", @@ -338,13 +350,68 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) { SSHKeyName: aws.String("foo-keyname"), VersionNumber: aws.Int64(1), }, - wantHash: testUserDataHash, + wantHash: testUserDataHash, + wantDataSecretKey: nil, // respective tag is not given + }, + { + name: "tag of bootstrap secret", + input: &ec2.LaunchTemplateVersion{ + LaunchTemplateId: aws.String("lt-12345"), + LaunchTemplateName: aws.String("foo"), + LaunchTemplateData: &ec2.ResponseLaunchTemplateData{ + ImageId: aws.String("foo-image"), + IamInstanceProfile: &ec2.LaunchTemplateIamInstanceProfileSpecification{ + Arn: aws.String("instance-profile/foo-profile"), + }, + KeyName: aws.String("foo-keyname"), + BlockDeviceMappings: []*ec2.LaunchTemplateBlockDeviceMapping{ + { + DeviceName: aws.String("foo-device"), + Ebs: &ec2.LaunchTemplateEbsBlockDevice{ + Encrypted: aws.Bool(true), + VolumeSize: aws.Int64(16), + VolumeType: aws.String("cool"), + }, + }, + }, + NetworkInterfaces: []*ec2.LaunchTemplateInstanceNetworkInterfaceSpecification{ + { + DeviceIndex: aws.Int64(1), + Groups: []*string{aws.String("foo-group")}, + }, + }, + TagSpecifications: []*ec2.LaunchTemplateTagSpecification{ + { + ResourceType: aws.String(ec2.ResourceTypeInstance), + Tags: []*ec2.Tag{ + { + Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/bootstrap-data-secret"), + Value: aws.String("bootstrap-secret-ns/bootstrap-secret"), + }, + }, + }, + }, + UserData: aws.String(base64.StdEncoding.EncodeToString([]byte(testUserData))), + }, + VersionNumber: aws.Int64(1), + }, + wantLT: &expinfrav1.AWSLaunchTemplate{ + Name: "foo", + AMI: infrav1.AMIReference{ + ID: aws.String("foo-image"), + }, + IamInstanceProfile: "foo-profile", + SSHKeyName: aws.String("foo-keyname"), + VersionNumber: aws.Int64(1), + }, + wantHash: testUserDataHash, + wantDataSecretKey: &types.NamespacedName{Namespace: "bootstrap-secret-ns", Name: "bootstrap-secret"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &Service{} - gotLT, gotHash, err := s.SDKToLaunchTemplate(tt.input) + gotLT, gotHash, gotDataSecretKey, err := s.SDKToLaunchTemplate(tt.input) if (err != nil) != tt.wantErr { t.Fatalf("error mismatch: got %v, wantErr %v", err, tt.wantErr) } @@ -354,6 +421,9 @@ func TestServiceSDKToLaunchTemplate(t *testing.T) { if !cmp.Equal(gotHash, tt.wantHash) { t.Fatalf("userDataHash mismatch: got %v, want %v", gotHash, tt.wantHash) } + if !cmp.Equal(gotDataSecretKey, tt.wantDataSecretKey) { + t.Fatalf("userDataSecretKey mismatch: got %v, want %v", gotDataSecretKey, tt.wantDataSecretKey) + } }) } } @@ -737,6 +807,10 @@ func TestCreateLaunchTemplate(t *testing.T) { } } + userDataSecretKey := types.NamespacedName{ + Namespace: "bootstrap-secret-ns", + Name: "bootstrap-secret", + } userData := []byte{1, 0, 0} testCases := []struct { name string @@ -771,7 +845,7 @@ func TestCreateLaunchTemplate(t *testing.T) { TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ { ResourceType: aws.String(ec2.ResourceTypeInstance), - Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), }, { ResourceType: aws.String(ec2.ResourceTypeVolume), @@ -795,7 +869,7 @@ func TestCreateLaunchTemplate(t *testing.T) { // formatting added to match arrays during cmp.Equal formatTagsInput(arg) if !cmp.Equal(expectedInput, arg) { - t.Fatalf("mismatch in input expected: %+v, got: %+v", expectedInput, arg) + t.Fatalf("mismatch in input expected: %+v, got: %+v, diff: %s", expectedInput, arg, cmp.Diff(expectedInput, arg)) } }) }, @@ -831,7 +905,7 @@ func TestCreateLaunchTemplate(t *testing.T) { TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ { ResourceType: aws.String(ec2.ResourceTypeInstance), - Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), }, { ResourceType: aws.String(ec2.ResourceTypeVolume), @@ -893,7 +967,7 @@ func TestCreateLaunchTemplate(t *testing.T) { TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ { ResourceType: aws.String(ec2.ResourceTypeInstance), - Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), }, { ResourceType: aws.String(ec2.ResourceTypeVolume), @@ -948,7 +1022,7 @@ func TestCreateLaunchTemplate(t *testing.T) { tc.expect(g, mockEC2Client.EXPECT()) } - launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userData) + launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, userData) tc.check(g, launchTemplate, err) }) } @@ -972,7 +1046,11 @@ func TestLaunchTemplateDataCreation(t *testing.T) { s := NewService(cs) - launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), nil) + userDataSecretKey := types.NamespacedName{ + Namespace: "bootstrap-secret-ns", + Name: "bootstrap-secret", + } + launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, nil) g.Expect(err).To(HaveOccurred()) g.Expect(launchTemplate).Should(BeEmpty()) }) @@ -987,6 +1065,10 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { sortTags(arg.LaunchTemplateData.TagSpecifications[index].Tags) } } + userDataSecretKey := types.NamespacedName{ + Namespace: "bootstrap-secret-ns", + Name: "bootstrap-secret", + } userData := []byte{1, 0, 0} testCases := []struct { name string @@ -1022,7 +1104,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ { ResourceType: aws.String(ec2.ResourceTypeInstance), - Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), }, { ResourceType: aws.String(ec2.ResourceTypeVolume), @@ -1041,7 +1123,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { // formatting added to match tags slice during cmp.Equal() formatTagsInput(arg) if !cmp.Equal(expectedInput, arg) { - t.Fatalf("mismatch in input expected: %+v, but got %+v", expectedInput, arg) + t.Fatalf("mismatch in input expected: %+v, but got %+v, diff: %s", expectedInput, arg, cmp.Diff(expectedInput, arg)) } }) }, @@ -1073,7 +1155,7 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ { ResourceType: aws.String(ec2.ResourceTypeInstance), - Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), }, { ResourceType: aws.String(ec2.ResourceTypeVolume), @@ -1120,10 +1202,10 @@ func TestCreateLaunchTemplateVersion(t *testing.T) { tc.expect(mockEC2Client.EXPECT()) } if tc.wantErr { - g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userData)).To(HaveOccurred()) + g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData)).To(HaveOccurred()) return } - g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userData)).NotTo(HaveOccurred()) + g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData)).NotTo(HaveOccurred()) }) } } @@ -1132,6 +1214,10 @@ func TestBuildLaunchTemplateTagSpecificationRequest(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() + userDataSecretKey := types.NamespacedName{ + Namespace: "bootstrap-secret-ns", + Name: "bootstrap-secret", + } testCases := []struct { name string check func(g *WithT, m []*ec2.LaunchTemplateTagSpecificationRequest) @@ -1142,7 +1228,7 @@ func TestBuildLaunchTemplateTagSpecificationRequest(t *testing.T) { expected := []*ec2.LaunchTemplateTagSpecificationRequest{ { ResourceType: aws.String(ec2.ResourceTypeInstance), - Tags: defaultEC2Tags("aws-mp-name", "cluster-name"), + Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey), }, { ResourceType: aws.String(ec2.ResourceTypeVolume), @@ -1172,7 +1258,7 @@ func TestBuildLaunchTemplateTagSpecificationRequest(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) s := NewService(cs) - tc.check(g, s.buildLaunchTemplateTagSpecificationRequest(ms)) + tc.check(g, s.buildLaunchTemplateTagSpecificationRequest(ms, userDataSecretKey)) }) } } diff --git a/pkg/cloud/services/interfaces.go b/pkg/cloud/services/interfaces.go index df6b4c179e..f6bbd3b169 100644 --- a/pkg/cloud/services/interfaces.go +++ b/pkg/cloud/services/interfaces.go @@ -17,6 +17,8 @@ limitations under the License. package services import ( + apimachinerytypes "k8s.io/apimachinery/pkg/types" + infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" @@ -65,15 +67,12 @@ type EC2Interface interface { TerminateInstanceAndWait(instanceID string) error DetachSecurityGroupsFromNetworkInterface(groups []string, interfaceID string) error - ReconcileLaunchTemplate(scope scope.LaunchTemplateScope, canUpdateLaunchTemplate func() (bool, error), runPostLaunchTemplateUpdateOperation func() error) error - ReconcileTags(scope scope.LaunchTemplateScope, resourceServicesToUpdate []scope.ResourceServiceToUpdate) error - DiscoverLaunchTemplateAMI(scope scope.LaunchTemplateScope) (*string, error) - GetLaunchTemplate(id string) (lt *expinfrav1.AWSLaunchTemplate, userDataHash string, err error) + GetLaunchTemplate(id string) (lt *expinfrav1.AWSLaunchTemplate, userDataHash string, userDataSecretKey *apimachinerytypes.NamespacedName, err error) GetLaunchTemplateID(id string) (string, error) GetLaunchTemplateLatestVersion(id string) (string, error) - CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userData []byte) (string, error) - CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userData []byte) error + CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) (string, error) + CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) error PruneLaunchTemplateVersions(id string) error DeleteLaunchTemplate(id string) error LaunchTemplateNeedsUpdate(scope scope.LaunchTemplateScope, incoming *expinfrav1.AWSLaunchTemplate, existing *expinfrav1.AWSLaunchTemplate) (bool, error) @@ -81,6 +80,14 @@ type EC2Interface interface { ReconcileBastion() error } +// MachinePoolReconcileInterface encapsulates high-level reconciliation functions regarding EC2 reconciliation. It is +// separate from EC2Interface so that we can mock AWS requests separately. For example, by not mocking the +// ReconcileLaunchTemplate function, but mocking EC2Interface, we can test which EC2 API operations would have been called. +type MachinePoolReconcileInterface interface { + ReconcileLaunchTemplate(scope scope.LaunchTemplateScope, ec2svc EC2Interface, canUpdateLaunchTemplate func() (bool, error), runPostLaunchTemplateUpdateOperation func() error) error + ReconcileTags(scope scope.LaunchTemplateScope, resourceServicesToUpdate []scope.ResourceServiceToUpdate) error +} + // SecretInterface encapsulated the methods exposed to the // machine actuator. type SecretInterface interface { diff --git a/pkg/cloud/services/mock_services/doc.go b/pkg/cloud/services/mock_services/doc.go index 1783a0d750..04493e0002 100644 --- a/pkg/cloud/services/mock_services/doc.go +++ b/pkg/cloud/services/mock_services/doc.go @@ -17,6 +17,8 @@ limitations under the License. // Run go generate to regenerate this mock. //nolint:revive //go:generate ../../../../hack/tools/bin/mockgen -destination ec2_interface_mock.go -package mock_services sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services EC2Interface //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt ec2_interface_mock.go > _ec2_interface_mock.go && mv _ec2_interface_mock.go ec2_interface_mock.go" +//go:generate ../../../../hack/tools/bin/mockgen -destination reconcile_interface_mock.go -package mock_services sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services MachinePoolReconcileInterface +//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt reconcile_interface_mock.go > _reconcile_interface_mock.go && mv _reconcile_interface_mock.go reconcile_interface_mock.go" //go:generate ../../../../hack/tools/bin/mockgen -destination secretsmanager_machine_interface_mock.go -package mock_services sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services SecretInterface //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt secretsmanager_machine_interface_mock.go > _secretsmanager_machine_interface_mock.go && mv _secretsmanager_machine_interface_mock.go secretsmanager_machine_interface_mock.go" //go:generate ../../../../hack/tools/bin/mockgen -destination objectstore_machine_interface_mock.go -package mock_services sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services ObjectStoreInterface diff --git a/pkg/cloud/services/mock_services/ec2_interface_mock.go b/pkg/cloud/services/mock_services/ec2_interface_mock.go index 7f8cb07aaf..922d5f3360 100644 --- a/pkg/cloud/services/mock_services/ec2_interface_mock.go +++ b/pkg/cloud/services/mock_services/ec2_interface_mock.go @@ -24,6 +24,7 @@ import ( reflect "reflect" gomock "github.com/golang/mock/gomock" + types "k8s.io/apimachinery/pkg/types" v1beta2 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2" v1beta20 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" scope "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" @@ -68,32 +69,32 @@ func (mr *MockEC2InterfaceMockRecorder) CreateInstance(arg0, arg1, arg2 interfac } // CreateLaunchTemplate mocks base method. -func (m *MockEC2Interface) CreateLaunchTemplate(arg0 scope.LaunchTemplateScope, arg1 *string, arg2 []byte) (string, error) { +func (m *MockEC2Interface) CreateLaunchTemplate(arg0 scope.LaunchTemplateScope, arg1 *string, arg2 types.NamespacedName, arg3 []byte) (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateLaunchTemplate", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "CreateLaunchTemplate", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) return ret0, ret1 } // CreateLaunchTemplate indicates an expected call of CreateLaunchTemplate. -func (mr *MockEC2InterfaceMockRecorder) CreateLaunchTemplate(arg0, arg1, arg2 interface{}) *gomock.Call { +func (mr *MockEC2InterfaceMockRecorder) CreateLaunchTemplate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateLaunchTemplate", reflect.TypeOf((*MockEC2Interface)(nil).CreateLaunchTemplate), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateLaunchTemplate", reflect.TypeOf((*MockEC2Interface)(nil).CreateLaunchTemplate), arg0, arg1, arg2, arg3) } // CreateLaunchTemplateVersion mocks base method. -func (m *MockEC2Interface) CreateLaunchTemplateVersion(arg0 string, arg1 scope.LaunchTemplateScope, arg2 *string, arg3 []byte) error { +func (m *MockEC2Interface) CreateLaunchTemplateVersion(arg0 string, arg1 scope.LaunchTemplateScope, arg2 *string, arg3 types.NamespacedName, arg4 []byte) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateLaunchTemplateVersion", arg0, arg1, arg2, arg3) + ret := m.ctrl.Call(m, "CreateLaunchTemplateVersion", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(error) return ret0 } // CreateLaunchTemplateVersion indicates an expected call of CreateLaunchTemplateVersion. -func (mr *MockEC2InterfaceMockRecorder) CreateLaunchTemplateVersion(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +func (mr *MockEC2InterfaceMockRecorder) CreateLaunchTemplateVersion(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateLaunchTemplateVersion", reflect.TypeOf((*MockEC2Interface)(nil).CreateLaunchTemplateVersion), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateLaunchTemplateVersion", reflect.TypeOf((*MockEC2Interface)(nil).CreateLaunchTemplateVersion), arg0, arg1, arg2, arg3, arg4) } // DeleteBastion mocks base method. @@ -199,13 +200,14 @@ func (mr *MockEC2InterfaceMockRecorder) GetInstanceSecurityGroups(arg0 interface } // GetLaunchTemplate mocks base method. -func (m *MockEC2Interface) GetLaunchTemplate(arg0 string) (*v1beta20.AWSLaunchTemplate, string, error) { +func (m *MockEC2Interface) GetLaunchTemplate(arg0 string) (*v1beta20.AWSLaunchTemplate, string, *types.NamespacedName, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetLaunchTemplate", arg0) ret0, _ := ret[0].(*v1beta20.AWSLaunchTemplate) ret1, _ := ret[1].(string) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret2, _ := ret[2].(*types.NamespacedName) + ret3, _ := ret[3].(error) + return ret0, ret1, ret2, ret3 } // GetLaunchTemplate indicates an expected call of GetLaunchTemplate. @@ -331,34 +333,6 @@ func (mr *MockEC2InterfaceMockRecorder) ReconcileBastion() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReconcileBastion", reflect.TypeOf((*MockEC2Interface)(nil).ReconcileBastion)) } -// ReconcileLaunchTemplate mocks base method. -func (m *MockEC2Interface) ReconcileLaunchTemplate(arg0 scope.LaunchTemplateScope, arg1 func() (bool, error), arg2 func() error) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReconcileLaunchTemplate", arg0, arg1, arg2) - ret0, _ := ret[0].(error) - return ret0 -} - -// ReconcileLaunchTemplate indicates an expected call of ReconcileLaunchTemplate. -func (mr *MockEC2InterfaceMockRecorder) ReconcileLaunchTemplate(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReconcileLaunchTemplate", reflect.TypeOf((*MockEC2Interface)(nil).ReconcileLaunchTemplate), arg0, arg1, arg2) -} - -// ReconcileTags mocks base method. -func (m *MockEC2Interface) ReconcileTags(arg0 scope.LaunchTemplateScope, arg1 []scope.ResourceServiceToUpdate) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReconcileTags", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 -} - -// ReconcileTags indicates an expected call of ReconcileTags. -func (mr *MockEC2InterfaceMockRecorder) ReconcileTags(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReconcileTags", reflect.TypeOf((*MockEC2Interface)(nil).ReconcileTags), arg0, arg1) -} - // TerminateInstance mocks base method. func (m *MockEC2Interface) TerminateInstance(arg0 string) error { m.ctrl.T.Helper() diff --git a/pkg/cloud/services/mock_services/reconcile_interface_mock.go b/pkg/cloud/services/mock_services/reconcile_interface_mock.go new file mode 100644 index 0000000000..3771e81e3a --- /dev/null +++ b/pkg/cloud/services/mock_services/reconcile_interface_mock.go @@ -0,0 +1,80 @@ +/* +Copyright 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. +*/ + +// Code generated by MockGen. DO NOT EDIT. +// Source: sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services (interfaces: MachinePoolReconcileInterface) + +// Package mock_services is a generated GoMock package. +package mock_services + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + scope "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope" + services "sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services" +) + +// MockMachinePoolReconcileInterface is a mock of MachinePoolReconcileInterface interface. +type MockMachinePoolReconcileInterface struct { + ctrl *gomock.Controller + recorder *MockMachinePoolReconcileInterfaceMockRecorder +} + +// MockMachinePoolReconcileInterfaceMockRecorder is the mock recorder for MockMachinePoolReconcileInterface. +type MockMachinePoolReconcileInterfaceMockRecorder struct { + mock *MockMachinePoolReconcileInterface +} + +// NewMockMachinePoolReconcileInterface creates a new mock instance. +func NewMockMachinePoolReconcileInterface(ctrl *gomock.Controller) *MockMachinePoolReconcileInterface { + mock := &MockMachinePoolReconcileInterface{ctrl: ctrl} + mock.recorder = &MockMachinePoolReconcileInterfaceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockMachinePoolReconcileInterface) EXPECT() *MockMachinePoolReconcileInterfaceMockRecorder { + return m.recorder +} + +// ReconcileLaunchTemplate mocks base method. +func (m *MockMachinePoolReconcileInterface) ReconcileLaunchTemplate(arg0 scope.LaunchTemplateScope, arg1 services.EC2Interface, arg2 func() (bool, error), arg3 func() error) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ReconcileLaunchTemplate", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(error) + return ret0 +} + +// ReconcileLaunchTemplate indicates an expected call of ReconcileLaunchTemplate. +func (mr *MockMachinePoolReconcileInterfaceMockRecorder) ReconcileLaunchTemplate(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReconcileLaunchTemplate", reflect.TypeOf((*MockMachinePoolReconcileInterface)(nil).ReconcileLaunchTemplate), arg0, arg1, arg2, arg3) +} + +// ReconcileTags mocks base method. +func (m *MockMachinePoolReconcileInterface) ReconcileTags(arg0 scope.LaunchTemplateScope, arg1 []scope.ResourceServiceToUpdate) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ReconcileTags", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// ReconcileTags indicates an expected call of ReconcileTags. +func (mr *MockMachinePoolReconcileInterfaceMockRecorder) ReconcileTags(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReconcileTags", reflect.TypeOf((*MockMachinePoolReconcileInterface)(nil).ReconcileTags), arg0, arg1) +}