fix workload status for multiple revision case (crossplane#190)

* fix workload status for multiple revision case Signed-off-by: 天元 <jianbo.sjb@alibaba-inc.com> * add CurrentWorkingRevision flag in workload status Signed-off-by: 天元 <jianbo.sjb@alibaba-inc.com> Signed-off-by: zzxwill <zzxwill@gmail.com>
zzxwill · Sep 10, 2020 · 9547ba9 · 9547ba9
1 parent 98e7619
commit 9547ba9
Show file tree

Hide file tree

Showing 15 changed files with 369 additions and 9 deletions.
diff --git a/apis/core/v1alpha2/core_types.go b/apis/core/v1alpha2/core_types.go
@@ -364,6 +364,9 @@ type WorkloadStatus struct {
 	// if it needs a single place to summarize the entire status of the workload
 	Status string `json:"status,omitempty"`
 
+	// HistoryWorkingRevision is a flag showing if it's history revision but still working
+	HistoryWorkingRevision bool `json:"currentWorkingRevision,omitempty"`
+
 	// ComponentName that produced this workload.
 	ComponentName string `json:"componentName,omitempty"`
 

diff --git a/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml b/charts/oam-kubernetes-runtime/crds/core.oam.dev_applicationconfigurations.yaml
@@ -385,6 +385,10 @@ spec:
                     componentRevisionName:
                       description: ComponentRevisionName of current component
                       type: string
+                    currentWorkingRevision:
+                      description: HistoryWorkingRevision is a flag showing if it's
+                        history revision but still working
+                      type: boolean
                     scopes:
                       description: Scopes associated with this workload.
                       items:

diff --git a/go.mod b/go.mod
@@ -6,6 +6,7 @@ require (
 	github.com/crossplane/crossplane-runtime v0.8.0
 	github.com/davecgh/go-spew v1.1.1
 	github.com/gertd/go-pluralize v0.1.7
+	github.com/ghodss/yaml v1.0.0
 	github.com/go-logr/logr v0.1.0
 	github.com/google/go-cmp v0.4.0
 	github.com/json-iterator/go v1.1.8

diff --git a/go.sum b/go.sum
@@ -111,6 +111,7 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo
 github.com/gertd/go-pluralize v0.1.7 h1:RgvJTJ5W7olOoAks97BOwOlekBFsLEyh00W48Z6ZEZY=
 github.com/gertd/go-pluralize v0.1.7/go.mod h1:O4eNeeIf91MHh1GJ2I47DNtaesm66NYvjYgAahcqSDQ=
 github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
+github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
 github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
 github.com/globalsign/mgo v0.0.0-20180905125535-1ca0a4f7cbcb/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=
 github.com/globalsign/mgo v0.0.0-20181015135952-eeefdecb41b8/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=

diff --git a/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go b/pkg/controller/v1alpha2/applicationconfiguration/applicationconfiguration.go
@@ -22,6 +22,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/crossplane/oam-kubernetes-runtime/pkg/oam"
+
 	"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 	runtimev1alpha1 "github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
 	"github.com/crossplane/crossplane-runtime/pkg/event"
@@ -297,11 +299,8 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco
 	// patch the final status
 	acPatch := client.MergeFrom(ac.DeepCopyObject())
 
-	ac.Status.Workloads = make([]v1alpha2.WorkloadStatus, len(workloads))
-	for i := range workloads {
-		ac.Status.Workloads[i] = workloads[i].Status()
-	}
-	ac.SetConditions(v1alpha1.ReconcileSuccess())
+	r.updateStatus(ctx, ac, workloads)
+
 	ac.Status.Dependency = v1alpha2.DependencyStatus{}
 	waitTime := longWait
 	if len(depStatus.Unsatisfied) != 0 {
@@ -313,6 +312,44 @@ func (r *OAMApplicationReconciler) Reconcile(req reconcile.Request) (result reco
 		errors.Wrap(r.client.Status().Patch(ctx, ac, acPatch, client.FieldOwner(ac.GetUID())), errUpdateAppConfigStatus)
 }
 
+func (r *OAMApplicationReconciler) updateStatus(ctx context.Context, ac *v1alpha2.ApplicationConfiguration, workloads []Workload) {
+	ac.Status.Workloads = make([]v1alpha2.WorkloadStatus, len(workloads))
+	revisionStatus := make([]v1alpha2.WorkloadStatus, 0)
+	for i, w := range workloads {
+		ac.Status.Workloads[i] = workloads[i].Status()
+		if !w.RevisionEnabled {
+			continue
+		}
+		var ul unstructured.UnstructuredList
+		ul.SetKind(w.Workload.GetKind())
+		ul.SetAPIVersion(w.Workload.GetAPIVersion())
+		if err := r.client.List(ctx, &ul, client.MatchingLabels{oam.LabelAppName: ac.Name, oam.LabelAppComponent: w.ComponentName}); err != nil {
+			continue
+		}
+		for _, v := range ul.Items {
+			if v.GetName() == w.ComponentRevisionName {
+				continue
+			}
+			// These workload exists means the component is under progress of rollout
+			// Trait will not work for these remaining workload
+			revisionStatus = append(revisionStatus, v1alpha2.WorkloadStatus{
+				ComponentName:          w.ComponentName,
+				ComponentRevisionName:  v.GetName(),
+				HistoryWorkingRevision: true,
+				Reference: v1alpha1.TypedReference{
+					APIVersion: v.GetAPIVersion(),
+					Kind:       v.GetKind(),
+					Name:       v.GetName(),
+					UID:        v.GetUID(),
+				},
+			})
+		}
+	}
+	ac.Status.Workloads = append(ac.Status.Workloads, revisionStatus...)
+
+	ac.SetConditions(v1alpha1.ReconcileSuccess())
+}
+
 // if any finalizers newly registered, return true
 func registerFinalizers(ac *v1alpha2.ApplicationConfiguration) bool {
 	newFinalizer := false
@@ -349,6 +386,9 @@ type Workload struct {
 	// Traits associated with this workload.
 	Traits []*Trait
 
+	// RevisionEnabled means multiple workloads of same component will possibly be alive.
+	RevisionEnabled bool
+
 	// Scopes associated with this workload.
 	Scopes []unstructured.Unstructured
 }

diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go
@@ -174,7 +174,8 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati
 
 	addDataOutputsToDAG(dag, acc.DataOutputs, w)
 
-	return &Workload{ComponentName: acc.ComponentName, ComponentRevisionName: componentRevisionName, Workload: w, Traits: traits, Scopes: scopes}, nil
+	return &Workload{ComponentName: acc.ComponentName, ComponentRevisionName: componentRevisionName,
+		Workload: w, Traits: traits, RevisionEnabled: isRevisionEnabled(traitDefs), Scopes: scopes}, nil
 }
 
 func (r *components) renderTrait(ctx context.Context, ct v1alpha2.ComponentTrait, ac *v1alpha2.ApplicationConfiguration,

diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go
@@ -338,7 +338,8 @@ func TestRenderComponents(t *testing.T) {
 								return &Trait{Object: *t}
 							}(),
 						},
-						Scopes: []unstructured.Unstructured{},
+						RevisionEnabled: true,
+						Scopes:          []unstructured.Unstructured{},
 					},
 				},
 			},

diff --git a/test/e2e-test/component_version_test.go b/test/e2e-test/component_version_test.go
@@ -3,8 +3,14 @@ package controllers_test
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"io/ioutil"
 	"time"
 
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/ghodss/yaml"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
@@ -297,6 +303,184 @@ var _ = Describe("Versioning mechanism of components", func() {
 		})
 	})
 
-	//TODO(roywang) Components have componentName and have revision-enabled trait
-	//TODO(roywang) Components have componentName and have no revision-enabled trait
+	When("Components have componentName and have revision-enabled trait", func() {
+		It("should create workloads with name of revision and keep the old revision", func() {
+
+			By("Create trait definition")
+			var td v1alpha2.TraitDefinition
+			Expect(readYaml("testdata/revision/trait-def.yaml", &td)).Should(BeNil())
+
+			var gtd v1alpha2.TraitDefinition
+			if err := k8sClient.Get(ctx, client.ObjectKey{Name: td.Name, Namespace: td.Namespace}, &gtd); err != nil {
+				Expect(k8sClient.Create(ctx, &td)).Should(Succeed())
+			} else {
+				td.ResourceVersion = gtd.ResourceVersion
+				Expect(k8sClient.Update(ctx, &td)).Should(Succeed())
+			}
+
+			By("Create Component v1")
+			var comp1 v1alpha2.Component
+			Expect(readYaml("testdata/revision/comp-v1.yaml", &comp1)).Should(BeNil())
+			Expect(k8sClient.Create(ctx, &comp1)).Should(Succeed())
+
+			By("Create AppConfig with component")
+			var appconfig v1alpha2.ApplicationConfiguration
+			Expect(readYaml("testdata/revision/app.yaml", &appconfig)).Should(BeNil())
+			Expect(k8sClient.Create(ctx, &appconfig)).Should(Succeed())
+
+			By("Get Component latest status after ControllerRevision created")
+			Eventually(
+				func() *v1alpha2.Revision {
+					k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &comp1)
+					return comp1.Status.LatestRevision
+				},
+				time.Second*30, time.Millisecond*500).ShouldNot(BeNil())
+
+			revisionNameV1 := comp1.Status.LatestRevision.Name
+
+			By("Workload created with revisionName v1")
+			var w1 unstructured.Unstructured
+			Eventually(
+				func() error {
+					w1.SetAPIVersion("example.com/v1")
+					w1.SetKind("Bar")
+					return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: revisionNameV1}, &w1)
+				},
+				time.Second*15, time.Millisecond*500).Should(BeNil())
+			k1, _, _ := unstructured.NestedString(w1.Object, "spec", "key")
+			Expect(k1).Should(BeEquivalentTo("v1"), fmt.Sprintf("%v", w1.Object))
+
+			By("Create Component v2")
+			var comp2 v1alpha2.Component
+			Expect(readYaml("testdata/revision/comp-v2.yaml", &comp2)).Should(BeNil())
+			comp2.ResourceVersion = comp1.ResourceVersion
+			Expect(k8sClient.Update(ctx, &comp2)).Should(Succeed())
+
+			By("Get Component latest status after ControllerRevision created")
+			Eventually(
+				func() *v1alpha2.Revision {
+					k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &comp2)
+					if comp2.Status.LatestRevision != nil && comp2.Status.LatestRevision.Revision > 1 {
+						return comp2.Status.LatestRevision
+					}
+					return nil
+				},
+				time.Second*30, time.Millisecond*500).ShouldNot(BeNil())
+
+			revisionNameV2 := comp2.Status.LatestRevision.Name
+
+			By("Workload exist with revisionName v2")
+			var w2 unstructured.Unstructured
+			Eventually(
+				func() error {
+					w2.SetAPIVersion("example.com/v1")
+					w2.SetKind("Bar")
+					return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: revisionNameV2}, &w2)
+				},
+				time.Second*15, time.Millisecond*500).Should(BeNil())
+			k2, _, _ := unstructured.NestedString(w2.Object, "spec", "key")
+			Expect(k2).Should(BeEquivalentTo("v2"), fmt.Sprintf("%v", w2.Object))
+
+			By("Check AppConfig status")
+			Eventually(
+				func() error {
+					return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: appconfig.Name}, &appconfig)
+				},
+				time.Second*15, time.Millisecond*500).Should(BeNil())
+
+			Expect(len(appconfig.Status.Workloads)).Should(BeEquivalentTo(2))
+			Expect(appconfig.Status.Workloads[0].ComponentRevisionName).Should(BeEquivalentTo(revisionNameV2))
+			Expect(appconfig.Status.Workloads[0].HistoryWorkingRevision).Should(BeEquivalentTo(false))
+			Expect(appconfig.Status.Workloads[1].ComponentRevisionName).Should(BeEquivalentTo(revisionNameV1))
+			Expect(appconfig.Status.Workloads[1].HistoryWorkingRevision).Should(BeEquivalentTo(true))
+
+			//Clean
+			k8sClient.Delete(ctx, &appconfig)
+			k8sClient.Delete(ctx, &comp1)
+			k8sClient.Delete(ctx, &comp2)
+		})
+	})
+
+	When("Components have componentName and without revision-enabled trait", func() {
+		It("should create workloads with name of component and replace the old revision", func() {
+
+			By("Create trait definition")
+			var td v1alpha2.TraitDefinition
+			Expect(readYaml("testdata/revision/trait-def-no-revision.yaml", &td)).Should(BeNil())
+			var gtd v1alpha2.TraitDefinition
+			if err := k8sClient.Get(ctx, client.ObjectKey{Name: td.Name, Namespace: td.Namespace}, &gtd); err != nil {
+				Expect(k8sClient.Create(ctx, &td)).Should(Succeed())
+			} else {
+				td.ResourceVersion = gtd.ResourceVersion
+				Expect(k8sClient.Update(ctx, &td)).Should(Succeed())
+			}
+
+			By("Create Component v1")
+			var comp1 v1alpha2.Component
+			Expect(readYaml("testdata/revision/comp-v1.yaml", &comp1)).Should(BeNil())
+			Expect(k8sClient.Create(ctx, &comp1)).Should(Succeed())
+
+			By("Create AppConfig with component")
+			var appconfig v1alpha2.ApplicationConfiguration
+			Expect(readYaml("testdata/revision/app.yaml", &appconfig)).Should(BeNil())
+			Expect(k8sClient.Create(ctx, &appconfig)).Should(Succeed())
+
+			By("Workload created with component name")
+			var w1 unstructured.Unstructured
+			Eventually(
+				func() error {
+					w1.SetAPIVersion("example.com/v1")
+					w1.SetKind("Bar")
+					return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &w1)
+				},
+				time.Second*15, time.Millisecond*500).Should(BeNil())
+
+			k1, _, _ := unstructured.NestedString(w1.Object, "spec", "key")
+			Expect(k1).Should(BeEquivalentTo("v1"), fmt.Sprintf("%v", w1.Object))
+
+			By("Create Component v2")
+			var comp2 v1alpha2.Component
+			Expect(readYaml("testdata/revision/comp-v2.yaml", &comp2)).Should(BeNil())
+			k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &comp1)
+			comp2.ResourceVersion = comp1.ResourceVersion
+			Expect(k8sClient.Update(ctx, &comp2)).Should(Succeed())
+
+			By("Workload exist with revisionName v2")
+			var w2 unstructured.Unstructured
+			Eventually(
+				func() string {
+					w2.SetAPIVersion("example.com/v1")
+					w2.SetKind("Bar")
+					err := k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, &w2)
+					if err != nil {
+						return ""
+					}
+					k2, _, _ := unstructured.NestedString(w2.Object, "spec", "key")
+					return k2
+				},
+				time.Second*15, time.Millisecond*500).Should(BeEquivalentTo("v2"))
+
+			By("Check AppConfig status")
+			Eventually(
+				func() error {
+					return k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: appconfig.Name}, &appconfig)
+				},
+				time.Second*15, time.Millisecond*500).Should(BeNil())
+
+			Expect(len(appconfig.Status.Workloads)).Should(BeEquivalentTo(1))
+
+			//Clean
+			k8sClient.Delete(ctx, &appconfig)
+			k8sClient.Delete(ctx, &comp1)
+			k8sClient.Delete(ctx, &comp2)
+		})
+	})
 })
+
+func readYaml(path string, object runtime.Object) error {
+	data, err := ioutil.ReadFile(path)
+	if err != nil {
+		return err
+	}
+	return yaml.Unmarshal(data, object)
+}