diff --git a/pkg/addon/addon.go b/pkg/addon/addon.go index 879e91b6c..19dd83f02 100644 --- a/pkg/addon/addon.go +++ b/pkg/addon/addon.go @@ -53,6 +53,7 @@ const ( DefinitionsDirName string = "definitions" ) +// ListOptions contains flags mark what files should be read in an addon directory type ListOptions struct { GetDetail bool GetDefinition bool @@ -62,17 +63,22 @@ type ListOptions struct { } var ( - ListLevelOptions = ListOptions{} - GetLevelOptions = ListOptions{GetDetail: true, GetDefinition: true, GetParameter: true} + // GetLevelOptions used when get or list addons + GetLevelOptions = ListOptions{GetDetail: true, GetDefinition: true, GetParameter: true} + + // EnableLevelOptions used when enable addon EnableLevelOptions = ListOptions{GetDetail: true, GetDefinition: true, GetResource: true, GetTemplate: true, GetParameter: true} ) -type AddonErr error +// aError is internal error type of addon +type aError error var ( - AddonNotExist AddonErr = errors.New("addon not exist") + // ErrNotExist means addon not exists + ErrNotExist aError = errors.New("addon not exist") ) +// gitHelper helps get addon's file by git type gitHelper struct { Client *github.Client Meta *utils.Content @@ -85,14 +91,16 @@ type GitAddonSource struct { Token string `json:"token,omitempty"` } -type AddonReader struct { +// asyncReader helps async read files of addon +type asyncReader struct { addon *types.Addon h *gitHelper item *github.RepositoryContent errChan chan error } -func (r *AddonReader) SetReadContent(content *github.RepositoryContent) { +// SetReadContent set which file to read +func (r *asyncReader) SetReadContent(content *github.RepositoryContent) { r.item = content } @@ -159,8 +167,11 @@ func getSingleAddonFromGit(baseURL, dir, addonName, token string, opt ListOption return nil, err } _, items, err := gith.readRepo(gith.Meta.Path) + if err != nil { + return nil, err + } - reader := AddonReader{ + reader := asyncReader{ addon: &types.Addon{}, h: gith, errChan: make(chan error, 1), @@ -186,7 +197,7 @@ func getSingleAddonFromGit(baseURL, dir, addonName, token string, opt ListOption wg.Add(1) go readDefinitions(&wg, reader) case ResourcesDirName: - if !opt.GetResource { + if !opt.GetResource && !opt.GetParameter { break } reader.SetReadContent(item) @@ -213,7 +224,7 @@ func getSingleAddonFromGit(baseURL, dir, addonName, token string, opt ListOption } -func readTemplate(wg *sync.WaitGroup, reader AddonReader) { +func readTemplate(wg *sync.WaitGroup, reader asyncReader) { defer wg.Done() content, _, err := reader.h.readRepo(*reader.item.Path) if err != nil { @@ -234,7 +245,7 @@ func readTemplate(wg *sync.WaitGroup, reader AddonReader) { } } -func readResources(wg *sync.WaitGroup, reader AddonReader) { +func readResources(wg *sync.WaitGroup, reader asyncReader) { defer wg.Done() dirPath := strings.Split(reader.item.GetPath(), "/") dirPath, err := cutPathUntil(dirPath, ResourcesDirName) @@ -263,7 +274,7 @@ func readResources(wg *sync.WaitGroup, reader AddonReader) { } // readResFile read single resource file -func readResFile(wg *sync.WaitGroup, reader AddonReader, dirPath []string) { +func readResFile(wg *sync.WaitGroup, reader asyncReader, dirPath []string) { defer wg.Done() content, _, err := reader.h.readRepo(*reader.item.Path) if err != nil { @@ -288,7 +299,7 @@ func readResFile(wg *sync.WaitGroup, reader AddonReader, dirPath []string) { } } -func readDefinitions(wg *sync.WaitGroup, reader AddonReader) { +func readDefinitions(wg *sync.WaitGroup, reader asyncReader) { defer wg.Done() dirPath := strings.Split(reader.item.GetPath(), "/") dirPath, err := cutPathUntil(dirPath, DefinitionsDirName) @@ -316,7 +327,7 @@ func readDefinitions(wg *sync.WaitGroup, reader AddonReader) { } // readDefFile read single definition file -func readDefFile(wg *sync.WaitGroup, reader AddonReader, dirPath []string) { +func readDefFile(wg *sync.WaitGroup, reader asyncReader, dirPath []string) { defer wg.Done() content, _, err := reader.h.readRepo(*reader.item.Path) if err != nil { @@ -331,7 +342,7 @@ func readDefFile(wg *sync.WaitGroup, reader AddonReader, dirPath []string) { reader.addon.Definitions = append(reader.addon.Definitions, types.AddonElementFile{Data: b, Name: reader.item.GetName(), Path: dirPath}) } -func readMetadata(wg *sync.WaitGroup, reader AddonReader) { +func readMetadata(wg *sync.WaitGroup, reader asyncReader) { defer wg.Done() content, _, err := reader.h.readRepo(*reader.item.Path) if err != nil { @@ -350,7 +361,7 @@ func readMetadata(wg *sync.WaitGroup, reader AddonReader) { } } -func readReadme(wg *sync.WaitGroup, reader AddonReader) { +func readReadme(wg *sync.WaitGroup, reader asyncReader) { defer wg.Done() content, _, err := reader.h.readRepo(*reader.item.Path) if err != nil { @@ -358,6 +369,10 @@ func readReadme(wg *sync.WaitGroup, reader AddonReader) { return } reader.addon.Detail, err = content.GetContent() + if err != nil { + reader.errChan <- err + return + } } func createGitHelper(baseURL, dir, token string) (*gitHelper, error) { @@ -476,6 +491,9 @@ func RenderApplication(addon *types.Addon, args map[string]string) (*v1beta1.App } defObjs = append(defObjs, obj) } + if app.Spec.Workflow == nil { + app.Spec.Workflow = &v1beta1.Workflow{Steps: make([]v1beta1.WorkflowStep, 0)} + } app.Spec.Workflow.Steps = append(app.Spec.Workflow.Steps, v1beta1.WorkflowStep{ Name: "deploy-all", diff --git a/pkg/addon/error.go b/pkg/addon/error.go index c66d40d48..1a0c6f98c 100644 --- a/pkg/addon/error.go +++ b/pkg/addon/error.go @@ -20,8 +20,8 @@ var ( // WrapErrRateLimit return ErrRateLimit if is the situation, or return error directly func WrapErrRateLimit(err error) error { - var rateLimit *github.RateLimitError - if errors.As(err, &rateLimit) { + errRate := &github.RateLimitError{} + if errors.As(err, &errRate) { return ErrRateLimit } return err diff --git a/pkg/apiserver/rest/usecase/addon.go b/pkg/apiserver/rest/usecase/addon.go index 82bec2f05..5a1cb1e9d 100644 --- a/pkg/apiserver/rest/usecase/addon.go +++ b/pkg/apiserver/rest/usecase/addon.go @@ -51,12 +51,12 @@ func AddonImpl2AddonRes(impl *types.Addon) (*apis.DetailAddonResponse, error) { dec := k8syaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) _, _, err := dec.Decode([]byte(def.Data), nil, obj) if err != nil { - return nil, errors.New(fmt.Sprintf("convert %s file content to definition fail", def.Name)) + return nil, fmt.Errorf("convert %s file content to definition fail", def.Name) } defs = append(defs, &apis.AddonDefinition{ - obj.GetName(), - obj.GetKind(), - obj.GetAnnotations()["definition.oam.dev/description"], + Name: obj.GetName(), + DefType: obj.GetKind(), + Description: obj.GetAnnotations()["definition.oam.dev/description"], }) } return &apis.DetailAddonResponse{ @@ -104,23 +104,21 @@ func (u *addonUsecaseImpl) GetAddon(ctx context.Context, name string, registry s if addon, exist = u.tryGetAddonFromCache(r.Name, name); !exist { addon, err = pkgaddon.GetAddon(name, r.Git, pkgaddon.GetLevelOptions) } - if err != nil && !errors.Is(err, pkgaddon.AddonNotExist) { + if err != nil && !errors.Is(err, pkgaddon.ErrNotExist) { return nil, err } if addon != nil { break } } - } else { - if addon, exist = u.tryGetAddonFromCache(registry, name); !exist { - addonRegistry, err := u.GetAddonRegistry(ctx, registry) - if err != nil { - return nil, err - } - addon, err = pkgaddon.GetAddon(name, addonRegistry.Git, pkgaddon.GetLevelOptions) - if err != nil && !errors.Is(err, pkgaddon.AddonNotExist) { - return nil, err - } + } else if addon, exist = u.tryGetAddonFromCache(registry, name); !exist { + addonRegistry, err := u.GetAddonRegistry(ctx, registry) + if err != nil { + return nil, err + } + addon, err = pkgaddon.GetAddon(name, addonRegistry.Git, pkgaddon.GetLevelOptions) + if err != nil && !errors.Is(err, pkgaddon.ErrNotExist) { + return nil, err } } @@ -194,7 +192,7 @@ func (u *addonUsecaseImpl) ListAddons(ctx context.Context, registry, query strin } else { listAddons, err = pkgaddon.ListAddons(r.Git, pkgaddon.GetLevelOptions) if err != nil { - log.Logger.Errorf("fail to get addons from registry %s", r.Name) + log.Logger.Errorf("fail to get addons from registry %s, %v", r.Name, err) continue } // if list addons, details will be retrieved later @@ -332,7 +330,7 @@ func (u *addonUsecaseImpl) EnableAddon(ctx context.Context, name string, args ap if addon, exist = u.tryGetAddonFromCache(r.Name, name); !exist { addon, err = pkgaddon.GetAddon(name, r.Git, pkgaddon.EnableLevelOptions) } - if err != nil && !errors.Is(err, pkgaddon.AddonNotExist) { + if err != nil && !errors.Is(err, pkgaddon.ErrNotExist) { return bcode.WrapGithubRateLimitErr(err) } if addon == nil { diff --git a/test/e2e-apiserver-test/addon_test.go b/test/e2e-apiserver-test/addon_test.go index 9e78252c0..7df0ea650 100644 --- a/test/e2e-apiserver-test/addon_test.go +++ b/test/e2e-apiserver-test/addon_test.go @@ -1,23 +1,22 @@ -package e2e_apiserver +package e2e_apiserver_test import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "os" "time" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/wait" + "github.com/pkg/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" "github.com/oam-dev/kubevela/pkg/addon" apis "github.com/oam-dev/kubevela/pkg/apiserver/rest/apis/v1" - "github.com/oam-dev/kubevela/pkg/oam/util" - "github.com/oam-dev/kubevela/pkg/utils/common" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" ) const baseURL = "http://127.0.0.1:8000" @@ -52,8 +51,8 @@ var _ = Describe("Test addon rest api", func() { By("add registry") createRes := post("/api/v1/addon_registries", createReq) Expect(createRes).ShouldNot(BeNil()) - Expect(createRes.StatusCode).Should(Equal(200)) Expect(createRes.Body).ShouldNot(BeNil()) + Expect(createRes.StatusCode).Should(Equal(200)) defer createRes.Body.Close() @@ -77,17 +76,6 @@ var _ = Describe("Test addon rest api", func() { }) It("should enable and disable an addon", func() { - // todo(qiaozp) we should remove this namespace creation. This should be solved with a application template. - ns := v1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "flux-system", - }, - } - args := common.Args{} - k8sClient, err := args.GetClient() - Expect(err).Should(BeNil()) - Expect(k8sClient.Create(context.Background(), &ns)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{})) - defer GinkgoRecover() req := apis.EnableAddonRequest{ Args: map[string]string{ @@ -103,25 +91,30 @@ var _ = Describe("Test addon rest api", func() { defer res.Body.Close() var statusRes apis.AddonStatusResponse - err = json.NewDecoder(res.Body).Decode(&statusRes) + err := json.NewDecoder(res.Body).Decode(&statusRes) Expect(err).Should(BeNil()) Expect(statusRes.Phase).Should(Equal(apis.AddonPhaseEnabling)) // Wait for addon enabled - period := 20 * time.Second - timeout := 5 * time.Minute - err = wait.PollImmediate(period, timeout, func() (done bool, err error) { + period := 10 * time.Second + timeout := 2 * time.Minute + Eventually(func() error { res = get("/api/v1/addons/" + testAddon + "/status") err = json.NewDecoder(res.Body).Decode(&statusRes) Expect(err).Should(BeNil()) if statusRes.Phase == apis.AddonPhaseEnabled { - return true, nil + return nil } - return false, nil - }) - Expect(err).Should(BeNil()) + var app v1beta1.Application + err = k8sClient.Get(context.Background(), client.ObjectKey{Name: "addon-example", Namespace: "vela-system"}, &app) + Expect(err).Should(BeNil()) + data, err := json.Marshal(app) + Expect(err).Should(BeNil()) + fmt.Println(data) + return errors.New("not ready") + }, timeout, period).Should(BeNil()) res = post("/api/v1/addons/"+testAddon+"/disable", req) Expect(res).ShouldNot(BeNil())