From 5d49557ee75c279eea2b23355e544d1f0726ef7e Mon Sep 17 00:00:00 2001 From: Alexander <42068202+ale8k@users.noreply.github.com> Date: Tue, 16 Jul 2024 14:54:08 +0100 Subject: [PATCH] revert dial and call all (#1267) --- internal/jimm/controller.go | 39 +++++++++++++------------------------ internal/jimm/jimm.go | 33 ++++++++----------------------- internal/jimm/model.go | 32 ++++++++++-------------------- 3 files changed, 31 insertions(+), 73 deletions(-) diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go index a6acf5c83..569ffa5b3 100644 --- a/internal/jimm/controller.go +++ b/internal/jimm/controller.go @@ -7,6 +7,7 @@ import ( "database/sql" "encoding/json" "fmt" + "sync" "github.com/juju/juju/api/base" "github.com/juju/juju/api/controller/controller" @@ -45,43 +46,29 @@ var ( // access to. func (j *JIMM) AllModels(ctx context.Context, user *openfga.User) (jujuparams.UserModelList, error) { const op = errors.Op("jimm.AllModels") - - userModelList := jujuparams.UserModelList{} + var resultMut sync.Mutex + flattenedUml := jujuparams.UserModelList{} uuidToPerms, err := j.getControllersWithModelPermissionsForUser(ctx, user) if err != nil { - return userModelList, errors.E(op, err) + return flattenedUml, errors.E(op, err) } - umlCh := make(chan jujuparams.UserModelList) - errorsCh := make(chan error) - - dialAndCallControllers(j.dial, uuidToPerms, errorsCh, func(api API) { + err = j.forEachController(ctx, uuidToPerms.GetControllers(), func(c *dbmodel.Controller, api API) error { + resultMut.Lock() + defer resultMut.Unlock() uml, err := api.AllModels(ctx) if err != nil { - errorsCh <- err - return + return err } - umlCh <- uml + flattenedUml.UserModels = append(flattenedUml.UserModels, uml.UserModels...) + return nil }) - - errs := []error{} - - for range uuidToPerms { - select { - case uml := <-umlCh: - userModelList.UserModels = append(userModelList.UserModels, uml.UserModels...) - case err := <-errorsCh: - errs = append(errs, err) - } - } - - if len(errs) > 0 { - // What do we do with call errors? Just take the first? - return userModelList, errors.E(op, errs[0]) + if err != nil { + return flattenedUml, errors.E(err, op) } - return userModelList, nil + return flattenedUml, nil } // AddController adds the specified controller to JIMM. Only diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index 8a01b3205..a35aef4df 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -185,6 +185,14 @@ type controllerWithModelPermissions struct { type controllerPermSet map[string]controllerWithModelPermissions +func (c controllerPermSet) GetControllers() []dbmodel.Controller { + controllers := []dbmodel.Controller{} + for _, v := range c { + controllers = append(controllers, v.controller) + } + return controllers +} + type dialerFunc func(ctx context.Context, ctl *dbmodel.Controller, modelTag names.ModelTag, permissons ...permission) (API, error) // getControllersWithModelPermissionsForUser returns a map of controller uuids to: @@ -242,31 +250,6 @@ func (j *JIMM) getControllersWithModelPermissionsForUser(ctx context.Context, us return uuidToPerms, nil } -// dialAndCallControllers dials multiple controllers in their own routines -// from the controller permission set and reports errors on dial to an error channel. -// -// It takes a function which will pass the dialed controller to the function. -// Should you wish to handle concurrency waiting, please read from a channel -// outside the op or create a wait group within the op. -func dialAndCallControllers( - dialerFunc dialerFunc, - p controllerPermSet, - errCh chan error, - op func(api API), -) { - for _, perms := range p { - go func(c controllerWithModelPermissions) { - api, err := dialerFunc(context.Background(), &c.controller, names.ModelTag{}, c.permissions...) - if err != nil { - errCh <- err - return - } - defer api.Close() - op(api) - }(perms) - } -} - // dial dials the controller and model specified by the given Controller // and ModelTag. If no Dialer has been configured then an error with a // code of CodeConnectionFailed will be returned. diff --git a/internal/jimm/model.go b/internal/jimm/model.go index d6c65b733..1fd6e0557 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -9,6 +9,7 @@ import ( "slices" "sort" "strings" + "sync" "time" jujupermission "github.com/juju/juju/core/permission" @@ -764,6 +765,7 @@ func (j *JIMM) ModelStatus(ctx context.Context, user *openfga.User, mt names.Mod func (j *JIMM) GetAllModelSummariesForUser(ctx context.Context, user *openfga.User) (jujuparams.ModelSummaryResults, error) { const op = errors.Op("jimm.GetAllModelSummariesForUser") + var resultMut sync.Mutex flattenedSummaries := jujuparams.ModelSummaryResults{} uuidToPerms, err := j.getControllersWithModelPermissionsForUser(ctx, user) @@ -771,34 +773,20 @@ func (j *JIMM) GetAllModelSummariesForUser(ctx context.Context, user *openfga.Us return flattenedSummaries, errors.E(op, err) } - summariesCh := make(chan jujuparams.ModelSummaryResults) - errorsCh := make(chan error) - - dialAndCallControllers(j.dial, uuidToPerms, errorsCh, func(api API) { + err = j.forEachController(ctx, uuidToPerms.GetControllers(), func(c *dbmodel.Controller, api API) error { + resultMut.Lock() + defer resultMut.Unlock() var out jujuparams.ModelSummaryResults in := jujuparams.ModelSummariesRequest{UserTag: names.NewUserTag(user.Name).String(), All: true} if err := api.ListModelSummaries(context.Background(), &in, &out); err != nil { - errorsCh <- err - return + return err } - summariesCh <- out + flattenedSummaries.Results = append(flattenedSummaries.Results, out.Results...) + return nil }) - - errs := []error{} - - for range uuidToPerms { - select { - case sum := <-summariesCh: - flattenedSummaries.Results = append(flattenedSummaries.Results, sum.Results...) - case err := <-errorsCh: - errs = append(errs, err) - } - } - - if len(errs) > 0 { - // What do we do with call errors? Just take the first? - return flattenedSummaries, errors.E(op, errs[0]) + if err != nil { + return flattenedSummaries, errors.E(err, op) } // Flatten permissions into single slice for index searching