add tests

Signed-off-by: 天元 <jianbo.sjb@alibaba-inc.com>
crossplane · Jun 22, 2020 · bf5d471 · bf5d471
1 parent 9a89fdf
commit bf5d471
Show file tree

Hide file tree

Showing 10 changed files with 651 additions and 53 deletions.
diff --git a/apis/core/v1alpha2/core_types.go b/apis/core/v1alpha2/core_types.go
@@ -198,13 +198,19 @@ type ComponentStatus struct {
 
 	// LatestRevision of component
 	// +optional
-	LatestRevision string `json:"latestRevision"`
+	LatestRevision *Revision `json:"latestRevision,omitempty"`
 
 	// TODO(negz): Maintain references to any ApplicationConfigurations that
 	// reference this component? Doing so would allow us to queue a reconcile
 	// for consuming ApplicationConfigurations when this Component changed.
 }
 
+// Revision has name and revision number
+type Revision struct {
+	Name     string `json:"name"`
+	Revision int64  `json:"revision"`
+}
+
 // +kubebuilder:object:root=true
 
 // A Component describes how an OAM workload kind may be instantiated.
@@ -304,8 +310,8 @@ type WorkloadStatus struct {
 	// ComponentName that produced this workload.
 	ComponentName string `json:"componentName,omitempty"`
 
-	//ComponentRevision of current component
-	ComponentRevision string `json:"componentRevision,omitempty"`
+	//ComponentRevisionName of current component
+	ComponentRevisionName string `json:"componentRevisionName,omitempty"`
 
 	// Reference to a workload created by an ApplicationConfiguration.
 	Reference runtimev1alpha1.TypedReference `json:"workloadRef,omitempty"`

diff --git a/apis/core/v1alpha2/zz_generated.deepcopy.go b/apis/core/v1alpha2/zz_generated.deepcopy.go
diff --git a/go.mod b/go.mod
@@ -6,9 +6,10 @@ require (
 	github.com/crossplane/crossplane-runtime v0.8.0
 	github.com/crossplane/oam-controllers v0.0.0-00010101000000-000000000000
 	github.com/google/go-cmp v0.4.0
+	github.com/json-iterator/go v1.1.8
 	github.com/pkg/errors v0.8.1
 	github.com/rs/xid v1.2.1
-	golang.org/x/tools v0.0.0-20200325010219-a49f79bcc224
+	github.com/stretchr/testify v1.6.1
 	k8s.io/api v0.18.2
 	k8s.io/apiextensions-apiserver v0.18.2
 	k8s.io/apimachinery v0.18.2

diff --git a/go.sum b/go.sum
@@ -432,6 +432,8 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
+github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
@@ -748,6 +750,8 @@ gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
 gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.0-20190905181640-827449938966 h1:B0J02caTR6tpSJozBJyiAzT6CtBzjclw4pgm9gg8Ys0=
 gopkg.in/yaml.v3 v3.0.0-20190905181640-827449938966/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=

diff --git a/pkg/controller/oam/applicationconfiguration/applicationconfiguration.go b/pkg/controller/oam/applicationconfiguration/applicationconfiguration.go
@@ -240,8 +240,8 @@ type Workload struct {
 	// ComponentName that produced this workload.
 	ComponentName string
 
-	//ComponentRevision of current component
-	ComponentRevision string
+	//ComponentRevisionName of current component
+	ComponentRevisionName string
 
 	// A Workload object.
 	Workload *unstructured.Unstructured
@@ -254,8 +254,8 @@ type Workload struct {
 // in the status of an ApplicationConfiguration.
 func (w Workload) Status() v1alpha2.WorkloadStatus {
 	acw := v1alpha2.WorkloadStatus{
-		ComponentName:     w.ComponentName,
-		ComponentRevision: w.ComponentRevision,
+		ComponentName:         w.ComponentName,
+		ComponentRevisionName: w.ComponentRevisionName,
 		Reference: runtimev1alpha1.TypedReference{
 			APIVersion: w.Workload.GetAPIVersion(),
 			Kind:       w.Workload.GetKind(),

diff --git a/pkg/controller/oam/applicationconfiguration/applicationconfiguration_test.go b/pkg/controller/oam/applicationconfiguration/applicationconfiguration_test.go
@@ -424,3 +424,9 @@ func TestEligible(t *testing.T) {
 		})
 	}
 }
+
+func TestIsRevisionWorkload(t *testing.T) {
+	if true != IsRevisionWorkload(v1alpha2.WorkloadStatus{ComponentName: "compName", Reference: runtimev1alpha1.TypedReference{Name: "compName-rev1"}}) {
+		t.Error("workloadName has componentName as prefix is revisionWorkload")
+	}
+}
diff --git a/pkg/controller/oam/applicationconfiguration/component.go b/pkg/controller/oam/applicationconfiguration/component.go
@@ -32,17 +32,23 @@ type ComponentHandler struct {
 
 // Create implements EventHandler
 func (c *ComponentHandler) Create(evt event.CreateEvent, q workqueue.RateLimitingInterface) {
-	c.createControllerRevision(evt.Meta, evt.Object)
+	if !c.createControllerRevision(evt.Meta, evt.Object) {
+		// No revision created, return
+		return
+	}
 	for _, req := range c.getRelatedAppConfig(evt.Meta) {
 		q.Add(req)
 	}
 }
 
 // Update implements EventHandler
 func (c *ComponentHandler) Update(evt event.UpdateEvent, q workqueue.RateLimitingInterface) {
-	c.createControllerRevision(evt.MetaNew, evt.ObjectNew)
+	if !c.createControllerRevision(evt.MetaNew, evt.ObjectNew) {
+		// No revision created, return
+		return
+	}
 	//Note(wonderflow): MetaOld => MetaNew, requeue once is enough
-	for _, req := range c.getRelatedAppConfig(evt.MetaOld) {
+	for _, req := range c.getRelatedAppConfig(evt.MetaNew) {
 		q.Add(req)
 	}
 }
@@ -86,39 +92,54 @@ func (c *ComponentHandler) getRelatedAppConfig(object metav1.Object) []reconcile
 
 //IsRevisionDiff check whether there's any different between two component revision
 func (c *ComponentHandler) IsRevisionDiff(mt metav1.Object, curComp *v1alpha2.Component) (bool, int64) {
-	if curComp.Status.LatestRevision == "" {
-		return true, 1
+	if curComp.Status.LatestRevision == nil {
+		return true, 0
 	}
-	oldRev, err := c.appsClient.ControllerRevisions(mt.GetNamespace()).Get(context.Background(), curComp.Status.LatestRevision, metav1.GetOptions{})
+	oldRev, err := c.appsClient.ControllerRevisions(mt.GetNamespace()).Get(context.Background(), curComp.Status.LatestRevision.Name, metav1.GetOptions{})
 	if err != nil {
-		c.l.Info(fmt.Sprintf("get old controllerRevision %s error %v, will create new revision", curComp.Status.LatestRevision, err), "componentName", mt.GetName())
-		// Note(wonderflow) Use generation as revision number when fail to get old revision
-		return true, mt.GetGeneration()
+		c.l.Info(fmt.Sprintf("get old controllerRevision %s error %v, will create new revision", curComp.Status.LatestRevision.Name, err), "componentName", mt.GetName())
+		return true, curComp.Status.LatestRevision.Revision
 	}
-	var oldComp v1alpha2.Component
-	err = json.Unmarshal(oldRev.Data.Raw, &oldComp)
+	oldComp, err := UnpackRevisionData(oldRev)
 	if err != nil {
-		c.l.Info(fmt.Sprintf("Unmarshal old controllerRevision %s error %v, will create new revision", curComp.Status.LatestRevision, err), "componentName", mt.GetName())
-		return true, oldRev.Revision + 1
+		c.l.Info(fmt.Sprintf("Unmarshal old controllerRevision %s error %v, will create new revision", curComp.Status.LatestRevision.Name, err), "componentName", mt.GetName())
+		return true, oldRev.Revision
 	}
+
 	if reflect.DeepEqual(curComp.Spec, oldComp.Spec) {
-		return false, -1
+		return false, oldRev.Revision
 	}
-	return true, oldRev.Revision + 1
+	return true, oldRev.Revision
+}
+
+// UnpackRevisionData will unpack revision.Data to Component
+func UnpackRevisionData(rev *appsv1.ControllerRevision) (*v1alpha2.Component, error) {
+	var err error
+	if rev.Data.Object != nil {
+		comp, ok := rev.Data.Object.(*v1alpha2.Component)
+		if !ok {
+			return nil, fmt.Errorf("invalid type of revision %s, type should not be %v", rev.Name, reflect.TypeOf(rev.Data.Object))
+		}
+		return comp, nil
+	}
+	var comp v1alpha2.Component
+	err = json.Unmarshal(rev.Data.Raw, &comp)
+	return &comp, err
 }
 
 func newTrue() *bool {
 	b := true
 	return &b
 }
 
-func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtime.Object) {
+func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtime.Object) bool {
 	curComp := obj.(*v1alpha2.Component)
-	diff, newRevision := c.IsRevisionDiff(mt, curComp)
+	diff, curRevision := c.IsRevisionDiff(mt, curComp)
 	if !diff {
 		// No difference, no need to create new revision.
-		return
+		return false
 	}
+	nextRevision := curRevision + 1
 	revisionName := ConstructRevisionName(mt.GetName())
 	// set annotation to component
 	revision := appsv1.ControllerRevision{
@@ -134,21 +155,25 @@ func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtim
 				},
 			},
 		},
-		Revision: newRevision,
+		Revision: nextRevision,
 		Data:     runtime.RawExtension{Object: curComp},
 	}
 	_, err := c.appsClient.ControllerRevisions(mt.GetNamespace()).Create(context.Background(), &revision, metav1.CreateOptions{})
 	if err != nil {
 		c.l.Info(fmt.Sprintf("error create controllerRevision %v", err), "componentName", mt.GetName())
-		return
+		return false
+	}
+	curComp.Status.LatestRevision = &v1alpha2.Revision{
+		Name:     revisionName,
+		Revision: nextRevision,
 	}
-	curComp.Status.LatestRevision = revisionName
 	err = c.client.Status().Update(context.Background(), curComp)
 	if err != nil {
 		c.l.Info(fmt.Sprintf("update component status latestRevision %s err %v", revisionName, err), "componentName", mt.GetName())
-		return
+		return false
 	}
 	c.l.Info(fmt.Sprintf("ControllerRevision %s created", revisionName))
+	return true
 }
 
 // ConstructRevisionName will generate revisionName from componentName