Merge pull request kubernetes-sigs#4512 from philjb/pjb-4511-addition…

…altags-root-volumes

fix: propagate AdditionalTags from AWSCluster to storage volumes

controllers/awsmachine_controller.go

@@ -567,7 +567,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 {
@@ -1063,20 +1063,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

@@ -483,34 +483,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())