Merge pull request #460 from saurav-agarwalla/automated-cherry-pick-o…

…f-#448-release-1.21

Automated cherry pick of #448: Handle InvalidInstanceID.NotFound when tagging resources

pkg/providers/v1/aws_fakes.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/elb"
@@ -272,6 +273,10 @@ func (ec2i *FakeEC2Impl) CreateTags(input *ec2.CreateTagsInput) (*ec2.CreateTags
 		if *id == "i-error" {
 			return nil, errors.New("Unable to tag")
 		}
+
+		if *id == "i-not-found" {
+			return nil, awserr.New("InvalidInstanceID.NotFound", "Instance not found", nil)
+		}
 	}
 	return &ec2.CreateTagsOutput{}, nil
 }
@@ -282,6 +287,10 @@ func (ec2i *FakeEC2Impl) DeleteTags(input *ec2.DeleteTagsInput) (*ec2.DeleteTags
 		if *id == "i-error" {
 			return nil, errors.New("Unable to remove tag")
 		}
+
+		if *id == "i-not-found" {
+			return nil, awserr.New("InvalidInstanceID.NotFound", "Instance not found", nil)
+		}
 	}
 	return &ec2.DeleteTagsOutput{}, nil
 }

pkg/providers/v1/tags.go

@@ -325,6 +325,10 @@ func (c *Cloud) TagResource(resourceID string, tags map[string]string) error {
 	output, err := c.ec2.CreateTags(request)
 
 	if err != nil {
+		if isAWSErrorInstanceNotFound(err) {
+			klog.Infof("Couldn't find resource when trying to tag it hence skipping it, %v", err)
+			return nil
+		}
 		klog.Errorf("Error occurred trying to tag resources, %v", err)
 		return err
 	}
@@ -345,6 +349,10 @@ func (c *Cloud) UntagResource(resourceID string, tags map[string]string) error {
 	output, err := c.ec2.DeleteTags(request)
 
 	if err != nil {
+		if isAWSErrorInstanceNotFound(err) {
+			klog.Infof("Couldn't find resource when trying to untag it hence skipping it, %v", err)
+			return nil
+		}
 		klog.Errorf("Error occurred trying to untag resources, %v", err)
 		return err
 	}

pkg/providers/v1/tags_test.go

@@ -20,11 +20,16 @@ limitations under the License.
 package aws
 
 import (
+	"bytes"
+	"errors"
+	"flag"
+	"os"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/stretchr/testify/assert"
+	"k8s.io/klog/v2"
 )
 
 func TestFilterTags(t *testing.T) {
@@ -233,3 +238,107 @@ func TestHasNoClusterPrefixTag(t *testing.T) {
 		})
 	}
 }
+
+func TestTagResource(t *testing.T) {
+	testFlags := flag.NewFlagSet("TestTagResource", flag.ExitOnError)
+	klog.InitFlags(testFlags)
+	testFlags.Parse([]string{"--logtostderr=false"})
+	awsServices := NewFakeAWSServices(TestClusterID)
+	c, err := newAWSCloud(CloudConfig{}, awsServices)
+	if err != nil {
+		t.Errorf("Error building aws cloud: %v", err)
+		return
+	}
+
+	tests := []struct {
+		name            string
+		instanceID      string
+		err             error
+		expectedMessage string
+	}{
+		{
+			name:            "tagging successful",
+			instanceID:      "i-random",
+			err:             nil,
+			expectedMessage: "Done calling create-tags to EC2",
+		},
+		{
+			name:            "tagging failed due to unknown error",
+			instanceID:      "i-error",
+			err:             errors.New("Unable to tag"),
+			expectedMessage: "Error occurred trying to tag resources",
+		},
+		{
+			name:            "tagging failed due to resource not found error",
+			instanceID:      "i-not-found",
+			err:             nil,
+			expectedMessage: "Couldn't find resource when trying to tag it hence skipping it",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var logBuf bytes.Buffer
+			klog.SetOutput(&logBuf)
+			defer func() {
+				klog.SetOutput(os.Stderr)
+			}()
+
+			err := c.TagResource(tt.instanceID, nil)
+			assert.Equal(t, tt.err, err)
+			assert.Contains(t, logBuf.String(), tt.expectedMessage)
+		})
+	}
+}
+
+func TestUntagResource(t *testing.T) {
+	testFlags := flag.NewFlagSet("TestUntagResource", flag.ExitOnError)
+	klog.InitFlags(testFlags)
+	testFlags.Parse([]string{"--logtostderr=false"})
+	awsServices := NewFakeAWSServices(TestClusterID)
+	c, err := newAWSCloud(CloudConfig{}, awsServices)
+	if err != nil {
+		t.Errorf("Error building aws cloud: %v", err)
+		return
+	}
+
+	tests := []struct {
+		name            string
+		instanceID      string
+		err             error
+		expectedMessage string
+	}{
+		{
+			name:            "untagging successful",
+			instanceID:      "i-random",
+			err:             nil,
+			expectedMessage: "Done calling delete-tags to EC2",
+		},
+		{
+			name:            "untagging failed due to unknown error",
+			instanceID:      "i-error",
+			err:             errors.New("Unable to remove tag"),
+			expectedMessage: "Error occurred trying to untag resources",
+		},
+		{
+			name:            "untagging failed due to resource not found error",
+			instanceID:      "i-not-found",
+			err:             nil,
+			expectedMessage: "Couldn't find resource when trying to untag it hence skipping it",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var logBuf bytes.Buffer
+			klog.SetOutput(&logBuf)
+			defer func() {
+				klog.SetOutput(os.Stderr)
+			}()
+
+			err := c.UntagResource(tt.instanceID, nil)
+			assert.Equal(t, tt.err, err)
+			assert.Contains(t, logBuf.String(), tt.expectedMessage)
+		})
+	}
+}