Skip to content

Commit

Permalink
[v10] Add better error handling for ec2 labels (#13487)
Browse files Browse the repository at this point in the history
This change adds a more useful error message for EC2 labels when tags aren't available in EC2 instance metadata. It also adds a cleaner fallback for when individual tag values can't be fetched.
  • Loading branch information
atburke authored Jun 21, 2022
1 parent afedc5f commit f379555
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 8 deletions.
31 changes: 24 additions & 7 deletions lib/labels/ec2/ec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,25 +109,42 @@ func (l *EC2) Sync(ctx context.Context) error {

tags, err := l.c.Client.GetTagKeys(ctx)
if err != nil {
if trace.IsNotFound(err) {
l.c.Log.Warningf("Could not fetch tags, please ensure 'allow instance tags in metadata' is enabled on the instance")
return nil
}
return trace.Wrap(err)
}

currentLabels := l.Get()
var errors []error
for _, t := range tags {
if !types.IsValidLabelKey(t) {
l.c.Log.Debugf("Skipping EC2 tag %q, not a valid label key.", t)
continue
}

key := toAWSLabel(t)
value, err := l.c.Client.GetTagValue(ctx, t)
if err != nil {
return trace.Wrap(err)
}
if types.IsValidLabelKey(t) {
m[toAWSLabel(t)] = value
errors = append(errors, err)
// If we know the key exists but GetTagValue failed, use the current value
// if it exists.
if currentValue, ok := currentLabels[key]; ok {
m[key] = currentValue
} else {
m[key] = ""
}
} else {
l.c.Log.Debugf("Skipping EC2 tag %q, not a valid label key.", t)
m[key] = value
}

}

l.mu.Lock()
defer l.mu.Unlock()
l.labels = m
return nil
return trace.NewAggregate(errors...)
}

// Start will start a loop that continually keeps EC2 labels updated.
Expand All @@ -141,7 +158,7 @@ func (l *EC2) periodicUpdateLabels(ctx context.Context) {

for {
if err := l.Sync(ctx); err != nil {
l.c.Log.Errorf("Error fetching EC2 tags: %v", err)
l.c.Log.Warningf("Error fetching EC2 tags: %v", err)
}
select {
case <-ticker.Chan():
Expand Down
48 changes: 47 additions & 1 deletion lib/labels/ec2/ec2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,20 @@ import (
)

type mockIMDSClient struct {
tags map[string]string
tagsDisabled bool
tags map[string]string
// errorKeys are the keys that should return an error from GetTagValue.
errorKeys []string
}

func (m *mockIMDSClient) IsAvailable(ctx context.Context) bool {
return true
}

func (m *mockIMDSClient) GetTagKeys(ctx context.Context) ([]string, error) {
if m.tagsDisabled {
return nil, trace.NotFound("")
}
keys := make([]string, 0, len(m.tags))
for k := range m.tags {
keys = append(keys, k)
Expand All @@ -42,6 +48,11 @@ func (m *mockIMDSClient) GetTagKeys(ctx context.Context) ([]string, error) {
}

func (m *mockIMDSClient) GetTagValue(ctx context.Context, key string) (string, error) {
for _, k := range m.errorKeys {
if k == key {
return "", trace.NotFound("Tag %q not found", key)
}
}
if value, ok := m.tags[key]; ok {
return value, nil
}
Expand Down Expand Up @@ -121,3 +132,38 @@ func TestEC2LabelsValidKey(t *testing.T) {
require.NoError(t, ec2Labels.Sync(ctx))
require.Equal(t, expectedTags, ec2Labels.Get())
}

func TestEC2LabelsDisabled(t *testing.T) {
ctx := context.Background()
imdsClient := &mockIMDSClient{
tagsDisabled: true,
}
ec2Labels, err := New(ctx, &Config{
Client: imdsClient,
})
require.NoError(t, err)
require.NoError(t, ec2Labels.Sync(ctx))
require.Equal(t, map[string]string{}, ec2Labels.Get())
}

func TestEC2LabelsGetValueFail(t *testing.T) {
ctx := context.Background()
tags := map[string]string{"a": "1", "b": "2", "c": "3"}
updateTags := map[string]string{"b": "6", "c": "7", "d": "8"}
errorTags := []string{"b", "d"}
expectedTags := map[string]string{"aws/b": "2", "aws/c": "7", "aws/d": ""}

imdsClient := &mockIMDSClient{
tags: tags,
}
ec2Labels, err := New(ctx, &Config{
Client: imdsClient,
})
require.NoError(t, err)
require.NoError(t, ec2Labels.Sync(ctx))

imdsClient.tags = updateTags
imdsClient.errorKeys = errorTags
require.Error(t, ec2Labels.Sync(ctx))
require.Equal(t, expectedTags, ec2Labels.Get())
}

0 comments on commit f379555

Please sign in to comment.