fix comments

Signed-off-by: 楚岳 <wangyike.wyk@alibaba-inc.com>

pkg/addon/addon.go

@@ -1071,12 +1071,12 @@ func (h *Installer) dispatchAddonResource(addon *InstallPackage) error {
 	}
 
 	if !h.overrideDefs {
-		existDefs, err := checkConflictDefs(h.ctx, h.cli, defs, *app)
+		existDefs, err := checkConflictDefs(h.ctx, h.cli, defs, app.Name)
 		if err != nil {
 			return err
 		}
 		if len(existDefs) != 0 {
-			return fmt.Errorf(`definitions: %s in this addon already exist, if you want override them, Please use add "--override-defs" to re-enable or re-upgrade`, existDefs)
+			return produceDefConflictError(existDefs)
 		}
 	}
 

pkg/addon/addon_suite_test.go

@@ -418,17 +418,17 @@ var _ = Describe("test override defs of addon", func() {
 		u := unstructured.Unstructured{Object: compUnstructured}
 		u.SetAPIVersion(v1beta1.SchemeGroupVersion.String())
 		u.SetKind(v1beta1.ComponentDefinitionKind)
-		c, err := checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{&u}, app)
+		c, err := checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{&u}, app.GetName())
 		Expect(err).Should(BeNil())
 		Expect(len(c)).Should(BeEquivalentTo(1))
 
 		u.SetName("rollout")
-		c, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{&u}, app)
+		c, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{&u}, app.GetName())
 		Expect(err).Should(BeNil())
 		Expect(len(c)).Should(BeEquivalentTo(0))
 
 		u.SetKind("NotExistKind")
-		_, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{&u}, app)
+		_, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{&u}, app.GetName())
 		Expect(err).ShouldNot(BeNil())
 
 		compUnstructured2, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&comp2)
@@ -436,7 +436,7 @@ var _ = Describe("test override defs of addon", func() {
 		u2 := &unstructured.Unstructured{Object: compUnstructured2}
 		u2.SetAPIVersion(v1beta1.SchemeGroupVersion.String())
 		u2.SetKind(v1beta1.ComponentDefinitionKind)
-		c, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{u2}, app)
+		c, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{u2}, app.GetName())
 		Expect(err).Should(BeNil())
 		Expect(len(c)).Should(BeEquivalentTo(1))
 
@@ -445,7 +445,7 @@ var _ = Describe("test override defs of addon", func() {
 		u3 := &unstructured.Unstructured{Object: compUnstructured3}
 		u3.SetAPIVersion(v1beta1.SchemeGroupVersion.String())
 		u3.SetKind(v1beta1.ComponentDefinitionKind)
-		c, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{u3}, app)
+		c, err = checkConflictDefs(ctx, k8sClient, []*unstructured.Unstructured{u3}, app.GetName())
 		Expect(err).Should(BeNil())
 		Expect(len(c)).Should(BeEquivalentTo(0))
 	})

pkg/addon/addon_test.go

@@ -950,6 +950,11 @@ func TestCheckSemVer(t *testing.T) {
 			require: ">=1.5.0",
 			res:     false,
 		},
+		{
+			actual:  "1.5.0-alpha.2",
+			require: ">=1.5.0",
+			res:     false,
+		},
 	}
 	for _, testCase := range testCases {
 		result, err := checkSemVer(testCase.actual, testCase.require)
@@ -1311,3 +1316,15 @@ func TestMergeAddonInstallArgs(t *testing.T) {
 	}
 
 }
+
+func TestGenerateConflictError(t *testing.T) {
+	confictAddon := map[string]string{
+		"helm":      "definition: helm already exist and not belong to any addon \n",
+		"kustomize": "definition: %s in this addon already exist in fluxcd \n",
+	}
+	err := produceDefConflictError(confictAddon)
+	assert.Error(t, err)
+	strings.Contains(err.Error(), "in this addon already exist in fluxcd")
+
+	assert.NoError(t, produceDefConflictError(map[string]string{}))
+}

pkg/addon/utils.go

@@ -38,6 +38,7 @@ import (
 	"github.com/oam-dev/kubevela/pkg/definition"
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/pkg/utils/addon"
 	"github.com/oam-dev/kubevela/pkg/utils/common"
 )
 
@@ -440,15 +441,19 @@ func isErrorCueRenderPathNotFound(err error, path string) bool {
 	return err.Error() == fmt.Sprintf("var(path=%s) not exist", path)
 }
 
-func checkConflictDefs(ctx context.Context, k8sClient client.Client, defs []*unstructured.Unstructured, app v1beta1.Application) ([]string, error) {
-	var res []string
+func checkConflictDefs(ctx context.Context, k8sClient client.Client, defs []*unstructured.Unstructured, appName string) (map[string]string, error) {
+	res := map[string]string{}
 	for _, def := range defs {
 		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(def), def)
 		if err == nil {
 			owner := metav1.GetControllerOf(def)
-			if owner == nil || owner.Kind != v1beta1.ApplicationKind || owner.Name != app.Name {
+			if owner == nil || owner.Kind != v1beta1.ApplicationKind {
+				res[def.GetName()] = fmt.Sprintf("definition: %s already exist and not belong to any addon \n", def.GetName())
+				continue
+			}
+			if owner.Name != appName {
 				// if addon not belong to an addon or addon name is another one, we should put them in result
-				res = append(res, def.GetName())
+				res[def.GetName()] = fmt.Sprintf("definition: %s in this addon already exist in %s \n", def.GetName(), addon.AppName2Addon(appName))
 			}
 		}
 		if err != nil && !errors2.IsNotFound(err) {
@@ -457,3 +462,15 @@ func checkConflictDefs(ctx context.Context, k8sClient client.Client, defs []*uns
 	}
 	return res, nil
 }
+
+func produceDefConflictError(conflictDefs map[string]string) error {
+	if len(conflictDefs) == 0 {
+		return nil
+	}
+	var errorInfo string
+	for _, s := range conflictDefs {
+		errorInfo += s
+	}
+	errorInfo += "if you want override them, Please use arguments '--override' to enable \n"
+	return errors.New(errorInfo)
+}

references/cli/addon.go

@@ -205,7 +205,7 @@ func NewAddonEnableCommand(c common.Args, ioStream cmdutil.IOStreams) *cobra.Com
 	cmd.Flags().StringVarP(&addonVersion, "version", "v", "", "specify the addon version to enable")
 	cmd.Flags().StringVarP(&addonClusters, types.ClustersArg, "c", "", "specify the runtime-clusters to enable")
 	cmd.Flags().BoolVarP(&skipValidate, "skip-version-validating", "s", false, "skip validating system version requirement")
-	cmd.Flags().BoolVarP(&overrideDefs, "override-defs", "o", false, "override already exist definitions within addon")
+	cmd.Flags().BoolVarP(&overrideDefs, "override-definitions", "", false, "override existing definitions if conflict with those contained in this addon")
 	return cmd
 }
 
@@ -326,7 +326,7 @@ non-empty new arg
 	}
 	cmd.Flags().StringVarP(&addonVersion, "version", "v", "", "specify the addon version to upgrade")
 	cmd.Flags().BoolVarP(&skipValidate, "skip-version-validating", "s", false, "skip validating system version requirement")
-	cmd.Flags().BoolVarP(&overrideDefs, "override-defs", "o", false, "override already exist definitions within addon")
+	cmd.Flags().BoolVarP(&overrideDefs, "override-definitions", "", false, "override existing definitions if conflict with those contained in this addon")
 	return cmd
 }