AWSCluster AdditionalTags are not propagated to AWSMachine storage volumes

[philjb]

Bug Description

The AWSCluster resources has a spec.AdditionalTags k-v map that is a place end users (aka me!) can place kvs that I want to use as resource tags in AWS (mostly for cost tracking).

The idea is that these tags are propagated to all resources descendent from the AWSCluster, such as ec2 instances and their root storage volumes.

Today, these tags make it to ec2 instances but not the root storage volumes. Only the awsmachine.spec.AdditionalTags make it to the root storage volumes. The layering of AdditionalTags is great for providing overrides (or additions) at various levels but the AdditionalTags from the AWSCluster should propagate to all resources.

I believe this is a bug; but I could imagine someone arguing that it is a design choice - it would be a surprising one and one that creates more work for the end user (as the user would need to explicitly also set AdditionalTags on the AWSMachine manifest).

Fix

This is an easy fix. See PR shortly.

Here's the ensureStorageTags method which only pulls tags from the machine L1121.

cluster-api-provider-aws/controllers/awsmachine_controller.go

Lines 1114 to 1140 in b90c18c

	func (r *AWSMachineReconciler) ensureStorageTags(ec2svc services.EC2Interface, instance *infrav1.Instance, machine *infrav1.AWSMachine) {
	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)
	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)
	if err != nil {
	r.Log.Error(err, "Failed to fetch the changed volume tags in EC2 instance")
	}
	annotations[volumeID] = newAnnotation
	}
	// We also need to update the annotation if anything changed.
	err = r.updateMachineAnnotationJSON(machine, VolumeTagsLastAppliedAnnotation, annotations)
	if err != nil {
	r.Log.Error(err, "Failed to fetch the changed volume tags in EC2 instance")
	}
	}
	}

Just above its callsite, you can see that for ec2 instances the ensureTags method merges tags from the machine with those from the cluster through machineScope.AdditionalTags().

cluster-api-provider-aws/controllers/awsmachine_controller.go

Lines 585 to 593 in b90c18c

	_, err = r.ensureTags(ec2svc, machineScope.AWSMachine, machineScope.GetInstanceID(), machineScope.AdditionalTags())
	if err != nil {
	machineScope.Error(err, "failed to ensure tags")
	return ctrl.Result{}, err
	}
	if instance != nil {
	r.ensureStorageTags(ec2svc, instance, machineScope.AWSMachine)
	}

Prior work

The ensureStorageTags method was introduced in this PR: #2463

[Skarlso]

/triage accepted