From a598272491e9ccaa77806026e0082e7cceea7f49 Mon Sep 17 00:00:00 2001 From: Charlie Chiang Date: Tue, 10 May 2022 13:35:12 +0800 Subject: [PATCH] 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 * Style: revert incorrect auto-formatting Signed-off-by: Charlie Chiang * Refactor: change original variable name to avoid confusions Signed-off-by: Charlie Chiang * 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 * Refactor: use more concise method to check length Signed-off-by: Charlie Chiang * Test: add one more test condition for dual addons i.e. local and registry Signed-off-by: Charlie Chiang * Refactor: simplify testing logic by removing unneeded looping Signed-off-by: Charlie Chiang * Style: add missing license header Signed-off-by: Charlie Chiang --- references/cli/addon.go | 61 +++++++------- references/cli/addon_suite_test.go | 125 +++++++++++++++++++++++++++++ references/cli/addon_test.go | 2 +- references/cli/uninstall_test.go | 4 +- 4 files changed, 163 insertions(+), 29 deletions(-) create mode 100644 references/cli/addon_suite_test.go diff --git a/references/cli/addon.go b/references/cli/addon.go index 06ca8dea8..f6296c355 100644 --- a/references/cli/addon.go +++ b/references/cli/addon.go @@ -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 Enable addon with specify version: - vela addon enable --version + vela addon enable --version Enable addon for specific clusters, (local means control plane): vela addon enable --clusters={local,cluster1,cluster2} `, @@ -220,7 +220,7 @@ func NewAddonUpgradeCommand(c common.Args, ioStream cmdutil.IOStreams) *cobra.Co Upgrade addon by: vela addon upgrade Upgrade addon with specify version: - vela addon upgrade --version + vela addon upgrade --version Upgrade addon for specific clusters, (local means control plane): vela addon upgrade --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 { diff --git a/references/cli/addon_suite_test.go b/references/cli/addon_suite_test.go new file mode 100644 index 000000000..0b5ae252d --- /dev/null +++ b/references/cli/addon_suite_test.go @@ -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")) + }) + }) +}) diff --git a/references/cli/addon_test.go b/references/cli/addon_test.go index ebaa7110e..d06a559ea 100644 --- a/references/cli/addon_test.go +++ b/references/cli/addon_test.go @@ -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) } } diff --git a/references/cli/uninstall_test.go b/references/cli/uninstall_test.go index 57d9f5928..ecfaa64d3 100644 --- a/references/cli/uninstall_test.go +++ b/references/cli/uninstall_test.go @@ -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