From 142242a02dc02bb25897d2cb2baf3fe4584bd14d Mon Sep 17 00:00:00 2001 From: zhaohuihui Date: Fri, 21 Apr 2023 21:28:53 +0800 Subject: [PATCH] Fix: install dependency is invalid for runtime addon when it's clusters arg is nil Signed-off-by: zhaohuihui --- e2e/addon/addon_test.go | 158 ++++++++++++++++++++++++++-------- pkg/addon/addon.go | 51 ++++++++--- pkg/addon/addon_suite_test.go | 24 ++++++ 3 files changed, 187 insertions(+), 46 deletions(-) diff --git a/e2e/addon/addon_test.go b/e2e/addon/addon_test.go index 3e230d184..9042c2d38 100644 --- a/e2e/addon/addon_test.go +++ b/e2e/addon/addon_test.go @@ -142,11 +142,128 @@ var _ = Describe("Addon Test", func() { Expect(output).To(ContainSubstring("you can try another version by command")) Expect(err).NotTo(HaveOccurred()) }) + }) + + Context("Addon registry test", func() { + It("List all addon registry", func() { + output, err := e2e.Exec("vela addon registry list") + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("KubeVela")) + }) + + It("Get addon registry", func() { + output, err := e2e.Exec("vela addon registry get KubeVela") + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("KubeVela")) + }) + + It("Add test addon registry", func() { + output, err := e2e.LongTimeExec("vela addon registry add my-repo --type=git --endpoint=https://github.com/oam-dev/catalog --path=/experimental/addons", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("Successfully add an addon registry my-repo")) + + Eventually(func() error { + output, err := e2e.LongTimeExec("vela addon registry update my-repo --type=git --endpoint=https://github.com/oam-dev/catalog --path=/addons", 300*time.Second) + if err != nil { + return err + } + if !strings.Contains(output, "Successfully update an addon registry my-repo") { + return fmt.Errorf("cannot update addon registry") + } + return nil + }, 30*time.Second, 300*time.Millisecond).Should(BeNil()) + + output, err = e2e.LongTimeExec("vela addon registry delete my-repo", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("Successfully delete an addon registry my-repo")) + }) + }) + + Context("Enable dependency addon test", func() { + It(" enable mock-dependence-rely without specified clusters when mock-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.")) + Eventually(func(g Gomega) { + 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["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) + }, 30*time.Second).Should(Succeed()) + // disable mock-dependence-rely and mock-dependence addon + /*output1, err := e2e.LongTimeExec("vela addon disable mock-dependence-rely", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output1).To(ContainSubstring("Successfully disable addon")) + output2, err := e2e.LongTimeExec("vela addon disable mock-dependence", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output2).To(ContainSubstring("Successfully disable addon"))*/ + }) + + It("enable mock-dependence-rely without specified clusters when mock-dependence addon was enabled with specified clusters", func() { + // 1. enable mock-dependence addon with local clusters + 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()) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(ContainSubstring("enabled successfully.")) + Eventually(func(g Gomega) { + // 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(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") + 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) { + 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["clusterLabelSelector"]).Should(Equal(map[string]interface{}{})) + Expect(topologyPolicyValue["clusters"]).Should(BeNil()) + }, 30*time.Second).Should(Succeed()) + // 4. disable mock-dependence-rely and mock-dependence addon + /*output2, err := e2e.LongTimeExec("vela addon disable mock-dependence-rely", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output2).To(ContainSubstring("Successfully disable addon")) + output3, err := e2e.LongTimeExec("vela addon disable mock-dependence", 600*time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(output3).To(ContainSubstring("Successfully disable addon"))*/ + }) It("Test addon dependency with specified clusters", func() { const clusterName = "k3s-default" // enable addon - output, err := e2e.Exec("vela addon enable mock-dependence --clusters local myparam=test") + 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()) + }) Expect(err).NotTo(HaveOccurred()) Expect(output).To(ContainSubstring("enabled successfully.")) output1, err := e2e.Exec("vela ls -A") @@ -185,7 +302,10 @@ var _ = Describe("Addon Test", func() { Expect(topologyPolicyValue["clusters"]).Should(Equal([]interface{}{"local"})) }, 600*time.Second).Should(Succeed()) // enable addon which rely on mock-dependence addon - e2e.Exec("vela addon enable mock-dependence-rely --clusters local," + clusterName) + e2e.InteractiveExec("vela addon enable mock-dependence-rely --clusters local,"+clusterName, func(c *expect.Console) { + _, err = c.SendLine("y") + Expect(err).NotTo(HaveOccurred()) + }) // check mock-dependence application parameter Eventually(func(g Gomega) { sec := &v1.Secret{} @@ -210,38 +330,4 @@ var _ = Describe("Addon Test", func() { }) }) - Context("Addon registry test", func() { - It("List all addon registry", func() { - output, err := e2e.Exec("vela addon registry list") - Expect(err).NotTo(HaveOccurred()) - Expect(output).To(ContainSubstring("KubeVela")) - }) - - It("Get addon registry", func() { - output, err := e2e.Exec("vela addon registry get KubeVela") - Expect(err).NotTo(HaveOccurred()) - Expect(output).To(ContainSubstring("KubeVela")) - }) - - It("Add test addon registry", func() { - output, err := e2e.LongTimeExec("vela addon registry add my-repo --type=git --endpoint=https://github.com/oam-dev/catalog --path=/experimental/addons", 600*time.Second) - Expect(err).NotTo(HaveOccurred()) - Expect(output).To(ContainSubstring("Successfully add an addon registry my-repo")) - - Eventually(func() error { - output, err := e2e.LongTimeExec("vela addon registry update my-repo --type=git --endpoint=https://github.com/oam-dev/catalog --path=/addons", 300*time.Second) - if err != nil { - return err - } - if !strings.Contains(output, "Successfully update an addon registry my-repo") { - return fmt.Errorf("cannot update addon registry") - } - return nil - }, 30*time.Second, 300*time.Millisecond).Should(BeNil()) - - output, err = e2e.LongTimeExec("vela addon registry delete my-repo", 600*time.Second) - Expect(err).NotTo(HaveOccurred()) - Expect(output).To(ContainSubstring("Successfully delete an addon registry my-repo")) - }) - }) }) diff --git a/pkg/addon/addon.go b/pkg/addon/addon.go index 81d4edc49..113dba7e4 100644 --- a/pkg/addon/addon.go +++ b/pkg/addon/addon.go @@ -1015,18 +1015,11 @@ func (h *Installer) installDependency(addon *InstallPackage) error { continue } depHandler := *h - // get dependency addon original parameters - depArgs, depArgsErr := GetAddonLegacyParameters(h.ctx, h.cli, dep.Name) + // reset dependency addon clusters parameter + depArgs, depArgsErr := getDependencyArgs(h.ctx, h.cli, dep.Name, depClusters) if depArgsErr != nil { - if !apierrors.IsNotFound(depArgsErr) { - return depArgsErr - } + return depArgsErr } - if depArgs == nil { - depArgs = map[string]interface{}{} - } - // reset the cluster arg - depArgs[types.ClustersArg] = depClusters depHandler.args = depArgs @@ -1092,6 +1085,23 @@ func checkDependencyNeedInstall(ctx context.Context, k8sClient client.Client, de needInstallAddonDep = true depClusters = addonClusters } else { + // if addon clusters is nil, meaning to deploy to all clusters + if addonClusters == nil { + hasTopologyPolicy := false + topologyPolicyValue := map[string]interface{}{} + for _, policy := range depApp.Spec.Policies { + if policy.Type == "topology" { + hasTopologyPolicy = true + unmarshalErr := json.Unmarshal(policy.Properties.Raw, &topologyPolicyValue) + if unmarshalErr != nil { + return true, nil, unmarshalErr + } + break + } + } + need := hasTopologyPolicy && topologyPolicyValue["clusterLabelSelector"] == nil + return need, nil, nil + } // get the runtime clusters of current dependency addon for _, r := range depApp.Status.AppliedResources { if r.Cluster != "" && !stringslices.Contains(depClusters, r.Cluster) { @@ -1110,6 +1120,27 @@ func checkDependencyNeedInstall(ctx context.Context, k8sClient client.Client, de 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 +} + // checkDependency checks if addon's dependency func (h *Installer) checkDependency(addon *InstallPackage) ([]string, error) { var app v1beta1.Application diff --git a/pkg/addon/addon_suite_test.go b/pkg/addon/addon_suite_test.go index e22a1162f..6104199e5 100644 --- a/pkg/addon/addon_suite_test.go +++ b/pkg/addon/addon_suite_test.go @@ -194,6 +194,30 @@ var _ = Describe("Addon test", func() { Expect(needInstallAddonDep).Should(BeTrue()) Expect(depClusters).Should(Equal(addonClusters)) }, 30*time.Second).Should(Succeed()) + + // case3: addonClusters is nil + needInstallAddonDep2, depClusters2, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, nil) + Expect(needInstallAddonDep2).Should(BeFalse()) + Expect(depClusters2).Should(BeNil()) + Expect(err).Should(BeNil()) + }) + + 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()) + 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()) }) It(" determineAddonAppName func test", func() {