Skip to content

Commit

Permalink
Reduce number of calls to EC2 API
Browse files Browse the repository at this point in the history
  • Loading branch information
Claes Mogren committed Apr 11, 2020
1 parent 599fed3 commit 480a226
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 107 deletions.
57 changes: 17 additions & 40 deletions pkg/awsutils/awsutils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ec2"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/wait"
)

Expand Down Expand Up @@ -165,9 +164,6 @@ type EC2InstanceMetadataCache struct {
region string
accountID string

// dynamic
currentENIs int

ec2Metadata ec2metadata.EC2Metadata
ec2SVC ec2wrapper.EC2
}
Expand All @@ -188,9 +184,6 @@ type ENIMetadata struct {

// The ip addresses allocated for the network interface
IPv4Addresses []*ec2.NetworkInterfacePrivateIpAddress

// Tags are the tags associated with this ENI in AWS
Tags map[string]string
}

func (eni ENIMetadata) PrimaryIPv4Address() string {
Expand Down Expand Up @@ -370,7 +363,6 @@ func (cache *EC2InstanceMetadataCache) setPrimaryENI() error {
}
eniMACs := strings.Fields(metadataENImacs)
log.Debugf("Discovered %d interfaces.", len(eniMACs))
cache.currentENIs = len(eniMACs)

// retrieve the attached ENIs
for _, eniMAC := range eniMACs {
Expand Down Expand Up @@ -429,7 +421,6 @@ func (cache *EC2InstanceMetadataCache) GetAttachedENIs() (eniList []ENIMetadata,
}
macsStrs := strings.Fields(macs)
log.Debugf("Total number of interfaces found: %d ", len(macsStrs))
cache.currentENIs = len(macsStrs)

var enis []ENIMetadata
// retrieve the attached ENIs
Expand All @@ -446,7 +437,7 @@ func (cache *EC2InstanceMetadataCache) GetAttachedENIs() (eniList []ENIMetadata,
func (cache *EC2InstanceMetadataCache) getENIMetadata(macStr string) (ENIMetadata, error) {
eniMACList := strings.Split(macStr, "/")
eniMAC := eniMACList[0]
log.Debugf("Found ENI mac address : %s", eniMAC)
log.Debugf("Found ENI MAC address: %s", eniMAC)

eni, deviceNum, err := cache.getENIDeviceNumber(eniMAC)
if err != nil {
Expand All @@ -458,42 +449,17 @@ func (cache *EC2InstanceMetadataCache) getENIMetadata(macStr string) (ENIMetadat
if err != nil {
return ENIMetadata{}, errors.Wrapf(err, "get ENI metadata: failed to retrieve IPs and CIDR for ENI: %s", eniMAC)
}
privateIPv4s, tags, _, err := cache.DescribeENI(eni)
if err != nil {
return ENIMetadata{}, errors.Wrapf(err, "get ENI metadata: failed to describe ENI: %s, %v", eniMAC, err)
}
// getIPsAndCIDR() queries IMDS for IPv4 addresses attached to the ENI.
// DescribeENI() calls the DescribeNetworkInterfaces AWS API call, which
// technically should be the source of truth and contain the freshest
// information. Let's just do a quick scan here and output some diagnostic
// messages if we find stale info in the IMDS result.
imdsIPv4Set := sets.NewString(imdsIPv4s...)
privateIPv4Set := sets.String{}
for _, privateIPv4 := range privateIPv4s {
privateIPv4Set.Insert(aws.StringValue(privateIPv4.PrivateIpAddress))
}
missingIMDS := privateIPv4Set.Difference(imdsIPv4Set).List()
missingDNI := imdsIPv4Set.Difference(privateIPv4Set).List()
if len(missingIMDS) > 0 {
strMissing := strings.Join(missingIMDS, ",")
log.Debugf("getENIMetadata: DescribeNetworkInterfaces(%s) yielded private IPv4 addresses %s that were not yet found in IMDS.", eni, strMissing)
}
if len(missingDNI) > 0 {
strMissing := strings.Join(missingDNI, ",")
log.Debugf("getENIMetadata: IMDS query yielded stale IPv4 addresses %s that were not found in DescribeNetworkInterfaces(%s).", strMissing, eni)
}
return ENIMetadata{
ENIID: eni,
MAC: eniMAC,
DeviceNumber: deviceNum,
SubnetIPv4CIDR: cidr,
IPv4Addresses: privateIPv4s,
Tags: tags,
IPv4Addresses: imdsIPv4s,
}, nil
}

// getIPsAndCIDR return list of IPs, CIDR, error
func (cache *EC2InstanceMetadataCache) getIPsAndCIDR(eniMAC string) ([]string, string, error) {
func (cache *EC2InstanceMetadataCache) getIPsAndCIDR(eniMAC string) ([]*ec2.NetworkInterfacePrivateIpAddress, string, error) {
start := time.Now()
cidr, err := cache.ec2Metadata.GetMetadata(metadataMACPath + eniMAC + metadataSubnetCIDR)
awsAPILatency.WithLabelValues("GetMetadata", fmt.Sprint(err != nil)).Observe(msSince(start))
Expand All @@ -506,17 +472,28 @@ func (cache *EC2InstanceMetadataCache) getIPsAndCIDR(eniMAC string) ([]string, s
log.Debugf("Found CIDR %s for ENI %s", cidr, eniMAC)

start = time.Now()
ipv4s, err := cache.ec2Metadata.GetMetadata(metadataMACPath + eniMAC + metadataIPv4s)
ipv4sAsString, err := cache.ec2Metadata.GetMetadata(metadataMACPath + eniMAC + metadataIPv4s)
awsAPILatency.WithLabelValues("GetMetadata", fmt.Sprint(err != nil)).Observe(msSince(start))
if err != nil {
awsAPIErrInc("GetMetadata", err)
log.Errorf("Failed to retrieve ENI %s local-ipv4s from instance metadata service, %v", eniMAC, err)
return nil, "", errors.Wrapf(err, "failed to retrieve ENI %s local-ipv4s", eniMAC)
}

ipv4Strs := strings.Fields(ipv4s)
ipv4Strs := strings.Fields(ipv4sAsString)
log.Debugf("Found IP addresses %v on ENI %s", ipv4Strs, eniMAC)
return ipv4Strs, cidr, nil
ipv4s := make([]*ec2.NetworkInterfacePrivateIpAddress, 0, len(ipv4Strs))
// network/interfaces/macs/mac/public-ipv4s The public IP address or Elastic IP addresses associated with the interface.
// There may be multiple IPv4 addresses on an instance. https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instancedata-data-categories.html
isFirst := true
for _, ipv4 := range ipv4Strs {
// TODO: Verify that the first IP is always the primary
primary := isFirst
ip := ipv4
ipv4s = append(ipv4s, &ec2.NetworkInterfacePrivateIpAddress{PrivateIpAddress: &ip, Primary: &primary})
isFirst = false
}
return ipv4s, cidr, nil
}

// getENIDeviceNumber returns ENI ID, device number, error
Expand Down
37 changes: 0 additions & 37 deletions pkg/awsutils/awsutils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,43 +251,6 @@ func TestGetAttachedENIs(t *testing.T) {

mockMetadata.EXPECT().GetMetadata(metadataMACPath).Return(primaryMAC+" "+eni2MAC, nil)

mockEC2.EXPECT().DescribeNetworkInterfaces(gomock.Any()).
DoAndReturn(func(input *ec2.DescribeNetworkInterfacesInput) (*ec2.DescribeNetworkInterfacesOutput, error) {
output := []*ec2.NetworkInterface{}
for _, in := range input.NetworkInterfaceIds {
ip := ""
attachID := ""
switch *in {
case eniID:
ip, attachID = eni1PrivateIP, eniAttachID
case eni2ID:
ip, attachID = eni2PrivateIP, eni2AttachID
default:
panic("no such id " + *in)
}

output = append(output, &ec2.NetworkInterface{
PrivateIpAddresses: []*ec2.NetworkInterfacePrivateIpAddress{
{
PrivateIpAddress: &ip,
},
},
Attachment: &ec2.NetworkInterfaceAttachment{
AttachmentId: &attachID,
},
TagSet: []*ec2.Tag{
{
Key: aws.String("foo"),
Value: aws.String("foo-value"),
},
},
})
}
return &ec2.DescribeNetworkInterfacesOutput{
NetworkInterfaces: output,
}, nil
}).Times(2)

gomock.InOrder(
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataDeviceNum).Return(eni1Device, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataInterface).Return(eniID, nil),
Expand Down
102 changes: 72 additions & 30 deletions pkg/ipamd/ipamd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,19 +165,22 @@ var (

// IPAMContext contains node level control information
type IPAMContext struct {
awsClient awsutils.APIs
dataStore *datastore.DataStore
k8sClient k8sapi.K8SAPIs
useCustomNetworking bool
eniConfig eniconfig.ENIConfig
criClient cri.APIs
networkClient networkutils.NetworkAPIs
maxIPsPerENI int
maxENI int
unmanagedENI int
warmENITarget int
warmIPTarget int
minimumIPTarget int
awsClient awsutils.APIs
dataStore *datastore.DataStore
k8sClient k8sapi.K8SAPIs
useCustomNetworking bool
eniConfig eniconfig.ENIConfig
criClient cri.APIs
networkClient networkutils.NetworkAPIs
maxIPsPerENI int
maxENI int
// unmanagedENIs is a list of ENIs tagged with
unmanagedENIs []string
unmanagedENIUpdated time.Time
warmENITarget int
warmIPTarget int
minimumIPTarget int
// primaryIP is map from ENI ID to primary IP of that ENI
primaryIP map[string]string
lastNodeIPPoolAction time.Time
lastDecreaseIPPool time.Time
Expand Down Expand Up @@ -278,19 +281,17 @@ func (c *IPAMContext) nodeInit() error {
if err != nil {
return errors.New("ipamd init: failed to retrieve attached ENIs info")
}
enis, numUnmanaged := filterUnmanagedENIs(allENIs)
enis := c.filterUnmanagedENIs(allENIs)
nodeMaxENI, err := c.getMaxENI()
if err != nil {
log.Error("Failed to get ENI limit")
return err
}
c.maxENI = nodeMaxENI
c.unmanagedENI = numUnmanaged
c.maxIPsPerENI, err = c.awsClient.GetENIipLimit()
if err != nil {
return err
}
c.updateIPStats(numUnmanaged)

var pbVPCcidrs []string
vpcCIDRs := c.awsClient.GetVPCIPv4CIDRs()
Expand Down Expand Up @@ -627,12 +628,12 @@ func (c *IPAMContext) increaseIPPool() {
c.updateLastNodeIPPoolAction()
} else {
// If we did not add an IP, try to add an ENI instead.
if c.dataStore.GetENIs() < (c.maxENI - c.unmanagedENI) {
if c.dataStore.GetENIs() < (c.maxENI - len(c.unmanagedENIs)) {
if err = c.tryAllocateENI(); err == nil {
c.updateLastNodeIPPoolAction()
}
} else {
log.Debugf("Skipping ENI allocation as the instance's max ENI limit of %d is already reached (accounting for %d unmanaged ENIs)", c.maxENI, c.unmanagedENI)
log.Debugf("Skipping ENI allocation as the instance's max ENI limit of %d is already reached (accounting for %d unmanaged ENIs)", c.maxENI, len(c.unmanagedENIs))
}
}
}
Expand Down Expand Up @@ -723,7 +724,7 @@ func (c *IPAMContext) tryAssignIPs() (increasedPool bool, err error) {
return false, errors.Wrap(err, fmt.Sprintf("failed to allocate one IP addresses on ENI %s, err: %v", eni.ID, err))
}
}

// This call to EC2 is needed to verify which IPs got attached to this ENI.
ec2Addrs, _, err := c.getENIaddresses(eni.ID)
if err != nil {
ipamdErrInc("increaseIPPoolGetENIaddressesFailed")
Expand All @@ -748,7 +749,6 @@ func (c *IPAMContext) setupENI(eni string, eniMetadata awsutils.ENIMetadata) err

// For secondary ENIs, set up the network
if eni != c.awsClient.GetPrimaryENI() {

eniPrimaryIP := eniMetadata.PrimaryIPv4Address()
err = c.networkClient.SetupENINetwork(eniPrimaryIP, eniMetadata.MAC, eniMetadata.DeviceNumber, eniMetadata.SubnetIPv4CIDR)
if err != nil {
Expand Down Expand Up @@ -780,7 +780,7 @@ func (c *IPAMContext) addENIaddressesToDataStore(ec2Addrs []*ec2.NetworkInterfac
return primaryIP
}

// returns all addresses on ENI, the primary address on ENI, error
// getENIaddresses calls EC2 and returns a list of all addresses on the ENI, the primary address on ENI, error
func (c *IPAMContext) getENIaddresses(eni string) ([]*ec2.NetworkInterfacePrivateIpAddress, string, error) {
ec2Addrs, _, _, err := c.awsClient.DescribeENI(eni)
if err != nil {
Expand Down Expand Up @@ -936,9 +936,7 @@ func (c *IPAMContext) nodeIPPoolReconcile(interval time.Duration) {
ipamdErrInc("reconcileFailedGetENIs")
return
}
attachedENIs, numUnmanaged := filterUnmanagedENIs(allENIs)
c.updateIPStats(numUnmanaged)
c.unmanagedENI = numUnmanaged
attachedENIs := c.filterUnmanagedENIs(allENIs)
curENIs := c.dataStore.GetENIInfos()

// Mark phase
Expand Down Expand Up @@ -1102,18 +1100,62 @@ func getMinimumIPTarget() int {
return noMinimumIPTarget
}

func filterUnmanagedENIs(enis []awsutils.ENIMetadata) ([]awsutils.ENIMetadata, int) {
numFiltered := 0
// filterUnmanagedENIs filters out ENIs marked with the "node.k8s.amazonaws.com/no_manage" tag
func (c *IPAMContext) filterUnmanagedENIs(enis []awsutils.ENIMetadata) []awsutils.ENIMetadata {
ret := make([]awsutils.ENIMetadata, 0, len(enis))
noManageENIs := make([]string, 0, len(enis))
// Update filter if needed
if c.needTagRefresh() {
log.Debug("Refreshing ENI tags")
for _, eni := range enis {
// The primary ENI can never be noManageENIs out
if eni.ENIID == c.awsClient.GetPrimaryENI() {
continue
}
// Call EC2 to get the tags
_, tags, _, err := c.awsClient.DescribeENI(eni.ENIID)
if err != nil {
log.Errorf("Failed to get tags: %v, not updating the tag filter", err)
noManageENIs = c.unmanagedENIs
break
} else if tags[eniNoManageTagKey] == "true" {
log.Debugf("ENI %s is tagged with: %s", eni.ENIID, eniNoManageTagKey)
noManageENIs = append(noManageENIs, eni.ENIID)
}
}
c.setUnmanagedENIs(noManageENIs)
}
for _, eni := range enis {
if eni.Tags[eniNoManageTagKey] == "true" {
log.Debugf("skipping ENI %s: tagged with %s", eni.ENIID, eniNoManageTagKey)
numFiltered++
// If we have known, unmanaged ENIs, skip them
if c.isUnmanagedENIs(eni.ENIID) {
log.Debugf("Skipping ENI %s: tagged with %s", eni.ENIID, eniNoManageTagKey)
continue
}
ret = append(ret, eni)
}
return ret, numFiltered
return ret
}

func (c *IPAMContext) isUnmanagedENIs(eniID string) bool {
if len(c.unmanagedENIs) > 0 {
for _, unmanagedENIID := range c.unmanagedENIs {
if unmanagedENIID == eniID {
return true
}
}
}
return false
}

func (c *IPAMContext) setUnmanagedENIs(unmanagedENIs []string) {
c.unmanagedENIs = unmanagedENIs
c.unmanagedENIUpdated = time.Now()
c.updateIPStats(len(unmanagedENIs))
}

// Refresh tags at most once a minute
func (c *IPAMContext) needTagRefresh() bool {
return time.Since(c.unmanagedENIUpdated) > nodeIPPoolReconcileInterval
}

// ipTargetState determines the number of IPs `short` or `over` our WARM_IP_TARGET,
Expand Down

0 comments on commit 480a226

Please sign in to comment.