diff --git a/pkg/addon/helper.go b/pkg/addon/helper.go index 86fdbc4fd..caf6b8426 100644 --- a/pkg/addon/helper.go +++ b/pkg/addon/helper.go @@ -188,8 +188,8 @@ func GetAddonStatus(ctx context.Context, cli client.Client, name string) (Status } } -// FindWholeAddonPackagesFromRegistry find addons' WholeInstallPackage from registries, empty registryName indicates matching all -func FindWholeAddonPackagesFromRegistry(ctx context.Context, k8sClient client.Client, addonNames []string, registryNames []string) ([]*WholeAddonPackage, error) { +// FindAddonPackagesDetailFromRegistry find addons' WholeInstallPackage from registries, empty registryName indicates matching all +func FindAddonPackagesDetailFromRegistry(ctx context.Context, k8sClient client.Client, addonNames []string, registryNames []string) ([]*WholeAddonPackage, error) { var addons []*WholeAddonPackage var registries []Registry diff --git a/pkg/addon/helper_test.go b/pkg/addon/helper_test.go index 616a9bf93..30e42e729 100644 --- a/pkg/addon/helper_test.go +++ b/pkg/addon/helper_test.go @@ -29,21 +29,21 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { +var _ = Describe("test FindAddonPackagesDetailFromRegistry", func() { Describe("when no registry is added, no matter what you do, it will just return error", func() { Context("when empty addonNames and registryNames is supplied", func() { It("should return error", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{}, []string{}) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{}) Expect(err).To(HaveOccurred()) }) It("should return error", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, nil, nil) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, nil) Expect(err).To(HaveOccurred()) }) }) Context("when non-empty addonNames and registryNames is supplied", func() { It("should return error saying ErrRegistryNotExist", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"fluxcd"}, []string{"some-registry"}) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"fluxcd"}, []string{"some-registry"}) Expect(errors.Is(err, ErrRegistryNotExist)).To(BeTrue()) }) }) @@ -70,18 +70,18 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("when empty addonNames and registryNames is supplied", func() { It("should return error, empty addonNames are not allowed", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{}, []string{"KubeVela"}) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{"KubeVela"}) Expect(err).To(HaveOccurred()) }) It("should return error, empty addonNames are not allowed", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, nil, []string{"KubeVela"}) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, []string{"KubeVela"}) Expect(err).To(HaveOccurred()) }) }) Context("one existing addon name provided", func() { It("should return one valid result, matching all registries", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux"}, nil) Expect(err).To(Succeed()) Expect(res).To(HaveLen(1)) Expect(res[0].Name).To(Equal("velaux")) @@ -89,7 +89,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Expect(res[0].APISchema).ToNot(BeNil()) }) It("should return one valid result, matching one registry", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux"}, []string{"KubeVela"}) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux"}, []string{"KubeVela"}) Expect(err).To(Succeed()) Expect(res).To(HaveLen(1)) Expect(res[0].Name).To(Equal("velaux")) @@ -100,7 +100,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("one non-existent addon name provided", func() { It("should return error as ErrNotExist", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil) Expect(errors.Is(err, ErrNotExist)).To(BeTrue()) Expect(res).To(BeNil()) }) @@ -108,7 +108,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("two existing addon names provided", func() { It("should return two valid result", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux", "traefik"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux", "traefik"}, nil) Expect(err).To(Succeed()) Expect(res).To(HaveLen(2)) Expect(res[0].Name).To(Equal("velaux")) @@ -122,7 +122,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("one existing addon name and one non-existent addon name provided", func() { It("should return only one valid result", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"velaux", "non-existent-addon"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux", "non-existent-addon"}, nil) Expect(err).To(Succeed()) Expect(res).To(HaveLen(1)) Expect(res[0].Name).To(Equal("velaux")) @@ -151,25 +151,25 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("when empty addonNames and registryNames is supplied", func() { It("should return error, empty addonNames are not allowed", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{}, []string{}) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{}) Expect(err).To(HaveOccurred()) }) It("should return error, empty addonNames are not allowed", func() { - _, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, nil, []string{"testreg"}) + _, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, []string{"testreg"}) Expect(err).To(HaveOccurred()) }) }) Context("one existing addon name provided", func() { It("should return one valid result, matching all registries", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"example"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"example"}, nil) Expect(err).To(Succeed()) Expect(res).To(HaveLen(1)) Expect(res[0].Name).To(Equal("example")) Expect(res[0].InstallPackage).ToNot(BeNil()) }) It("should return one valid result, matching one registry", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"example"}, []string{"testreg"}) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"example"}, []string{"testreg"}) Expect(err).To(Succeed()) Expect(res).To(HaveLen(1)) Expect(res[0].Name).To(Equal("example")) @@ -179,7 +179,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("one non-existent addon name provided", func() { It("should return error as ErrNotExist", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"non-existent-addon"}, nil) Expect(errors.Is(err, ErrNotExist)).To(BeTrue()) Expect(res).To(BeNil()) }) @@ -187,7 +187,7 @@ var _ = Describe("test FindWholeAddonPackagesFromRegistry", func() { Context("one existing addon name and one non-existent addon name provided", func() { It("should return only one valid result", func() { - res, err := FindWholeAddonPackagesFromRegistry(context.Background(), k8sClient, []string{"example", "non-existent-addon"}, nil) + res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"example", "non-existent-addon"}, nil) Expect(err).To(Succeed()) Expect(res).To(HaveLen(1)) Expect(res[0].Name).To(Equal("example")) diff --git a/references/cli/addon.go b/references/cli/addon.go index 6a6f308a6..193c3ea9f 100644 --- a/references/cli/addon.go +++ b/references/cli/addon.go @@ -654,6 +654,16 @@ func statusAddon(name string, ioStreams cmdutil.IOStreams, cmd *cobra.Command, c return nil } +func addonNotExist(err error) bool { + if errors.Is(err, pkgaddon.ErrNotExist) || errors.Is(err, pkgaddon.ErrRegistryNotExist) { + return true + } + if strings.Contains(err.Error(), "not found") { + return true + } + return false +} + // generateAddonInfo will get addon status, description, version, dependencies (and whether they are installed), // and parameters (and their current values). // The first return value is the formatted string for printing. @@ -664,12 +674,21 @@ func generateAddonInfo(c client.Client, name string) (string, pkgaddon.Status, e var installed bool var addonPackage *pkgaddon.WholeAddonPackage + // Check current addon status + status, err := pkgaddon.GetAddonStatus(context.Background(), c, name) + if err != nil { + return res, status, err + } + // Get addon install package - if verboseStatus { + if verboseStatus || status.AddonPhase == statusDisabled { // We need the metadata to get descriptions about parameters - addonPackages, err := pkgaddon.FindWholeAddonPackagesFromRegistry(context.Background(), c, []string{name}, nil) - // Not found error can be ignored, because the user can define their own addon. Others can't. - if err != nil && !errors.Is(err, pkgaddon.ErrNotExist) && !errors.Is(err, pkgaddon.ErrRegistryNotExist) { + addonPackages, err := pkgaddon.FindAddonPackagesDetailFromRegistry(context.Background(), c, []string{name}, nil) + // If the state of addon is not disabled, we don't check the error, because it could be installed from local. + if status.AddonPhase == statusDisabled && err != nil { + if addonNotExist(err) { + return "", pkgaddon.Status{}, fmt.Errorf("addon '%s' not found in cluster or any registry", name) + } return "", pkgaddon.Status{}, err } if len(addonPackages) != 0 { @@ -677,12 +696,6 @@ func generateAddonInfo(c client.Client, name string) (string, pkgaddon.Status, e } } - // Check current addon status - status, err := pkgaddon.GetAddonStatus(context.Background(), c, name) - if err != nil { - return res, status, err - } - switch status.AddonPhase { case statusEnabled: installed = true @@ -785,12 +798,23 @@ func generateParameterString(status pkgaddon.Status, addonPackage *pkgaddon.Whol if addonPackage.APISchema == nil { return ret } - ret = printSchema(addonPackage.APISchema, status.Parameters, 0) return ret } +func convertInterface2StringList(l []interface{}) []string { + var strl []string + for _, s := range l { + str, ok := s.(string) + if !ok { + continue + } + strl = append(strl, str) + } + return strl +} + // printSchema prints the parameters in an addon recursively to a string // Deeper the parameter is nested, more the indentations. func printSchema(ref *openapi3.Schema, currentParams map[string]interface{}, indent int) string { @@ -853,19 +877,25 @@ func printSchema(ref *openapi3.Schema, currentParams map[string]interface{}, ind // Show current value if currentValue != "" { ret += addedIndent - ret += "\tcurrent: " + color.New(color.FgGreen).Sprintf("%s\n", currentValue) - } - // Show default value - if defaultValue != "" { - ret += addedIndent - ret += "\tdefault: " + fmt.Sprintf("%#v\n", defaultValue) + ret += "\tcurrent value: " + color.New(color.FgGreen).Sprintf("%s\n", currentValue) } + // Show required or not if required { ret += addedIndent ret += "\trequired: " ret += color.GreenString("✔\n") } + // Show Enum options + if len(propValue.Value.Enum) > 0 { + ret += addedIndent + ret += "\toptions: \"" + strings.Join(convertInterface2StringList(propValue.Value.Enum), "\", \"") + "\"\n" + } + // Show default value + if defaultValue != "" && currentValue == "" { + ret += addedIndent + ret += "\tdefault: " + fmt.Sprintf("%#v\n", defaultValue) + } // Object type param, we will get inside the object. // To show what's inside nested objects. diff --git a/references/cli/addon_suite_test.go b/references/cli/addon_suite_test.go index 484a2b816..21d33f896 100644 --- a/references/cli/addon_suite_test.go +++ b/references/cli/addon_suite_test.go @@ -269,7 +269,7 @@ var _ = Describe("Addon status or info", func() { BeforeEach(func() { // Delete KubeVela registry ds := pkgaddon.NewRegistryDataStore(k8sClient) - Expect(ds.DeleteRegistry(context.Background(), "KubeVela")).To(Succeed()) + Expect(ds.DeleteRegistry(context.Background(), "KubeVela")).Should(SatisfyAny(Succeed(), util.NotFoundMatcher{})) // Install fluxcd locally Expect(k8sClient.Create(context.Background(), &fluxcd)).Should(SatisfyAny(BeNil(), util.AlreadyExistMatcher{})) }) @@ -287,6 +287,7 @@ var _ = Describe("Addon status or info", func() { if err != nil { return err } + fmt.Println(addonName, res, err) // Should include enabled status, like: // fluxcd: enabled (1.1.0) if !strings.Contains(res, @@ -349,6 +350,11 @@ var _ = Describe("Addon status or info", func() { "KubeVela", )) }) + It("should report addon not exist in any registry name", func() { + addonName := "not-exist" + _, _, err := generateAddonInfo(k8sClient, addonName) + Expect(err.Error()).Should(BeEquivalentTo("addon 'not-exist' not found in cluster or any registry")) + }) }) }) }) diff --git a/references/cli/addon_test.go b/references/cli/addon_test.go index 441355b4b..1364ffc21 100644 --- a/references/cli/addon_test.go +++ b/references/cli/addon_test.go @@ -19,7 +19,6 @@ package cli import ( "fmt" "os" - "strings" "testing" . "github.com/onsi/ginkgo" @@ -27,7 +26,7 @@ import ( "github.com/fatih/color" "github.com/getkin/kin-openapi/openapi3" - "gotest.tools/assert" + "github.com/stretchr/testify/assert" pkgaddon "github.com/oam-dev/kubevela/pkg/addon" "github.com/oam-dev/kubevela/pkg/utils/common" @@ -91,8 +90,8 @@ func TestParseMap(t *testing.T) { for _, s := range testcase { r, err := parseAddonArgsToMap(s.args) if s.nilError { - assert.NilError(t, err) - assert.DeepEqual(t, s.res, r) + assert.NoError(t, err) + assert.Equal(t, s.res, r) } else { assert.Error(t, err, fmt.Sprintf("%v should be error case", s.args)) } @@ -208,7 +207,7 @@ func TestTransCluster(t *testing.T) { }, } for _, s := range testcase { - assert.DeepEqual(t, transClusters(s.str), s.res) + assert.Equal(t, transClusters(s.str), s.res) } } @@ -341,7 +340,7 @@ func TestPackageValidAddon(t *testing.T) { cmd := NewAddonPackageCommand(commandArgs) cmd.SetArgs([]string{"./test-data/addon/sample"}) err := cmd.Execute() - assert.NilError(t, err) + assert.NoError(t, err) defer func() { _ = os.RemoveAll("sample-1.0.1.tgz") }() @@ -380,13 +379,13 @@ func TestGenerateParameterString(t *testing.T) { "dbURL": &openapi3.SchemaRef{ Value: &openapi3.Schema{ Description: "Specify the MongoDB URL. it only enabled where DB type is MongoDB.", - Default: nil, + Default: "abc.com", }, }, "dbType": &openapi3.SchemaRef{ Value: &openapi3.Schema{ Description: "Specify the database type, current support KubeAPI(default) and MongoDB.", - Default: "kubeapi", + Enum: []interface{}{"kubeapi", "mongodb"}, }, }, }, @@ -397,18 +396,19 @@ func TestGenerateParameterString(t *testing.T) { color.New(color.FgCyan).Sprintf("-> ") + color.New(color.Bold).Sprint("dbType") + ": " + "Specify the database type, current support KubeAPI(default) and MongoDB.\n" + - "\tcurrent: " + color.New(color.FgGreen).Sprint("\"kubeapi\"\n") + - "\tdefault: " + "\"kubeapi\"\n" + - "\trequired: " + color.GreenString("✔\n"), + "\tcurrent value: " + color.New(color.FgGreen).Sprint("\"kubeapi\"\n") + + "\trequired: " + color.GreenString("✔\n") + + "\toptions: \"kubeapi\", \"mongodb\"\n", // dbURL color.New(color.FgCyan).Sprintf("-> ") + color.New(color.Bold).Sprint("dbURL") + ": " + - "Specify the MongoDB URL. it only enabled where DB type is MongoDB.", + "Specify the MongoDB URL. it only enabled where DB type is MongoDB.\n" + + "\tdefault: " + "\"abc.com\"\n", // database color.New(color.FgCyan).Sprintf("-> ") + color.New(color.Bold).Sprint("database") + ": " + "Specify the database name, for the kubeapi db type, it represents namespace.\n" + - "\tcurrent: " + color.New(color.FgGreen).Sprint("\"kubevela\""), + "\tcurrent value: " + color.New(color.FgGreen).Sprint("\"kubevela\""), }, }, } @@ -416,7 +416,7 @@ func TestGenerateParameterString(t *testing.T) { for _, s := range testcase { res := generateParameterString(s.status, s.addonPackage) for _, o := range s.outputs { - assert.Check(t, strings.Contains(res, o)) + assert.Contains(t, res, o) } } @@ -434,12 +434,12 @@ func TestNewAddonCreateCommand(t *testing.T) { cmd.SetArgs([]string{"test-addon", "--chart", "a", "--helm-repo", "https://some.com", "--chart-version", "c"}) err = cmd.Execute() - assert.NilError(t, err) + assert.NoError(t, err) _ = os.RemoveAll("test-addon") cmd.SetArgs([]string{"test-addon"}) err = cmd.Execute() - assert.NilError(t, err) + assert.NoError(t, err) _ = os.RemoveAll("test-addon") }