From 4833767627dc2805e19437375fd7b5b5830c5ac8 Mon Sep 17 00:00:00 2001 From: zhuhuijun Date: Sun, 13 Sep 2020 22:31:56 +0800 Subject: [PATCH 1/4] feat(component):Add observedGeneration to represent Component Add observedGeneration to represent Component Fix #91 Signed-off-by: zhuhuijun --- apis/core/v1alpha2/core_types.go | 4 ++++ .../oam-kubernetes-runtime/crds/core.oam.dev_components.yaml | 4 ++++ pkg/controller/v1alpha2/applicationconfiguration/component.go | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/apis/core/v1alpha2/core_types.go b/apis/core/v1alpha2/core_types.go index a42faa6e..43719b5b 100644 --- a/apis/core/v1alpha2/core_types.go +++ b/apis/core/v1alpha2/core_types.go @@ -221,6 +221,10 @@ type ComponentSpec struct { // A ComponentStatus represents the observed state of a Component. type ComponentStatus struct { + // The generation observed by the component controller. + // +optional + ObservedGeneration int64 `json:"observedGeneration"` + runtimev1alpha1.ConditionedStatus `json:",inline"` // LatestRevision of component diff --git a/charts/oam-kubernetes-runtime/crds/core.oam.dev_components.yaml b/charts/oam-kubernetes-runtime/crds/core.oam.dev_components.yaml index 5947b071..2bcda26b 100644 --- a/charts/oam-kubernetes-runtime/crds/core.oam.dev_components.yaml +++ b/charts/oam-kubernetes-runtime/crds/core.oam.dev_components.yaml @@ -139,6 +139,10 @@ spec: - name - revision type: object + observedGeneration: + description: The generation observed by the component controller. + format: int64 + type: integer type: object type: object served: true diff --git a/pkg/controller/v1alpha2/applicationconfiguration/component.go b/pkg/controller/v1alpha2/applicationconfiguration/component.go index 51fe5d6e..fd99a30e 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/component.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/component.go @@ -171,6 +171,10 @@ func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtim c.Logger.Info(fmt.Sprintf("error create controllerRevision %v", err), "componentName", mt.GetName()) return false } + + if curComp.Status.ObservedGeneration != curComp.Generation { + curComp.Status.ObservedGeneration = curComp.Generation + } err = c.Client.Status().Update(context.Background(), curComp) if err != nil { c.Logger.Info(fmt.Sprintf("update component status latestRevision %s err %v", revisionName, err), "componentName", mt.GetName()) From 263dd2aec89e2b12e6f075117f6f543e18020ebc Mon Sep 17 00:00:00 2001 From: zhuhuijun Date: Wed, 16 Sep 2020 22:18:25 +0800 Subject: [PATCH 2/4] feat(component):Add observedGeneration to represent Component Add observedGeneration to represent Component Fix #205 Signed-off-by: zhuhuijun --- apis/core/v1alpha2/core_types.go | 4 ++ ...ore.oam.dev_applicationconfigurations.yaml | 4 ++ .../applicationconfiguration.go | 7 +++ .../applicationconfiguration_test.go | 55 +++++++++++++++++++ .../applicationconfiguration/component.go | 2 + .../component_test.go | 8 ++- 6 files changed, 78 insertions(+), 2 deletions(-) diff --git a/apis/core/v1alpha2/core_types.go b/apis/core/v1alpha2/core_types.go index 43719b5b..b35b9043 100644 --- a/apis/core/v1alpha2/core_types.go +++ b/apis/core/v1alpha2/core_types.go @@ -409,6 +409,10 @@ type ApplicationConfigurationStatus struct { // Workloads created by this ApplicationConfiguration. Workloads []WorkloadStatus `json:"workloads,omitempty"` + + // The generation observed by the appConfig controller. + // +optional + ObservedGeneration int64 `json:"observedGeneration"` } // DependencyStatus represents the observed state of the dependency of diff --git a/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml b/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml index a9580035..e7556662 100644 --- a/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml +++ b/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml @@ -369,6 +369,10 @@ spec: type: object type: array type: object + observedGeneration: + description: The generation observed by the appConfig controller. + format: int64 + type: integer status: description: Status is a place holder for a customized controller to fill if it needs a single place to summarize the status of the diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go index 7ef233d9..7a4213ea 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go @@ -239,6 +239,7 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco } r.record.Event(ac, event.Normal(reasonExecutePosthook, "Successfully executed a posthook", "posthook name", name)) } + updateObservedGeneration(ac) returnErr = errors.Wrap(r.client.Status().Update(ctx, ac), errUpdateAppConfigStatus) }() @@ -350,6 +351,12 @@ func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac *v1alpha ac.SetConditions(v1alpha1.ReconcileSuccess()) } +func updateObservedGeneration(ac *v1alpha2.ApplicationConfiguration) { + if ac.Status.ObservedGeneration != ac.Generation { + ac.Status.ObservedGeneration = ac.Generation + } +} + // if any finalizers newly registered, return true func registerFinalizers(ac *v1alpha2.ApplicationConfiguration) bool { newFinalizer := false diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go index c1707d78..a65ec5f9 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration_test.go @@ -22,6 +22,10 @@ import ( "testing" "time" + "k8s.io/apimachinery/pkg/types" + + "k8s.io/apimachinery/pkg/util/intstr" + runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1" "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/resource" @@ -29,6 +33,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/pkg/errors" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -1394,3 +1399,53 @@ func TestAddDataOutputsToDAG(t *testing.T) { t.Errorf("didn't add conditions to source correctly: %s", diff) } } + +func TestUpdateStatus(t *testing.T) { + + mockGetAppConfigFn := func(_ context.Context, key client.ObjectKey, obj runtime.Object) error { + if o, ok := obj.(*v1alpha2.ApplicationConfiguration); ok { + *o = v1alpha2.ApplicationConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-appconfig", + Generation: 1, + }, + Spec: v1alpha2.ApplicationConfigurationSpec{ + Components: []v1alpha2.ApplicationConfigurationComponent{ + { + ComponentName: "example-component", + ParameterValues: []v1alpha2.ComponentParameterValue{ + { + Name: "image", + Value: intstr.IntOrString{ + StrVal: "wordpress:php7.3", + }, + }, + }, + }, + }, + }, + Status: v1alpha2.ApplicationConfigurationStatus{ + ObservedGeneration: 0, + }, + } + } + return nil + } + + m := &mock.Manager{ + Client: &test.MockClient{ + MockGet: mockGetAppConfigFn, + }, + } + + r := NewReconciler(m) + + ac := &v1alpha2.ApplicationConfiguration{} + err := r.client.Get(context.Background(), types.NamespacedName{Name: "example-appconfig"}, ac) + assert.Equal(t, err, nil) + assert.Equal(t, ac.Status.ObservedGeneration, int64(0)) + + updateObservedGeneration(ac) + assert.Equal(t, ac.Status.ObservedGeneration, int64(1)) + +} diff --git a/pkg/controller/v1alpha2/applicationconfiguration/component.go b/pkg/controller/v1alpha2/applicationconfiguration/component.go index fd99a30e..9f159cea 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/component.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/component.go @@ -166,6 +166,7 @@ func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtim Revision: nextRevision, Data: runtime.RawExtension{Object: curComp}, } + err := c.Client.Create(context.TODO(), &revision) if err != nil { c.Logger.Info(fmt.Sprintf("error create controllerRevision %v", err), "componentName", mt.GetName()) @@ -175,6 +176,7 @@ func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtim if curComp.Status.ObservedGeneration != curComp.Generation { curComp.Status.ObservedGeneration = curComp.Generation } + err = c.Client.Status().Update(context.Background(), curComp) if err != nil { c.Logger.Info(fmt.Sprintf("update component status latestRevision %s err %v", revisionName, err), "componentName", mt.GetName()) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/component_test.go b/pkg/controller/v1alpha2/applicationconfiguration/component_test.go index e393c963..0829bfa8 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/component_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/component_test.go @@ -26,7 +26,9 @@ import ( func TestComponentHandler(t *testing.T) { q := controllertest.Queue{Interface: workqueue.New()} - var curComp = &v1alpha2.Component{} + var curComp = &v1alpha2.Component{ + ObjectMeta: metav1.ObjectMeta{Generation: 1}, + } var createdRevisions = []appsv1.ControllerRevision{} var instance = ComponentHandler{ Client: &test.MockClient{ @@ -109,7 +111,7 @@ func TestComponentHandler(t *testing.T) { RevisionLimit: 2, } comp := &v1alpha2.Component{ - ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "comp1"}, + ObjectMeta: metav1.ObjectMeta{Namespace: "biz", Name: "comp1", Generation: 1}, Spec: v1alpha2.ComponentSpec{Workload: runtime.RawExtension{Object: &v1.Deployment{Spec: v1.DeploymentSpec{Template: v12.PodTemplateSpec{Spec: v12.PodSpec{Containers: []v12.Container{{Image: "nginx:v1"}}}}}}}}, } @@ -137,6 +139,8 @@ func TestComponentHandler(t *testing.T) { // check component's status saved in corresponding controllerRevision assert.Equal(t, gotComp.Status.LatestRevision.Name, revisions.Items[0].Name) assert.Equal(t, gotComp.Status.LatestRevision.Revision, revisions.Items[0].Revision) + // check component's status ObservedGeneration + assert.Equal(t, gotComp.Status.ObservedGeneration, comp.Generation) q.Done(item) // ============ Test Create Event End =================== From dc0dfea301247713cabcfcb0cfecc5770c0c1363 Mon Sep 17 00:00:00 2001 From: zhuhuijun Date: Mon, 28 Sep 2020 18:41:38 +0800 Subject: [PATCH 3/4] mv updateObservedGeneration Signed-off-by: zhuhuijun --- .../applicationconfiguration/applicationconfiguration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go index 7a4213ea..259d9f51 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go @@ -227,6 +227,7 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco // execute the posthooks at the end no matter what defer func() { + updateObservedGeneration(ac) for name, hook := range r.postHooks { exeResult, err := hook.Exec(ctx, ac, log) if err != nil { @@ -239,7 +240,6 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco } r.record.Event(ac, event.Normal(reasonExecutePosthook, "Successfully executed a posthook", "posthook name", name)) } - updateObservedGeneration(ac) returnErr = errors.Wrap(r.client.Status().Update(ctx, ac), errUpdateAppConfigStatus) }() From 4af60f2dc9a6c029bc3a0e0884f8a9c326aaee1e Mon Sep 17 00:00:00 2001 From: zhuhuijun Date: Mon, 28 Sep 2020 19:16:26 +0800 Subject: [PATCH 4/4] fix check-diff Signed-off-by: zhuhuijun --- .../crds/core.oam.dev_applicationconfigurations.yaml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml b/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml index c1a3f074..7a15b908 100644 --- a/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml +++ b/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml @@ -369,12 +369,6 @@ spec: type: object type: array type: object -<<<<<<< HEAD - observedGeneration: - description: The generation observed by the appConfig controller. - format: int64 - type: integer -======= historyWorkloads: description: HistoryWorkloads will record history but still working revision workloads. @@ -407,7 +401,10 @@ spec: type: object type: object type: array ->>>>>>> upstream/master + observedGeneration: + description: The generation observed by the appConfig controller. + format: int64 + type: integer status: description: Status is a place holder for a customized controller to fill if it needs a single place to summarize the status of the