'juju list-models' loads each Model object 3 times

[John A Meinel]

Tracking through the code, we have a couple bugs.

List models does this:

    for _, modelUUID := range modelUUIDs {
  st, release, err := m.state.GetBackend(modelUUID)

^- GetBackend is a modelManagerStateShim which requires both a 'state' and an 'model' so it does:
 otherState, release, err := st.pool.Get(modelUUID)
...
 otherModel, err := otherState.Model()

...
 return modelManagerStateShim{otherState, otherModel, st.pool}, release, nil
that is the first load. AFAICT ListModels does nothing with this object, but we still do all the work to read it from the DB.

  if err != nil {
   // This model could have been removed.
   if errors.IsNotFound(err) {
    continue
   }
   return result, errors.Trace(err)
  }
  defer release()

  model, err := st.Model()
  if err != nil {
   return result, errors.Trace(err)
  }
^- our first use to load it directly

  var lastConn *time.Time
  userLastConn, err := model.LastModelConnection(userTag)
  if err != nil {
   if !state.IsNeverConnectedError(err) {
    return result, errors.Trace(err)
   }
  } else {
   lastConn = &userLastConn
  }

  model, err = st.Model()
  if err != nil {
   return result, errors.Trace(err)
  }
^- we then load it one more time, just to be sure.

  result.UserModels = append(result.UserModels, params.UserModel{
   Model: params.Model{
    Name: model.Name(),
    UUID: model.UUID(),
    OwnerTag: model.Owner().String(),
   },
   LastConnection: lastConn,
  })
 }

There might be a different function we could use:

func (st modelManagerStateShim) GetModel(modelUUID string) (Model, func() bool, error) {
 model, release, err := st.pool.GetModel(modelUUID)
...
 return modelShim{model}, release, nil
}

But the above is calling st.Model() and not st.GetModel.

[John A Meinel (jameinel)]

https://github.com/juju/juju/pull/8128