[Backport release-1.8] Fix: install dependency is invalid for runtime addon when it's cluster arg is nil (#5935)

* Fix: install dependency is invalid for runtime addon when it's clusters arg is nil

Signed-off-by: zhaohuihui <zhaohuihui_yewu@cmss.chinamobile.com>
(cherry picked from commit 142242a02d)

* Fix: add unit test for getDependencyArgs and checkDependencyNeedInstall

Signed-off-by: zhaohuihui <zhaohuihui_yewu@cmss.chinamobile.com>
(cherry picked from commit c38f6e43b7)

* Fix: Simplified the checkDependencyNeedInstall func logic

Signed-off-by: zhaohuihui <zhaohuihui_yewu@cmss.chinamobile.com>
(cherry picked from commit f038023a98)

* Fix: add comments for checkDependencyNeedInstall

Signed-off-by: zhaohuihui <zhaohuihui_yewu@cmss.chinamobile.com>
(cherry picked from commit 1009de2fc5)

---------

Co-authored-by: zhaohuihui <zhaohuihui_yewu@cmss.chinamobile.com>
This commit is contained in:
github-actions[bot]
2023-04-28 10:46:32 +08:00
committed by GitHub
co-authored by zhaohuihui
parent c3b736f753
commit e528902bea
11 changed files with 385 additions and 68 deletions
+126 -36
View File
@@ -142,11 +142,132 @@ 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())
})
It("enable mock-dependence-rely with specified clusters when mock-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."))
Eventually(func(g Gomega) {
app := &v1beta1.Application{}
Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "addon-mock-dependence2", 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"}))
}, 30*time.Second).Should(Succeed())
})
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())
})
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 +306,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 +334,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"))
})
})
})
@@ -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
@@ -0,0 +1,10 @@
description: An addon for testing addon dependency with specified clusters.
icon: ""
invisible: false
name: mock-dependence-rely2
tags:
- my-tag
version: 1.0.0
dependencies:
# install controller by helm.
- name: mock-dependence2
@@ -0,0 +1,4 @@
parameter: {
// +usage=Custom parameter description
myparam: *"mynsrely" | string
}
@@ -0,0 +1,9 @@
package main
output: {
apiVersion: "core.oam.dev/v1beta1"
kind: "Application"
spec: {
components: []
policies: []
}
}
+3
View File
@@ -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
@@ -0,0 +1,7 @@
description: An addon for testing addon dependency with specified clusters.
icon: ""
invisible: false
name: mock-dependence2
tags:
- my-tag
version: 1.0.0
@@ -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]
}
+24
View File
@@ -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: {}
}
}
},
]
}
}
+56 -28
View File
@@ -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
@@ -1082,34 +1075,69 @@ func (h *Installer) installDependency(addon *InstallPackage) error {
// 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) {
depApp, err := FetchAddonRelatedApp(ctx, k8sClient, depName)
var needInstallAddonDep = false
var depClusters []string
if err != nil {
if !apierrors.IsNotFound(err) {
return needInstallAddonDep, depClusters, err
return false, nil, err
}
// depApp is not exist
needInstallAddonDep = true
depClusters = addonClusters
} else {
// get the runtime clusters of current dependency addon
for _, r := range depApp.Status.AppliedResources {
if r.Cluster != "" && !stringslices.Contains(depClusters, r.Cluster) {
depClusters = append(depClusters, r.Cluster)
// dependent addon is not exist
return true, addonClusters, 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
}
// determine if there are no dependencies on the cluster to be installed
for _, addonCluster := range addonClusters {
if !stringslices.Contains(depClusters, addonCluster) {
depClusters = append(depClusters, addonCluster)
needInstallAddonDep = true
}
}
// nil clusters indicates that the dependent addon is installed on all clusters
if topologyPolicyValue["clusters"] == nil {
return false, nil, 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
if addonClusters == nil {
return true, nil, nil
}
// Determine whether the dependent addon's existing clusters can cover the new addon's clusters
var needInstallAddonDep = false
var depClusters []string
originClusters := topologyPolicyValue["clusters"].([]interface{})
for _, r := range originClusters {
depClusters = append(depClusters, r.(string))
}
for _, addonCluster := range addonClusters {
if !stringslices.Contains(depClusters, addonCluster) {
depClusters = append(depClusters, addonCluster)
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
}
// checkDependency checks if addon's dependency
func (h *Installer) checkDependency(addon *InstallPackage) ([]string, error) {
var app v1beta1.Application
+137 -4
View File
@@ -175,7 +175,7 @@ var _ = Describe("Addon test", func() {
})
It("checkDependencyNeedInstall func test", func() {
// case1: dependency addon not enable
// case1: dependency addon not exist, adonClusters is not nil
depAddonName := "legacy-addon"
addonClusters := []string{"cluster1", "cluster2"}
needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, addonClusters)
@@ -183,7 +183,13 @@ var _ = Describe("Addon test", func() {
Expect(depClusters).Should(Equal(addonClusters))
Expect(err).Should(BeNil())
// case2: dependency addon enable
// 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
app = v1beta1.Application{}
Expect(yaml.Unmarshal([]byte(legacyAppYaml), &app)).Should(BeNil())
app.SetNamespace(testns)
@@ -191,9 +197,82 @@ var _ = Describe("Addon test", func() {
Eventually(func(g Gomega) {
needInstallAddonDep, depClusters, err := checkDependencyNeedInstall(ctx, k8sClient, depAddonName, addonClusters)
Expect(err).Should(BeNil())
Expect(needInstallAddonDep).Should(BeTrue())
Expect(depClusters).Should(Equal(addonClusters))
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
app = v1beta1.Application{}
Expect(yaml.Unmarshal([]byte(legacy3AppYaml), &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)
Expect(err).Should(BeNil())
Expect(needInstallAddonDep).Should(BeFalse())
Expect(depClusters).Should(BeNil())
}, 60*time.Second).Should(Succeed())
// case5: clusters is not nil, addonClusters is nil,
app = v1beta1.Application{}
Expect(yaml.Unmarshal([]byte(legacy2AppYaml), &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)
Expect(err).Should(BeNil())
Expect(needInstallAddonDep).Should(BeTrue())
Expect(depClusters).Should(BeNil())
}, 60*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())
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())
// 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())
// 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())
})
It(" determineAddonAppName func test", func() {
@@ -599,6 +678,60 @@ spec:
properties:
image: crccheck/hello-world
port: 8000
`
legacy2AppYaml = `apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
name: legacy-addon2
spec:
components:
- name: express-server
type: webservice
properties:
image: crccheck/hello-world
port: 8000
policies:
- name: target-default
type: topology
properties:
clusters: ["local"]
namespace: "default"
`
legacy3AppYaml = `apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
name: legacy-addon3
spec:
components:
- name: express-server
type: webservice
properties:
image: crccheck/hello-world
port: 8000
policies:
- name: target-default
type: topology
properties:
clusterLabelSelector: {}
namespace: "default"
`
secretYaml = `apiVersion: v1
data:
addonParameterDataKey: eyJjbHVzdGVycyI6WyJsb2NhbCIsInZlbGEtbTEiXX0K
kind: Secret
metadata:
name: addon-secret-fluxcd
namespace: vela-system
type: Opaque
`
secretErrorYaml = `apiVersion: v1
data:
addonParameterDataKey: eyJjbHVzdGVycyI6WyJsb2NhbCIsInZlbGEtbTEiXQo=
kind: Secret
metadata:
name: addon-secret-fluxcd1
namespace: vela-system
type: Opaque
`
deployYaml = `apiVersion: apps/v1
kind: Deployment