Merge pull request #4380 from nojnhuh/aso-tags-and-ready-error

[release-1.11] ASO: Return readyErr over not done err when tags fail

Author: k8s-ci-robot

azure/services/aso/aso.go

@@ -65,6 +65,8 @@ func New(ctrlClient client.Client, clusterName string) *Service {
 
 // CreateOrUpdateResource implements the logic for creating a new or updating an
 // existing resource with ASO.
+//
+//nolint:gocyclo // This function is necessarily complex.
 func (s *Service) CreateOrUpdateResource(ctx context.Context, spec azure.ASOResourceSpecGetter, serviceName string) (result genruntime.MetaObject, err error) {
 	ctx, log, done := tele.StartSpanWithLogger(ctx, "services.aso.CreateOrUpdateResource")
 	defer done()
@@ -155,6 +157,9 @@ func (s *Service) CreateOrUpdateResource(ctx context.Context, spec azure.ASOReso
 
 	if t, ok := spec.(TagsGetterSetter); ok {
 		if err := reconcileTags(t, existing, parameters); err != nil {
+			if azure.IsOperationNotDoneError(err) && readyErr != nil {
+				return nil, readyErr
+			}
 			return nil, errors.Wrap(err, "failed to reconcile tags")
 		}
 	}

azure/services/aso/aso_test.go

@@ -822,6 +822,71 @@ 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(c, clusterName)
+
+		mockCtrl := gomock.NewController(t)
+		specMock := struct {
+			*mock_azure.MockASOResourceSpecGetter
+			*mock_aso.MockTagsGetterSetter
+		}{
+			MockASOResourceSpecGetter: mock_azure.NewMockASOResourceSpecGetter(mockCtrl),
+			MockTagsGetterSetter:      mock_aso.NewMockTagsGetterSetter(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, nil)
+		specMock.MockTagsGetterSetter.EXPECT().GetDesiredTags(gomock.Any()).Return(existing.Spec.Tags, nil)
+
+		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)