diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go index 6b63cfc2a..99040c0e6 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go @@ -28,6 +28,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" @@ -108,6 +109,12 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, util.PatchCondition(ctx, r, &(componentDefinition), cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrCreateOrUpdateDefinitionRevision, defRev.Name, err))) } + klog.InfoS("Successfully update DefinitionRevision", "name", defRev.Name) + + if err := coredef.CleanUpDefinitionRevision(ctx, r.Client, &componentDefinition, r.defRevLimit); err != nil { + klog.Error("[Garbage collection]") + r.record.Event(&componentDefinition, event.Warning("failed to garbage collect DefinitionRevision of type ComponentDefinition", err)) + } return ctrl.Result{}, nil } @@ -150,6 +157,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, util.PatchCondition(ctx, r, &(def.ComponentDefinition), cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrCreateOrUpdateDefinitionRevision, defRev.Name, err))) } + klog.InfoS("Successfully create DefinitionRevision", "name", defRev.Name) def.ComponentDefinition.Status.LatestRevision = &common.Revision{ Name: defRev.Name, @@ -157,14 +165,16 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { RevisionHash: defRev.Spec.RevisionHash, } - if err := r.Status().Update(ctx, &def.ComponentDefinition); err != nil { + if err := r.UpdateStatus(ctx, &def.ComponentDefinition); err != nil { klog.ErrorS(err, "cannot update componentDefinition Status") r.record.Event(&(def.ComponentDefinition), event.Warning("cannot update ComponentDefinition Status", err)) - return ctrl.Result{}, err + return ctrl.Result{}, util.PatchCondition(ctx, r, &(def.ComponentDefinition), + cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrUpdateComponentDefinition, def.ComponentDefinition.Name, err))) } if err := coredef.CleanUpDefinitionRevision(ctx, r.Client, &def.ComponentDefinition, r.defRevLimit); err != nil { klog.Error("[Garbage collection]") + r.record.Event(&def.ComponentDefinition, event.Warning("failed to garbage collect DefinitionRevision of type ComponentDefinition", err)) } return ctrl.Result{}, nil @@ -203,6 +213,18 @@ func (r *Reconciler) createOrUpdateComponentDefRevision(ctx context.Context, nam return r.Update(ctx, rev) } +// UpdateStatus updates v1beta1.ComponentDefinition's Status with retry.RetryOnConflict +func (r *Reconciler) UpdateStatus(ctx context.Context, def *v1beta1.ComponentDefinition, opts ...client.UpdateOption) error { + status := def.DeepCopy().Status + return retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { + if err = r.Get(ctx, client.ObjectKey{Namespace: def.Namespace, Name: def.Name}, def); err != nil { + return + } + def.Status = status + return r.Status().Update(ctx, def, opts...) + }) +} + // SetupWithManager will setup with event recorder func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { r.record = event.NewAPIRecorder(mgr.GetEventRecorderFor("ComponentDefinition")). diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componetrevision_test.go b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componetrevision_test.go index 7a99a7409..b192bf836 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componetrevision_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componetrevision_test.go @@ -169,6 +169,8 @@ var _ = Describe("Test DefinitionRevision created by ComponentDefinition", func( Context("Test ComponentDefinition Controller clean up", func() { It("Test clean up definitionRevision", func() { + var revKey client.ObjectKey + var defRev v1beta1.DefinitionRevision cdName := "test-clean-up" revisionNum := 1 defKey := client.ObjectKey{Namespace: namespace, Name: cdName} @@ -179,17 +181,10 @@ var _ = Describe("Test DefinitionRevision created by ComponentDefinition", func( cd.Name = cdName cd.Spec.Schematic.CUE.Template = fmt.Sprintf(cdTemplate, fmt.Sprintf("test-v%d", revisionNum)) Expect(k8sClient.Create(ctx, cd)).Should(BeNil()) - reconcileRetry(&r, req) - revKey := client.ObjectKey{Namespace: namespace, Name: fmt.Sprintf("%s-v%d", cdName, revisionNum)} - revisionNum++ - var defRev v1beta1.DefinitionRevision - Eventually(func() error { - return k8sClient.Get(ctx, revKey, &defRev) - }, 10*time.Second, time.Second).Should(BeNil()) By("update componentDefinition") checkComp := new(v1beta1.ComponentDefinition) - for i := 0; i < defRevisionLimit; i++ { + for i := 0; i < defRevisionLimit+1; i++ { Eventually(func() error { err := k8sClient.Get(ctx, defKey, checkComp) if err != nil { @@ -217,9 +212,14 @@ var _ = Describe("Test DefinitionRevision created by ComponentDefinition", func( checkComp.Spec.Schematic.CUE.Template = fmt.Sprintf(cdTemplate, fmt.Sprintf("test-v%d", revisionNum)) return k8sClient.Update(ctx, checkComp) }, 10*time.Second, time.Second).Should(BeNil()) - revisionNum++ reconcileRetry(&r, req) + revKey = client.ObjectKey{Namespace: namespace, Name: fmt.Sprintf("%s-v%d", cdName, revisionNum)} + revisionNum++ + Eventually(func() error { + return k8sClient.Get(ctx, revKey, &defRev) + }, 10*time.Second, time.Second).Should(BeNil()) + deletedRevision := new(v1beta1.DefinitionRevision) deleteRevKey := types.NamespacedName{Namespace: namespace, Name: cdName + "-v1"} listOpts := []client.ListOption{ @@ -255,6 +255,11 @@ var _ = Describe("Test DefinitionRevision created by ComponentDefinition", func( }, 10*time.Second, time.Second).Should(BeNil()) reconcileRetry(&r, req) + revKey = client.ObjectKey{Namespace: namespace, Name: fmt.Sprintf("%s-v%d", cdName, revisionNum)} + Eventually(func() error { + return k8sClient.Get(ctx, revKey, &defRev) + }, 10*time.Second, time.Second).Should(BeNil()) + deleteRevKey = types.NamespacedName{Namespace: namespace, Name: cdName + "-v2"} Eventually(func() error { err := k8sClient.List(ctx, defRevList, listOpts...) diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_controller.go b/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_controller.go index 8a4263d1a..a8c42cd3d 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_controller.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_controller.go @@ -28,6 +28,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" @@ -101,6 +102,12 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, util.PatchCondition(ctx, r, &(traitdefinition), cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrCreateOrUpdateDefinitionRevision, defRev.Name, err))) } + klog.InfoS("Successfully update DefinitionRevision", "name", defRev.Name) + + if err := coredef.CleanUpDefinitionRevision(ctx, r.Client, &traitdefinition, r.defRevLimit); err != nil { + klog.Error("[Garbage collection]") + r.record.Event(&traitdefinition, event.Warning("failed to garbage collect DefinitionRevision of type TraitDefinition", err)) + } return ctrl.Result{}, nil } @@ -123,6 +130,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { return ctrl.Result{}, util.PatchCondition(ctx, r, &(def.TraitDefinition), cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrCreateOrUpdateDefinitionRevision, defRev.Name, err))) } + klog.InfoS("Successfully create DefinitionRevision", "name", defRev.Name) def.TraitDefinition.Status.LatestRevision = &common.Revision{ Name: defRev.Name, @@ -130,14 +138,16 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { RevisionHash: defRev.Spec.RevisionHash, } - if err := r.Status().Update(ctx, &def.TraitDefinition); err != nil { + if err := r.UpdateStatus(ctx, &def.TraitDefinition); err != nil { klog.ErrorS(err, "cannot update TraitDefinition Status") r.record.Event(&(def.TraitDefinition), event.Warning("cannot update TraitDefinition Status", err)) - return ctrl.Result{}, err + return ctrl.Result{}, util.PatchCondition(ctx, r, &(def.TraitDefinition), + cpv1alpha1.ReconcileError(fmt.Errorf(util.ErrUpdateTraitDefinition, def.TraitDefinition.Name, err))) } if err := coredef.CleanUpDefinitionRevision(ctx, r.Client, &def.TraitDefinition, r.defRevLimit); err != nil { klog.Error("[Garbage collection]") + r.record.Event(&def.TraitDefinition, event.Warning("failed to garbage collect DefinitionRevision of type TraitDefinition", err)) } return ctrl.Result{}, nil @@ -176,6 +186,18 @@ func (r *Reconciler) createOrUpdateTraitDefRevision(ctx context.Context, namespa return r.Update(ctx, rev) } +// UpdateStatus updates v1beta1.TraitDefinition's Status with retry.RetryOnConflict +func (r *Reconciler) UpdateStatus(ctx context.Context, def *v1beta1.TraitDefinition, opts ...client.UpdateOption) error { + status := def.DeepCopy().Status + return retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { + if err = r.Get(ctx, client.ObjectKey{Namespace: def.Namespace, Name: def.Name}, def); err != nil { + return + } + def.Status = status + return r.Status().Update(ctx, def, opts...) + }) +} + // SetupWithManager will setup with event recorder func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { r.record = event.NewAPIRecorder(mgr.GetEventRecorderFor("TraitDefinition")). diff --git a/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitrevision_test.go b/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitrevision_test.go index 1118eb490..8b103653d 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitrevision_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitrevision_test.go @@ -131,6 +131,8 @@ var _ = Describe("Test DefinitionRevision created by TraitDefinition", func() { Context("Test TraitDefinition Controller clean up", func() { It("Test clean up definitionRevision", func() { + var revKey client.ObjectKey + var defRev v1beta1.DefinitionRevision tdName := "test-clean-up" revisionNum := 1 defKey := client.ObjectKey{Namespace: namespace, Name: tdName} @@ -141,17 +143,10 @@ var _ = Describe("Test DefinitionRevision created by TraitDefinition", func() { td.Name = tdName td.Spec.Schematic.CUE.Template = fmt.Sprintf(tdTemplate, fmt.Sprintf("test-v%d", revisionNum)) Expect(k8sClient.Create(ctx, td)).Should(BeNil()) - reconcileRetry(&r, req) - revKey := client.ObjectKey{Namespace: namespace, Name: fmt.Sprintf("%s-v%d", tdName, revisionNum)} - revisionNum++ - var defRev v1beta1.DefinitionRevision - Eventually(func() error { - return k8sClient.Get(ctx, revKey, &defRev) - }, 10*time.Second, time.Second).Should(BeNil()) By("update TraitDefinition") checkComp := new(v1beta1.TraitDefinition) - for i := 0; i < defRevisionLimit; i++ { + for i := 0; i < defRevisionLimit+1; i++ { Eventually(func() error { err := k8sClient.Get(ctx, defKey, checkComp) if err != nil { @@ -179,9 +174,14 @@ var _ = Describe("Test DefinitionRevision created by TraitDefinition", func() { checkComp.Spec.Schematic.CUE.Template = fmt.Sprintf(tdTemplate, fmt.Sprintf("test-v%d", revisionNum)) return k8sClient.Update(ctx, checkComp) }, 10*time.Second, time.Second).Should(BeNil()) - revisionNum++ reconcileRetry(&r, req) + revKey = client.ObjectKey{Namespace: namespace, Name: fmt.Sprintf("%s-v%d", tdName, revisionNum)} + revisionNum++ + Eventually(func() error { + return k8sClient.Get(ctx, revKey, &defRev) + }, 10*time.Second, time.Second).Should(BeNil()) + deletedRevision := new(v1beta1.DefinitionRevision) deleteRevKey := types.NamespacedName{Namespace: namespace, Name: tdName + "-v1"} listOpts := []client.ListOption{ @@ -217,6 +217,11 @@ var _ = Describe("Test DefinitionRevision created by TraitDefinition", func() { }, 10*time.Second, time.Second).Should(BeNil()) reconcileRetry(&r, req) + revKey = client.ObjectKey{Namespace: namespace, Name: fmt.Sprintf("%s-v%d", tdName, revisionNum)} + Eventually(func() error { + return k8sClient.Get(ctx, revKey, &defRev) + }, 10*time.Second, time.Second).Should(BeNil()) + deleteRevKey = types.NamespacedName{Namespace: namespace, Name: tdName + "-v2"} Eventually(func() error { err := k8sClient.List(ctx, defRevList, listOpts...) diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 709f40af2..f2e49917b 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -97,6 +97,11 @@ const ( // ErrUpdateCapabilityInConfigMap is the error while creating or updating a capability ErrUpdateCapabilityInConfigMap = "cannot create or update capability %s in ConfigMap: %v" + // ErrUpdateComponentDefinition is the error while update ComponentDefinition + ErrUpdateComponentDefinition = "cannot update ComponentDefinition %s: %v" + // ErrUpdateTraitDefinition is the error while update TraitDefinition + ErrUpdateTraitDefinition = "cannot update TraitDefinition %s: %v" + // ErrCreateConvertedWorklaodDefinition is the error while apply a WorkloadDefinition ErrCreateConvertedWorklaodDefinition = "cannot create converted WorkloadDefinition %s: %v"