Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix aws cp caching empty instance types set #785

Merged
merged 2 commits into from
Nov 10, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/apis/provisioning/v1alpha5/provisioner.go
Original file line number Diff line number Diff line change
@@ -155,7 +155,7 @@ func (r Requirements) WithPod(pod *v1.Pod) Requirements {
// Consolidate combines In and NotIn requirements for each unique key, producing
// an equivalent minimal representation of the requirements. This is useful as
// requirements may be appended from a variety of sources and then consolidated.
// Caution: If a key has contains a `NotIn` operator without a corresponding
// Caution: If a key contains a `NotIn` operator without a corresponding
// `In` operator, the requirement will permanently be [] after consolidation. To
// avoid this, include the broadest `In` requirements before consolidating.
func (r Requirements) Consolidate() (requirements Requirements) {
2 changes: 1 addition & 1 deletion pkg/cloudprovider/aws/cloudprovider.go
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ const (
// resources. This value represents the maximum eventual consistency between
// AWS actual state and the controller's ability to provision those
// resources. Cache hits enable faster provisioning and reduced API load on
// AWS APIs, which can have a serious import on performance and scalability.
// AWS APIs, which can have a serious impact on performance and scalability.
// DO NOT CHANGE THIS VALUE WITHOUT DUE CONSIDERATION
CacheTTL = 60 * time.Second
// CacheCleanupInterval triggers cache cleanup (lazy eviction) at this interval.
20 changes: 11 additions & 9 deletions pkg/cloudprovider/aws/instancetypes.go
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ package aws
import (
"context"
"fmt"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
@@ -30,8 +31,9 @@ import (
)

const (
instanceTypesCacheKey = "types"
instanceTypeZonesCacheKey = "zones"
instanceTypesCacheKey = "types"
instanceTypeZonesCacheKey = "zones"
instanceTypesAndZonesCacheTTL = 5 * time.Minute
)

type InstanceTypeProvider struct {
@@ -44,7 +46,7 @@ func NewInstanceTypeProvider(ec2api ec2iface.EC2API, subnetProvider *SubnetProvi
return &InstanceTypeProvider{
ec2api: ec2api,
subnetProvider: subnetProvider,
cache: cache.New(CacheTTL, CacheCleanupInterval),
cache: cache.New(instanceTypesAndZonesCacheTTL, CacheCleanupInterval),
}
}

@@ -53,12 +55,12 @@ func (p *InstanceTypeProvider) Get(ctx context.Context, constraints *v1alpha1.Co
// Get InstanceTypes from EC2
instanceTypes, err := p.getInstanceTypes(ctx)
if err != nil {
return nil, fmt.Errorf("retrieving all instance types, %w", err)
return nil, err
}
// Get Viable AZs from subnets
subnets, err := p.subnetProvider.Get(ctx, constraints)
if err != nil {
return nil, fmt.Errorf("getting subnets, %w", err)
return nil, err
}
subnetZones := sets.NewString()
for _, subnet := range subnets {
@@ -86,11 +88,10 @@ func (p *InstanceTypeProvider) Get(ctx context.Context, constraints *v1alpha1.Co
return result, nil
}

func (p *InstanceTypeProvider) getInstanceTypeZones(ctx context.Context) (result map[string]sets.String, err error) {
func (p *InstanceTypeProvider) getInstanceTypeZones(ctx context.Context) (map[string]sets.String, error) {
if cached, ok := p.cache.Get(instanceTypeZonesCacheKey); ok {
return cached.(map[string]sets.String), nil
}
defer func() { p.cache.SetDefault(instanceTypeZonesCacheKey, result) }()
bwagner5 marked this conversation as resolved.
Show resolved Hide resolved
zones := map[string]sets.String{}
if err := p.ec2api.DescribeInstanceTypeOfferingsPagesWithContext(ctx, &ec2.DescribeInstanceTypeOfferingsInput{LocationType: aws.String("availability-zone")},
func(output *ec2.DescribeInstanceTypeOfferingsOutput, lastPage bool) bool {
@@ -105,15 +106,15 @@ func (p *InstanceTypeProvider) getInstanceTypeZones(ctx context.Context) (result
return nil, fmt.Errorf("describing instance type zone offerings, %w", err)
}
logging.FromContext(ctx).Debugf("Discovered EC2 instance types zonal offerings")
p.cache.SetDefault(instanceTypeZonesCacheKey, zones)
return zones, nil
}

// getInstanceTypes retrieves all instance types from the ec2 DescribeInstanceTypes API using some opinionated filters
func (p *InstanceTypeProvider) getInstanceTypes(ctx context.Context) (result map[string]*InstanceType, err error) {
func (p *InstanceTypeProvider) getInstanceTypes(ctx context.Context) (map[string]*InstanceType, error) {
if cached, ok := p.cache.Get(instanceTypesCacheKey); ok {
return cached.(map[string]*InstanceType), nil
}
defer func() { p.cache.SetDefault(instanceTypesCacheKey, result) }()
instanceTypes := map[string]*InstanceType{}
bwagner5 marked this conversation as resolved.
Show resolved Hide resolved
if err := p.ec2api.DescribeInstanceTypesPagesWithContext(ctx, &ec2.DescribeInstanceTypesInput{
Filters: []*ec2.Filter{
@@ -133,6 +134,7 @@ func (p *InstanceTypeProvider) getInstanceTypes(ctx context.Context) (result map
return nil, fmt.Errorf("fetching instance types using ec2.DescribeInstanceTypes, %w", err)
}
logging.FromContext(ctx).Debugf("Discovered %d EC2 instance types", len(instanceTypes))
p.cache.SetDefault(instanceTypesCacheKey, instanceTypes)
return instanceTypes, nil
}