diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component.go index 39957df06..c02e0d68d 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component.go @@ -5,6 +5,7 @@ import ( "fmt" "sort" + "github.com/crossplane/crossplane-runtime/pkg/logging" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -17,8 +18,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/crossplane/crossplane-runtime/pkg/logging" - "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2" "github.com/oam-dev/kubevela/pkg/controller/utils" ) @@ -80,7 +79,7 @@ func (c *ComponentHandler) Generic(_ event.GenericEvent, _ workqueue.RateLimitin func isMatch(appConfigs *v1alpha2.ApplicationConfigurationList, compName string) (bool, types.NamespacedName) { for _, app := range appConfigs.Items { for _, comp := range app.Spec.Components { - if comp.ComponentName == compName { + if comp.ComponentName == compName || utils.ExtractComponentName(comp.RevisionName) == compName { return true, types.NamespacedName{Namespace: app.Namespace, Name: app.Name} } } @@ -125,6 +124,10 @@ func (c *ComponentHandler) IsRevisionDiff(mt klog.KMetadata, curComp *v1alpha2.C func (c *ComponentHandler) createControllerRevision(mt metav1.Object, obj runtime.Object) ([]reconcile.Request, bool) { curComp := obj.(*v1alpha2.Component) comp := curComp.DeepCopy() + // No generation changed, will not create revision + if comp.Generation == comp.Status.ObservedGeneration { + return nil, false + } diff, curRevision := c.IsRevisionDiff(mt, comp) if !diff { // No difference, no need to create new revision. diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component_custom_revision_test.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component_custom_revision_test.go index c2c6f5bd6..07f6d78cd 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component_custom_revision_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/component_custom_revision_test.go @@ -18,50 +18,68 @@ package applicationconfiguration import ( "encoding/json" + "fmt" "io/ioutil" "net/http" "net/http/httptest" + "strings" "testing" + "time" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2" ) -func TestCustomRevisionHook(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var req RevisionHookRequest - data, err := ioutil.ReadAll(r.Body) - if err != nil { - w.WriteHeader(400) - return - } - err = json.Unmarshal(data, &req) - if err != nil { - w.WriteHeader(401) - return - } - if len(req.RelatedApps) != 1 { - w.WriteHeader(400) - w.Write([]byte("we should have only one relatedApps")) - return - } - if req.Comp.Annotations == nil { - req.Comp.Annotations = make(map[string]string) +var RevisionHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req RevisionHookRequest + data, err := ioutil.ReadAll(r.Body) + if err != nil { + w.WriteHeader(400) + return + } + err = json.Unmarshal(data, &req) + if err != nil { + w.WriteHeader(401) + return + } + fmt.Println("got request from", req.Comp.Name) + + if len(req.RelatedApps) != 1 { + var abc []string + for _, v := range req.RelatedApps { + abc = append(abc, v.Name) } + // we can add a check here for real world handler + fmt.Printf("we should have only one relatedApps, but now %d: %s\n", len(req.RelatedApps), strings.Join(abc, ", ")) + } + if req.Comp.Annotations == nil { + req.Comp.Annotations = make(map[string]string) + } + if len(req.RelatedApps) > 0 { req.Comp.Annotations["app-name"] = req.RelatedApps[0].Name req.Comp.Annotations["app-namespace"] = req.RelatedApps[0].Namespace + } + a := &unstructured.Unstructured{} + err = json.Unmarshal(req.Comp.Spec.Workload.Raw, a) + fmt.Println("XX:", err) + a.SetAnnotations(map[string]string{"time": time.Now().Format(time.RFC3339Nano)}) + data, _ = json.Marshal(a) + req.Comp.Spec.Workload.Raw = data + newdata, err := json.Marshal(req.Comp) + if err != nil { + w.WriteHeader(500) + return + } + w.WriteHeader(200) + w.Write(newdata) +}) - newdata, err := json.Marshal(req.Comp) - if err != nil { - w.WriteHeader(500) - return - } - w.WriteHeader(200) - w.Write(newdata) - })) +func TestCustomRevisionHook(t *testing.T) { + srv := httptest.NewServer(RevisionHandler) defer srv.Close() compHandler := ComponentHandler{ CustomRevisionHookURL: srv.URL, @@ -71,7 +89,4 @@ func TestCustomRevisionHook(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "app1", comp.Annotations["app-name"]) assert.Equal(t, "default1", comp.Annotations["app-namespace"]) - - err = compHandler.customComponentRevisionHook([]reconcile.Request{{NamespacedName: types.NamespacedName{Name: "app1", Namespace: "default1"}}, {NamespacedName: types.NamespacedName{Name: "app2", Namespace: "default2"}}}, comp) - assert.Equal(t, err.Error(), "httpcode(400) err: we should have only one relatedApps") } diff --git a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go index 894894d2c..d7e10354c 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/applicationconfiguration/revision_enable_test.go @@ -19,18 +19,20 @@ package applicationconfiguration import ( "context" "fmt" + "net/http/httptest" "strconv" "time" - v1 "k8s.io/api/apps/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/crossplane/crossplane-runtime/pkg/logging" + v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -340,3 +342,165 @@ var _ = Describe("Test ApplicationConfiguration Component Revision Enabled trait }) }) + +var _ = Describe("Test Component Revision Enabled with custom component revision hook", func() { + const ( + namespace = "revision-enable-test2" + compName = "revision-test-comp2" + ) + var ( + ctx = context.Background() + component v1alpha2.Component + ns = corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + }, + } + ) + + BeforeEach(func() {}) + + AfterEach(func() { + // delete the namespace with all its resources + Expect(k8sClient.Delete(ctx, &ns, client.PropagationPolicy(metav1.DeletePropagationForeground))). + Should(SatisfyAny(BeNil(), &util.NotFoundMatcher{})) + }) + + It("custom component change revision lead to revision difference, it should not loop infinitely create", func() { + srv := httptest.NewServer(RevisionHandler) + defer srv.Close() + customComponentHandler := &ComponentHandler{Client: k8sClient, RevisionLimit: 100, Logger: logging.NewLogrLogger(ctrl.Log.WithName("component-handler")), CustomRevisionHookURL: srv.URL} + getDeploy := func(image string) *v1.Deployment { + return &v1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, + Spec: v1.DeploymentSpec{ + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{ + "app": compName, + }}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "app": compName, + }}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Name: "wordpress", + Image: image, + Ports: []corev1.ContainerPort{ + { + Name: "wordpress", + ContainerPort: 80, + }, + }, + }, + }}}, + }, + } + } + component = v1alpha2.Component{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "core.oam.dev/v1alpha2", + Kind: "Component", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: compName, + Namespace: namespace, + }, + Spec: v1alpha2.ComponentSpec{ + Workload: runtime.RawExtension{ + Object: getDeploy("wordpress:4.6.1-apache"), + }, + }, + } + + By("Create namespace") + Eventually( + func() error { + return k8sClient.Create(ctx, &ns) + }, + time.Second*3, time.Millisecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) + + By("Create Component") + Expect(k8sClient.Create(ctx, &component)).Should(Succeed()) + + By("component handler will automatically create controller revision") + Expect(func() bool { + _, ok := customComponentHandler.createControllerRevision(component.DeepCopy(), component.DeepCopy()) + return ok + }()).Should(BeTrue()) + + By("it should not create again for the same generation component") + cmpV1 := &v1alpha2.Component{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: compName}, cmpV1)).Should(Succeed()) + Expect(func() bool { + _, ok := customComponentHandler.createControllerRevision(cmpV1, cmpV1) + return ok + }()).Should(BeFalse()) + + var crList v1.ControllerRevisionList + By("Check controller revision created successfully") + Eventually(func() error { + labels := &metav1.LabelSelector{ + MatchLabels: map[string]string{ + ControllerRevisionComponentLabel: compName, + }, + } + selector, err := metav1.LabelSelectorAsSelector(labels) + if err != nil { + return err + } + err = k8sClient.List(ctx, &crList, &client.ListOptions{ + LabelSelector: selector, + }) + if err != nil { + return err + } + if len(crList.Items) != 1 { + return fmt.Errorf("want only 1 revision created but got %d", len(crList.Items)) + } + return nil + }, time.Second, 300*time.Millisecond).Should(BeNil()) + + By("===================================== Start to Update =========================================") + cmpV2 := &v1alpha2.Component{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: compName}, cmpV2)).Should(Succeed()) + cmpV2.Spec.Workload = runtime.RawExtension{ + Object: getDeploy("wordpress:v2"), + } + By("Update Component") + Expect(k8sClient.Update(ctx, cmpV2)).Should(Succeed()) + By("component handler will automatically create a ne controller revision") + Expect(func() bool { _, ok := componentHandler.createControllerRevision(cmpV2, cmpV2); return ok }()).Should(BeTrue()) + + cmpV3 := &v1alpha2.Component{} + Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: compName}, cmpV3)).Should(Succeed()) + Expect(func() bool { _, ok := componentHandler.createControllerRevision(cmpV3, cmpV3); return ok }()).Should(BeFalse()) + + By("Check controller revision created successfully") + Eventually(func() error { + labels := &metav1.LabelSelector{ + MatchLabels: map[string]string{ + ControllerRevisionComponentLabel: compName, + }, + } + selector, err := metav1.LabelSelectorAsSelector(labels) + if err != nil { + return err + } + err = k8sClient.List(ctx, &crList, &client.ListOptions{ + LabelSelector: selector, + }) + if err != nil { + return err + } + if len(crList.Items) != 2 { + return fmt.Errorf("there should be exactly 2 revision created but got %d", len(crList.Items)) + } + return nil + }, time.Second, 300*time.Millisecond).Should(BeNil()) + }) +})