Revert "tests"

This reverts commit 9e2b48e.
aws · Feb 8, 2022 · 25d3050 · 25d3050
1 parent 9e2b48e
commit 25d3050
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 61 deletions.
diff --git a/pkg/cloudprovider/aws/fake/ec2api.go b/pkg/cloudprovider/aws/fake/ec2api.go
@@ -49,7 +49,6 @@ type EC2Behavior struct {
 	DescribeAvailabilityZonesOutput     *ec2.DescribeAvailabilityZonesOutput
 	CalledWithCreateFleetInput          set.Set
 	CalledWithCreateLaunchTemplateInput set.Set
-	CalledWithDeleteLaunchTemplateInput set.Set
 	Instances                           sync.Map
 	LaunchTemplates                     sync.Map
 	InsufficientCapacityPools           []CapacityPool
@@ -69,7 +68,6 @@ func (e *EC2API) Reset() {
 	e.EC2Behavior = EC2Behavior{
 		CalledWithCreateFleetInput:          set.NewSet(),
 		CalledWithCreateLaunchTemplateInput: set.NewSet(),
-		CalledWithDeleteLaunchTemplateInput: set.NewSet(),
 		Instances:                           sync.Map{},
 		LaunchTemplates:                     sync.Map{},
 		InsufficientCapacityPools:           []CapacityPool{},
@@ -171,30 +169,12 @@ func (e *EC2API) DescribeLaunchTemplatesWithContext(_ context.Context, input *ec
 		}
 		return true
 	})
-	if len(output.LaunchTemplates) == 0 && len(input.LaunchTemplateNames) != 0 {
+	if len(output.LaunchTemplates) == 0 {
 		return nil, awserr.New("InvalidLaunchTemplateName.NotFoundException", "not found", nil)
 	}
 	return output, nil
 }
 
-func (e *EC2API) DescribeLaunchTemplatesPagesWithContext(ctx context.Context, input *ec2.DescribeLaunchTemplatesInput, fn func(*ec2.DescribeLaunchTemplatesOutput, bool) bool, opts ...request.Option) error {
-	output, err := e.DescribeLaunchTemplatesWithContext(ctx, input, opts...)
-	if err != nil {
-		return err
-	}
-	fn(output, false)
-	return nil
-}
-
-func (e *EC2API) DeleteLaunchTemplate(input *ec2.DeleteLaunchTemplateInput) (*ec2.DeleteLaunchTemplateOutput, error) {
-	e.CalledWithDeleteLaunchTemplateInput.Add(input)
-	launchTemplate, ok := e.LaunchTemplates.LoadAndDelete(input.LaunchTemplateName)
-	if !ok {
-		return nil, awserr.New("InvalidLaunchTemplateName.NotFoundException", "not found", nil)
-	}
-	return &ec2.DeleteLaunchTemplateOutput{LaunchTemplate: launchTemplate.(*ec2.LaunchTemplate)}, nil
-}
-
 func (e *EC2API) DescribeSubnetsWithContext(context.Context, *ec2.DescribeSubnetsInput, ...request.Option) (*ec2.DescribeSubnetsOutput, error) {
 	if e.DescribeSubnetsOutput != nil {
 		return e.DescribeSubnetsOutput, nil

diff --git a/pkg/cloudprovider/aws/instance.go b/pkg/cloudprovider/aws/instance.go
@@ -44,15 +44,6 @@ type InstanceProvider struct {
 	launchTemplateProvider *LaunchTemplateProvider
 }
 
-func NewInstanceProvider(ec2api ec2iface.EC2API, instanceTypeProvider *InstanceTypeProvider, subnetProvider *SubnetProvider, launchTemplateProvider *LaunchTemplateProvider) *InstanceProvider {
-	return &InstanceProvider{
-		ec2api:                 ec2api,
-		instanceTypeProvider:   instanceTypeProvider,
-		subnetProvider:         subnetProvider,
-		launchTemplateProvider: launchTemplateProvider,
-	}
-}
-
 // Create an instance given the constraints.
 // instanceTypes should be sorted by priority for spot capacity type.
 // If spot is not used, the instanceTypes are not required to be sorted

diff --git a/pkg/cloudprovider/aws/suite_test.go b/pkg/cloudprovider/aws/suite_test.go
@@ -18,7 +18,6 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
-	"fmt"
 	"math"
 	"strings"
 	"testing"
@@ -36,6 +35,7 @@ import (
 	"github.com/aws/karpenter/pkg/utils/injection"
 	"github.com/aws/karpenter/pkg/utils/options"
 	"github.com/aws/karpenter/pkg/utils/resources"
+	"github.com/patrickmn/go-cache"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -53,10 +53,9 @@ import (
 var ctx context.Context
 var opts options.Options
 var env *test.Environment
-var launchTemplateProvider *LaunchTemplateProvider
-var instanceTypeProvider *InstanceTypeProvider
-var securityGroupProvider *SecurityGroupProvider
-var subnetProvider *SubnetProvider
+var launchTemplateCache *cache.Cache
+var securityGroupCache *cache.Cache
+var unavailableOfferingsCache *cache.Cache
 var fakeEC2API *fake.EC2API
 var provisioners *provisioning.Controller
 var selectionController *selection.Controller
@@ -78,16 +77,33 @@ var _ = BeforeSuite(func() {
 		}
 		Expect(opts.Validate()).To(Succeed(), "Failed to validate options")
 		ctx = injection.WithOptions(ctx, opts)
-		clientSet := kubernetes.NewForConfigOrDie(e.Config)
+		launchTemplateCache = cache.New(CacheTTL, CacheCleanupInterval)
+		unavailableOfferingsCache = cache.New(InsufficientCapacityErrorCacheTTL, InsufficientCapacityErrorCacheCleanupInterval)
+		securityGroupCache = cache.New(CacheTTL, CacheCleanupInterval)
 		fakeEC2API = &fake.EC2API{}
-		subnetProvider = NewSubnetProvider(fakeEC2API)
-		instanceTypeProvider = NewInstanceTypeProvider(fakeEC2API, subnetProvider)
-		securityGroupProvider = NewSecurityGroupProvider(fakeEC2API)
-		launchTemplateProvider = NewLaunchTemplateProvider(ctx, fakeEC2API, NewAMIProvider(&fake.SSMAPI{}, clientSet), securityGroupProvider)
+		subnetProvider := NewSubnetProvider(fakeEC2API)
+		instanceTypeProvider := &InstanceTypeProvider{
+			ec2api:               fakeEC2API,
+			subnetProvider:       subnetProvider,
+			cache:                cache.New(InstanceTypesAndZonesCacheTTL, CacheCleanupInterval),
+			unavailableOfferings: unavailableOfferingsCache,
+		}
+		clientSet := kubernetes.NewForConfigOrDie(e.Config)
+		securityGroupProvider := &SecurityGroupProvider{
+			ec2api: fakeEC2API,
+			cache:  securityGroupCache,
+		}
 		cloudProvider := &CloudProvider{
 			subnetProvider:       subnetProvider,
 			instanceTypeProvider: instanceTypeProvider,
-			instanceProvider:     NewInstanceProvider(fakeEC2API, instanceTypeProvider, subnetProvider, launchTemplateProvider),
+			instanceProvider: &InstanceProvider{
+				fakeEC2API, instanceTypeProvider, subnetProvider, &LaunchTemplateProvider{
+					ec2api:                fakeEC2API,
+					amiProvider:           NewAMIProvider(&fake.SSMAPI{}, clientSet),
+					securityGroupProvider: securityGroupProvider,
+					cache:                 launchTemplateCache,
+				},
+			},
 		}
 		registry.RegisterOrDie(ctx, cloudProvider)
 		provisioners = provisioning.NewController(ctx, e.Client, clientSet.CoreV1(), cloudProvider)
@@ -113,10 +129,9 @@ var _ = Describe("Allocation", func() {
 		provisioner = ProvisionerWithProvider(&v1alpha5.Provisioner{ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}}, provider)
 		provisioner.SetDefaults(ctx)
 		fakeEC2API.Reset()
-		launchTemplateProvider.cache.Flush()
-		instanceTypeProvider.cache.Flush()
-		instanceTypeProvider.unavailableOfferings.Flush()
-		securityGroupProvider.cache.Flush()
+		launchTemplateCache.Flush()
+		securityGroupCache.Flush()
+		unavailableOfferingsCache.Flush()
 	})
 
 	AfterEach(func() {
@@ -318,7 +333,7 @@ var _ = Describe("Allocation", func() {
 				ExpectNotScheduled(ctx, env.Client, pod)
 				// capacity shortage is over - expire the item from the cache and try again
 				fakeEC2API.InsufficientCapacityPools = []fake.CapacityPool{}
-				instanceTypeProvider.unavailableOfferings.Delete(UnavailableOfferingsCacheKey(v1alpha1.CapacityTypeOnDemand, "inf1.6xlarge", "test-zone-1a"))
+				unavailableOfferingsCache.Delete(UnavailableOfferingsCacheKey(v1alpha1.CapacityTypeOnDemand, "inf1.6xlarge", "test-zone-1a"))
 				pod = ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, pod)[0]
 				node := ExpectScheduled(ctx, env.Client, pod)
 				Expect(node.Labels).To(HaveKeyWithValue(v1.LabelInstanceTypeStable, "inf1.6xlarge"))
@@ -354,21 +369,6 @@ var _ = Describe("Allocation", func() {
 			})
 		})
 		Context("LaunchTemplates", func() {
-			It("should hydrate the launch template cache on instantiation of the LaunchTemplateProvider", func() {
-				launchTemplateProvider.hydrateCache(ctx)
-				Expect(launchTemplateProvider.cache.ItemCount()).To(Equal(0))
-				fakeEC2API.DescribeLaunchTemplatesOutput = &ec2.DescribeLaunchTemplatesOutput{
-					LaunchTemplates: []*ec2.LaunchTemplate{{LaunchTemplateName: aws.String(fmt.Sprintf("Karpenter-%s-abcdef", opts.ClusterName))}},
-				}
-				launchTemplateProvider.hydrateCache(ctx)
-				Expect(launchTemplateProvider.cache.ItemCount()).To(Equal(1))
-			})
-			It("should delete launch templates when evicted from cache", func() {
-				launchTemplate := &ec2.LaunchTemplate{LaunchTemplateName: aws.String(fmt.Sprintf("Karpenter-%s-abcdef", opts.ClusterName))}
-				launchTemplateProvider.cache.SetDefault(*launchTemplate.LaunchTemplateName, launchTemplate)
-				launchTemplateProvider.cache.Delete(*launchTemplate.LaunchTemplateName)
-				Expect(fakeEC2API.CalledWithDeleteLaunchTemplateInput.Cardinality()).To(Equal(1))
-			})
 			It("should use same launch template for equivalent constraints", func() {
 				t1 := v1.Toleration{
 					Key:      "Abacus",
@@ -409,6 +409,7 @@ var _ = Describe("Allocation", func() {
 				name2 := fakeEC2API.CalledWithCreateFleetInput.Pop().(*ec2.CreateFleetInput).LaunchTemplateConfigs[0].LaunchTemplateSpecification.LaunchTemplateName
 				Expect(name1).To(Equal(name2))
 			})
+
 			It("should default to a generated launch template", func() {
 				pod := ExpectProvisioned(ctx, env.Client, selectionController, provisioners, provisioner, test.UnschedulablePod())[0]
 				ExpectScheduled(ctx, env.Client, pod)