From 4bf580f8334df0bcba83d1dba22c0a08edfa0ab9 Mon Sep 17 00:00:00 2001 From: Kubernetes Prow Robot Date: Sat, 23 Sep 2023 14:04:56 -0700 Subject: [PATCH] Merge pull request #4512 from philjb/pjb-4511-additionaltags-root-volumes fix: propagate AdditionalTags from AWSCluster to storage volumes --- controllers/awsmachine_controller.go | 8 +++--- .../awsmachine_controller_unit_test.go | 25 ++++++++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/controllers/awsmachine_controller.go b/controllers/awsmachine_controller.go index 7b6dc2204b..5a8c8bc8fd 100644 --- a/controllers/awsmachine_controller.go +++ b/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") } diff --git a/controllers/awsmachine_controller_unit_test.go b/controllers/awsmachine_controller_unit_test.go index 0931ad4a9e..f276f14146 100644 --- a/controllers/awsmachine_controller_unit_test.go +++ b/controllers/awsmachine_controller_unit_test.go @@ -483,7 +483,7 @@ 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) @@ -491,26 +491,33 @@ func TestAWSMachineReconciler(t *testing.T) { 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())