Fix: resolve locally installed addons not being displayed (#3827)

* Fix: resolve locally installed addons not being displayed Addressed an issue where locally installed addons may not be displayed if one with the same name is in the registry Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Style: revert incorrect auto-formatting Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Refactor: change original variable name to avoid confusions Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Test: add tests for outputs from `vela addon list` when an addon with the same as registry one is locally installed Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Refactor: use more concise method to check length Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Test: add one more test condition for dual addons i.e. local and registry Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Refactor: simplify testing logic by removing unneeded looping Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com> * Style: add missing license header Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com>
2026-08-19 04:26:39 +00:00 · 2022-05-10 13:35:12 +08:00
parent b260348f30
commit a598272491
4 changed files with 163 additions and 29 deletions
@@ -30,7 +30,6 @@ import (

 	"helm.sh/helm/v3/pkg/strvals"

-	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2"
 	"github.com/oam-dev/kubevela/pkg/oam"

 	"k8s.io/client-go/rest"
@@ -109,10 +108,11 @@ func NewAddonListCommand(c common.Args) *cobra.Command {
 			if err != nil {
 				return err
 			}
-			err = listAddons(context.Background(), k8sClient, "")
+			table, err := listAddons(context.Background(), k8sClient, "")
 			if err != nil {
 				return err
 			}
+			fmt.Println(table.String())
 			return nil
 		},
 	}
@@ -129,7 +129,7 @@ func NewAddonEnableCommand(c common.Args, ioStream cmdutil.IOStreams) *cobra.Com
 Enable addon by:
 	vela addon enable <addon-name>
 Enable addon with specify version:
-    vela addon enable <addon-name> --version <addon-version>
+	vela addon enable <addon-name> --version <addon-version>
 Enable addon for specific clusters, (local means control plane):
 	vela addon enable <addon-name> --clusters={local,cluster1,cluster2}
 `,
@@ -220,7 +220,7 @@ func NewAddonUpgradeCommand(c common.Args, ioStream cmdutil.IOStreams) *cobra.Co
 Upgrade addon by:
 	vela addon upgrade <addon-name>
 Upgrade addon with specify version:
-    vela addon upgrade <addon-name> --version <addon-version>
+	vela addon upgrade <addon-name> --version <addon-version>
 Upgrade addon for specific clusters, (local means control plane):
 	vela addon upgrade <addon-name> --clusters={local,cluster1,cluster2}
 `,
@@ -443,15 +443,15 @@ func generateAddonInfo(name string, status pkgaddon.Status) string {
 	return res
 }

-func listAddons(ctx context.Context, clt client.Client, registry string) error {
+func listAddons(ctx context.Context, clt client.Client, registry string) (*uitable.Table, error) {
 	var addons []*pkgaddon.UIData
 	var err error
 	registryDS := pkgaddon.NewRegistryDataStore(clt)
 	registries, err := registryDS.ListRegistries(ctx)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	onlineAddon := map[string]bool{}
+
 	for _, r := range registries {
 		if registry != "" && r.Name != registry {
 			continue
@@ -480,31 +480,38 @@ func listAddons(ctx context.Context, clt client.Client, registry string) error {
 	table := uitable.New()
 	table.AddRow("NAME", "REGISTRY", "DESCRIPTION", "AVAILABLE-VERSIONS", "STATUS")

+	// get locally installed addons first
+	locallyInstalledAddons := map[string]bool{}
+	appList := v1beta1.ApplicationList{}
+	if err := clt.List(ctx, &appList, client.MatchingLabels{oam.LabelAddonRegistry: pkgaddon.LocalAddonRegistryName}); err != nil {
+		return table, err
+	}
+	for _, app := range appList.Items {
+		labels := app.GetLabels()
+		addonName := labels[oam.LabelAddonName]
+		addonVersion := labels[oam.LabelAddonVersion]
+		table.AddRow(addonName, app.GetLabels()[oam.LabelAddonRegistry], "", genAvailableVersionInfo([]string{addonVersion}, addonVersion), statusEnabled)
+		locallyInstalledAddons[addonName] = true
+	}
+
 	for _, addon := range addons {
+		// if the addon with same name has already installed locally, display the registry one as not installed
+		if locallyInstalledAddons[addon.Name] {
+			table.AddRow(addon.Name, addon.RegistryName, addon.Description, addon.AvailableVersions, "disabled")
+			continue
+		}
 		status, err := pkgaddon.GetAddonStatus(ctx, clt, addon.Name)
 		if err != nil {
-			return err
+			return table, err
 		}
 		statusRow := status.AddonPhase
 		if len(status.InstalledVersion) != 0 {
 			statusRow += fmt.Sprintf(" (%s)", status.InstalledVersion)
 		}
-		table.AddRow(addon.Name, addon.RegistryName, addon.Description, genAvailableVersionInfo(addon.AvailableVersions, status), statusRow)
-		onlineAddon[addon.Name] = true
+		table.AddRow(addon.Name, addon.RegistryName, addon.Description, genAvailableVersionInfo(addon.AvailableVersions, status.InstalledVersion), statusRow)
 	}
-	appList := v1alpha2.ApplicationList{}
-	if err := clt.List(ctx, &appList, client.MatchingLabels{oam.LabelAddonRegistry: pkgaddon.LocalAddonRegistryName}); err != nil {
-		return err
-	}
-	for _, app := range appList.Items {
-		addonName := app.GetLabels()[oam.LabelAddonName]
-		if onlineAddon[addonName] {
-			continue
-		}
-		table.AddRow(addonName, app.GetLabels()[oam.LabelAddonRegistry], "", statusEnabled)
-	}
-	fmt.Println(table.String())
-	return nil
+
+	return table, nil
 }

 func waitApplicationRunning(k8sClient client.Client, addonName string) error {
@@ -540,13 +547,13 @@ func waitApplicationRunning(k8sClient client.Client, addonName string) error {
 // generate the available version
 // this func put the installed version as the first version and keep the origin order
 // print ... if available version too much
-func genAvailableVersionInfo(versions []string, status pkgaddon.Status) string {
+func genAvailableVersionInfo(versions []string, installedVersion string) string {
 	var v []string

 	// put installed-version as the first version and keep the origin order
-	if len(status.InstalledVersion) != 0 {
+	if len(installedVersion) != 0 {
 		for i, version := range versions {
-			if version == status.InstalledVersion {
+			if version == installedVersion {
 				v = append(v, version)
 				versions = append(versions[:i], versions[i+1:]...)
 			}
@@ -562,7 +569,7 @@ func genAvailableVersionInfo(versions []string, status pkgaddon.Status) string {
 			res += "..."
 			break
 		}
-		if version == status.InstalledVersion {
+		if version == installedVersion {
 			col := color.New(color.Bold, color.FgGreen)
 			res += col.Sprintf("%s", version)
 		} else {
@@ -0,0 +1,125 @@
+/*
+Copyright 2021 The KubeVela Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cli
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"sigs.k8s.io/yaml"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
+	"github.com/oam-dev/kubevela/pkg/oam/util"
+
+	"github.com/fatih/color"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	pkgaddon "github.com/oam-dev/kubevela/pkg/addon"
+
+	"github.com/gosuri/uitable"
+)
+
+var _ = Describe("Output of listing addons tests", func() {
+	// Output of function listAddons to test
+	var actualTable *uitable.Table
+
+	// getRowsByName extracts every rows with its NAME matching name
+	getRowsByName := func(name string) []*uitable.Row {
+		matchedRows := []*uitable.Row{}
+		for _, row := range actualTable.Rows {
+			// Check column NAME(0) = name
+			if row.Cells[0].Data == name {
+				matchedRows = append(matchedRows, row)
+			}
+		}
+		return matchedRows
+	}
+
+	BeforeEach(func() {
+		// Prepare KubeVela registry
+		reg := &pkgaddon.Registry{
+			Name: "KubeVela",
+			Helm: &pkgaddon.HelmSource{
+				URL: "https://addons.kubevela.net",
+			},
+		}
+		ds := pkgaddon.NewRegistryDataStore(k8sClient)
+		Expect(ds.AddRegistry(context.Background(), *reg)).To(Succeed())
+	})
+
+	JustBeforeEach(func() {
+		// Print addon list to table for later comparison
+		ret, err := listAddons(context.Background(), k8sClient, "")
+		Expect(err).Should(BeNil())
+		actualTable = ret
+	})
+
+	When("there is no addons installed", func() {
+		It("should not have any enabled addon", func() {
+			Expect(actualTable.Rows).ToNot(HaveLen(0))
+			for idx, row := range actualTable.Rows {
+				// Skip header
+				if idx == 0 {
+					continue
+				}
+				// Check column STATUS(4) = disabled
+				Expect(row.Cells[4].Data).To(Equal("disabled"))
+			}
+		})
+	})
+
+	When("there is locally installed addons", func() {
+		BeforeEach(func() {
+			// Install fluxcd locally
+			fluxcd := v1beta1.Application{}
+			err := yaml.Unmarshal([]byte(fluxcdYaml), &fluxcd)
+			Expect(err).Should(BeNil())
+			Expect(k8sClient.Create(context.Background(), &fluxcd)).Should(SatisfyAny(BeNil(), util.AlreadyExistMatcher{}))
+		})
+
+		It("should print fluxcd addon as local", func() {
+			matchedRows := getRowsByName("fluxcd")
+			Expect(matchedRows).ToNot(HaveLen(0))
+			// Only use first row (local first), check column REGISTRY(1) = local
+			Expect(matchedRows[0].Cells[1].Data).To(Equal("local"))
+			Eventually(func() error {
+				matchedRows = getRowsByName("fluxcd")
+				// Check column STATUS(4) = enabled
+				if matchedRows[0].Cells[4].Data != "enabled" {
+					return fmt.Errorf("fluxcd is not enabled yet")
+				}
+				// Check column AVAILABLE-VERSIONS(3) = 1.1.0
+				if versionString := matchedRows[0].Cells[3].Data; versionString != fmt.Sprintf("[%s]", color.New(color.Bold, color.FgGreen).Sprintf("1.1.0")) {
+					return fmt.Errorf("fluxcd version string is incorrect: %s", versionString)
+				}
+				return nil
+			}, 30*time.Second, 300*time.Millisecond).Should(BeNil())
+		})
+
+		It("should print fluxcd in the registry as disabled", func() {
+			matchedRows := getRowsByName("fluxcd")
+			// There should be a local one and a registry one
+			Expect(len(matchedRows)).To(Equal(2))
+			// The registry one should be disabled
+			Expect(matchedRows[1].Cells[1].Data).To(Equal("KubeVela"))
+			Expect(matchedRows[1].Cells[4].Data).To(Equal("disabled"))
+		})
+	})
+})
@@ -261,7 +261,7 @@ func TestGenerateAvailableVersions(t *testing.T) {
 		},
 	}
 	for _, s := range testcases {
-		re := genAvailableVersionInfo(s.c.versions, pkgaddon.Status{InstalledVersion: s.c.inVersion})
+		re := genAvailableVersionInfo(s.c.versions, s.c.inVersion)
 		assert.Equal(t, re, s.res)
 	}
 }
@@ -70,7 +70,9 @@ metadata:
  name: addon-fluxcd
  namespace: vela-system
  labels:
-     addons.oam.dev/name: fluxcd
+    addons.oam.dev/name: fluxcd
+    addons.oam.dev/registry: local
+    addons.oam.dev/version: 1.1.0
 spec:
  components:
  - name: ns-flux-system