From 189555ba1634529d3320cc406de4edd07ccf6178 Mon Sep 17 00:00:00 2001 From: zhaohuiweixiao Date: Thu, 15 Jun 2023 11:20:50 +0800 Subject: [PATCH] Refactor: addon dependency installation logic (#6045) Signed-off-by: zhaohuihui --- e2e/addon/addon_test.go | 252 +++++++++++++++++- .../mock-dependence-locally/README.md | 3 + .../mock-dependence-locally/metadata.yaml | 7 + .../mock-dependence-locally/parameter.cue | 12 + .../mock-dependence-locally/template.cue | 24 ++ .../mock-dependence-no-clusters-arg/README.md | 3 + .../metadata.yaml | 7 + .../parameter.cue | 10 + .../template.cue | 10 + .../testdata/mock-dependence-rely3/README.md | 3 + .../mock-dependence-rely3/metadata.yaml | 10 + .../mock-dependence-rely3/parameter.cue | 4 + .../mock-dependence-rely3/template.cue | 9 + .../README.md | 3 + .../metadata.yaml | 10 + .../parameter.cue | 12 + .../template.cue | 24 ++ .../README.md | 3 + .../metadata.yaml | 10 + .../parameter.cue | 12 + .../template.cue | 24 ++ .../testdata/mock-dependence/parameter.cue | 2 +- .../mock/testdata/mock-dependence3/README.md | 3 + .../testdata/mock-dependence3/metadata.yaml | 7 + .../testdata/mock-dependence3/parameter.cue | 6 + .../testdata/mock-dependence3/template.cue | 24 ++ .../helm-repo/has-clusters-arg-v1.0.0.tgz | Bin 0 -> 775 bytes e2e/addon/mock/testrepo/helm-repo/index.yaml | 9 + pkg/addon/addon.go | 178 ++++++++++--- pkg/addon/addon_suite_test.go | 168 +++++++----- pkg/addon/addon_test.go | 15 ++ pkg/addon/helper.go | 1 - pkg/addon/helper_test.go | 1 + 33 files changed, 745 insertions(+), 121 deletions(-) create mode 100644 e2e/addon/mock/testdata/mock-dependence-locally/README.md create mode 100644 e2e/addon/mock/testdata/mock-dependence-locally/metadata.yaml create mode 100644 e2e/addon/mock/testdata/mock-dependence-locally/parameter.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-locally/template.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/README.md create mode 100644 e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/metadata.yaml create mode 100644 e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/parameter.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/template.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-rely3/README.md create mode 100644 e2e/addon/mock/testdata/mock-dependence-rely3/metadata.yaml create mode 100644 e2e/addon/mock/testdata/mock-dependence-rely3/parameter.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-rely3/template.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-locally/README.md create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-locally/metadata.yaml create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-locally/parameter.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-locally/template.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/README.md create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/metadata.yaml create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/parameter.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/template.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence3/README.md create mode 100644 e2e/addon/mock/testdata/mock-dependence3/metadata.yaml create mode 100644 e2e/addon/mock/testdata/mock-dependence3/parameter.cue create mode 100644 e2e/addon/mock/testdata/mock-dependence3/template.cue create mode 100644 e2e/addon/mock/testrepo/helm-repo/has-clusters-arg-v1.0.0.tgz diff --git a/e2e/addon/addon_test.go b/e2e/addon/addon_test.go index 2adc98746..af61efedc 100644 --- a/e2e/addon/addon_test.go +++ b/e2e/addon/addon_test.go @@ -33,6 +33,7 @@ import ( "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/e2e" "github.com/oam-dev/kubevela/pkg/addon" + addonutil "github.com/oam-dev/kubevela/pkg/utils/addon" "github.com/oam-dev/kubevela/pkg/utils/common" ) @@ -180,7 +181,7 @@ var _ = Describe("Addon Test", func() { }) Context("Enable dependency addon test", func() { - It(" enable mock-dependence-rely without specified clusters when mock-dependence addon is not enabled", func() { + It("enable upstream addon without specified clusters when dependence addon is not enabled", func() { output, err := e2e.Exec("vela addon enable mock-dependence-rely") Expect(err).NotTo(HaveOccurred()) Expect(output).To(ContainSubstring("enabled successfully.")) @@ -198,7 +199,7 @@ var _ = Describe("Addon Test", func() { }, 30*time.Second).Should(Succeed()) }) - It("enable mock-dependence-rely with specified clusters when mock-dependence addon is not enabled ", func() { + It("enable upstream addon with specified clusters when dependence addon is not enabled ", func() { output, err := e2e.Exec("vela addon enable mock-dependence-rely2 --clusters local") Expect(err).NotTo(HaveOccurred()) Expect(output).To(ContainSubstring("enabled successfully.")) @@ -216,8 +217,133 @@ var _ = Describe("Addon Test", func() { }, 30*time.Second).Should(Succeed()) }) - It("enable mock-dependence-rely without specified clusters when mock-dependence addon was enabled with specified clusters", func() { + It("enable upstream addon without specified clusters when dependence addon is enabled with specified clusters", func() { // 1. enable mock-dependence addon with local clusters + dependentName := "mock-dependence3" + addonName := "mock-dependence-rely3" + output, err := e2e.Exec("vela addon enable " + dependentName + " --clusters local myparam=test") + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("enabled successfully.")) + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: addonutil.Addon2SecName(dependentName), Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "clusters": []interface{}{"local"}, + "myparam": "test", + })) + // check application render cluster + app := &v1beta1.Application{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: addonutil.Addon2AppName(dependentName), Namespace: "vela-system"}, app)).Should(Succeed()) + topologyPolicyValue := map[string]interface{}{} + for _, policy := range app.Spec.Policies { + if policy.Type == "topology" { + Expect(json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue)).Should(Succeed()) + break + } + } + Expect(topologyPolicyValue["clusters"]).Should(Equal([]interface{}{"local"})) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(BeNil()) + }, 600*time.Second).Should(Succeed()) + // 2. enable mock-dependence-rely addon without clusters + output1, err1 := e2e.Exec("vela addon enable " + addonName) + Expect(err1).NotTo(HaveOccurred()) + Expect(output1).To(ContainSubstring("enabled successfully.")) + // 3. enable mock-dependence-rely addon changes the mock-dependence topology policy + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: addonutil.Addon2SecName(dependentName), Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "myparam": "test", + })) + app := &v1beta1.Application{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: addonutil.Addon2AppName(dependentName), Namespace: "vela-system"}, app)).Should(Succeed()) + topologyPolicyValue := map[string]interface{}{} + for _, policy := range app.Spec.Policies { + if policy.Type == "topology" { + Expect(json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue)).Should(Succeed()) + break + } + } + Expect(topologyPolicyValue["clusters"]).Should(BeNil()) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) + }, 30*time.Second).Should(Succeed()) + }) + + It("enable upstream addon with specified clusters when dependence addon is enabled with clusters value is nil", func() { + // enable fluxcd + // enable rollout --clusters={local} + // 1. enable mock-dependence addon with nil clusters parameter + output, err := e2e.InteractiveExec("vela addon enable mock-dependence myparam=test", func(c *expect.Console) { + _, err = c.SendLine("y") + Expect(err).NotTo(HaveOccurred()) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("enabled successfully.")) + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "myparam": "test", + })) + // check application render cluster + app := &v1beta1.Application{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence", Namespace: "vela-system"}, app)).Should(Succeed()) + topologyPolicyValue := map[string]interface{}{} + for _, policy := range app.Spec.Policies { + if policy.Type == "topology" { + Expect(json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue)).Should(Succeed()) + break + } + } + Expect(topologyPolicyValue["clusters"]).Should(BeNil()) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) + }, 600*time.Second).Should(Succeed()) + + // 2. enable mock-dependence-rely addon with local clusters + output1, err := e2e.InteractiveExec("vela addon enable mock-dependence-rely --clusters local", func(c *expect.Console) { + _, err = c.SendLine("y") + Expect(err).NotTo(HaveOccurred()) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(output1).To(ContainSubstring("enabled successfully.")) + // 3. enable mock-dependence-rely addon changes the mock-dependence topology policy + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "myparam": "test", + })) + // check application render cluster + app := &v1beta1.Application{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence", Namespace: "vela-system"}, app)).Should(Succeed()) + topologyPolicyValue := map[string]interface{}{} + for _, policy := range app.Spec.Policies { + if policy.Type == "topology" { + Expect(json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue)).Should(Succeed()) + break + } + } + Expect(topologyPolicyValue["clusters"]).Should(BeNil()) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) + }, 30*time.Second).Should(Succeed()) + }) + + It("enable upstream addon without clusters when dependence addon is enabled with clusters value is not nil", func() { + // enable fluxcd --clusters={local} + // enable rollout + // 1. enable mock-dependence addon with local clusters and myparam parameter output, err := e2e.InteractiveExec("vela addon enable mock-dependence --clusters local myparam=test", func(c *expect.Console) { _, err = c.SendLine("y") Expect(err).NotTo(HaveOccurred()) @@ -225,6 +351,15 @@ var _ = Describe("Addon Test", func() { Expect(err).NotTo(HaveOccurred()) Expect(output).To(ContainSubstring("enabled successfully.")) Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "clusters": []interface{}{"local"}, + "myparam": "test", + })) // check application render cluster app := &v1beta1.Application{} Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence", Namespace: "vela-system"}, app)).Should(Succeed()) @@ -238,6 +373,7 @@ var _ = Describe("Addon Test", func() { Expect(topologyPolicyValue["clusters"]).Should(Equal([]interface{}{"local"})) Expect(topologyPolicyValue["clusterLabelSelector"]).Should(BeNil()) }, 600*time.Second).Should(Succeed()) + // 2. enable mock-dependence-rely addon without clusters output1, err := e2e.InteractiveExec("vela addon enable mock-dependence-rely", func(c *expect.Console) { _, err = c.SendLine("y") @@ -247,6 +383,15 @@ var _ = Describe("Addon Test", func() { Expect(output1).To(ContainSubstring("enabled successfully.")) // 3. enable mock-dependence-rely addon changes the mock-dependence topology policy Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "myparam": "test", + })) + // check application render cluster app := &v1beta1.Application{} Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence", Namespace: "vela-system"}, app)).Should(Succeed()) topologyPolicyValue := map[string]interface{}{} @@ -256,12 +401,12 @@ var _ = Describe("Addon Test", func() { break } } - Expect(topologyPolicyValue["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) Expect(topologyPolicyValue["clusters"]).Should(BeNil()) - }, 30*time.Second).Should(Succeed()) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) + }, 60*time.Second).Should(Succeed()) }) - It("Test addon dependency with specified clusters", func() { + It("enable upstream addon with two clusters when dependence addon is enabled with one cluster", func() { const clusterName = "k3s-default" // enable addon output, err := e2e.InteractiveExec("vela addon enable mock-dependence --clusters local myparam=test", func(c *expect.Console) { @@ -330,6 +475,101 @@ var _ = Describe("Addon Test", func() { } } Expect(topologyPolicyValue["clusters"]).Should(Equal([]interface{}{"local", clusterName})) + }, 60*time.Second).Should(Succeed()) + }) + + It("enable upstream addon without clusters when dependence addon which is enabled locally", func() { + // enable ./fluxcd + // enable rollout + // enable addon locally + output, err := e2e.LongTimeExec("vela addon enable ../../e2e/addon/mock/testdata/mock-dependence-locally --clusters local myparam=test", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("enabled successfully.")) + // check dependence application parameter + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence-locally", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "clusters": []interface{}{"local"}, + "myparam": "test", + })) + // check application render cluster + app := &v1beta1.Application{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence-locally", Namespace: "vela-system"}, app)).Should(Succeed()) + topologyPolicyValue := map[string]interface{}{} + for _, policy := range app.Spec.Policies { + if policy.Type == "topology" { + Expect(json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue)).Should(Succeed()) + break + } + } + Expect(topologyPolicyValue["clusters"]).Should(Equal([]interface{}{"local"})) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(BeNil()) + }, 600*time.Second).Should(Succeed()) + // enable addon which rely on mock-dependence-locally + output1, err1 := e2e.Exec("vela addon enable mock-dependence-upstream-locally") + Expect(err1).NotTo(HaveOccurred()) + Expect(output1).To(ContainSubstring("enabled successfully.")) + // check mock-dependence-locally application parameter + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence-locally", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "clusters": []interface{}{"local"}, + "myparam": "test", + })) + // check application render cluster + app := &v1beta1.Application{} + Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence-locally", Namespace: "vela-system"}, app)).Should(Succeed()) + topologyPolicyValue := map[string]interface{}{} + for _, policy := range app.Spec.Policies { + if policy.Type == "topology" { + Expect(json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue)).Should(Succeed()) + break + } + } + Expect(topologyPolicyValue["clusters"]).Should(Equal([]interface{}{"local"})) + Expect(topologyPolicyValue["clusterLabelSelector"]).Should(BeNil()) + }, 30*time.Second).Should(Succeed()) + }) + + It("enable upstream addon with specified clusters when dependence addon which without clusters arg is enabled", func() { + // enable vela-prism + // enable o11 + output, err := e2e.LongTimeExec("vela addon enable mock-dependence-no-clusters-arg myparam=test", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("enabled successfully.")) + // check dependence application parameter + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence-no-clusters-arg", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "myparam": "test", + })) + }, 600*time.Second).Should(Succeed()) + // enable addon which rely on mock-dependence-no-clusters-arg + output1, err1 := e2e.Exec("vela addon enable mock-dependence-upstream-no-clusters-arg --clusters local") + Expect(err1).NotTo(HaveOccurred()) + Expect(output1).To(ContainSubstring("enabled successfully.")) + // check mock-dependence-locally application parameter + Eventually(func(g Gomega) { + // check parameter + sec := &v1.Secret{} + g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-secret-mock-dependence-no-clusters-arg", Namespace: "vela-system"}, sec)).Should(Succeed()) + parameters := map[string]interface{}{} + json.Unmarshal(sec.Data[addon.AddonParameterDataKey], ¶meters) + g.Expect(parameters).Should(BeEquivalentTo(map[string]interface{}{ + "myparam": "test", + })) }, 30*time.Second).Should(Succeed()) }) }) diff --git a/e2e/addon/mock/testdata/mock-dependence-locally/README.md b/e2e/addon/mock/testdata/mock-dependence-locally/README.md new file mode 100644 index 000000000..56dfa31f0 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-locally/README.md @@ -0,0 +1,3 @@ +# mock-dependence + +This is an addon template. Check how to build your own addon: https://kubevela.net/docs/platform-engineers/addon/intro diff --git a/e2e/addon/mock/testdata/mock-dependence-locally/metadata.yaml b/e2e/addon/mock/testdata/mock-dependence-locally/metadata.yaml new file mode 100644 index 000000000..cb60934b3 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-locally/metadata.yaml @@ -0,0 +1,7 @@ +description: An addon for testing addon dependency with installed locally. +icon: "" +invisible: false +name: mock-dependence-locally +tags: +- my-tag +version: 1.0.0 diff --git a/e2e/addon/mock/testdata/mock-dependence-locally/parameter.cue b/e2e/addon/mock/testdata/mock-dependence-locally/parameter.cue new file mode 100644 index 000000000..b05024892 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-locally/parameter.cue @@ -0,0 +1,12 @@ +// parameter.cue is used to store addon parameters. +// +// You can use these parameters in template.cue or in resources/ by 'parameter.myparam' +// +// For example, you can use parameters to allow the user to customize +// container images, ports, and etc. +parameter: { + // +usage=Custom parameter description + myparam: *"myns" | string + // +usage=Deploy to specified clusters. Leave empty to deploy to all clusters. + clusters?: [...string] +} diff --git a/e2e/addon/mock/testdata/mock-dependence-locally/template.cue b/e2e/addon/mock/testdata/mock-dependence-locally/template.cue new file mode 100644 index 000000000..709d70a9d --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-locally/template.cue @@ -0,0 +1,24 @@ +package main +_targetNamespace: parameter.myparam +output: { + apiVersion: "core.oam.dev/v1beta1" + kind: "Application" + spec: { + components: [], + policies: [ + { + type: "topology" + name: "deploy-mock-dependency-ns" + properties: { + namespace: _targetNamespace + if parameter.clusters != _|_ { + clusters: parameter.clusters + } + if parameter.clusters == _|_ { + clusterLabelSelector: {} + } + } + }, + ] + } +} diff --git a/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/README.md b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/README.md new file mode 100644 index 000000000..56dfa31f0 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/README.md @@ -0,0 +1,3 @@ +# mock-dependence + +This is an addon template. Check how to build your own addon: https://kubevela.net/docs/platform-engineers/addon/intro diff --git a/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/metadata.yaml b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/metadata.yaml new file mode 100644 index 000000000..f89978f57 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/metadata.yaml @@ -0,0 +1,7 @@ +description: An addon for testing addon dependency witch is not defined clusters arg. +icon: "" +invisible: false +name: mock-dependence-no-clusters-arg +tags: +- my-tag +version: 1.0.0 diff --git a/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/parameter.cue b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/parameter.cue new file mode 100644 index 000000000..01184b38d --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/parameter.cue @@ -0,0 +1,10 @@ +// parameter.cue is used to store addon parameters. +// +// You can use these parameters in template.cue or in resources/ by 'parameter.myparam' +// +// For example, you can use parameters to allow the user to customize +// container images, ports, and etc. +parameter: { + // +usage=Custom parameter description + myparam: *"myns" | string +} diff --git a/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/template.cue b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/template.cue new file mode 100644 index 000000000..14edeb335 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-no-clusters-arg/template.cue @@ -0,0 +1,10 @@ +package main +_targetNamespace: parameter.myparam +output: { + apiVersion: "core.oam.dev/v1beta1" + kind: "Application" + spec: { + components: [], + policies: [] + } +} diff --git a/e2e/addon/mock/testdata/mock-dependence-rely3/README.md b/e2e/addon/mock/testdata/mock-dependence-rely3/README.md new file mode 100644 index 000000000..9f001c0bb --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-rely3/README.md @@ -0,0 +1,3 @@ +# mock-dependence-rely2 + +This is an addon template. Check how to build your own addon: https://kubevela.net/docs/platform-engineers/addon/intro diff --git a/e2e/addon/mock/testdata/mock-dependence-rely3/metadata.yaml b/e2e/addon/mock/testdata/mock-dependence-rely3/metadata.yaml new file mode 100644 index 000000000..9929d14ee --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-rely3/metadata.yaml @@ -0,0 +1,10 @@ +description: An addon for testing addon dependency with specified clusters. +icon: "" +invisible: false +name: mock-dependence-rely3 +tags: +- my-tag +version: 1.0.0 +dependencies: + # install controller by helm. + - name: mock-dependence3 diff --git a/e2e/addon/mock/testdata/mock-dependence-rely3/parameter.cue b/e2e/addon/mock/testdata/mock-dependence-rely3/parameter.cue new file mode 100644 index 000000000..ce1f3aba1 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-rely3/parameter.cue @@ -0,0 +1,4 @@ +parameter: { + // +usage=Custom parameter description + myparam: *"mynsrely" | string +} diff --git a/e2e/addon/mock/testdata/mock-dependence-rely3/template.cue b/e2e/addon/mock/testdata/mock-dependence-rely3/template.cue new file mode 100644 index 000000000..2ac3c733f --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-rely3/template.cue @@ -0,0 +1,9 @@ +package main +output: { + apiVersion: "core.oam.dev/v1beta1" + kind: "Application" + spec: { + components: [] + policies: [] + } +} diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-locally/README.md b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/README.md new file mode 100644 index 000000000..53ffa22ba --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/README.md @@ -0,0 +1,3 @@ +# mock-dependence-rely + +This is an addon template. Check how to build your own addon: https://kubevela.net/docs/platform-engineers/addon/intro diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-locally/metadata.yaml b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/metadata.yaml new file mode 100644 index 000000000..86017537f --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/metadata.yaml @@ -0,0 +1,10 @@ +description: An addon for testing addon dependency with installed locally. +icon: "" +invisible: false +name: mock-dependence-upstream-locally +tags: +- my-tag +version: 1.0.0 +dependencies: + # install controller by helm. + - name: mock-dependence-locally diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-locally/parameter.cue b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/parameter.cue new file mode 100644 index 000000000..68c2c4d1e --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/parameter.cue @@ -0,0 +1,12 @@ +// parameter.cue is used to store addon parameters. +// +// You can use these parameters in template.cue or in resources/ by 'parameter.myparam' +// +// For example, you can use parameters to allow the user to customize +// container images, ports, and etc. +parameter: { + // +usage=Custom parameter description + myparam: *"mynsrely" | string + // +usage=Deploy to specified clusters. Leave empty to deploy to all clusters. + clusters?: [...string] +} diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-locally/template.cue b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/template.cue new file mode 100644 index 000000000..f5172f232 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-locally/template.cue @@ -0,0 +1,24 @@ +package main +_targetNamespace: parameter.myparam +output: { + apiVersion: "core.oam.dev/v1beta1" + kind: "Application" + spec: { + components: [] + policies: [ + { + type: "topology" + name: "deploy-mock-dependency-ns" + properties: { + namespace: _targetNamespace + if parameter.clusters != _|_ { + clusters: parameter.clusters + } + if parameter.clusters == _|_ { + clusterLabelSelector: {} + } + } + }, + ] + } +} diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/README.md b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/README.md new file mode 100644 index 000000000..53ffa22ba --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/README.md @@ -0,0 +1,3 @@ +# mock-dependence-rely + +This is an addon template. Check how to build your own addon: https://kubevela.net/docs/platform-engineers/addon/intro diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/metadata.yaml b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/metadata.yaml new file mode 100644 index 000000000..ce33c337d --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/metadata.yaml @@ -0,0 +1,10 @@ +description: An addon for testing addon dependency witch is not defined clusters arg. +icon: "" +invisible: false +name: mock-dependence-upstream-no-clusters-arg +tags: +- my-tag +version: 1.0.0 +dependencies: + # install controller by helm. + - name: mock-dependence-no-clusters-arg diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/parameter.cue b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/parameter.cue new file mode 100644 index 000000000..68c2c4d1e --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/parameter.cue @@ -0,0 +1,12 @@ +// parameter.cue is used to store addon parameters. +// +// You can use these parameters in template.cue or in resources/ by 'parameter.myparam' +// +// For example, you can use parameters to allow the user to customize +// container images, ports, and etc. +parameter: { + // +usage=Custom parameter description + myparam: *"mynsrely" | string + // +usage=Deploy to specified clusters. Leave empty to deploy to all clusters. + clusters?: [...string] +} diff --git a/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/template.cue b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/template.cue new file mode 100644 index 000000000..f5172f232 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence-upstream-no-clusters-arg/template.cue @@ -0,0 +1,24 @@ +package main +_targetNamespace: parameter.myparam +output: { + apiVersion: "core.oam.dev/v1beta1" + kind: "Application" + spec: { + components: [] + policies: [ + { + type: "topology" + name: "deploy-mock-dependency-ns" + properties: { + namespace: _targetNamespace + if parameter.clusters != _|_ { + clusters: parameter.clusters + } + if parameter.clusters == _|_ { + clusterLabelSelector: {} + } + } + }, + ] + } +} diff --git a/e2e/addon/mock/testdata/mock-dependence/parameter.cue b/e2e/addon/mock/testdata/mock-dependence/parameter.cue index a9899b757..b05024892 100644 --- a/e2e/addon/mock/testdata/mock-dependence/parameter.cue +++ b/e2e/addon/mock/testdata/mock-dependence/parameter.cue @@ -7,6 +7,6 @@ parameter: { // +usage=Custom parameter description myparam: *"myns" | string - //+usage=Deploy to specified clusters. Leave empty to deploy to all clusters. + // +usage=Deploy to specified clusters. Leave empty to deploy to all clusters. clusters?: [...string] } diff --git a/e2e/addon/mock/testdata/mock-dependence3/README.md b/e2e/addon/mock/testdata/mock-dependence3/README.md new file mode 100644 index 000000000..619074308 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence3/README.md @@ -0,0 +1,3 @@ +# mock-dependence2 + +This is an addon template. Check how to build your own addon: https://kubevela.net/docs/platform-engineers/addon/intro diff --git a/e2e/addon/mock/testdata/mock-dependence3/metadata.yaml b/e2e/addon/mock/testdata/mock-dependence3/metadata.yaml new file mode 100644 index 000000000..d665f166a --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence3/metadata.yaml @@ -0,0 +1,7 @@ +description: An addon for testing addon dependency with specified clusters. +icon: "" +invisible: false +name: mock-dependence3 +tags: +- my-tag +version: 1.0.0 diff --git a/e2e/addon/mock/testdata/mock-dependence3/parameter.cue b/e2e/addon/mock/testdata/mock-dependence3/parameter.cue new file mode 100644 index 000000000..09680c914 --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence3/parameter.cue @@ -0,0 +1,6 @@ +parameter: { + //+usage=Custom parameter description + myparam: *"myns" | string + //+usage=Deploy to specified clusters. Leave empty to deploy to all clusters. + clusters?: [...string] +} diff --git a/e2e/addon/mock/testdata/mock-dependence3/template.cue b/e2e/addon/mock/testdata/mock-dependence3/template.cue new file mode 100644 index 000000000..709d70a9d --- /dev/null +++ b/e2e/addon/mock/testdata/mock-dependence3/template.cue @@ -0,0 +1,24 @@ +package main +_targetNamespace: parameter.myparam +output: { + apiVersion: "core.oam.dev/v1beta1" + kind: "Application" + spec: { + components: [], + policies: [ + { + type: "topology" + name: "deploy-mock-dependency-ns" + properties: { + namespace: _targetNamespace + if parameter.clusters != _|_ { + clusters: parameter.clusters + } + if parameter.clusters == _|_ { + clusterLabelSelector: {} + } + } + }, + ] + } +} diff --git a/e2e/addon/mock/testrepo/helm-repo/has-clusters-arg-v1.0.0.tgz b/e2e/addon/mock/testrepo/helm-repo/has-clusters-arg-v1.0.0.tgz new file mode 100644 index 0000000000000000000000000000000000000000..f7892fd489223b27b4be5d93f47d5c3d94b5ab35 GIT binary patch literal 775 zcmV+i1Ni(OiwFP!000001MQdJZ__Xk$NSny`5%t(1~fmKHkBexXae4V2SB`_X>yZW zYc;X0*zFWS|937w+Kp~(5-J60{-{!8`*KcvIp5byrbk(^)&?s*V$~uT9G{SwO{aq} z38UFGYX8G9YWsTz(PR=QVKSb@WEYJm<7fcWzwj(~pjT%Vz@R`OH}VuS{#Psy<6oj- zIWx?!Sy>!sjwZm#Wb)hikK>#8C&`TBKPCGCgvXiwVV-#WbJSVImElsPa4vx5xfF0E zD=?@H7mL0pM};Csk=1a`%@TIsjPE);Ywq+)2%lmCiU6fxWioq!mXGFy#q zPAwzSD4m(Aq{7OyUVDG-t^ErhEuWl{HE@9x=8C?Fj+`HLJl=L3s-e&ut!?XJx zzGrhRK4XEIk&RN@{_3pz+qO$af~aX*QDm`LEEbE!VzF2(7K_DVu~;lmo1gMNBO?GP F006q@jCcS5 literal 0 HcmV?d00001 diff --git a/e2e/addon/mock/testrepo/helm-repo/index.yaml b/e2e/addon/mock/testrepo/helm-repo/index.yaml index 275e3e843..78946b3a0 100644 --- a/e2e/addon/mock/testrepo/helm-repo/index.yaml +++ b/e2e/addon/mock/testrepo/helm-repo/index.yaml @@ -69,4 +69,13 @@ entries: urls: - http://127.0.0.1:9098/helm/mock-be-dep-addon-v1.0.0.tgz version: v1.0.0 + has-clusters-arg: + - created: "2023-06-01T09:11:16.865230605Z" + description: An addon for testing addon dependency with clusters. + home: https://www.has-clusters-arg.com/icon + icon: https://www.has-clusters-arg.com + name: has-clusters-arg + urls: + - http://127.0.0.1:9098/helm/has-clusters-arg-v1.0.0.tgz + version: v1.0.0 generated: "2022-06-15T13:17:04.733573+08:00" \ No newline at end of file diff --git a/pkg/addon/addon.go b/pkg/addon/addon.go index 68898a314..71468b4ae 100644 --- a/pkg/addon/addon.go +++ b/pkg/addon/addon.go @@ -1040,7 +1040,7 @@ func (h *Installer) installDependency(addon *InstallPackage) error { var dependencies []string var addonClusters = getClusters(h.args) for _, dep := range addon.Dependencies { - needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(h.ctx, h.cli, dep.Name, addonClusters) + needInstallAddonDep, err := checkDependencyNeedInstall(h.ctx, h.cli, dep.Name, addonClusters) if err != nil { return err } @@ -1054,7 +1054,7 @@ func (h *Installer) installDependency(addon *InstallPackage) error { } depHandler := *h // reset dependency addon clusters parameter - depArgs, depArgsErr := getDependencyArgs(h.ctx, h.cli, dep.Name, depClusters) + depArgs, depArgsErr := getDependencyArgs(h.ctx, h.cli, dep.Name, addonClusters) if depArgsErr != nil { return depArgsErr } @@ -1291,39 +1291,150 @@ func listInstalledAddons(ctx context.Context, k8sClient client.Client) (itemInfo return installedAddons, nil } -// checkDependencyNeedInstall checks whether dependency addon needs to be installed on other clusters -func checkDependencyNeedInstall(ctx context.Context, k8sClient client.Client, depName string, addonClusters []string) (bool, []string, error) { +// checkDependencyNeedInstall checks whether dependency addon needs to be reinstalled +// If the dep addon is not installed, need to install +// If the dep addon is installed locally, don't need to install +// If the dep addon is installed from registry and not defined clusters, don't need to install +// If the dep addon is installed from registry and is defined clusters, and clusters value is nil, don't need to install +// If the dep addon is installed from registry and is defined clusters, and clusters value is not nil, +// and the upstream addon's clusters is nil, need to install +// If the dep addon is installed from registry and is defined clusters, and clusters value is not nil, +// and the upstream addon's clusters is not nil, the re-installation is based on whether the dep clusters value can contain the upstream clusters value +func checkDependencyNeedInstall(ctx context.Context, k8sClient client.Client, depName string, addonClusters []string) (bool, error) { depApp, err := FetchAddonRelatedApp(ctx, k8sClient, depName) if err != nil { if !apierrors.IsNotFound(err) { - return false, nil, err + return false, err } - // dependent addon is not exist - return true, addonClusters, nil + // dependent addon is not exist, need to install it + return true, nil } - topologyPolicyValue := map[string]interface{}{} - for _, policy := range depApp.Spec.Policies { - if policy.Type == "topology" { - unmarshalErr := json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue) - if unmarshalErr != nil { - return false, nil, unmarshalErr - } - break - } + + // We should not automatically override addons installed locally by the user, so skip to reinstall it + labels := depApp.GetLabels() + installedRegistry := labels[oam.LabelAddonRegistry] + isLocalRegistry := installedRegistry == LocalAddonRegistryName + if isLocalRegistry { + klog.Warningf("%v is installed locally. Please ensure that it has been installed to the required clusters, if not, please manually install it.", depName) + return false, nil } + + // Addons without the clusters parameter can only be installed on the local cluster. So we don't need to reinstall it + hasClustersArg, hasClustersArgsErr := hasClustersParameters(ctx, k8sClient, depName) + if hasClustersArgsErr != nil { + return false, hasClustersArgsErr + } + if !hasClustersArg { + return false, nil + } + + // get addon current parameter value + depArgs, depArgsErr := GetAddonLegacyParameters(ctx, k8sClient, depName) + if depArgsErr != nil && !apierrors.IsNotFound(depArgsErr) { + return false, depArgsErr + } + clusterArgValue := depArgs[types.ClustersArg] + // nil clusters indicates that the dependent addon is installed on all clusters - if topologyPolicyValue["clusters"] == nil { - return false, nil, nil + if clusterArgValue == nil { + return false, nil } - // nil addonClusters indicates the addon will be installed, - // thus we should set the dependent addon's clusters arg to be nil so that it is installed on all clusters + + // nil addonClusters indicates the addon will be installed all clusters, thus the dependent addon should also to be installed on all clusters. if addonClusters == nil { - return true, nil, nil + return true, nil } + // Determine whether the dependent addon's existing clusters can cover the new addon's clusters + needInstallAddonDep, _ := hasNotCoveredClusters(clusterArgValue, addonClusters) + return needInstallAddonDep, nil +} + +// getDependencyArgs get the dependent addon's install args according to the upstream addon's clusters parameter's value +// If dep addon has not defined clusters parameter, don't need to set clusters parameter value, +// If dep addon has not installed, set the clusters value same to addon's clusters +// If dep addon clusters parameter's value is nil, the dependent addon is installed on all clusters, +// don't need to reset clusters parameter value +// If dep addon has defined clusters parameter, and clusters is not nil, and addon clusters is nil, +// set clusters value as nil +// If dep addon has defined clusters parameter, and clusters is not nil, and addon clusters is not nil, +// set clusters value as the union of the dependent addon's and the upstream addon's clusters +func getDependencyArgs(ctx context.Context, k8sClient client.Client, depName string, addonClusters []string) (map[string]interface{}, error) { + hasClustersArg, err := hasClustersParameters(ctx, k8sClient, depName) + if err != nil { + return nil, err + } + // dep addon is not install, installed it by assigning clusters value + _, depErr := FetchAddonRelatedApp(ctx, k8sClient, depName) + if depErr != nil { + if !apierrors.IsNotFound(depErr) { + return nil, err + } + depArgs := map[string]interface{}{} + if addonClusters != nil { + depArgs = map[string]interface{}{ + types.ClustersArg: addonClusters, + } + } + return depArgs, nil + } + + // dep addon is installed + depArgs, depArgsErr := GetAddonLegacyParameters(ctx, k8sClient, depName) + if depArgsErr != nil && !apierrors.IsNotFound(depArgsErr) { + return nil, depArgsErr + } + if !hasClustersArg || depArgs[types.ClustersArg] == nil { + return depArgs, nil + } + if addonClusters == nil { + delete(depArgs, types.ClustersArg) + } else { + clusterArgValue := depArgs[types.ClustersArg] + notCovered, depClusters := hasNotCoveredClusters(clusterArgValue, addonClusters) + if notCovered { + depArgs[types.ClustersArg] = depClusters + } + } + return depArgs, nil +} + +// hasClustersParameters checks whether the addon defines the clusters parameter. +// If the addon has been installed, get addon package from the installed registry, +// Otherwise, get addon package from one of registries where this addon exists. +func hasClustersParameters(ctx context.Context, k8sClient client.Client, addonName string) (bool, error) { + var installedRegistry []string + depApp, err := FetchAddonRelatedApp(ctx, k8sClient, addonName) + if err == nil { + labels := depApp.GetLabels() + registryName, ok := labels[oam.LabelAddonRegistry] + if ok { + installedRegistry = []string{registryName} + } + } + addonPackages, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{addonName}, installedRegistry) + // If the state of addon is not disabled, we don't check the error, because it could be installed from local. + if err != nil { + return false, err + } + var addonPackage *WholeAddonPackage + if len(addonPackages) != 0 { + addonPackage = addonPackages[0] + } + if addonPackage.APISchema == nil { + return false, nil + } + schemas := addonPackage.APISchema.Properties + _, hasClusters := schemas[types.ClustersArg] + return hasClusters, nil +} + +// hasNotCoveredClusters check if the clusterArgsValue can cover the values of addonClusters, +// and if not covered, also return the merged clusters array +func hasNotCoveredClusters(clusterArgValue interface{}, addonClusters []string) (bool, []string) { var needInstallAddonDep = false var depClusters []string - originClusters := topologyPolicyValue["clusters"].([]interface{}) + originClusters := clusterArgValue.([]interface{}) for _, r := range originClusters { depClusters = append(depClusters, r.(string)) } @@ -1333,28 +1444,7 @@ func checkDependencyNeedInstall(ctx context.Context, k8sClient client.Client, de needInstallAddonDep = true } } - return needInstallAddonDep, depClusters, nil -} - -// getDependencyArgs resets the dependency clusters arg according needed install depClusters -func getDependencyArgs(ctx context.Context, k8sClient client.Client, depName string, depClusters []string) (map[string]interface{}, error) { - depArgs, depArgsErr := GetAddonLegacyParameters(ctx, k8sClient, depName) - if depArgsErr != nil && !apierrors.IsNotFound(depArgsErr) { - return nil, depArgsErr - } - // reset the cluster arg - if depClusters == nil { - // delete clusters args, when render addon, it will use clusterLabelSelector then render addon to all clusters - if depArgs != nil && depArgs[types.ClustersArg] != nil { - delete(depArgs, types.ClustersArg) - } - } else { - if depArgs == nil { - depArgs = map[string]interface{}{} - } - depArgs[types.ClustersArg] = depClusters - } - return depArgs, nil + return needInstallAddonDep, depClusters } // checkDependency checks if addon's dependency diff --git a/pkg/addon/addon_suite_test.go b/pkg/addon/addon_suite_test.go index 3fc2b7f98..907999bf1 100644 --- a/pkg/addon/addon_suite_test.go +++ b/pkg/addon/addon_suite_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "io" + "net/http/httptest" "time" . "github.com/onsi/ginkgo/v2" @@ -51,9 +52,27 @@ import ( var _ = Describe("Addon test", func() { ctx := context.Background() var app v1beta1.Application + var s *httptest.Server + + BeforeEach(func() { + s = setupMockServer() + // Prepare registry + reg := &Registry{ + Name: "addon_helper_test", + Helm: &HelmSource{ + URL: s.URL, + }, + } + ds := NewRegistryDataStore(k8sClient) + Expect(ds.AddRegistry(context.Background(), *reg)).To(Succeed()) + }) AfterEach(func() { Expect(k8sClient.Delete(ctx, &app)).Should(BeNil()) + // Clean up registry + ds := NewRegistryDataStore(k8sClient) + Expect(ds.DeleteRegistry(context.Background(), "addon_helper_test")).To(Succeed()) + s.Close() }) It("continueOrRestartWorkflow func test", func() { @@ -175,104 +194,68 @@ var _ = Describe("Addon test", func() { }) It("checkDependencyNeedInstall func test", func() { - // case1: dependency addon not exist, adonClusters is not nil + // case1: dependency addon not exist depAddonName := "legacy-addon" addonClusters := []string{"cluster1", "cluster2"} - needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, addonClusters) + needInstallAddonDep, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, addonClusters) Expect(needInstallAddonDep).Should(BeTrue()) - Expect(depClusters).Should(Equal(addonClusters)) Expect(err).Should(BeNil()) - // case1.1: dependency addon not exist, adonClusters is nil - needInstallAddonDep1, depClusters1, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, nil) - Expect(needInstallAddonDep1).Should(BeTrue()) - Expect(depClusters1).Should(BeNil()) - Expect(err).Should(BeNil()) - - // case2: dependency addon exist, no topology policy, addonClusters is not nil + // case2: dependency addon is installed locally app = v1beta1.Application{} - Expect(yaml.Unmarshal([]byte(legacyAppYaml), &app)).Should(BeNil()) + Expect(yaml.Unmarshal([]byte(legacyAppYamlLocal), &app)).Should(BeNil()) app.SetNamespace(testns) Expect(k8sClient.Create(ctx, &app)).Should(BeNil()) Eventually(func(g Gomega) { - needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, addonClusters) + needInstallAddonDep, err := checkDependencyNeedInstall(ctx, k8sClient, "legacy-addon-local", addonClusters) Expect(err).Should(BeNil()) Expect(needInstallAddonDep).Should(BeFalse()) - Expect(depClusters).Should(BeNil()) }, 30*time.Second).Should(Succeed()) - // case3: clusters is nil (no topology policy), addonClusters is nil - needInstallAddonDep2, depClusters2, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, nil) - Expect(needInstallAddonDep2).Should(BeFalse()) - Expect(depClusters2).Should(BeNil()) - Expect(err).Should(BeNil()) - - // case4: clusters is nil, addonClusters is nil + // case3: dependency addon has no clusters arg + noClusterArgAddonName := "vela-workflow" app = v1beta1.Application{} - Expect(yaml.Unmarshal([]byte(legacy3AppYaml), &app)).Should(BeNil()) + Expect(yaml.Unmarshal([]byte(legacyAppYamlNoClustersArg), &app)).Should(BeNil()) app.SetNamespace(testns) Expect(k8sClient.Create(ctx, &app)).Should(BeNil()) Eventually(func(g Gomega) { - needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, "legacy-addon3", nil) + needInstallAddonDep, err := checkDependencyNeedInstall(ctx, k8sClient, noClusterArgAddonName, addonClusters) Expect(err).Should(BeNil()) Expect(needInstallAddonDep).Should(BeFalse()) - Expect(depClusters).Should(BeNil()) - }, 60*time.Second).Should(Succeed()) + }, 30*time.Second).Should(Succeed()) - // case5: clusters is not nil, addonClusters is nil, + // case3: dependency addon has clusters arg, but clusters value is nil + hasClusterArgAddonName := "has-clusters-arg" app = v1beta1.Application{} - Expect(yaml.Unmarshal([]byte(legacy2AppYaml), &app)).Should(BeNil()) + Expect(yaml.Unmarshal([]byte(legacyAppYamlHasClustersArg), &app)).Should(BeNil()) app.SetNamespace(testns) Expect(k8sClient.Create(ctx, &app)).Should(BeNil()) Eventually(func(g Gomega) { - needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, "legacy-addon2", nil) + needInstallAddonDep, err := checkDependencyNeedInstall(ctx, k8sClient, hasClusterArgAddonName, addonClusters) Expect(err).Should(BeNil()) - Expect(needInstallAddonDep).Should(BeTrue()) - Expect(depClusters).Should(BeNil()) - }, 60*time.Second).Should(Succeed()) + Expect(needInstallAddonDep).Should(BeFalse()) + }, 30*time.Second).Should(Succeed()) - // case6: clusters is [local], addonClusters is ["cluster1", "cluster2"] - Eventually(func(g Gomega) { - needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, "legacy-addon2", addonClusters) - Expect(err).Should(BeNil()) - Expect(needInstallAddonDep).Should(BeTrue()) - Expect(depClusters).Should(Equal(append([]string{"local"}, addonClusters...))) - }, 60*time.Second).Should(Succeed()) - }) - - It("getDependencyArgs func test", func() { - // case1: depClusters is nil - depAddonName := "legacy-addon" - depArgs, err := getDependencyArgs(ctx, k8sClient, depAddonName, nil) - Expect(depArgs).Should(BeNil()) - Expect(err).Should(BeNil()) - - // case2: depClusters is not nil - app = v1beta1.Application{} - Expect(yaml.Unmarshal([]byte(legacyAppYaml), &app)).Should(BeNil()) + // case4: dependency addon has clusters arg, clusters value is local, addonClusters is nil + secret := v1.Secret{} + Expect(yaml.Unmarshal([]byte(legacySecretYamlHasClustersArg), &secret)).Should(BeNil()) app.SetNamespace(testns) - //Expect(k8sClient.Create(ctx, &app)).Should(BeNil()) - depClusters := []string{"cluster1", "cluster2"} - depArgs2, err := getDependencyArgs(ctx, k8sClient, depAddonName, depClusters) - Expect(depArgs2["clusters"]).Should(Equal(depClusters)) - Expect(err).Should(BeNil()) + Expect(k8sClient.Create(ctx, &secret)).Should(BeNil()) + Eventually(func(g Gomega) { + needInstallAddonDep, err := checkDependencyNeedInstall(ctx, k8sClient, hasClusterArgAddonName, nil) + Expect(err).Should(BeNil()) + Expect(needInstallAddonDep).Should(BeTrue()) + }, 60*time.Second).Should(Succeed()) - // clusters exist, depClusters is nil - sec := v1.Secret{} - Expect(yaml.Unmarshal([]byte(secretYaml), &sec)).Should(BeNil()) - Expect(k8sClient.Create(ctx, &sec)).Should(BeNil()) - depArgs3, err := getDependencyArgs(ctx, k8sClient, "fluxcd", nil) - Expect(depArgs3).ToNot(BeNil()) - Expect(depArgs3["clusters"]).Should(BeNil()) - Expect(err).Should(BeNil()) + // case5: dependency addon has clusters arg, clusters value is ["local"], addonClusters is ["local"] + needInstallAddonDep1, err1 := checkDependencyNeedInstall(ctx, k8sClient, hasClusterArgAddonName, []string{"local"}) + Expect(err1).Should(BeNil()) + Expect(needInstallAddonDep1).Should(BeFalse()) - // getArgs throw exception - sec1 := v1.Secret{} - Expect(yaml.Unmarshal([]byte(secretErrorYaml), &sec1)).Should(BeNil()) - Expect(k8sClient.Create(ctx, &sec1)).Should(BeNil()) - depArgs4, err := getDependencyArgs(ctx, k8sClient, "fluxcd1", nil) - Expect(depArgs4).Should(BeNil()) - Expect(err).ToNot(BeNil()) + // case6: dependency addon has clusters arg, clusters value is local, addonClusters is ["cluster1", "cluster2"] + needInstallAddonDep2, err2 := checkDependencyNeedInstall(ctx, k8sClient, hasClusterArgAddonName, addonClusters) + Expect(err2).Should(BeNil()) + Expect(needInstallAddonDep2).Should(BeTrue()) }) It(" determineAddonAppName func test", func() { @@ -678,6 +661,53 @@ spec: properties: image: crccheck/hello-world port: 8000 +` + legacyAppYamlLocal = `apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: legacy-addon-local + labels: + addons.oam.dev/registry: local +spec: + components: + - name: express-server + type: webservice + properties: + image: crccheck/hello-world + port: 8000 +` + legacyAppYamlNoClustersArg = `apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: addon-vela-workflow +spec: + components: + - name: express-server + type: webservice + properties: + image: crccheck/hello-world + port: 8000 +` + legacyAppYamlHasClustersArg = `apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: addon-has-clusters-arg +spec: + components: + - name: express-server + type: webservice + properties: + image: crccheck/hello-world + port: 8000 +` + legacySecretYamlHasClustersArg = `apiVersion: v1 +data: + addonParameterDataKey: eyJjbHVzdGVycyI6WyJsb2NhbCJdfQ== +kind: Secret +metadata: + name: addon-secret-has-clusters-arg + namespace: vela-system +type: Opaque ` legacy2AppYaml = `apiVersion: core.oam.dev/v1beta1 kind: Application diff --git a/pkg/addon/addon_test.go b/pkg/addon/addon_test.go index 7ac8f8fce..532e6bb08 100644 --- a/pkg/addon/addon_test.go +++ b/pkg/addon/addon_test.go @@ -426,6 +426,21 @@ func TestGetAddonVersionMeetSystemRequirement(t *testing.T) { assert.Equal(t, version, "") } +func TestHasNotCoveredClusters(t *testing.T) { + // case1: clusterArgValue can cover addonClusters + cav := []interface{}{"local"} + addonClusters := []string{"local"} + notCovered, mergedClusters := hasNotCoveredClusters(cav, addonClusters) + assert.False(t, notCovered) + assert.Equal(t, []string{"local"}, mergedClusters) + + // case2: clusterArgValue can not cover addonClusters + addonClusters = []string{"local", "c1"} + notCovered1, mergedClusters1 := hasNotCoveredClusters(cav, addonClusters) + assert.True(t, notCovered1) + assert.Equal(t, addonClusters, mergedClusters1) +} + var baseAddon = InstallPackage{ Meta: Meta{ Name: "test-render-cue-definition-addon", diff --git a/pkg/addon/helper.go b/pkg/addon/helper.go index b65da5327..27cc02ea3 100644 --- a/pkg/addon/helper.go +++ b/pkg/addon/helper.go @@ -193,7 +193,6 @@ func FindAddonPackagesDetailFromRegistry(ctx context.Context, k8sClient client.C if len(addonNames) == 0 { return nil, fmt.Errorf("no addon name specified") } - registryDataStore := NewRegistryDataStore(k8sClient) // Find matched registries diff --git a/pkg/addon/helper_test.go b/pkg/addon/helper_test.go index 7db2dc43d..15c6dbcf9 100644 --- a/pkg/addon/helper_test.go +++ b/pkg/addon/helper_test.go @@ -44,6 +44,7 @@ func setupMockServer() *httptest.Server { "bar-v1.0.0.tgz", "bar-v2.0.0.tgz", "mock-be-dep-addon-v1.0.0.tgz", + "has-clusters-arg-v1.0.0.tgz", } for _, f := range fileList { if strings.Contains(req.URL.Path, f) {