diff --git a/applicationset/controllers/applicationset_controller.go b/applicationset/controllers/applicationset_controller.go index 1ba523ef74b1d..70fa60e97a0f6 100644 --- a/applicationset/controllers/applicationset_controller.go +++ b/applicationset/controllers/applicationset_controller.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -427,20 +428,29 @@ func (r *ApplicationSetReconciler) setApplicationSetStatusCondition(ctx context. if needToUpdateConditions || len(applicationSet.Status.Conditions) < len(newConditions) { // fetch updated Application Set object before updating it - namespacedName := types.NamespacedName{Namespace: applicationSet.Namespace, Name: applicationSet.Name} - if err := r.Get(ctx, namespacedName, applicationSet); err != nil { - if client.IgnoreNotFound(err) != nil { - return nil + // DefaultRetry will retry 5 times with a backoff factor of 1, jitter of 0.1 and a duration of 10ms + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + namespacedName := types.NamespacedName{Namespace: applicationSet.Namespace, Name: applicationSet.Name} + updatedAppset := &argov1alpha1.ApplicationSet{} + if err := r.Get(ctx, namespacedName, updatedAppset); err != nil { + if client.IgnoreNotFound(err) != nil { + return nil + } + return fmt.Errorf("error fetching updated application set: %w", err) } - return fmt.Errorf("error fetching updated application set: %w", err) - } - applicationSet.Status.SetConditions( - newConditions, evaluatedTypes, - ) + updatedAppset.Status.SetConditions( + newConditions, evaluatedTypes, + ) - // Update the newly fetched object with new set of conditions - err := r.Client.Status().Update(ctx, applicationSet) + // Update the newly fetched object with new set of conditions + err := r.Client.Status().Update(ctx, updatedAppset) + if err != nil { + return err + } + updatedAppset.DeepCopyInto(applicationSet) + return nil + }) if err != nil && !apierr.IsNotFound(err) { return fmt.Errorf("unable to set application set condition: %w", err) } @@ -1249,15 +1259,29 @@ func (r *ApplicationSetReconciler) migrateStatus(ctx context.Context, appset *ar } if update { - namespacedName := types.NamespacedName{Namespace: appset.Namespace, Name: appset.Name} - if err := r.Client.Status().Update(ctx, appset); err != nil { - return fmt.Errorf("unable to set application set status: %w", err) - } - if err := r.Get(ctx, namespacedName, appset); err != nil { - if client.IgnoreNotFound(err) != nil { - return nil + // DefaultRetry will retry 5 times with a backoff factor of 1, jitter of 0.1 and a duration of 10ms + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + namespacedName := types.NamespacedName{Namespace: appset.Namespace, Name: appset.Name} + updatedAppset := &argov1alpha1.ApplicationSet{} + if err := r.Get(ctx, namespacedName, updatedAppset); err != nil { + if client.IgnoreNotFound(err) != nil { + return nil + } + return fmt.Errorf("error fetching updated application set: %w", err) } - return fmt.Errorf("error fetching updated application set: %w", err) + + updatedAppset.Status.ApplicationStatus = appset.Status.ApplicationStatus + + // Update the newly fetched object with new set of ApplicationStatus + err := r.Client.Status().Update(ctx, updatedAppset) + if err != nil { + return err + } + updatedAppset.DeepCopyInto(appset) + return nil + }) + if err != nil && !apierr.IsNotFound(err) { + return fmt.Errorf("unable to set application set condition: %w", err) } } return nil @@ -1272,21 +1296,31 @@ func (r *ApplicationSetReconciler) updateResourcesStatus(ctx context.Context, lo statuses = append(statuses, status) } appset.Status.Resources = statuses + // DefaultRetry will retry 5 times with a backoff factor of 1, jitter of 0.1 and a duration of 10ms + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + namespacedName := types.NamespacedName{Namespace: appset.Namespace, Name: appset.Name} + updatedAppset := &argov1alpha1.ApplicationSet{} + if err := r.Get(ctx, namespacedName, updatedAppset); err != nil { + if client.IgnoreNotFound(err) != nil { + return nil + } + return fmt.Errorf("error fetching updated application set: %w", err) + } + + updatedAppset.Status.Resources = appset.Status.Resources - namespacedName := types.NamespacedName{Namespace: appset.Namespace, Name: appset.Name} - err := r.Client.Status().Update(ctx, appset) + // Update the newly fetched object with new status resources + err := r.Client.Status().Update(ctx, updatedAppset) + if err != nil { + return err + } + updatedAppset.DeepCopyInto(appset) + return nil + }) if err != nil { logCtx.Errorf("unable to set application set status: %v", err) return fmt.Errorf("unable to set application set status: %w", err) } - - if err := r.Get(ctx, namespacedName, appset); err != nil { - if client.IgnoreNotFound(err) != nil { - return nil - } - return fmt.Errorf("error fetching updated application set: %w", err) - } - return nil } @@ -1321,20 +1355,30 @@ func (r *ApplicationSetReconciler) setAppSetApplicationStatus(ctx context.Contex for i := range applicationStatuses { applicationSet.Status.SetApplicationStatus(applicationStatuses[i]) } + // DefaultRetry will retry 5 times with a backoff factor of 1, jitter of 0.1 and a duration of 10ms + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + updatedAppset := &argov1alpha1.ApplicationSet{} + if err := r.Get(ctx, namespacedName, updatedAppset); err != nil { + if client.IgnoreNotFound(err) != nil { + return nil + } + return fmt.Errorf("error fetching updated application set: %w", err) + } + + updatedAppset.Status.ApplicationStatus = applicationSet.Status.ApplicationStatus - // Update the newly fetched object with new set of ApplicationStatus - err := r.Client.Status().Update(ctx, applicationSet) + // Update the newly fetched object with new set of ApplicationStatus + err := r.Client.Status().Update(ctx, updatedAppset) + if err != nil { + return err + } + updatedAppset.DeepCopyInto(applicationSet) + return nil + }) if err != nil { logCtx.Errorf("unable to set application set status: %v", err) return fmt.Errorf("unable to set application set status: %w", err) } - - if err := r.Get(ctx, namespacedName, applicationSet); err != nil { - if client.IgnoreNotFound(err) != nil { - return nil - } - return fmt.Errorf("error fetching updated application set: %w", err) - } } return nil diff --git a/applicationset/controllers/applicationset_controller_test.go b/applicationset/controllers/applicationset_controller_test.go index f11131dc33ef0..d5813d5f114a8 100644 --- a/applicationset/controllers/applicationset_controller_test.go +++ b/applicationset/controllers/applicationset_controller_test.go @@ -2397,7 +2397,7 @@ func TestSetApplicationSetStatusCondition(t *testing.T) { argoObjs := []runtime.Object{} for _, testCase := range testCases { - client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&testCase.appset).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).Build() + client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&testCase.appset).WithIndex(&v1alpha1.Application{}, ".metadata.controller", appControllerIndexer).WithStatusSubresource(&testCase.appset).Build() metrics := appsetmetrics.NewFakeAppsetMetrics(client) r := ApplicationSetReconciler{