diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go index a69b6c2b4..dafc815f2 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go @@ -302,6 +302,11 @@ func (r *Reconciler) gcResourceTrackers(logCtx monitorContext.Context, handler * })) defer subCtx.Commit("finish gc resourceTrackers") + statusUpdater := r.patchStatus + if isUpdate { + statusUpdater = r.updateStatus + } + var options []resourcekeeper.GCOption if !gcOutdated { options = append(options, resourcekeeper.DisableMarkStageGCOption{}, resourcekeeper.DisableGCComponentRevisionOption{}, resourcekeeper.DisableLegacyGCOption{}) @@ -309,8 +314,10 @@ func (r *Reconciler) gcResourceTrackers(logCtx monitorContext.Context, handler * finished, waiting, err := handler.resourceKeeper.GarbageCollect(logCtx, options...) if err != nil { logCtx.Error(err, "Failed to gc resourcetrackers") - r.Recorder.Event(handler.app, event.Warning(velatypes.ReasonFailedGC, err)) - return r.endWithNegativeCondition(logCtx, handler.app, condition.ReconcileError(err), phase) + cond := condition.Deleting() + cond.Message = fmt.Sprintf("error encountered during garbage collection: %s", err.Error()) + handler.app.Status.SetConditions(cond) + return r.result(statusUpdater(logCtx, handler.app, phase)).ret() } if !finished { logCtx.Info("GarbageCollecting resourcetrackers unfinished") @@ -319,13 +326,10 @@ func (r *Reconciler) gcResourceTrackers(logCtx monitorContext.Context, handler * cond.Message = fmt.Sprintf("Waiting for %s to delete. (At least %d resources are deleting.)", waiting[0].DisplayName(), len(waiting)) } handler.app.Status.SetConditions(cond) - return r.result(r.patchStatus(logCtx, handler.app, phase)).requeue(baseGCBackoffWaitTime).ret() + return r.result(statusUpdater(logCtx, handler.app, phase)).requeue(baseGCBackoffWaitTime).ret() } logCtx.Info("GarbageCollected resourcetrackers") - if isUpdate { - return r.result(r.updateStatus(logCtx, handler.app, phase)).ret() - } - return r.result(r.patchStatus(logCtx, handler.app, phase)).ret() + return r.result(statusUpdater(logCtx, handler.app, phase)).ret() } type reconcileResult struct { diff --git a/test/e2e-multicluster-test/multicluster_test.go b/test/e2e-multicluster-test/multicluster_test.go index 185d610e8..b6e64db97 100644 --- a/test/e2e-multicluster-test/multicluster_test.go +++ b/test/e2e-multicluster-test/multicluster_test.go @@ -714,5 +714,87 @@ var _ = Describe("Test multicluster scenario", func() { Expect(cm.Data["cluster"]).Should(Equal("cluster-worker")) Expect(k8sClient.Delete(hubCtx, def)).Should(Succeed()) }) + + It("Test application with failed gc and restart workflow", func() { + By("duplicate cluster") + secret := &corev1.Secret{} + const secretName = "disconnection-test" + Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: kubevelatypes.DefaultKubeVelaNS, Name: WorkerClusterName}, secret)).Should(Succeed()) + secret.SetName(secretName) + secret.SetResourceVersion("") + Expect(k8sClient.Create(hubCtx, secret)).Should(Succeed()) + defer func() { + _ = k8sClient.Delete(hubCtx, secret) + }() + + By("create cluster normally") + bs, err := os.ReadFile("./testdata/app/app-disconnection-test.yaml") + Expect(err).Should(Succeed()) + app := &v1beta1.Application{} + Expect(yaml.Unmarshal(bs, app)).Should(Succeed()) + app.SetNamespace(namespace) + Expect(k8sClient.Create(hubCtx, app)).Should(Succeed()) + key := client.ObjectKeyFromObject(app) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(hubCtx, key, app)).Should(Succeed()) + g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunning)) + }).WithTimeout(10 * time.Second).WithPolling(2 * time.Second).Should(Succeed()) + + By("disconnect cluster") + Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: kubevelatypes.DefaultKubeVelaNS, Name: secretName}, secret)).Should(Succeed()) + secret.Data["endpoint"] = []byte("https://1.2.3.4:9999") + Expect(k8sClient.Update(hubCtx, secret)).Should(Succeed()) + + By("update application") + Expect(k8sClient.Get(hubCtx, key, app)).Should(Succeed()) + app.Spec.Policies = nil + Expect(k8sClient.Update(hubCtx, app)).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(hubCtx, key, app)).Should(Succeed()) + g.Expect(app.Status.ObservedGeneration).Should(Equal(app.Generation)) + g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunning)) + rts := &v1beta1.ResourceTrackerList{} + g.Expect(k8sClient.List(hubCtx, rts, client.MatchingLabels{oam.LabelAppName: key.Name, oam.LabelAppNamespace: key.Namespace})).Should(Succeed()) + cnt := 0 + for _, item := range rts.Items { + if item.Spec.Type == v1beta1.ResourceTrackerTypeVersioned { + cnt++ + } + } + g.Expect(cnt).Should(Equal(2)) + }).WithTimeout(10 * time.Second).WithPolling(2 * time.Second).Should(Succeed()) + + By("try update application again") + Expect(k8sClient.Get(hubCtx, key, app)).Should(Succeed()) + if app.Annotations == nil { + app.Annotations = map[string]string{} + } + app.Annotations[oam.AnnotationPublishVersion] = "test" + Expect(k8sClient.Update(hubCtx, app)).Should(Succeed()) + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(hubCtx, key, app)).Should(Succeed()) + g.Expect(app.Status.LatestRevision).ShouldNot(BeNil()) + g.Expect(app.Status.LatestRevision.Revision).Should(Equal(int64(3))) + g.Expect(app.Status.ObservedGeneration).Should(Equal(app.Generation)) + g.Expect(app.Status.Phase).Should(Equal(common.ApplicationRunning)) + }).WithTimeout(1 * time.Minute).WithPolling(2 * time.Second).Should(Succeed()) + + By("clear disconnection cluster secret") + Expect(k8sClient.Get(hubCtx, types.NamespacedName{Namespace: kubevelatypes.DefaultKubeVelaNS, Name: secretName}, secret)).Should(Succeed()) + Expect(k8sClient.Delete(hubCtx, secret)).Should(Succeed()) + + By("wait gc application completed") + Eventually(func(g Gomega) { + rts := &v1beta1.ResourceTrackerList{} + g.Expect(k8sClient.List(hubCtx, rts, client.MatchingLabels{oam.LabelAppName: key.Name, oam.LabelAppNamespace: key.Namespace})).Should(Succeed()) + cnt := 0 + for _, item := range rts.Items { + if item.Spec.Type == v1beta1.ResourceTrackerTypeVersioned { + cnt++ + } + } + g.Expect(cnt).Should(Equal(1)) + }).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should(Succeed()) + }) }) }) diff --git a/test/e2e-multicluster-test/testdata/app/app-disconnection-test.yaml b/test/e2e-multicluster-test/testdata/app/app-disconnection-test.yaml new file mode 100644 index 000000000..2eb02cd50 --- /dev/null +++ b/test/e2e-multicluster-test/testdata/app/app-disconnection-test.yaml @@ -0,0 +1,17 @@ +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: app-disconnection-test +spec: + components: + - type: k8s-objects + name: app-dis-cm + properties: + objects: + - apiVersion: v1 + kind: ConfigMap + policies: + - type: topology + name: disconnection-test + properties: + clusters: ["disconnection-test"] \ No newline at end of file