diff --git a/azure/services/aso/aso.go b/azure/services/aso/aso.go index dfc6a6846f7..e8f14ee8912 100644 --- a/azure/services/aso/aso.go +++ b/azure/services/aso/aso.go @@ -152,9 +152,6 @@ func (r *reconciler[T]) CreateOrUpdateResource(ctx context.Context, spec azure.A if t, ok := spec.(TagsGetterSetter[T]); ok { if err := reconcileTags(t, existing, resourceExists, parameters); err != nil { - if azure.IsOperationNotDoneError(err) && readyErr != nil { - return zero, readyErr - } return zero, errors.Wrap(err, "failed to reconcile tags") } } diff --git a/azure/services/aso/aso_test.go b/azure/services/aso/aso_test.go index a96bc95112b..dd270b8bc86 100644 --- a/azure/services/aso/aso_test.go +++ b/azure/services/aso/aso_test.go @@ -731,7 +731,6 @@ func TestCreateOrUpdateResource(t *testing.T) { }) specMock.MockASOResourceSpecGetter.EXPECT().WasManaged(gomock.Any()).Return(false) - specMock.MockTagsGetterSetter.EXPECT().GetActualTags(gomock.Any()).Return(nil) specMock.MockTagsGetterSetter.EXPECT().GetAdditionalTags().Return(nil) specMock.MockTagsGetterSetter.EXPECT().GetDesiredTags(gomock.Any()).Return(nil).Times(2) specMock.MockTagsGetterSetter.EXPECT().SetTags(gomock.Any(), gomock.Any()) @@ -823,71 +822,6 @@ func TestCreateOrUpdateResource(t *testing.T) { g.Expect(err.Error()).To(ContainSubstring("failed to reconcile tags")) }) - t.Run("with tags not done error and readyErr", func(t *testing.T) { - g := NewGomegaWithT(t) - - sch := runtime.NewScheme() - g.Expect(asoresourcesv1.AddToScheme(sch)).To(Succeed()) - c := fakeclient.NewClientBuilder(). - WithScheme(sch). - Build() - s := New[*asoresourcesv1.ResourceGroup](c, clusterName) - - mockCtrl := gomock.NewController(t) - specMock := struct { - *mock_azure.MockASOResourceSpecGetter[*asoresourcesv1.ResourceGroup] - *mock_aso.MockTagsGetterSetter[*asoresourcesv1.ResourceGroup] - }{ - MockASOResourceSpecGetter: mock_azure.NewMockASOResourceSpecGetter[*asoresourcesv1.ResourceGroup](mockCtrl), - MockTagsGetterSetter: mock_aso.NewMockTagsGetterSetter[*asoresourcesv1.ResourceGroup](mockCtrl), - } - specMock.MockASOResourceSpecGetter.EXPECT().ResourceRef().Return(&asoresourcesv1.ResourceGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "name", - Namespace: "namespace", - }, - }) - specMock.MockASOResourceSpecGetter.EXPECT().Parameters(gomockinternal.AContext(), gomock.Any()).DoAndReturn(func(_ context.Context, group *asoresourcesv1.ResourceGroup) (*asoresourcesv1.ResourceGroup, error) { - return group, nil - }) - - existing := &asoresourcesv1.ResourceGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "name", - Namespace: "namespace", - Labels: map[string]string{ - infrav1.OwnedByClusterLabelKey: clusterName, - }, - Annotations: map[string]string{ - asoannotations.ReconcilePolicy: string(asoannotations.ReconcilePolicyManage), - }, - }, - Spec: asoresourcesv1.ResourceGroup_Spec{ - Tags: map[string]string{"desired": "tags"}, - }, - Status: asoresourcesv1.ResourceGroup_STATUS{ - Tags: map[string]string{"actual": "tags"}, - Conditions: []conditions.Condition{ - { - Type: conditions.ConditionTypeReady, - Status: metav1.ConditionFalse, - Message: "not ready :(", - }, - }, - }, - } - - specMock.MockTagsGetterSetter.EXPECT().GetActualTags(gomock.Any()).Return(existing.Status.Tags) - specMock.MockTagsGetterSetter.EXPECT().GetDesiredTags(gomock.Any()).Return(existing.Spec.Tags) - - ctx := context.Background() - g.Expect(c.Create(ctx, existing)).To(Succeed()) - - result, err := s.CreateOrUpdateResource(ctx, specMock, "service") - g.Expect(result).To(BeNil()) - g.Expect(err.Error()).To(ContainSubstring("not ready :(")) - }) - t.Run("reconcile policy annotation resets after un-pause", func(t *testing.T) { g := NewGomegaWithT(t) diff --git a/azure/services/aso/interfaces.go b/azure/services/aso/interfaces.go index 92c059b9447..1c410cb705e 100644 --- a/azure/services/aso/interfaces.go +++ b/azure/services/aso/interfaces.go @@ -36,7 +36,6 @@ type Reconciler[T genruntime.MetaObject] interface { type TagsGetterSetter[T genruntime.MetaObject] interface { GetAdditionalTags() infrav1.Tags GetDesiredTags(resource T) infrav1.Tags - GetActualTags(resource T) infrav1.Tags SetTags(resource T, tags infrav1.Tags) } diff --git a/azure/services/aso/mock_aso/aso_mock.go b/azure/services/aso/mock_aso/aso_mock.go index 598fa2fcb8f..868ed35fdb2 100644 --- a/azure/services/aso/mock_aso/aso_mock.go +++ b/azure/services/aso/mock_aso/aso_mock.go @@ -127,20 +127,6 @@ func (m *MockTagsGetterSetter[T]) EXPECT() *MockTagsGetterSetterMockRecorder[T] return m.recorder } -// GetActualTags mocks base method. -func (m *MockTagsGetterSetter[T]) GetActualTags(resource T) v1beta1.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetActualTags", resource) - ret0, _ := ret[0].(v1beta1.Tags) - return ret0 -} - -// GetActualTags indicates an expected call of GetActualTags. -func (mr *MockTagsGetterSetterMockRecorder[T]) GetActualTags(resource any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetActualTags", reflect.TypeOf((*MockTagsGetterSetter[T])(nil).GetActualTags), resource) -} - // GetAdditionalTags mocks base method. func (m *MockTagsGetterSetter[T]) GetAdditionalTags() v1beta1.Tags { m.ctrl.T.Helper() diff --git a/azure/services/aso/tags.go b/azure/services/aso/tags.go index 1216a5babe9..0a830fc1093 100644 --- a/azure/services/aso/tags.go +++ b/azure/services/aso/tags.go @@ -18,13 +18,10 @@ 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" @@ -49,17 +46,7 @@ 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) - } + existingTags = t.GetDesiredTags(existing) } existingTagsMap := converters.TagsToMap(existingTags) diff --git a/azure/services/aso/tags_test.go b/azure/services/aso/tags_test.go index c2672e96de6..9fc5a87f256 100644 --- a/azure/services/aso/tags_test.go +++ b/azure/services/aso/tags_test.go @@ -18,16 +18,13 @@ 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" ) @@ -97,7 +94,6 @@ func TestReconcileTags(t *testing.T) { tagsLastAppliedAnnotation: string(lastAppliedTagsJSON), }) } - tag.EXPECT().GetActualTags(existing).Return(test.existingTags) tag.EXPECT().GetDesiredTags(existing).Return(test.existingTags) tag.EXPECT().GetAdditionalTags().Return(test.additionalTagsSpec) @@ -128,27 +124,4 @@ 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()) - }) } diff --git a/azure/services/groups/spec.go b/azure/services/groups/spec.go index 53d3146f347..96eafcb9eb9 100644 --- a/azure/services/groups/spec.go +++ b/azure/services/groups/spec.go @@ -86,11 +86,6 @@ func (*GroupSpec) GetDesiredTags(resource *asoresourcesv1.ResourceGroup) infrav1 return resource.Spec.Tags } -// GetActualTags implements aso.TagsGetterSetter. -func (*GroupSpec) GetActualTags(resource *asoresourcesv1.ResourceGroup) infrav1.Tags { - return resource.Status.Tags -} - // SetTags implements aso.TagsGetterSetter. func (*GroupSpec) SetTags(resource *asoresourcesv1.ResourceGroup, tags infrav1.Tags) { resource.Spec.Tags = tags diff --git a/azure/services/managedclusters/spec.go b/azure/services/managedclusters/spec.go index b7330099ccb..06ef6222bf7 100644 --- a/azure/services/managedclusters/spec.go +++ b/azure/services/managedclusters/spec.go @@ -601,11 +601,6 @@ func (*ManagedClusterSpec) GetDesiredTags(resource *asocontainerservicev1.Manage return resource.Spec.Tags } -// GetActualTags implements aso.TagsGetterSetter. -func (*ManagedClusterSpec) GetActualTags(resource *asocontainerservicev1.ManagedCluster) infrav1.Tags { - return resource.Status.Tags -} - // SetTags implements aso.TagsGetterSetter. func (*ManagedClusterSpec) SetTags(resource *asocontainerservicev1.ManagedCluster, tags infrav1.Tags) { resource.Spec.Tags = tags