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.

Sign up for GitHub

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

Jump to bottom

Ignore ENIs returning 404 on metadata request #194

Closed
wants to merge 2 commits into from
Closed

Ignore ENIs returning 404 on metadata request #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e614c4
Test current behavior on failed ENI metadata request
deiwin Oct 5, 2018
7fd3b3e
Ignore ENIs returning 404 on metadata request
deiwin Oct 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 20 additions & 2 deletions pkg/awsutils/awsutils.go
View file
Open in desktop
Expand Up @@ -84,7 +84,8 @@ var (
},
[]string{"fn", "error"},
)
prometheusRegistered = false
prometheusRegistered = false
errENIMetadataNotFound = fmt.Errorf("ENI metadata not found")
)

// APIs defines interfaces calls for adding/getting/deleting ENIs/secondary IPs. The APIs are not thread-safe.
Expand Down Expand Up @@ -385,6 +386,10 @@ func (cache *EC2InstanceMetadataCache) GetAttachedENIs() (eniList []ENIMetadata,
for _, macStr := range macsStrs {
eniMetadata, err := cache.getENIMetadata(macStr)
if err != nil {
if errors.Cause(err) == errENIMetadataNotFound {
log.Debugf("Metadata not found for eni %s. Ignoring, assuming stale metadata cache to be the cause.", macStr)
continue
}
return nil, errors.Wrapf(err, "get attached enis: failed to retrieve eni metadata for eni: %s", macStr)
}

Expand Down Expand Up @@ -456,7 +461,13 @@ func (cache *EC2InstanceMetadataCache) getENIDeviceNumber(eniMAC string) (string
awsAPILatency.WithLabelValues("GetMetadata", fmt.Sprint(err != nil)).Observe(msSince(start))
if err != nil {
awsAPIErrInc("GetMetadata", err)
log.Errorf("Failed to retrieve the device-number of eni %s, %v", eniMAC, err)
logMessage := fmt.Sprintf("Failed to retrieve the device-number of eni %s, %v", eniMAC, err)
if isMetadataNotFoundError(err) {
log.Warn(logMessage)
err = errENIMetadataNotFound
} else {
log.Errorf(logMessage)
}
return "", 0, errors.Wrapf(err, "failed to retrieve device-number for eni %s",
eniMAC)
}
Expand Down Expand Up @@ -692,6 +703,13 @@ func containsPrivateIPAddressLimitExceededError(err error) bool {
return false
}

func isMetadataNotFoundError(err error) bool {
aerr, ok := err.(awserr.Error)
return ok &&
aerr.Code() == "EC2MetadataError" &&
strings.Contains(aerr.OrigErr().Error(), "404 - Not Found")
}

func awsAPIErrInc(api string, err error) {
if aerr, ok := err.(awserr.Error); ok {
awsAPIErr.With(prometheus.Labels{"api": api, "error": aerr.Code()}).Inc()
Expand Down
55 changes: 55 additions & 0 deletions pkg/awsutils/awsutils_test.go
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/stretchr/testify/assert"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"

"github.com/aws/amazon-vpc-cni-k8s/pkg/ec2metadata/mocks"
"github.com/aws/amazon-vpc-cni-k8s/pkg/ec2wrapper/mocks"
Expand Down Expand Up @@ -274,6 +275,60 @@ func TestGetAttachedENIs(t *testing.T) {
assert.Equal(t, len(ens), 2)
}

func TestGetAttachedENIsOnErr(t *testing.T) {
ctrl, mockMetadata, _ := setup(t)
defer ctrl.Finish()

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

gomock.InOrder(
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataDeviceNum).Return(eni1Device, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataInterface).Return(primaryMAC, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataSubnetCIDR).Return(subnetCIDR, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataIPv4s).Return("", nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+eni2MAC+metadataDeviceNum).Return("", errors.New("Arbitrary error")),
)
ins := &EC2InstanceMetadataCache{ec2Metadata: mockMetadata}
_, err := ins.GetAttachedENIs()

assert.Error(t, err)
}

func TestGetAttachedENIsOn404(t *testing.T) {
ctrl, mockMetadata, _ := setup(t)
defer ctrl.Finish()

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

eniNotFoundErr := awserr.New("EC2MetadataError", "failed to make EC2Metadata request", errors.New(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is constructed exactly like it is in the AWS SDK, with the HTML copied from logs (visible in the PR message, if expanded).

"<?xml version=\"1.0\" encoding=\"iso-8859-1\"?>\n"+
"<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"\n"+
" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">\n"+
"<html xmlns=\"http://www.w3.org/1999/xhtml\" xml:lang=\"en\" lang=\"en\">\n"+
" <head>\n"+
" <title>404 - Not Found</title>\n"+
" </head>\n"+
" <body>\n"+
" <h1>404 - Not Found</h1>\n"+
" </body>\n"+
"</html>\n",
))

gomock.InOrder(
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataDeviceNum).Return(eni1Device, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataInterface).Return(primaryMAC, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataSubnetCIDR).Return(subnetCIDR, nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+primaryMAC+metadataIPv4s).Return("", nil),
mockMetadata.EXPECT().GetMetadata(metadataMACPath+eni2MAC+metadataDeviceNum).Return("", eniNotFoundErr),
)
ins := &EC2InstanceMetadataCache{ec2Metadata: mockMetadata}
ens, err := ins.GetAttachedENIs()

assert.NoError(t, err)
assert.Equal(t, len(ens), 1)
assert.Equal(t, ens[0].MAC, primaryMAC)
}

func TestAWSGetFreeDeviceNumberOnErr(t *testing.T) {
ctrl, _, mockEC2 := setup(t)
defer ctrl.Finish()
Expand Down