Merge pull request #4512 from philjb/pjb-4511-additionaltags-root-volumes

fix: propagate AdditionalTags from AWSCluster to storage volumes

Author: k8s-ci-robot

controllers/awsmachine_controller.go

@@ -589,7 +589,7 @@ func (r *AWSMachineReconciler) reconcileNormal(_ context.Context, machineScope *
 		}
 
 		if instance != nil {
-			r.ensureStorageTags(ec2svc, instance, machineScope.AWSMachine)
+			r.ensureStorageTags(ec2svc, instance, machineScope.AWSMachine, machineScope.AdditionalTags())
 		}
 
 		if err := r.reconcileLBAttachment(machineScope, elbScope, instance); err != nil {
@@ -1111,20 +1111,20 @@ func (r *AWSMachineReconciler) indexAWSMachineByInstanceID(o client.Object) []st
 	return nil
 }
 
-func (r *AWSMachineReconciler) ensureStorageTags(ec2svc services.EC2Interface, instance *infrav1.Instance, machine *infrav1.AWSMachine) {
+func (r *AWSMachineReconciler) ensureStorageTags(ec2svc services.EC2Interface, instance *infrav1.Instance, machine *infrav1.AWSMachine, additionalTags map[string]string) {
 	annotations, err := r.machineAnnotationJSON(machine, VolumeTagsLastAppliedAnnotation)
 	if err != nil {
 		r.Log.Error(err, "Failed to fetch the annotations for volume tags")
 	}
 	for _, volumeID := range instance.VolumeIDs {
 		if subAnnotation, ok := annotations[volumeID].(map[string]interface{}); ok {
-			newAnnotation, err := r.ensureVolumeTags(ec2svc, aws.String(volumeID), subAnnotation, machine.Spec.AdditionalTags)
+			newAnnotation, err := r.ensureVolumeTags(ec2svc, aws.String(volumeID), subAnnotation, additionalTags)
 			if err != nil {
 				r.Log.Error(err, "Failed to fetch the changed volume tags in EC2 instance")
 			}
 			annotations[volumeID] = newAnnotation
 		} else {
-			newAnnotation, err := r.ensureVolumeTags(ec2svc, aws.String(volumeID), make(map[string]interface{}), machine.Spec.AdditionalTags)
+			newAnnotation, err := r.ensureVolumeTags(ec2svc, aws.String(volumeID), make(map[string]interface{}), additionalTags)
 			if err != nil {
 				r.Log.Error(err, "Failed to fetch the changed volume tags in EC2 instance")
 			}

controllers/awsmachine_controller_unit_test.go

@@ -481,34 +481,41 @@ func TestAWSMachineReconciler(t *testing.T) {
 					}
 				})
 
-				t.Run("should tag instances from machine and cluster tags", func(t *testing.T) {
+				t.Run("should tag instances and volumes with machine and cluster tags", func(t *testing.T) {
 					g := NewWithT(t)
 					awsMachine := getAWSMachine()
 					setup(t, g, awsMachine)
 					defer teardown(t, g)
 					instanceCreate(t, g)
 					getCoreSecurityGroups(t, g)
 
-					ms.AWSMachine.Spec.AdditionalTags = infrav1.Tags{"kind": "alicorn"}
-					cs.AWSCluster.Spec.AdditionalTags = infrav1.Tags{"colour": "lavender"}
+					ms.AWSMachine.Spec.AdditionalTags = infrav1.Tags{"kind": "alicorn", "colour": "pink"} // takes precedence
+					cs.AWSCluster.Spec.AdditionalTags = infrav1.Tags{"colour": "lavender", "shape": "round"}
 
 					ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
+
+					// expect one call first to tag the instance and two calls for tagging each of two volumes
+					// the volumes get the tags from the AWSCluster _and_ the AWSMachine
+
 					ec2Svc.EXPECT().UpdateResourceTags(
-						gomock.Any(),
+						PointsTo("myMachine"),
 						map[string]string{
-							"kind": "alicorn",
+							"colour": "pink",
+							"shape":  "round",
+							"kind":   "alicorn",
 						},
 						map[string]string{},
-					).Return(nil).Times(2)
+					).Return(nil)
 
 					ec2Svc.EXPECT().UpdateResourceTags(
-						PointsTo("myMachine"),
+						gomock.Any(),
 						map[string]string{
-							"colour": "lavender",
+							"colour": "pink",
+							"shape":  "round",
 							"kind":   "alicorn",
 						},
 						map[string]string{},
-					).Return(nil)
+					).Return(nil).Times(2)
 
 					_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
 					g.Expect(err).To(BeNil())