revert dial and call all (#1267)

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

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.

internal/jimm/model.go

@@ -9,6 +9,7 @@ import (
 	"slices"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	jujupermission "github.com/juju/juju/core/permission"
@@ -764,41 +765,28 @@ 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)
 	if err != nil {
 		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