wait for ASO tags to converge before updating

azure/services/aso/aso_test.go

@@ -710,7 +710,7 @@ func TestCreateOrUpdateResource(t *testing.T) {
 
 		specMock.MockTagsGetterSetter.EXPECT().GetActualTags(gomock.Any()).Return(nil)
 		specMock.MockTagsGetterSetter.EXPECT().GetAdditionalTags().Return(nil)
-		specMock.MockTagsGetterSetter.EXPECT().GetDesiredTags(gomock.Any()).Return(nil)
+		specMock.MockTagsGetterSetter.EXPECT().GetDesiredTags(gomock.Any()).Return(nil).Times(2)
 		specMock.MockTagsGetterSetter.EXPECT().SetTags(gomock.Any(), gomock.Any())
 
 		ctx := context.Background()

azure/services/aso/tags.go

@@ -18,10 +18,13 @@ package aso
 
 import (
 	"encoding/json"
+	"reflect"
 
+	asoannotations "github.com/Azure/azure-service-operator/v2/pkg/common/annotations"
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
 	"github.com/pkg/errors"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/tags"
 	"sigs.k8s.io/cluster-api-provider-azure/util/maps"
@@ -47,6 +50,16 @@ func reconcileTags[T genruntime.MetaObject](t TagsGetterSetter[T], existing T, r
 		}
 
 		existingTags = t.GetActualTags(existing)
+		// Wait for tags to converge so we know for sure which ones are deleted from additionalTags (and
+		// should be deleted) and which were added manually (and should be kept).
+		if !reflect.DeepEqual(t.GetDesiredTags(existing), existingTags) &&
+			existing.GetAnnotations()[asoannotations.ReconcilePolicy] == string(asoannotations.ReconcilePolicyManage) {
+			return azure.WithTransientError(azure.NewOperationNotDoneError(&infrav1.Future{
+				Type:          createOrUpdateFutureType,
+				ResourceGroup: existing.GetNamespace(),
+				Name:          existing.GetName(),
+			}), requeueInterval)
+		}
 	}
 
 	existingTagsMap := converters.TagsToMap(existingTags)

azure/services/aso/tags_test.go

@@ -18,13 +18,16 @@ package aso
 
 import (
 	"encoding/json"
+	"errors"
 	"testing"
 
 	asoresourcesv1 "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601"
+	asoannotations "github.com/Azure/azure-service-operator/v2/pkg/common/annotations"
 	. "github.com/onsi/gomega"
 	"go.uber.org/mock/gomock"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
+	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/aso/mock_aso"
 )
 
@@ -43,6 +46,7 @@ func TestReconcileTags(t *testing.T) {
 				"oldAdditionalTag": "oldAdditionalVal",
 			},
 			existingTags: infrav1.Tags{
+				"oldAdditionalTag": "oldAdditionalVal",
 				"nonAdditionalTag": "nonAdditionalVal",
 			},
 			additionalTagsSpec: infrav1.Tags{
@@ -94,6 +98,7 @@ func TestReconcileTags(t *testing.T) {
 				})
 			}
 			tag.EXPECT().GetActualTags(existing).Return(test.existingTags)
+			tag.EXPECT().GetDesiredTags(existing).Return(test.existingTags)
 			tag.EXPECT().GetAdditionalTags().Return(test.additionalTagsSpec)
 
 			parameters := &asoresourcesv1.ResourceGroup{}
@@ -123,4 +128,27 @@ func TestReconcileTags(t *testing.T) {
 		err := reconcileTags[*asoresourcesv1.ResourceGroup](tag, existing, existing != nil, nil)
 		g.Expect(err).To(HaveOccurred())
 	})
+
+	t.Run("existing tags not up to date", func(t *testing.T) {
+		g := NewWithT(t)
+
+		mockCtrl := gomock.NewController(t)
+		tag := mock_aso.NewMockTagsGetterSetter[*asoresourcesv1.ResourceGroup](mockCtrl)
+
+		existing := &asoresourcesv1.ResourceGroup{
+			ObjectMeta: metav1.ObjectMeta{
+				Annotations: map[string]string{
+					asoannotations.ReconcilePolicy: string(asoannotations.ReconcilePolicyManage),
+				},
+			},
+		}
+		tag.EXPECT().GetActualTags(existing).Return(infrav1.Tags{"new": "value"})
+		tag.EXPECT().GetDesiredTags(existing).Return(infrav1.Tags{"old": "tag"})
+
+		err := reconcileTags[*asoresourcesv1.ResourceGroup](tag, existing, existing != nil, nil)
+		g.Expect(azure.IsOperationNotDoneError(err)).To(BeTrue())
+		var recerr azure.ReconcileError
+		g.Expect(errors.As(err, &recerr)).To(BeTrue())
+		g.Expect(recerr.IsTransient()).To(BeTrue())
+	})
 }