CFE-68 : initial code commit to read tags & update for exisiting ins…

…tace

Updating infrastructure in ec2 instance update flow

CFE-68: revert the tag updation

Updating infrastructure in ec2 instance update flow

 incorporate review comments

adding logs to test the tags for update flow

Correcting formate changes for logs

fix panic: panic while creating infracture tags

fix panic: panic while creating infracture tags

update the missed tags and update the resource tags from Infracture cluster

fixing CI failure

fixing unit testcases

Unit test case correction & enhance delta tags

Reading only from infracture CR and update EC2 instance

CFE-68 : MOdifying the infrastructure resource tag logic

CFE-68 : MOdifying the infrastructure resource tag logic

CFE-68 : MOdifying the infrastructure resource tag logic

CFE-68 : MOdifying the infrastructure resource tag logic

Adding few more logs to check the

Code enhancement or creating custom tags

Code enhancement to create custom tags

Code enhancement to create custom tags

Code enhancement to create custom tags

Code enhancement to create custom tags

Code enhancement to create custom tags

incorporating Codereview comments

On change in cluster tags EC2 instance will get update with latest tags

ginkgo version having few issues related version support, so Included RC flag as suggested

onsi/ginkgo#711

 adding ACK_GINKGO_DEPRECATIONS=1.16.5

removing GinkGo  flag g for unit test

pkg/actuators/machine/instances_test.go

@@ -1008,6 +1008,7 @@ func TestCorrectExistingTags(t *testing.T) {
 	testCases := []struct {
 		name               string
 		tags               []*ec2.Tag
+		userTags           map[string]string
 		expectedCreateTags bool
 	}{
 		{
@@ -1024,6 +1025,40 @@ func TestCorrectExistingTags(t *testing.T) {
 			},
 			expectedCreateTags: false,
 		},
+		{
+			name: "Valid Tags and Create",
+			tags: []*ec2.Tag{
+				{
+					Key:   aws.String("kubernetes.io/cluster/" + clusterID),
+					Value: aws.String("owned"),
+				},
+				{
+					Key:   aws.String("Name"),
+					Value: aws.String(machine.Name),
+				},
+			},
+			expectedCreateTags: true,
+			userTags:           map[string]string{"UserDefinedTag2": "UserDefinedTagValue2"},
+		},
+		{
+			name: "Valid Tags and Update",
+			tags: []*ec2.Tag{
+				{
+					Key:   aws.String("kubernetes.io/cluster/" + clusterID),
+					Value: aws.String("owned"),
+				},
+				{
+					Key:   aws.String("Name"),
+					Value: aws.String(machine.Name),
+				},
+				{
+					Key:   aws.String("UserDefinedTag1"),
+					Value: aws.String("UserDefinedTagValue1"),
+				},
+			},
+			expectedCreateTags: true,
+			userTags:           map[string]string{"UserDefinedTag1": "ModifiedValue"},
+		},
 		{
 			name: "Invalid Name Tag Correct Cluster",
 			tags: []*ec2.Tag{
@@ -1084,7 +1119,7 @@ func TestCorrectExistingTags(t *testing.T) {
 				mockAWSClient.EXPECT().CreateTags(gomock.Any()).Return(&ec2.CreateTagsOutput{}, nil).MinTimes(1)
 			}
 
-			err := correctExistingTags(machine, &instance, mockAWSClient)
+			err := correctExistingTags(machine, &instance, mockAWSClient, tc.userTags)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}

pkg/actuators/machine/reconciler.go

@@ -211,6 +211,13 @@ func (r *Reconciler) update() error {
 	runningLen := len(runningInstances)
 	var newestInstance *ec2.Instance
 
+	//Prepare the tag list with infrastructure tags.
+	//these tags will update to the ec2 instance.
+	tagList, err := r.getTagsFromInfrastructure()
+	if err != nil {
+		return err
+	}
+
 	if runningLen > 0 {
 		// It would be very unusual to have more than one here, but it is
 		// possible if someone manually provisions a machine with same tag name.
@@ -240,7 +247,7 @@ func (r *Reconciler) update() error {
 		return fmt.Errorf("failed to set machine cloud provider specifics: %w", err)
 	}
 
-	if err = correctExistingTags(r.machine, newestInstance, r.awsClient); err != nil {
+	if err = correctExistingTags(r.machine, newestInstance, r.awsClient, tagList); err != nil {
 		return fmt.Errorf("failed to correct existing instance tags: %w", err)
 	}
 
@@ -251,6 +258,25 @@ func (r *Reconciler) update() error {
 	return r.requeueIfInstancePending(newestInstance)
 }
 
+func (r *Reconciler) getTagsFromInfrastructure() (map[string]string, error) {
+	infra := &configv1.Infrastructure{}
+	infraName := client.ObjectKey{Name: awsclient.GlobalInfrastuctureName}
+
+	if err := r.client.Get(r.Context, infraName, infra); err != nil {
+		return nil, err
+	}
+	tags := make(map[string]string)
+	ok, resourceTags := fetchInfraResourceTags(infra)
+	if !ok {
+		return tags, nil
+	}
+
+	for _, value := range resourceTags {
+		tags[value.Key] = value.Value
+	}
+	return tags, nil
+}
+
 // exists returns true if machine exists.
 func (r *Reconciler) exists() (bool, error) {
 	// Get all instances not terminated.

pkg/actuators/machine/utils.go

@@ -22,6 +22,8 @@ import (
 	"net"
 	"strings"
 
+	configv1 "github.com/openshift/api/config/v1"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
@@ -134,7 +136,7 @@ func getInstanceByID(id string, client awsclient.Client, instanceStateFilter []*
 
 // correctExistingTags validates Name and clusterID tags are correct on the instance
 // and sets them if they are not.
-func correctExistingTags(machine *machinev1.Machine, instance *ec2.Instance, client awsclient.Client) error {
+func correctExistingTags(machine *machinev1.Machine, instance *ec2.Instance, client awsclient.Client, tags map[string]string) error {
 	// https://docs.aws.amazon.com/sdk-for-go/api/service/ec2/#EC2.CreateTags
 	if instance == nil || instance.InstanceId == nil {
 		return fmt.Errorf("unexpected nil found in instance: %v", instance)
@@ -153,28 +155,41 @@ func correctExistingTags(machine *machinev1.Machine, instance *ec2.Instance, cli
 			if *tag.Key == "kubernetes.io/cluster/"+clusterID && *tag.Value == "owned" {
 				clusterTagOk = true
 			}
+			if tagValue, present := tags[*tag.Key]; present && *tag.Value == tagValue {
+				delete(tags, *tag.Key)
+			}
 		}
 	}
 
-	// Update our tags if they're not set or correct
+	tagsToAdd := []*ec2.Tag{}
+	for key, value := range tags {
+		tagsToAdd = append(tagsToAdd, &ec2.Tag{
+			Key:   aws.String(key),
+			Value: aws.String(value),
+		})
+	}
+
 	if !nameTagOk || !clusterTagOk {
+		tagsToAdd = append(tagsToAdd, &ec2.Tag{
+			Key:   aws.String("kubernetes.io/cluster/" + clusterID),
+			Value: aws.String("owned"),
+		})
+		tagsToAdd = append(tagsToAdd, &ec2.Tag{
+			Key:   aws.String("Name"),
+			Value: aws.String(machine.Name),
+		})
+	}
+
+	if len(tagsToAdd) != 0 {
 		// Create tags only adds/replaces what is present, does not affect other tags.
 		input := &ec2.CreateTagsInput{
 			Resources: []*string{
 				aws.String(*instance.InstanceId),
 			},
-			Tags: []*ec2.Tag{
-				{
-					Key:   aws.String("kubernetes.io/cluster/" + clusterID),
-					Value: aws.String("owned"),
-				},
-				{
-					Key:   aws.String("Name"),
-					Value: aws.String(machine.Name),
-				},
-			},
+			Tags: tagsToAdd,
 		}
-		klog.Infof("Invalid or missing instance tags for machine: %v; instanceID: %v, updating", machine.Name, *instance.InstanceId)
+		klog.Infof("updating Tags for machine: %v; instanceID: %v, tags: %+v",
+			machine.Name, *instance.InstanceId, tagsToAdd)
 		_, err := client.CreateTags(input)
 		return err
 	}
@@ -470,3 +485,12 @@ func ProviderStatusFromRawExtension(rawExtension *runtime.RawExtension) (*machin
 	klog.V(5).Infof("Got provider Status from raw extension: %+v", providerStatus)
 	return providerStatus, nil
 }
+
+func fetchInfraResourceTags(infra *configv1.Infrastructure) (bool, []configv1.AWSResourceTag) {
+	// TODO : https://github.com/openshift/api/pull/1064 , we should consider the spec over status if spec contains the user tags
+	if infra != nil && infra.Status.PlatformStatus != nil &&
+		infra.Status.PlatformStatus.AWS != nil && infra.Status.PlatformStatus.AWS.ResourceTags != nil {
+		return true, infra.Status.PlatformStatus.AWS.ResourceTags
+	}
+	return false, nil
+}