From 5cfea2c7732f95e59d71ac53bb3d029cb3ceec5b Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Tue, 23 Apr 2024 00:48:07 -0700 Subject: [PATCH] Use status vaules for security groups --- pkg/cloudprovider/drift.go | 2 +- pkg/cloudprovider/suite_test.go | 88 +++++++++++---- .../nodeclass/status/controller.go | 5 +- .../nodeclass/status/securitygroup.go | 6 +- .../nodeclass/status/securitygroup_test.go | 10 -- .../nodeclass/status/suite_test.go | 1 - .../providers/securtiygroup/controller.go | 60 ----------- .../providers/securtiygroup/suite_test.go | 59 ----------- pkg/operator/operator.go | 2 +- pkg/providers/instance/suite_test.go | 15 ++- pkg/providers/instancetype/suite_test.go | 55 +++++++++- .../launchtemplate/launchtemplate.go | 14 ++- pkg/providers/launchtemplate/suite_test.go | 100 +++++++++++++++++- pkg/providers/securitygroup/securitygroup.go | 77 +++++++------- pkg/providers/securitygroup/suite_test.go | 36 +++---- pkg/test/environment.go | 2 +- 16 files changed, 296 insertions(+), 236 deletions(-) delete mode 100644 pkg/controllers/providers/securtiygroup/controller.go delete mode 100644 pkg/controllers/providers/securtiygroup/suite_test.go diff --git a/pkg/cloudprovider/drift.go b/pkg/cloudprovider/drift.go index 69dc5aad1783..c5a5ddd924b2 100644 --- a/pkg/cloudprovider/drift.go +++ b/pkg/cloudprovider/drift.go @@ -123,7 +123,7 @@ func (c *CloudProvider) areSecurityGroupsDrifted(ctx context.Context, ec2Instanc if err != nil { return "", err } - securityGroupIds := sets.New(lo.Map(securitygroup, func(sg *ec2.SecurityGroup, _ int) string { return aws.StringValue(sg.GroupId) })...) + securityGroupIds := sets.New(lo.Map(securitygroup, func(sg v1beta1.SecurityGroup, _ int) string { return sg.ID })...) if len(securityGroupIds) == 0 { return "", fmt.Errorf("no security groups are discovered") } diff --git a/pkg/cloudprovider/suite_test.go b/pkg/cloudprovider/suite_test.go index b07f7f6c0d18..d47b6b7f647a 100644 --- a/pkg/cloudprovider/suite_test.go +++ b/pkg/cloudprovider/suite_test.go @@ -39,15 +39,19 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/apis" "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "github.com/aws/karpenter-provider-aws/pkg/cloudprovider" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status" "github.com/aws/karpenter-provider-aws/pkg/fake" "github.com/aws/karpenter-provider-aws/pkg/operator/options" "github.com/aws/karpenter-provider-aws/pkg/test" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" corecloudproivder "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/controllers/provisioning" "sigs.k8s.io/karpenter/pkg/controllers/state" "sigs.k8s.io/karpenter/pkg/events" + "sigs.k8s.io/karpenter/pkg/operator/controller" coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" "sigs.k8s.io/karpenter/pkg/operator/scheme" coretest "sigs.k8s.io/karpenter/pkg/test" @@ -67,6 +71,7 @@ var cloudProvider *cloudprovider.CloudProvider var cluster *state.Cluster var fakeClock *clock.FakeClock var recorder events.Recorder +var statusController controller.Controller func TestAWS(t *testing.T) { ctx = TestContextWithLogger(t) @@ -86,6 +91,14 @@ var _ = BeforeSuite(func() { env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider, awsEnv.SubnetProvider) cluster = state.NewCluster(fakeClock, env.Client, cloudProvider) prov = provisioning.NewProvisioner(env.Client, recorder, cloudProvider, cluster) + statusController = status.NewController( + env.Client, + awsEnv.SubnetProvider, + awsEnv.SecurityGroupProvider, + awsEnv.AMIProvider, + awsEnv.InstanceProfileProvider, + awsEnv.LaunchTemplateProvider, + ) }) var _ = AfterSuite(func() { @@ -140,7 +153,6 @@ var _ = Describe("CloudProvider", func() { }) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed()) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed()) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) }) It("should return an ICE error when there are no instance types to launch", func() { // Specify no instance types and expect to receive a capacity error @@ -154,12 +166,15 @@ var _ = Describe("CloudProvider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim) Expect(corecloudproivder.IsInsufficientCapacityError(err)).To(BeTrue()) Expect(cloudProviderNodeClaim).To(BeNil()) }) It("should set ImageID in the status field of the nodeClaim", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim) Expect(err).To(BeNil()) Expect(cloudProviderNodeClaim).ToNot(BeNil()) @@ -167,6 +182,7 @@ var _ = Describe("CloudProvider", func() { }) It("should return NodeClass Hash on the nodeClaim", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim) Expect(err).To(BeNil()) Expect(cloudProviderNodeClaim).ToNot(BeNil()) @@ -175,6 +191,7 @@ var _ = Describe("CloudProvider", func() { }) It("should return NodeClass Hash Version on the nodeClaim", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim) Expect(err).To(BeNil()) Expect(cloudProviderNodeClaim).ToNot(BeNil()) @@ -187,6 +204,7 @@ var _ = Describe("CloudProvider", func() { It("should set context on the CreateFleet request if specified on the NodePool", func() { nodeClass.Spec.Context = aws.String(contextID) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -196,6 +214,7 @@ var _ = Describe("CloudProvider", func() { }) It("should default to no EC2 Context", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -269,6 +288,7 @@ var _ = Describe("CloudProvider", func() { }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) // 2 pods are created with resources such that both fit together only in one of the 2 InstanceTypes created above. pod1 := coretest.UnschedulablePod( @@ -365,6 +385,7 @@ var _ = Describe("CloudProvider", func() { }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) // 2 pods are created with resources such that both fit together only in one of the 2 InstanceTypes created above. pod1 := coretest.UnschedulablePod( @@ -471,6 +492,7 @@ var _ = Describe("CloudProvider", func() { }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod1 := coretest.UnschedulablePod( coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{Requests: v1.ResourceList{ @@ -566,8 +588,10 @@ var _ = Describe("CloudProvider", func() { awsEnv.EC2API.DescribeSubnetsOutput.Set(&ec2.DescribeSubnetsOutput{ Subnets: []*ec2.Subnet{ { - SubnetId: aws.String(validSubnet1), - AvailabilityZone: aws.String("zone-1"), + SubnetId: aws.String(validSubnet1), + AvailabilityZone: aws.String("zone-1"), + AvailableIpAddressCount: aws.Int64(100), + MapPublicIpOnLaunch: aws.Bool(false), Tags: []*ec2.Tag{ { Key: aws.String("sn-key-1"), @@ -576,8 +600,10 @@ var _ = Describe("CloudProvider", func() { }, }, { - SubnetId: aws.String(validSubnet2), - AvailabilityZone: aws.String("zone-2"), + SubnetId: aws.String(validSubnet2), + AvailabilityZone: aws.String("zone-2"), + AvailableIpAddressCount: aws.Int64(100), + MapPublicIpOnLaunch: aws.Bool(false), Tags: []*ec2.Tag{ { Key: aws.String("sn-key-2"), @@ -588,6 +614,7 @@ var _ = Describe("CloudProvider", func() { }, }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) instanceTypes, err := cloudProvider.GetInstanceTypes(ctx, nodePool) Expect(err).ToNot(HaveOccurred()) selectedInstanceType = instanceTypes[0] @@ -620,9 +647,10 @@ var _ = Describe("CloudProvider", func() { v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, }) nodeClaim.Labels = lo.Assign(nodeClaim.Labels, map[string]string{v1.LabelInstanceTypeStable: selectedInstanceType.Name}) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) }) It("should not fail if NodeClass does not exist", func() { + controllerutil.RemoveFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) ExpectDeleted(ctx, env.Client, nodeClass) drifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).ToNot(HaveOccurred()) @@ -651,6 +679,7 @@ var _ = Describe("CloudProvider", func() { v1beta1.AnnotationEC2NodeClassHash: "abcdefghijkl", }) ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).ToNot(HaveOccurred()) Expect(isDrifted).To(Equal(cloudprovider.NodeClassDrift)) @@ -674,7 +703,8 @@ var _ = Describe("CloudProvider", func() { }) It("should return an error if the security groups are empty", func() { awsEnv.EC2API.DescribeSecurityGroupsOutput.Set(&ec2.DescribeSecurityGroupsOutput{SecurityGroups: []*ec2.SecurityGroup{}}) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) + nodeClass.Status.SecurityGroups = []v1beta1.SecurityGroup{} + ExpectApplied(ctx, env.Client, nodeClass) // Instance is a reference to what we return in the GetInstances call instance.SecurityGroups = []*ec2.GroupIdentifier{{GroupId: aws.String(fake.SecurityGroupID())}} _, err := cloudProvider.IsDrifted(ctx, nodeClaim) @@ -695,19 +725,17 @@ var _ = Describe("CloudProvider", func() { Expect(isDrifted).To(Equal(cloudprovider.SecurityGroupDrift)) }) It("should return drifted if more security groups are present than instance security groups then discovered from nodeclass", func() { - awsEnv.EC2API.DescribeSecurityGroupsOutput.Set(&ec2.DescribeSecurityGroupsOutput{ - SecurityGroups: []*ec2.SecurityGroup{ - { - GroupId: aws.String(validSecurityGroup), - GroupName: aws.String("test-securitygroup"), - }, - { - GroupId: aws.String(fake.SecurityGroupID()), - GroupName: aws.String("test-securitygroup"), - }, + nodeClass.Status.SecurityGroups = []v1beta1.SecurityGroup{ + { + ID: validSecurityGroup, + Name: "test-securitygroup", }, - }) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) + { + ID: fake.SecurityGroupID(), + Name: "test-securitygroup", + }, + } + ExpectApplied(ctx, env.Client, nodeClass) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).ToNot(HaveOccurred()) Expect(isDrifted).To(Equal(cloudprovider.SecurityGroupDrift)) @@ -738,6 +766,7 @@ var _ = Describe("CloudProvider", func() { It("should return drifted if the AMI no longer matches the existing NodeClaims instance type", func() { nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{ID: amdAMIID}} ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).ToNot(HaveOccurred()) Expect(isDrifted).To(Equal(cloudprovider.AMIDrift)) @@ -788,6 +817,7 @@ var _ = Describe("CloudProvider", func() { DescribeTable("should return drifted if a statically drifted EC2NodeClass.Spec field is updated", func(changes v1beta1.EC2NodeClass) { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -796,6 +826,7 @@ var _ = Describe("CloudProvider", func() { nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash()}) ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err = cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(Equal(cloudprovider.NodeClassDrift)) @@ -828,6 +859,7 @@ var _ = Describe("CloudProvider", func() { nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash()}) ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(Equal(cloudprovider.NodeClassDrift)) @@ -835,7 +867,7 @@ var _ = Describe("CloudProvider", func() { DescribeTable("should not return drifted if dynamic fields are updated", func(changes v1beta1.EC2NodeClass) { ExpectApplied(ctx, env.Client, nodePool, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -844,7 +876,7 @@ var _ = Describe("CloudProvider", func() { nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash()}) ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err = cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -861,6 +893,7 @@ var _ = Describe("CloudProvider", func() { "Test Key": "Test Value", } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -875,6 +908,7 @@ var _ = Describe("CloudProvider", func() { v1beta1.AnnotationEC2NodeClassHashVersion: "test-hash-version-2", } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -892,6 +926,7 @@ var _ = Describe("CloudProvider", func() { "Test Key": "Test Value", } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -909,6 +944,8 @@ var _ = Describe("CloudProvider", func() { "Test Key": "Test Value", } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim) Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) @@ -920,6 +957,7 @@ var _ = Describe("CloudProvider", func() { // hard coded fixture data (ex. what the aws api will return) is maintained in fake/ec2api.go It("should default to the cluster's subnets", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod( coretest.PodOptions{NodeSelector: map[string]string{v1.LabelArchStable: corev1beta1.ArchitectureAmd64}}) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) @@ -951,6 +989,7 @@ var _ = Describe("CloudProvider", func() { Tags: []*ec2.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-2")}}}, }}) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{v1.LabelTopologyZone: "test-zone-1a"}}) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -966,6 +1005,7 @@ var _ = Describe("CloudProvider", func() { }}) nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{MaxPods: aws.Int32(1)} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod1 := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{v1.LabelTopologyZone: "test-zone-1a"}}) pod2 := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{v1.LabelTopologyZone: "test-zone-1a"}}) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod1, pod2) @@ -1000,6 +1040,7 @@ var _ = Describe("CloudProvider", func() { }}) nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{{Tags: map[string]string{"Name": "test-subnet-1"}}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) podSubnet1 := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, podSubnet1) ExpectScheduled(ctx, env.Client, podSubnet1) @@ -1032,6 +1073,7 @@ var _ = Describe("CloudProvider", func() { }, }) ExpectApplied(ctx, env.Client, nodePool2, nodeClass2) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass2)) podSubnet2 := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{corev1beta1.NodePoolLabelKey: nodePool2.Name}}) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, podSubnet2) ExpectScheduled(ctx, env.Client, podSubnet2) @@ -1073,6 +1115,8 @@ var _ = Describe("CloudProvider", func() { }, }) ExpectApplied(ctx, env.Client, nodePool, nodePool2, nodeClass, misconfiguredNodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(misconfiguredNodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1091,6 +1135,7 @@ var _ = Describe("CloudProvider", func() { } nodeClaim.Spec.Resources.Requests = v1.ResourceList{v1beta1.ResourceEFA: resource.MustParse("1")} ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim) Expect(err).To(BeNil()) Expect(lo.Keys(cloudProviderNodeClaim.Status.Allocatable)).To(ContainElement(v1beta1.ResourceEFA)) @@ -1106,6 +1151,7 @@ var _ = Describe("CloudProvider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodeClaim) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim) Expect(err).To(BeNil()) Expect(lo.Keys(cloudProviderNodeClaim.Status.Allocatable)).ToNot(ContainElement(v1beta1.ResourceEFA)) diff --git a/pkg/controllers/nodeclass/status/controller.go b/pkg/controllers/nodeclass/status/controller.go index da4aeca04dc0..037125d4ab70 100644 --- a/pkg/controllers/nodeclass/status/controller.go +++ b/pkg/controllers/nodeclass/status/controller.go @@ -63,7 +63,7 @@ func NewController(kubeClient client.Client, subnetProvider subnet.Provider, sec ami: &AMI{amiProvider: amiProvider}, subnet: &Subnet{subnetProvider: subnetProvider}, - securitygroup: &SecurityGroup{securityGroupProvider: securityGroupProvider}, + securitygroup: &SecurityGroup{SecurityGroupProvider: securityGroupProvider}, instanceprofile: &InstanceProfile{instanceProfileProvider: instanceProfileProvider}, launchtemplate: &LaunchTemplate{launchTemplateProvider: launchTemplateProvider}, }) @@ -77,6 +77,9 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl return reconcile.Result{}, err } } + c.securitygroup.LockProvider() + defer c.securitygroup.UnlockProvider() + stored := nodeClass.DeepCopy() var results []reconcile.Result diff --git a/pkg/controllers/nodeclass/status/securitygroup.go b/pkg/controllers/nodeclass/status/securitygroup.go index 378398ef04c0..04bc99ab8048 100644 --- a/pkg/controllers/nodeclass/status/securitygroup.go +++ b/pkg/controllers/nodeclass/status/securitygroup.go @@ -29,11 +29,11 @@ import ( ) type SecurityGroup struct { - securityGroupProvider securitygroup.Provider + SecurityGroupProvider securitygroup.Provider } func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { - securityGroups, err := sg.securityGroupProvider.List(ctx, nodeClass) + securityGroups, err := sg.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) if err != nil { return reconcile.Result{}, err } @@ -51,4 +51,4 @@ func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2No } }) return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil -} +} \ No newline at end of file diff --git a/pkg/controllers/nodeclass/status/securitygroup_test.go b/pkg/controllers/nodeclass/status/securitygroup_test.go index aa0791b3b180..ad0589122921 100644 --- a/pkg/controllers/nodeclass/status/securitygroup_test.go +++ b/pkg/controllers/nodeclass/status/securitygroup_test.go @@ -50,7 +50,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }) It("Should update EC2NodeClass status for Security Groups", func() { ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -78,7 +77,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }, } ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -99,7 +97,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }, } ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -111,7 +108,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }) It("Should update Security Groups status when the Security Groups selector gets updated by tags", func() { ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -138,7 +134,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }, } ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -154,7 +149,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }) It("Should update Security Groups status when the Security Groups selector gets updated by ids", func() { ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -178,7 +172,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }, } ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -195,14 +188,12 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }, } ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(BeNil()) }) It("Should not resolve a invalid selectors for an updated Security Groups selector", func() { ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ @@ -226,7 +217,6 @@ var _ = Describe("NodeClass Security Group Status Controller", func() { }, } ExpectApplied(ctx, env.Client, nodeClass) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass = ExpectExists(ctx, env.Client, nodeClass) Expect(nodeClass.Status.SecurityGroups).To(BeNil()) diff --git a/pkg/controllers/nodeclass/status/suite_test.go b/pkg/controllers/nodeclass/status/suite_test.go index 1e02644abbe3..545c9ba6cfd5 100644 --- a/pkg/controllers/nodeclass/status/suite_test.go +++ b/pkg/controllers/nodeclass/status/suite_test.go @@ -73,7 +73,6 @@ var _ = BeforeEach(func() { ctx = coreoptions.ToContext(ctx, coretest.Options()) nodeClass = test.EC2NodeClass() awsEnv.Reset() - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) }) var _ = AfterEach(func() { diff --git a/pkg/controllers/providers/securtiygroup/controller.go b/pkg/controllers/providers/securtiygroup/controller.go deleted file mode 100644 index 50e72ec8112e..000000000000 --- a/pkg/controllers/providers/securtiygroup/controller.go +++ /dev/null @@ -1,60 +0,0 @@ -/* -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 securitygroup - -import ( - "context" - "time" - - controllerruntime "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" - - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" - "github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup" -) - -type Controller struct { - securitygroupProvider securitygroup.Provider -} - -func NewController(securitygroupProvider securitygroup.Provider) *Controller { - return &Controller{ - securitygroupProvider: securitygroupProvider, - } -} - -func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { - if err := c.securitygroupProvider.UpdateSecurityGroup(ctx, nodeClass); err != nil { - return reconcile.Result{}, err - } - return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil -} - -func (c *Controller) Name() string { - return "providers.securitygroup" -} - -func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { - return corecontroller.Adapt(controllerruntime. - NewControllerManagedBy(m). - For(&v1beta1.EC2NodeClass{}). - WithOptions(controller.Options{ - MaxConcurrentReconciles: 10, - })) -} diff --git a/pkg/controllers/providers/securtiygroup/suite_test.go b/pkg/controllers/providers/securtiygroup/suite_test.go deleted file mode 100644 index df09685b6a96..000000000000 --- a/pkg/controllers/providers/securtiygroup/suite_test.go +++ /dev/null @@ -1,59 +0,0 @@ -/* -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 securitygroup_test - -import ( - "context" - "testing" - - coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" - "sigs.k8s.io/karpenter/pkg/operator/scheme" - coretest "sigs.k8s.io/karpenter/pkg/test" - - "github.com/aws/karpenter-provider-aws/pkg/apis" - controllerspricing "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/pricing" - "github.com/aws/karpenter-provider-aws/pkg/operator/options" - "github.com/aws/karpenter-provider-aws/pkg/test" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - . "knative.dev/pkg/logging/testing" -) - -var ctx context.Context -var stop context.CancelFunc -var env *coretest.Environment -var awsEnv *test.Environment -var controller *controllerspricing.Controller - -func TestAWS(t *testing.T) { - ctx = TestContextWithLogger(t) - RegisterFailHandler(Fail) - RunSpecs(t, "SecurityGroup") -} - -var _ = BeforeSuite(func() { - env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...)) - ctx = coreoptions.ToContext(ctx, coretest.Options()) - ctx = options.ToContext(ctx, test.Options()) - ctx, stop = context.WithCancel(ctx) - awsEnv = test.NewEnvironment(ctx, env) - controller = controllerspricing.NewController(awsEnv.PricingProvider) -}) - -var _ = AfterSuite(func() { - stop() - Expect(env.Stop()).To(Succeed(), "Failed to stop environment") -}) diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 39be55788322..572d5ec79843 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -133,7 +133,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont unavailableOfferingsCache := awscache.NewUnavailableOfferings() subnetProvider := subnet.NewDefaultProvider(ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - securityGroupProvider := securitygroup.NewDefaultProvider(ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + securityGroupProvider := securitygroup.NewDefaultProvider(operator.GetClient(), ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) instanceProfileProvider := instanceprofile.NewDefaultProvider(*sess.Config.Region, iam.New(sess), cache.New(awscache.InstanceProfileTTL, awscache.DefaultCleanupInterval)) pricingProvider := pricing.NewDefaultProvider( ctx, diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index 6758984a04d3..3b788535457f 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -26,9 +26,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/events" + "sigs.k8s.io/karpenter/pkg/operator/controller" coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" "sigs.k8s.io/karpenter/pkg/operator/scheme" coretest "sigs.k8s.io/karpenter/pkg/test" @@ -36,6 +38,7 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/apis" "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "github.com/aws/karpenter-provider-aws/pkg/cloudprovider" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status" "github.com/aws/karpenter-provider-aws/pkg/fake" "github.com/aws/karpenter-provider-aws/pkg/operator/options" "github.com/aws/karpenter-provider-aws/pkg/providers/instance" @@ -51,6 +54,7 @@ var ctx context.Context var env *coretest.Environment var awsEnv *test.Environment var cloudProvider *cloudprovider.CloudProvider +var statusController controller.Controller func TestAWS(t *testing.T) { ctx = TestContextWithLogger(t) @@ -65,6 +69,14 @@ var _ = BeforeSuite(func() { awsEnv = test.NewEnvironment(ctx, env) cloudProvider = cloudprovider.New(awsEnv.InstanceTypesProvider, awsEnv.InstanceProvider, events.NewRecorder(&record.FakeRecorder{}), env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider, awsEnv.SubnetProvider) + statusController = status.NewController( + env.Client, + awsEnv.SubnetProvider, + awsEnv.SecurityGroupProvider, + awsEnv.AMIProvider, + awsEnv.InstanceProfileProvider, + awsEnv.LaunchTemplateProvider, + ) }) var _ = AfterSuite(func() { @@ -117,7 +129,8 @@ var _ = Describe("InstanceProvider", func() { {CapacityType: corev1beta1.CapacityTypeSpot, InstanceType: "m5.xlarge", Zone: "test-zone-1a"}, {CapacityType: corev1beta1.CapacityTypeSpot, InstanceType: "m5.xlarge", Zone: "test-zone-1b"}, }) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) instanceTypes, err := cloudProvider.GetInstanceTypes(ctx, nodePool) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/providers/instancetype/suite_test.go b/pkg/providers/instancetype/suite_test.go index f8859e828f0f..0e8074feb285 100644 --- a/pkg/providers/instancetype/suite_test.go +++ b/pkg/providers/instancetype/suite_test.go @@ -39,11 +39,13 @@ import ( . "knative.dev/pkg/logging/testing" "knative.dev/pkg/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider" "sigs.k8s.io/karpenter/pkg/controllers/provisioning" "sigs.k8s.io/karpenter/pkg/controllers/state" "sigs.k8s.io/karpenter/pkg/events" + "sigs.k8s.io/karpenter/pkg/operator/controller" coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" "sigs.k8s.io/karpenter/pkg/operator/scheme" "sigs.k8s.io/karpenter/pkg/scheduling" @@ -54,6 +56,7 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/apis" "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "github.com/aws/karpenter-provider-aws/pkg/cloudprovider" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status" "github.com/aws/karpenter-provider-aws/pkg/fake" "github.com/aws/karpenter-provider-aws/pkg/operator/options" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily" @@ -68,6 +71,7 @@ var fakeClock *clock.FakeClock var prov *provisioning.Provisioner var cluster *state.Cluster var cloudProvider *cloudprovider.CloudProvider +var statusController controller.Controller func TestAWS(t *testing.T) { ctx = TestContextWithLogger(t) @@ -85,6 +89,14 @@ var _ = BeforeSuite(func() { env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider, awsEnv.SubnetProvider) cluster = state.NewCluster(fakeClock, env.Client, cloudProvider) prov = provisioning.NewProvisioner(env.Client, events.NewRecorder(&record.FakeRecorder{}), cloudProvider, cluster) + statusController = status.NewController( + env.Client, + awsEnv.SubnetProvider, + awsEnv.SecurityGroupProvider, + awsEnv.AMIProvider, + awsEnv.InstanceProfileProvider, + awsEnv.LaunchTemplateProvider, + ) }) var _ = AfterSuite(func() { @@ -157,12 +169,12 @@ var _ = Describe("InstanceTypeProvider", func() { }) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed()) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed()) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, windowsNodeClass)).To(BeNil()) }) It("should support individual instance type labels", func() { ExpectApplied(ctx, env.Client, nodePool, windowsNodePool, nodeClass, windowsNodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(windowsNodeClass)) nodeSelector := map[string]string{ // Well known @@ -216,6 +228,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should support combined instance type labels", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeSelector := map[string]string{ // Well known @@ -267,6 +280,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should support instance type labels with accelerator", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeSelector := map[string]string{ // Well known @@ -317,6 +331,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should not launch AWS Pod ENI on a t3", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ NodeSelector: map[string]string{ v1.LabelInstanceTypeStable: "t3.large", @@ -340,6 +355,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed()) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed()) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("1")}, @@ -402,6 +418,7 @@ var _ = Describe("InstanceTypeProvider", func() { Expect(awsEnv.PricingProvider.UpdateSpotPricing(ctx)).To(Succeed()) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed()) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed()) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ @@ -461,6 +478,7 @@ var _ = Describe("InstanceTypeProvider", func() { // Apply requirements and schedule pods. ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("1")}, @@ -485,6 +503,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should de-prioritize metal", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("1")}, @@ -504,6 +523,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should de-prioritize gpu types", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceCPU: resource.MustParse("1")}, @@ -530,6 +550,7 @@ var _ = Describe("InstanceTypeProvider", func() { }, }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ NodeSelector: map[string]string{ v1beta1.LabelInstanceSize: "metal", @@ -544,6 +565,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should launch AWS Pod ENI on a compatible instance type", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1beta1.ResourceAWSPodENI: resource.MustParse("1")}, @@ -562,6 +584,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("should launch instances for Nvidia GPU resource requests", func() { nodeNames := sets.NewString() ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ @@ -595,6 +618,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("should launch instances for Habana GPU resource requests", func() { nodeNames := sets.NewString() ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ @@ -626,6 +650,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("should launch instances for AWS Neuron resource requests", func() { nodeNames := sets.NewString() ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ @@ -668,6 +693,7 @@ var _ = Describe("InstanceTypeProvider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ @@ -695,6 +721,7 @@ var _ = Describe("InstanceTypeProvider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ @@ -720,6 +747,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should not launch instances w/ instance storage for ephemeral storage resource requests when exceeding blockDeviceMapping", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceEphemeralStorage: resource.MustParse("5000Gi")}, @@ -731,6 +759,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("should launch instances w/ instance storage for ephemeral storage resource requests when disks are mounted for ephemeral-storage", func() { nodeClass.Spec.InstanceStorePolicy = lo.ToPtr(v1beta1.InstanceStorePolicyRAID0) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1.ResourceEphemeralStorage: resource.MustParse("5000Gi")}, @@ -854,6 +883,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should launch instances in local zones", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ NodeRequirements: []v1.NodeSelectorRequirement{{ Key: v1.LabelTopologyZone, @@ -1675,6 +1705,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) its, err := cloudProvider.GetInstanceTypes(ctx, nodePool) Expect(err).To(BeNil()) @@ -1728,6 +1759,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("should launch instances of different type on second reconciliation attempt with Insufficient Capacity Error Cache fallback", func() { awsEnv.EC2API.InsufficientCapacityPools.Set([]fake.CapacityPool{{CapacityType: corev1beta1.CapacityTypeOnDemand, InstanceType: "inf1.6xlarge", Zone: "test-zone-1a"}}) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ NodeSelector: map[string]string{v1.LabelTopologyZone: "test-zone-1a"}, @@ -1775,6 +1807,7 @@ var _ = Describe("InstanceTypeProvider", func() { }, }}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) // it should've tried to pack them in test-zone-1a on a p3.8xlarge then hit insufficient capacity, the next attempt will try test-zone-1b ExpectNotScheduled(ctx, env.Client, pod) @@ -1809,6 +1842,7 @@ var _ = Describe("InstanceTypeProvider", func() { } // Provisions 2 m5.large instances since m5.xlarge was ICE'd ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pods...) for _, pod := range pods { ExpectNotScheduled(ctx, env.Client, pod) @@ -1822,6 +1856,7 @@ var _ = Describe("InstanceTypeProvider", func() { It("should launch instances on later reconciliation attempt with Insufficient Capacity Error Cache expiry", func() { awsEnv.EC2API.InsufficientCapacityPools.Set([]fake.CapacityPool{{CapacityType: corev1beta1.CapacityTypeOnDemand, InstanceType: "inf1.6xlarge", Zone: "test-zone-1a"}}) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ NodeSelector: map[string]string{v1.LabelInstanceTypeStable: "inf1.6xlarge"}, ResourceRequirements: v1.ResourceRequirements{ @@ -1855,6 +1890,7 @@ var _ = Describe("InstanceTypeProvider", func() { }, }}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) // it should've tried to pack them in test-zone-1a on a dl1.24xlarge then hit insufficient capacity, the next attempt will try test-zone-1b ExpectNotScheduled(ctx, env.Client, pod) @@ -1878,6 +1914,7 @@ var _ = Describe("InstanceTypeProvider", func() { } // Spot Unavailable ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectNotScheduled(ctx, env.Client, pod) @@ -1912,6 +1949,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) for _, ct := range []string{corev1beta1.CapacityTypeOnDemand, corev1beta1.CapacityTypeSpot} { for _, zone := range []string{"test-zone-1a", "test-zone-1b"} { ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, @@ -1943,6 +1981,7 @@ var _ = Describe("InstanceTypeProvider", func() { Context("CapacityType", func() { It("should default to on-demand", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -1952,6 +1991,7 @@ var _ = Describe("InstanceTypeProvider", func() { nodePool.Spec.Template.Spec.Requirements = []corev1beta1.NodeSelectorRequirementWithMinValues{ {NodeSelectorRequirement: v1.NodeSelectorRequirement{Key: corev1beta1.CapacityTypeLabelKey, Operator: v1.NodeSelectorOpIn, Values: []string{corev1beta1.CapacityTypeSpot, corev1beta1.CapacityTypeOnDemand}}}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -1979,6 +2019,7 @@ var _ = Describe("InstanceTypeProvider", func() { // Instance type with no zonal availability for spot shouldn't be scheduled ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectNotScheduled(ctx, env.Client, pod) @@ -2004,6 +2045,7 @@ var _ = Describe("InstanceTypeProvider", func() { } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -2032,6 +2074,7 @@ var _ = Describe("InstanceTypeProvider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -2045,6 +2088,7 @@ var _ = Describe("InstanceTypeProvider", func() { }) It("should default to EBS defaults when volumeSize is not defined in blockDeviceMappings for AL2 Root volume", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -2061,6 +2105,7 @@ var _ = Describe("InstanceTypeProvider", func() { awsEnv.LaunchTemplateProvider.CABundle = lo.ToPtr("Y2EtYnVuZGxlCg==") awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(lo.ToPtr("10.100.0.0/16")) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -2076,6 +2121,7 @@ var _ = Describe("InstanceTypeProvider", func() { nodeClass.Spec.AMIFamily = aws.String(v1beta1.AMIFamilyBottlerocket) nodeClass.Spec.BlockDeviceMappings[0].DeviceName = aws.String("/dev/xvdb") ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -2092,6 +2138,7 @@ var _ = Describe("InstanceTypeProvider", func() { nodeClass.Spec.AMIFamily = aws.String(v1beta1.AMIFamilyUbuntu) nodeClass.Spec.BlockDeviceMappings[0].DeviceName = aws.String("/dev/sda1") ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -2107,6 +2154,7 @@ var _ = Describe("InstanceTypeProvider", func() { Context("Metadata Options", func() { It("should default metadata options on generated launch template", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -2126,6 +2174,7 @@ var _ = Describe("InstanceTypeProvider", func() { HTTPTokens: aws.String(ec2.LaunchTemplateHttpTokensStateOptional), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -2164,6 +2213,7 @@ var _ = Describe("InstanceTypeProvider", func() { } var instanceTypeResult [][]*corecloudprovider.InstanceType ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) // Adding the general set of to the instancetype into the cache fullInstanceTypeList, err := cloudProvider.GetInstanceTypes(ctx, nodePool) Expect(err).To(BeNil()) @@ -2239,6 +2289,7 @@ var _ = Describe("InstanceTypeProvider", func() { var instanceTypeResult [][]*corecloudprovider.InstanceType ExpectApplied(ctx, env.Client, nodeClass) nodePool.Spec.Template.Spec.NodeClassRef.Name = nodeClass.Name + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) // Adding the general set of to the instancetype into the cache fullInstanceTypeList, err := cloudProvider.GetInstanceTypes(ctx, nodePool) Expect(err).To(BeNil()) diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index f7d692c6f0de..87fab12960ea 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -177,14 +177,12 @@ func (p *DefaultProvider) createAMIOptions(ctx context.Context, nodeClass *v1bet ClusterCIDR: p.ClusterCIDR.Load(), InstanceProfile: instanceProfile, InstanceStorePolicy: nodeClass.Spec.InstanceStorePolicy, - SecurityGroups: lo.Map(securityGroups, func(s *ec2.SecurityGroup, _ int) v1beta1.SecurityGroup { - return v1beta1.SecurityGroup{ID: aws.StringValue(s.GroupId), Name: aws.StringValue(s.GroupName)} - }), - Tags: tags, - Labels: labels, - CABundle: p.CABundle, - KubeDNSIP: p.KubeDNSIP, - NodeClassName: nodeClass.Name, + SecurityGroups: securityGroups, + Tags: tags, + Labels: labels, + CABundle: p.CABundle, + KubeDNSIP: p.KubeDNSIP, + NodeClassName: nodeClass.Name, } if nodeClass.Spec.AssociatePublicIPAddress != nil { options.AssociatePublicIPAddress = nodeClass.Spec.AssociatePublicIPAddress diff --git a/pkg/providers/launchtemplate/suite_test.go b/pkg/providers/launchtemplate/suite_test.go index bf4e13e4052f..2c655c08fe34 100644 --- a/pkg/providers/launchtemplate/suite_test.go +++ b/pkg/providers/launchtemplate/suite_test.go @@ -49,6 +49,7 @@ import ( "sigs.k8s.io/karpenter/pkg/controllers/provisioning" "sigs.k8s.io/karpenter/pkg/controllers/state" "sigs.k8s.io/karpenter/pkg/events" + "sigs.k8s.io/karpenter/pkg/operator/controller" coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" "sigs.k8s.io/karpenter/pkg/operator/scheme" coretest "sigs.k8s.io/karpenter/pkg/test" @@ -57,6 +58,7 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/apis" "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "github.com/aws/karpenter-provider-aws/pkg/cloudprovider" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status" "github.com/aws/karpenter-provider-aws/pkg/fake" "github.com/aws/karpenter-provider-aws/pkg/operator/options" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily" @@ -75,6 +77,7 @@ var fakeClock *clock.FakeClock var prov *provisioning.Provisioner var cluster *state.Cluster var cloudProvider *cloudprovider.CloudProvider +var statusController controller.Controller func TestAWS(t *testing.T) { ctx = TestContextWithLogger(t) @@ -94,6 +97,14 @@ var _ = BeforeSuite(func() { env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider, awsEnv.SubnetProvider) cluster = state.NewCluster(fakeClock, env.Client, cloudProvider) prov = provisioning.NewProvisioner(env.Client, events.NewRecorder(&record.FakeRecorder{}), cloudProvider, cluster) + statusController = status.NewController( + env.Client, + awsEnv.SubnetProvider, + awsEnv.SecurityGroupProvider, + awsEnv.AMIProvider, + awsEnv.InstanceProfileProvider, + awsEnv.LaunchTemplateProvider, + ) }) var _ = AfterSuite(func() { @@ -148,7 +159,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypes(ctx)).To(Succeed()) Expect(awsEnv.InstanceTypesProvider.UpdateInstanceTypeOfferings(ctx)).To(Succeed()) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) }) It("should create unique launch templates for multiple identical nodeClasses", func() { nodeClass2 := test.EC2NodeClass() @@ -191,6 +202,8 @@ var _ = Describe("LaunchTemplate Provider", func() { }), } ExpectApplied(ctx, env.Client, nodePool, nodeClass, nodePool2, nodeClass2) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass2)) ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pods...) ltConfigCount := len(awsEnv.EC2API.CreateFleetBehavior.CalledWithInput.Pop().LaunchTemplateConfigs) + len(awsEnv.EC2API.CreateFleetBehavior.CalledWithInput.Pop().LaunchTemplateConfigs) Expect(ltConfigCount).To(BeNumerically("==", awsEnv.EC2API.CalledWithCreateLaunchTemplateInput.Len())) @@ -205,6 +218,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should default to a generated launch template", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -223,6 +237,8 @@ var _ = Describe("LaunchTemplate Provider", func() { }) }) It("should fail to provision if the instance profile isn't defined", func() { + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodeClass.Status.InstanceProfile = "" ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() @@ -233,6 +249,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodeClass.Spec.Role = "" nodeClass.Spec.InstanceProfile = aws.String("overridden-profile") ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -272,6 +289,7 @@ var _ = Describe("LaunchTemplate Provider", func() { } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod1 := coretest.UnschedulablePod(coretest.PodOptions{ Tolerations: []v1.Toleration{t1, t2, t3}, ResourceRequirements: rr, @@ -303,6 +321,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should recover from an out-of-sync launch template cache", func() { nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{MaxPods: aws.Int32(1)} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -430,6 +449,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Context("Labels", func() { It("should apply labels to the node", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) node := ExpectScheduled(ctx, env.Client, pod) @@ -445,6 +465,7 @@ var _ = Describe("LaunchTemplate Provider", func() { "tag2": "tag2value", } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -477,6 +498,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -497,6 +519,7 @@ var _ = Describe("LaunchTemplate Provider", func() { "Name": "myname", } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -519,6 +542,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should default AL2 block device mappings", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2 ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -535,6 +559,7 @@ var _ = Describe("LaunchTemplate Provider", func() { awsEnv.LaunchTemplateProvider.CABundle = lo.ToPtr("Y2EtYnVuZGxlCg==") awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(lo.ToPtr("10.100.0.0/16")) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -573,6 +598,8 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -623,6 +650,8 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -636,6 +665,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should default bottlerocket second volume with root volume size", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyBottlerocket ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -656,6 +686,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyCustom nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -681,6 +712,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -706,12 +738,14 @@ var _ = Describe("LaunchTemplate Provider", func() { v1.ResourceEphemeralStorage: resource.MustParse("1Gi")}}, }}, )) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) }) It("should pack pods with any ephemeral-storage request", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ v1.ResourceEphemeralStorage: resource.MustParse("1G"), @@ -721,6 +755,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should pack pods with large ephemeral-storage request", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ v1.ResourceEphemeralStorage: resource.MustParse("10Gi"), @@ -730,6 +765,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should not pack pods if the sum of pod ephemeral-storage and overhead exceeds node capacity", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ v1.ResourceEphemeralStorage: resource.MustParse("19Gi"), @@ -740,6 +776,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should pack pods if the pod's ephemeral-storage exceeds node capacity and instance storage is mounted", func() { nodeClass.Spec.InstanceStorePolicy = lo.ToPtr(v1beta1.InstanceStorePolicyRAID0) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ // Default node ephemeral-storage capacity is 20Gi @@ -753,6 +790,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should launch multiple nodes if sum of pod ephemeral-storage requests exceeds a single nodes capacity", func() { var nodes []*v1.Node ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ @@ -775,6 +813,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should only pack pods with ephemeral-storage requests that will fit on an available node", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pods := []*v1.Pod{ coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ @@ -820,6 +859,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ v1.ResourceEphemeralStorage: resource.MustParse("25Gi"), @@ -849,6 +889,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ // this pod can only be satisfied if `/dev/xvdb` will house all the pods. @@ -886,6 +927,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ResourceRequirements: v1.ResourceRequirements{ Requests: map[v1.ResourceName]resource.Quantity{ // this pod can only be satisfied if `/dev/xvdb` will house all the pods. @@ -1033,6 +1075,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Context("User Data", func() { It("should specify --use-max-pods=false when using ENI-based pod density", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1041,6 +1084,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should specify --use-max-pods=false and --max-pods user value when user specifies maxPods in NodePool", func() { nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{MaxPods: aws.Int32(10)} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1055,6 +1099,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1082,6 +1127,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1109,6 +1155,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1141,6 +1188,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1173,6 +1221,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1196,6 +1245,7 @@ var _ = Describe("LaunchTemplate Provider", func() { EvictionMaxPodGracePeriod: aws.Int32(300), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1206,6 +1256,7 @@ var _ = Describe("LaunchTemplate Provider", func() { PodsPerCore: aws.Int32(2), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1217,6 +1268,7 @@ var _ = Describe("LaunchTemplate Provider", func() { MaxPods: aws.Int32(100), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1225,6 +1277,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should specify --dns-cluster-ip and --ip-family when running in an ipv6 cluster", func() { awsEnv.LaunchTemplateProvider.KubeDNSIP = net.ParseIP("fd4b:121b:812b::a") ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1233,6 +1286,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should specify --dns-cluster-ip when running in an ipv4 cluster", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1243,6 +1297,7 @@ var _ = Describe("LaunchTemplate Provider", func() { ImageGCHighThresholdPercent: aws.Int32(50), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1253,6 +1308,7 @@ var _ = Describe("LaunchTemplate Provider", func() { ImageGCLowThresholdPercent: aws.Int32(50), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1263,6 +1319,7 @@ var _ = Describe("LaunchTemplate Provider", func() { CPUCFSQuota: aws.Bool(false), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1275,6 +1332,7 @@ var _ = Describe("LaunchTemplate Provider", func() { "subdomain." + v1.LabelNamespaceNodeRestriction + "/custom-label": "custom-value", }) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1284,6 +1342,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2 nodeClass.Spec.InstanceStorePolicy = lo.ToPtr(v1beta1.InstanceStorePolicyRAID0) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1301,6 +1360,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodePool.Spec.Template.Spec.Taints = []v1.Taint{{Key: "foo", Value: "bar", Effect: v1.TaintEffectNoExecute}} nodePool.Spec.Template.Spec.StartupTaints = []v1.Taint{{Key: "baz", Value: "bin", Effect: v1.TaintEffectNoExecute}} ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.PodOptions{ Tolerations: []v1.Toleration{{Operator: v1.TolerationOpExists}}, }) @@ -1314,6 +1374,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodePool.Spec.Template.Spec.Taints = []v1.Taint{{Key: "foo", Value: "bar", Effect: v1.TaintEffectNoExecute}} nodePool.Spec.Template.Spec.StartupTaints = []v1.Taint{{Key: "baz", Value: "bin", Effect: v1.TaintEffectNoExecute}} ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodePool), nodePool)).To(Succeed()) pod := coretest.UnschedulablePod(coretest.PodOptions{ Tolerations: []v1.Toleration{{Operator: v1.TolerationOpExists}}, @@ -1335,6 +1396,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should not bootstrap on invalid toml user data", func() { nodeClass.Spec.UserData = aws.String("#/bin/bash\n ./not-toml.sh") ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) // This will not be scheduled since userData cannot be generated for the prospective node. @@ -1342,6 +1404,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should override system reserved values in user data", func() { ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{ SystemReserved: map[string]string{ string(v1.ResourceCPU): "2", @@ -1367,6 +1430,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should override kube reserved values in user data", func() { ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{ KubeReserved: map[string]string{ string(v1.ResourceCPU): "2", @@ -1392,6 +1456,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should override kube reserved values in user data", func() { ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{ EvictionHard: map[string]string{ "memory.available": "10%", @@ -1420,6 +1485,7 @@ var _ = Describe("LaunchTemplate Provider", func() { MaxPods: aws.Int32(10), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1438,6 +1504,7 @@ var _ = Describe("LaunchTemplate Provider", func() { ImageGCHighThresholdPercent: aws.Int32(50), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1458,6 +1525,7 @@ var _ = Describe("LaunchTemplate Provider", func() { ImageGCLowThresholdPercent: aws.Int32(50), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1475,6 +1543,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should pass ClusterDNSIP when discovered", func() { ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1493,6 +1562,7 @@ var _ = Describe("LaunchTemplate Provider", func() { CPUCFSQuota: aws.Bool(false), } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1516,6 +1586,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Expect(err).To(BeNil()) nodeClass.Spec.UserData = aws.String(string(content)) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1529,6 +1600,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Expect(err).To(BeNil()) nodeClass.Spec.UserData = aws.String(string(content)) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1542,6 +1614,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Expect(err).To(BeNil()) nodeClass.Spec.UserData = aws.String(string(content)) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1553,6 +1626,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should handle empty custom user data", func() { nodeClass.Spec.UserData = nil ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1584,6 +1658,7 @@ var _ = Describe("LaunchTemplate Provider", func() { } nodePool.Spec.Template.Spec.Taints = desiredTaints ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(coretest.UnscheduleablePodOptions(coretest.PodOptions{ Tolerations: []v1.Toleration{{ Operator: v1.TolerationOpExists, @@ -1612,6 +1687,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodePool.Spec.Template.Labels = desiredLabels ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1632,6 +1708,7 @@ var _ = Describe("LaunchTemplate Provider", func() { func(field string, kc corev1beta1.KubeletConfiguration) { nodePool.Spec.Template.Spec.Kubelet = &kc ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1722,6 +1799,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should set LocalDiskStrategy to Raid0 when specified by the InstanceStorePolicy", func() { nodeClass.Spec.InstanceStorePolicy = lo.ToPtr(v1beta1.InstanceStorePolicyRAID0) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1741,6 +1819,7 @@ var _ = Describe("LaunchTemplate Provider", func() { } nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{MaxPods: lo.ToPtr[int32](110)} ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1755,8 +1834,9 @@ var _ = Describe("LaunchTemplate Provider", func() { Entry("empty", nil, "al2023_userdata_unmerged.golden"), ) It("should fail to create launch templates if cluster CIDR is unresolved", func() { - awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(nil) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(nil) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectNotScheduled(ctx, env.Client, pod) @@ -1775,6 +1855,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, }}) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1796,6 +1877,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, }}) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1820,6 +1902,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, }}) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1850,6 +1933,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }}) nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1885,6 +1969,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }}) nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) nodePool.Spec.Template.Spec.Requirements = []corev1beta1.NodeSelectorRequirementWithMinValues{ { NodeSelectorRequirement: v1.NodeSelectorRequirement{ @@ -1908,6 +1993,7 @@ var _ = Describe("LaunchTemplate Provider", func() { awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{Images: []*ec2.Image{}}) nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectNotScheduled(ctx, env.Client, pod) @@ -1918,6 +2004,7 @@ var _ = Describe("LaunchTemplate Provider", func() { {Name: aws.String(coretest.RandomName()), ImageId: aws.String("ami-123"), Architecture: aws.String("newnew"), CreationDate: aws.String("2022-01-01T12:00:00Z")}}}) nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}} ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectNotScheduled(ctx, env.Client, pod) @@ -1937,6 +2024,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }, }}) ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) ExpectApplied(ctx, env.Client, nodePool) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) @@ -1952,6 +2040,7 @@ var _ = Describe("LaunchTemplate Provider", func() { {Tags: map[string]string{"Name": "test-subnet-3"}}, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1963,6 +2052,7 @@ var _ = Describe("LaunchTemplate Provider", func() { {Tags: map[string]string{"Name": "test-subnet-2"}}, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -1974,6 +2064,7 @@ var _ = Describe("LaunchTemplate Provider", func() { func(setValue, expectedValue, isEFA bool) { nodeClass.Spec.AssociatePublicIPAddress = lo.ToPtr(setValue) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod(lo.Ternary(isEFA, coretest.PodOptions{ ResourceRequirements: v1.ResourceRequirements{ Requests: v1.ResourceList{v1beta1.ResourceEFA: resource.MustParse("2")}, @@ -1995,6 +2086,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should specify the --dns-cluster-ip flag when clusterDNSIP is set", func() { nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{ClusterDNS: []string{"10.0.10.100"}} ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -2012,6 +2104,7 @@ var _ = Describe("LaunchTemplate Provider", func() { Expect(err).To(BeNil()) nodeClass.Spec.UserData = aws.String(string(content)) ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodePool), nodePool)).To(Succeed()) pod := coretest.UnschedulablePod(coretest.PodOptions{ NodeSelector: map[string]string{ @@ -2027,6 +2120,7 @@ var _ = Describe("LaunchTemplate Provider", func() { }) It("should bootstrap when custom user data is empty", func() { ExpectApplied(ctx, env.Client, nodeClass, nodePool) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) Expect(env.Client.Get(ctx, client.ObjectKeyFromObject(nodePool), nodePool)).To(Succeed()) pod := coretest.UnschedulablePod(coretest.PodOptions{ NodeSelector: map[string]string{ @@ -2046,6 +2140,7 @@ var _ = Describe("LaunchTemplate Provider", func() { It("should default detailed monitoring to off", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2 ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) @@ -2058,6 +2153,7 @@ var _ = Describe("LaunchTemplate Provider", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2 nodeClass.Spec.DetailedMonitoring = aws.Bool(true) ExpectApplied(ctx, env.Client, nodePool, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) pod := coretest.UnschedulablePod() ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod) ExpectScheduled(ctx, env.Client, pod) diff --git a/pkg/providers/securitygroup/securitygroup.go b/pkg/providers/securitygroup/securitygroup.go index 9cc61b27ce20..5fa9d7c608f7 100644 --- a/pkg/providers/securitygroup/securitygroup.go +++ b/pkg/providers/securitygroup/securitygroup.go @@ -25,7 +25,9 @@ import ( "github.com/mitchellh/hashstructure/v2" "github.com/patrickmn/go-cache" "github.com/samber/lo" + "k8s.io/apimachinery/pkg/types" "knative.dev/pkg/logging" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/karpenter/pkg/utils/pretty" @@ -33,80 +35,73 @@ import ( ) type Provider interface { - List(context.Context, *v1beta1.EC2NodeClass) ([]*ec2.SecurityGroup, error) - UpdateSecurityGroup(context.Context, *v1beta1.EC2NodeClass) error + List(context.Context, *v1beta1.EC2NodeClass) ([]v1beta1.SecurityGroup, error) + UpdateSecurityGroups(context.Context, *v1beta1.EC2NodeClass) ([]*ec2.SecurityGroup, error) } type DefaultProvider struct { sync.RWMutex - ec2api ec2iface.EC2API - cache *cache.Cache - cm *pretty.ChangeMonitor + ec2api ec2iface.EC2API + kubeClient client.Client + cache *cache.Cache + cm *pretty.ChangeMonitor } -func NewDefaultProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *DefaultProvider { +func NewDefaultProvider(kubeClient client.Client, ec2api ec2iface.EC2API, cache *cache.Cache) *DefaultProvider { return &DefaultProvider{ - ec2api: ec2api, - cm: pretty.NewChangeMonitor(), + ec2api: ec2api, + kubeClient: kubeClient, + cm: pretty.NewChangeMonitor(), // TODO: Remove cache cache when we utilize the security groups from the EC2NodeClass.status cache: cache, } } -func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([]*ec2.SecurityGroup, error) { - p.RLock() - defer p.RUnlock() +func (p *DefaultProvider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([]v1beta1.SecurityGroup, error) { + if err := p.kubeClient.Get(ctx, types.NamespacedName{Name: nodeClass.Name}, nodeClass); err != nil { + return nil, err + } + return nodeClass.Status.SecurityGroups, nil +} +// Use security groups provided by the nodeclass status +func (p *DefaultProvider) UpdateSecurityGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([]*ec2.SecurityGroup, error) { // Get SecurityGroups filterSets := getFilterSets(nodeClass.Spec.SecurityGroupSelectorTerms) - hash, err := p.securitygroupHash(filterSets) + securityGroups, err := p.getSecurityGroups(ctx, filterSets) if err != nil { return nil, err } - if sg, ok := p.cache.Get(hash); ok { - return sg.([]*ec2.SecurityGroup), nil + if p.cm.HasChanged(fmt.Sprintf("security-groups/%s", nodeClass.Name), securityGroups) { + logging.FromContext(ctx). + With("security-groups", lo.Map(securityGroups, func(s *ec2.SecurityGroup, _ int) string { + return aws.StringValue(s.GroupId) + })). + Debugf("discovered security groups") } - return []*ec2.SecurityGroup{}, nil + return securityGroups, nil } -func (p *DefaultProvider) UpdateSecurityGroup(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - p.Lock() - defer p.Unlock() - - filterSets := getFilterSets(nodeClass.Spec.SecurityGroupSelectorTerms) - hash, err := p.securitygroupHash(filterSets) +func (p *DefaultProvider) getSecurityGroups(ctx context.Context, filterSets [][]*ec2.Filter) ([]*ec2.SecurityGroup, error) { + hash, err := hashstructure.Hash(filterSets, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) if err != nil { - return err + return nil, err + } + if sg, ok := p.cache.Get(fmt.Sprint(hash)); ok { + return sg.([]*ec2.SecurityGroup), nil } securityGroups := map[string]*ec2.SecurityGroup{} for _, filters := range filterSets { output, err := p.ec2api.DescribeSecurityGroupsWithContext(ctx, &ec2.DescribeSecurityGroupsInput{Filters: filters}) if err != nil { - return fmt.Errorf("describing security groups %+v, %w", filterSets, err) + return nil, fmt.Errorf("describing security groups %+v, %w", filterSets, err) } for i := range output.SecurityGroups { securityGroups[lo.FromPtr(output.SecurityGroups[i].GroupId)] = output.SecurityGroups[i] } } p.cache.SetDefault(fmt.Sprint(hash), lo.Values(securityGroups)) - p.cache.DeleteExpired() // delete expired after we have successfully updated the cache - - if p.cm.HasChanged(fmt.Sprintf("security-groups/%s", nodeClass.Name), securityGroups) { - logging.FromContext(ctx). - With("security-groups", lo.Map(lo.Values(securityGroups), func(s *ec2.SecurityGroup, _ int) string { - return aws.StringValue(s.GroupId) - })). - Debugf("discovered security groups") - } - return nil -} - -func (p *DefaultProvider) securitygroupHash(filterSets [][]*ec2.Filter) (string, error) { - hash, err := hashstructure.Hash(filterSets, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) - if err != nil { - return "", err - } - return fmt.Sprintf("%d", hash), nil + return lo.Values(securityGroups), nil } func getFilterSets(terms []v1beta1.SecurityGroupSelectorTerm) (res [][]*ec2.Filter) { diff --git a/pkg/providers/securitygroup/suite_test.go b/pkg/providers/securitygroup/suite_test.go index ddcf9abb5244..1ef9772b0fdc 100644 --- a/pkg/providers/securitygroup/suite_test.go +++ b/pkg/providers/securitygroup/suite_test.go @@ -93,8 +93,7 @@ var _ = AfterEach(func() { var _ = Describe("SecurityGroupProvider", func() { It("should default to the clusters security groups", func() { - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -116,8 +115,7 @@ var _ = Describe("SecurityGroupProvider", func() { {GroupName: aws.String("test-sgName-1"), GroupId: aws.String("test-sg-1"), Tags: []*ec2.Tag{{Key: aws.String("kubernetes.io/cluster/test-cluster"), Value: aws.String("test-sg-1")}}}, {GroupName: aws.String("test-sgName-2"), GroupId: aws.String("test-sg-2"), Tags: []*ec2.Tag{{Key: aws.String("kubernetes.io/cluster/test-cluster"), Value: aws.String("test-sg-2")}}}, }}) - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -139,8 +137,7 @@ var _ = Describe("SecurityGroupProvider", func() { Tags: map[string]string{"Name": "test-security-group-2"}, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -159,8 +156,7 @@ var _ = Describe("SecurityGroupProvider", func() { ID: "sg-test1", }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -178,8 +174,7 @@ var _ = Describe("SecurityGroupProvider", func() { ID: "sg-test2", }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -203,8 +198,7 @@ var _ = Describe("SecurityGroupProvider", func() { Tags: map[string]string{"foo": "bar"}, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -224,8 +218,7 @@ var _ = Describe("SecurityGroupProvider", func() { Tags: map[string]string{"foo": "bar"}, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -243,8 +236,7 @@ var _ = Describe("SecurityGroupProvider", func() { Name: "securityGroup-test3", }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -264,8 +256,7 @@ var _ = Describe("SecurityGroupProvider", func() { Tags: map[string]string{"TestTag": "*"}, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) - securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + securityGroups, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) ExpectConsistsOfSecurityGroups([]*ec2.SecurityGroup{ { @@ -283,9 +274,8 @@ var _ = Describe("SecurityGroupProvider", func() { ID: *sg.GroupId, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) // Call list to request from aws and store in the cache - _, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + _, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) } @@ -303,9 +293,8 @@ var _ = Describe("SecurityGroupProvider", func() { Name: *sg.GroupName, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) // Call list to request from aws and store in the cache - _, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + _, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) } @@ -329,9 +318,8 @@ var _ = Describe("SecurityGroupProvider", func() { Tags: tag, }, } - Expect(awsEnv.SecurityGroupProvider.UpdateSecurityGroup(ctx, nodeClass)).To(BeNil()) // Call list to request from aws and store in the cache - _, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + _, err := awsEnv.SecurityGroupProvider.UpdateSecurityGroups(ctx, nodeClass) Expect(err).To(BeNil()) } diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 8c29b954ddf9..40e79eb7020b 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -101,7 +101,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment // Providers pricingProvider := pricing.NewDefaultProvider(ctx, fakePricingAPI, ec2api, fake.DefaultRegion) subnetProvider := subnet.NewDefaultProvider(ec2api, subnetCache) - securityGroupProvider := securitygroup.NewDefaultProvider(ec2api, securityGroupCache) + securityGroupProvider := securitygroup.NewDefaultProvider(env.Client, ec2api, securityGroupCache) versionProvider := version.NewDefaultProvider(env.KubernetesInterface, kubernetesVersionCache) instanceProfileProvider := instanceprofile.NewDefaultProvider(fake.DefaultRegion, iamapi, instanceProfileCache) amiProvider := amifamily.NewDefaultProvider(versionProvider, ssmapi, ec2api, ec2Cache)