diff --git a/cmd/jimmctl/cmd/relation_test.go b/cmd/jimmctl/cmd/relation_test.go index ac0dfd45c..d837176ea 100644 --- a/cmd/jimmctl/cmd/relation_test.go +++ b/cmd/jimmctl/cmd/relation_test.go @@ -10,7 +10,6 @@ import ( "os" "sort" "strings" - "time" petname "github.com/dustinkirkland/golang-petname" "github.com/google/uuid" @@ -542,13 +541,6 @@ func (s *relationSuite) TestCheckRelationViaSuperuser(c *gc.C) { CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, Life: "alive", - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().UTC().Truncate(time.Millisecond), - Valid: true, - }, - }, } err = db.AddModel(ctx, &model) diff --git a/cmd/jimmsrv/service/service.go b/cmd/jimmsrv/service/service.go index 83d9e6f6b..29008dae5 100644 --- a/cmd/jimmsrv/service/service.go +++ b/cmd/jimmsrv/service/service.go @@ -216,17 +216,6 @@ func (s *Service) ServeHTTP(w http.ResponseWriter, req *http.Request) { s.mux.ServeHTTP(w, req) } -// WatchControllers connects to all controllers and starts an AllWatcher -// monitoring all changes to models. WatchControllers finishes when the -// given context is canceled, or there is a fatal error watching models. -func (s *Service) WatchControllers(ctx context.Context) error { - w := jimm.Watcher{ - Database: s.jimm.Database, - Dialer: s.jimm.Dialer, - } - return w.Watch(ctx, 10*time.Minute) -} - // WatchModelSummaries connects to all controllers and starts a // ModelSummaryWatcher for all models. WatchModelSummaries finishes when // the given context is canceled, or there is a fatal error watching model @@ -275,6 +264,22 @@ func (s *Service) OpenFGACleanup(ctx context.Context, trigger <-chan time.Time) } } +// CleanupDyingModels triggers every `trigger` time and calls the jimm methods to cleanup dying models. +func (s *Service) CleanupDyingModels(ctx context.Context, trigger <-chan time.Time) error { + for { + select { + case <-trigger: + err := s.jimm.CleanupDyingModels(ctx) + if err != nil { + zapctx.Error(ctx, "dying models cleanup", zap.Error(err)) + continue + } + case <-ctx.Done(): + return nil + } + } +} + // Cleanup cleans up resources that need to be released on shutdown. func (s *Service) Cleanup() { // Iterating over clean up function in reverse-order to avoid early clean ups. @@ -498,11 +503,6 @@ func NewService(ctx context.Context, p Params) (*Service, error) { func (s *Service) StartServices(ctx context.Context, svc *service.Service) { // on the leader unit we start additional routines if s.isLeader { - // the leader unit connects to all controllers' AllWatcher - svc.Go(func() error { - return s.WatchControllers(ctx) - }) - // audit log cleanup routine if s.auditLogCleanupPeriod != 0 { svc.Go(func() error { @@ -524,6 +524,11 @@ func (s *Service) StartServices(ctx context.Context, svc *service.Service) { svc.Go(func() error { return s.OpenFGACleanup(ctx, time.NewTicker(6*time.Hour).C) }) + + // CleanupDyingModels cleanup - cleans up all dying models + svc.Go(func() error { + return s.CleanupDyingModels(ctx, time.NewTicker(time.Minute).C) + }) } // all units periodically update their controller/model metrics diff --git a/internal/db/applicationoffer_test.go b/internal/db/applicationoffer_test.go index bd0e151d0..c5938deb7 100644 --- a/internal/db/applicationoffer_test.go +++ b/internal/db/applicationoffer_test.go @@ -7,7 +7,6 @@ import ( "database/sql" "sort" "testing" - "time" qt "github.com/frankban/quicktest" "github.com/google/go-cmp/cmp/cmpopts" @@ -82,16 +81,7 @@ func initTestEnvironment(c *qt.C, db *db.Database) testEnvironment { Controller: env.controller, CloudRegion: env.cloud.Regions[0], CloudCredential: env.cred, - Type: "iaas", - IsController: false, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, - }, } c.Assert(db.DB.Create(&env.model).Error, qt.IsNil) @@ -105,16 +95,7 @@ func initTestEnvironment(c *qt.C, db *db.Database) testEnvironment { Controller: env.controller, CloudRegion: env.cloud.Regions[0], CloudCredential: env.cred, - Type: "iaas", - IsController: false, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, - }, } c.Assert(db.DB.Create(&env.model1).Error, qt.IsNil) diff --git a/internal/db/model_test.go b/internal/db/model_test.go index cef4c0a68..f80816bb4 100644 --- a/internal/db/model_test.go +++ b/internal/db/model_test.go @@ -7,7 +7,6 @@ import ( "database/sql" "sort" "testing" - "time" qt "github.com/frankban/quicktest" "github.com/juju/juju/state" @@ -72,16 +71,7 @@ func (s *dbSuite) TestAddModel(c *qt.C) { ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } m1 := model err = s.Database.AddModel(context.Background(), &model) @@ -147,16 +137,7 @@ func (s *dbSuite) TestGetModel(c *qt.C) { CloudRegion: cloud.Regions[0], CloudCredentialID: cred.ID, CloudCredential: cred, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } model.CloudCredential.Cloud = dbmodel.Cloud{} // We don't care about the cloud credential owner when @@ -240,16 +221,7 @@ func (s *dbSuite) TestUpdateModel(c *qt.C) { ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } err = s.Database.AddModel(context.Background(), &model) c.Assert(err, qt.Equals, nil) @@ -317,16 +289,7 @@ func (s *dbSuite) TestDeleteModel(c *qt.C) { Controller: controller, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } // model does not exist @@ -400,16 +363,7 @@ func (s *dbSuite) TestGetModelsUsingCredential(c *qt.C) { ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred1.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } err = s.Database.AddModel(context.Background(), &model1) c.Assert(err, qt.Equals, nil) @@ -424,16 +378,7 @@ func (s *dbSuite) TestGetModelsUsingCredential(c *qt.C) { ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred2.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } err = s.Database.AddModel(context.Background(), &model2) c.Assert(err, qt.Equals, nil) @@ -454,13 +399,7 @@ func (s *dbSuite) TestGetModelsUsingCredential(c *qt.C) { Controller: controller, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred1.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: model1.Status, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, }}) models, err = s.Database.GetModelsUsingCredential(context.Background(), 0) @@ -683,20 +622,7 @@ func (s *dbSuite) TestGetModelsByController(c *qt.C) { Controller: controller, CloudRegion: cloud.Regions[0], CloudCredential: cred, - Type: "iaas", - IsController: true, - DefaultSeries: "focal", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, }, { Name: "test-model-2", UUID: sql.NullString{ @@ -707,20 +633,7 @@ func (s *dbSuite) TestGetModelsByController(c *qt.C) { Controller: controller, CloudRegion: cloud.Regions[0], CloudCredential: cred, - Type: "iaas", - IsController: false, - DefaultSeries: "focal", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, }} for _, m := range models { c.Assert(s.Database.DB.Create(&m).Error, qt.IsNil) diff --git a/internal/db/resource_test.go b/internal/db/resource_test.go index 6786e5cbf..03e45149c 100644 --- a/internal/db/resource_test.go +++ b/internal/db/resource_test.go @@ -53,16 +53,7 @@ func SetupDB(c *qt.C, database *db.Database) (dbmodel.Model, dbmodel.Controller, ControllerID: controller.ID, CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, - Type: "iaas", - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: db.Now(), - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } err = database.AddModel(context.Background(), &model) c.Assert(err, qt.Equals, nil) diff --git a/internal/dbmodel/applicationoffer_test.go b/internal/dbmodel/applicationoffer_test.go index 718eda4ec..c338b63e8 100644 --- a/internal/dbmodel/applicationoffer_test.go +++ b/internal/dbmodel/applicationoffer_test.go @@ -43,9 +43,6 @@ func TestApplicationOfferUniqueConstraint(t *testing.T) { Controller: ctl, CloudRegion: cl.Regions[0], CloudCredential: cred, - Type: "iaas", - IsController: false, - DefaultSeries: "warty", Life: state.Alive.String(), } c.Assert(db.Create(&m).Error, qt.IsNil) diff --git a/internal/dbmodel/model.go b/internal/dbmodel/model.go index 333d6d8a5..a4c189872 100644 --- a/internal/dbmodel/model.go +++ b/internal/dbmodel/model.go @@ -7,10 +7,8 @@ import ( "time" "github.com/juju/juju/core/life" - "github.com/juju/juju/core/status" jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/names/v5" - "github.com/juju/version/v2" "github.com/canonical/jimm/v3/internal/errors" ) @@ -37,11 +35,6 @@ type Model struct { ControllerID uint Controller Controller - // (Unused) Currently model migrations are completed manually. - // MigrationControllerID is the controller that a model is migrating to. - // This is only filled if the new controller is within JIMM. - MigrationControllerID sql.NullInt32 - // CloudRegion is the cloud-region hosting the model. CloudRegionID uint CloudRegion CloudRegion @@ -50,33 +43,9 @@ type Model struct { CloudCredentialID uint CloudCredential CloudCredential `gorm:"foreignkey:CloudCredentialID;references:ID"` - // Type is the type of model. - Type string - - // IsController specifies if the model hosts the controller machines. - IsController bool - - // DefaultSeries holds the default series for the model. - DefaultSeries string - // Life holds the life status of the model. Life string - // Status holds the current status of the model. - Status Status `gorm:"embedded;embeddedPrefix:status_"` - - // SLA contains the SLA of the model. - SLA SLA `gorm:"embedded;embeddedPrefix:sla_"` - - // Cores contains the count of cores in the model. - Cores int64 - - // Machines contains the count of machines in the model. - Machines int64 - - // Units contains the count of machines in the model. - Units int64 - // Offers are the ApplicationOffers attached to the model. Offers []ApplicationOffer } @@ -115,10 +84,7 @@ func (m *Model) SetOwner(u *Identity) { // will need to be filled in manually by the caller of this function. func (m *Model) FromJujuModelInfo(info jujuparams.ModelInfo) error { m.Name = info.Name - m.Type = info.Type SetNullString(&m.UUID, &info.UUID) - m.IsController = info.IsController - m.DefaultSeries = info.DefaultSeries if info.OwnerTag != "" { ut, err := names.ParseUserTag(info.OwnerTag) if err != nil { @@ -127,7 +93,6 @@ func (m *Model) FromJujuModelInfo(info jujuparams.ModelInfo) error { m.OwnerIdentityName = ut.Id() } m.Life = string(info.Life) - m.Status.FromJujuEntityStatus(info.Status) m.CloudRegion.Name = info.CloudRegion if info.CloudTag != "" { @@ -147,13 +112,6 @@ func (m *Model) FromJujuModelInfo(info jujuparams.ModelInfo) error { m.CloudCredential.Owner.Name = cct.Owner().Id() } - if info.SLA != nil { - m.SLA.FromJujuModelSLAInfo(*info.SLA) - } - - if info.AgentVersion != nil { - m.Status.Version = info.AgentVersion.String() - } return nil } @@ -161,8 +119,6 @@ func (m *Model) FromJujuModelInfo(info jujuparams.ModelInfo) error { func (m *Model) FromJujuModelUpdate(info jujuparams.ModelUpdate) { m.Name = info.Name m.Life = string(info.Life) - m.Status.FromJujuStatusInfo(info.Status) - m.SLA.FromJujuModelSLAInfo(info.SLA) } // ToJujuModel converts a model into a jujuparams.Model. @@ -170,129 +126,31 @@ func (m Model) ToJujuModel() jujuparams.Model { var jm jujuparams.Model jm.Name = m.Name jm.UUID = m.UUID.String - jm.Type = m.Type jm.OwnerTag = names.NewUserTag(m.OwnerIdentityName).String() return jm } -// ToJujuModelSummary converts a model to a jujuparams.ModelSummary. The -// model must have its CloudRegion, CloudCredential, Controller, Machines, -// and Owner, associations fetched. The ModelSummary will not include the -// UserAccess or UserLastConnection fields, it is the caller's -// responsibility to complete these fields appropriately. -func (m Model) ToJujuModelSummary() jujuparams.ModelSummary { - var ms jujuparams.ModelSummary - ms.Name = m.Name - ms.Type = m.Type - ms.UUID = m.UUID.String - ms.ControllerUUID = m.Controller.UUID - ms.IsController = m.IsController - ms.ProviderType = m.CloudRegion.Cloud.Type - ms.DefaultSeries = m.DefaultSeries - ms.CloudTag = m.CloudRegion.Cloud.Tag().String() - ms.CloudRegion = m.CloudRegion.Name - ms.CloudCredentialTag = m.CloudCredential.Tag().String() - ms.OwnerTag = m.Owner.Tag().String() - ms.Life = life.Value(m.Life) - ms.Status = m.Status.ToJujuEntityStatus() - ms.Counts = []jujuparams.ModelEntityCount{{ - Entity: jujuparams.Machines, - Count: m.Machines, - }, { - Entity: jujuparams.Cores, - Count: m.Cores, - }, { - Entity: jujuparams.Units, - Count: m.Units, - }} - - // JIMM doesn't store information about Migrations so this is omitted. - ms.SLA = new(jujuparams.ModelSLAInfo) - *ms.SLA = m.SLA.ToJujuModelSLAInfo() - - v, err := version.Parse(m.Status.Version) - if err == nil { - // If there is an error parsing the version it is considered - // unavailable and therefore is not set. - ms.AgentVersion = &v +// MergeModelSummaryFromController converts a model to a jujuparams.ModelSummary. +// It uses the info from the controller and JIMM's db to fill the jujuparams.ModelSummary. +// maskingControllerUUID is used to mask the controllerUUID with the JIMM's one. +// access is the user access level got from JIMM. +func (m Model) MergeModelSummaryFromController(modelSummaryFromController *jujuparams.ModelSummary, maskingControllerUUID string, access jujuparams.UserAccessPermission) jujuparams.ModelSummary { + if modelSummaryFromController == nil { + modelSummaryFromController = &jujuparams.ModelSummary{} } - return ms -} - -// An SLA contains the details of the SLA associated with the model. -type SLA struct { - // Level contains the SLA level. - Level string - - // Owner contains the SLA owner. - Owner string -} - -// FromJujuModelSLAInfo converts jujuparams.ModelSLAInfo into SLA. -func (s *SLA) FromJujuModelSLAInfo(js jujuparams.ModelSLAInfo) { - s.Level = js.Level - s.Owner = js.Owner -} - -// ToJujuModelSLAInfo converts a SLA into a jujuparams.ModelSLAInfo. -func (s SLA) ToJujuModelSLAInfo() jujuparams.ModelSLAInfo { - var msi jujuparams.ModelSLAInfo - msi.Level = s.Level - msi.Owner = s.Owner - return msi -} - -// A Status holds the entity status of an object. -type Status struct { - Status string - Info string - Data Map - Since sql.NullTime - Version string -} - -// FromJujuEntityStatus converts jujuparams.EntityStatus into Status. -func (s *Status) FromJujuEntityStatus(js jujuparams.EntityStatus) { - s.Status = string(js.Status) - s.Info = js.Info - s.Data = Map(js.Data) - if js.Since == nil { - s.Since = sql.NullTime{Valid: false} - } else { - s.Since = sql.NullTime{ - Time: js.Since.UTC().Truncate(time.Millisecond), - Valid: true, - } - } -} - -// FromJujuStatusInfo updates the Status from the given -// jujuparams.StatusInfo. -func (s *Status) FromJujuStatusInfo(info jujuparams.StatusInfo) { - s.Status = string(info.Current) - s.Info = info.Message - s.Version = info.Version - if info.Since == nil { - s.Since = sql.NullTime{Valid: false} - } else { - s.Since = sql.NullTime{ - Time: info.Since.UTC().Truncate(time.Millisecond), - Valid: true, - } - } - s.Data = Map(info.Data) -} - -// ToJujuEntityStatus converts the status into a jujuparams.EntityStatus. -func (s Status) ToJujuEntityStatus() jujuparams.EntityStatus { - var es jujuparams.EntityStatus - es.Status = status.Status(s.Status) - es.Info = s.Info - es.Data = map[string]interface{}(s.Data) - if s.Since.Valid { - es.Since = &s.Since.Time + modelSummaryFromController.Name = m.Name + modelSummaryFromController.UUID = m.UUID.String + if maskingControllerUUID != "" { + modelSummaryFromController.ControllerUUID = maskingControllerUUID } else { - es.Since = nil + modelSummaryFromController.ControllerUUID = m.Controller.UUID } - return es + modelSummaryFromController.ProviderType = m.CloudRegion.Cloud.Type + modelSummaryFromController.CloudTag = m.CloudRegion.Cloud.Tag().String() + modelSummaryFromController.CloudRegion = m.CloudRegion.Name + modelSummaryFromController.CloudCredentialTag = m.CloudCredential.Tag().String() + modelSummaryFromController.OwnerTag = m.Owner.Tag().String() + modelSummaryFromController.Life = life.Value(m.Life) + modelSummaryFromController.UserAccess = access + return *modelSummaryFromController } diff --git a/internal/dbmodel/model_test.go b/internal/dbmodel/model_test.go index 536654154..f98d8be65 100644 --- a/internal/dbmodel/model_test.go +++ b/internal/dbmodel/model_test.go @@ -79,20 +79,7 @@ func TestModel(t *testing.T) { Controller: ctl, CloudRegion: cl.Regions[0], CloudCredential: cred, - Type: "iaas", - IsController: false, - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().Truncate(time.Millisecond), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } c.Assert(db.Create(&m).Error, qt.IsNil) @@ -145,20 +132,7 @@ func TestModelUniqueConstraint(t *testing.T) { Controller: ctl1, CloudRegion: cl1.Regions[0], CloudCredential: cred1, - Type: "iaas", - IsController: false, - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().Truncate(time.Millisecond), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } c.Assert(db.Create(&m1).Error, qt.IsNil) @@ -172,20 +146,7 @@ func TestModelUniqueConstraint(t *testing.T) { Controller: ctl2, CloudRegion: cl2.Regions[0], CloudCredential: cred2, - Type: "iaas", - IsController: false, - DefaultSeries: "jammy", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().Truncate(time.Millisecond), - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } c.Assert(db.Create(&m2).Error, qt.ErrorMatches, `ERROR: duplicate key value violates unique constraint .*`) @@ -207,7 +168,6 @@ func TestToJujuModel(t *testing.T) { c := qt.New(t) db := gormDB(c) cl, cred, ctl, u := initModelEnv(c, db) - now := time.Now().Truncate(time.Millisecond) m := dbmodel.Model{ Name: "test-model", UUID: sql.NullString{ @@ -219,20 +179,7 @@ func TestToJujuModel(t *testing.T) { Controller: ctl, CloudRegion: cl.Regions[0], CloudCredential: cred, - Type: "iaas", - IsController: false, - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: now, - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, } m.CloudRegion.Cloud = cl @@ -240,7 +187,6 @@ func TestToJujuModel(t *testing.T) { c.Check(jm, qt.DeepEquals, jujuparams.Model{ Name: "test-model", UUID: "00000001-0000-0000-0000-0000-000000000001", - Type: "iaas", OwnerTag: "user-bob@canonical.com", }) } @@ -260,27 +206,37 @@ func TestToJujuModelSummary(t *testing.T) { Controller: ctl, CloudRegion: cl.Regions[0], CloudCredential: cred, - Type: "iaas", - IsController: false, - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ + } + m.CloudRegion.Cloud = cl + modelSummaryFromController := jujuparams.ModelSummary{ + Name: "test-model", + Type: "iaas", + UUID: "00000001-0000-0000-0000-0000-000000000001", + ControllerUUID: "00000000-0000-0000-0000-0000-0000000000002", + Life: life.Value(state.Alive.String()), + IsController: false, + ProviderType: "test-provider", + DefaultSeries: "warty", + Status: jujuparams.EntityStatus{ Status: "available", - Since: sql.NullTime{ - Time: now, - Valid: true, - }, + Since: &now, }, - SLA: dbmodel.SLA{ + Counts: []jujuparams.ModelEntityCount{{ + Entity: "machines", + Count: 1, + }, { + Entity: "cores", + Count: 2, + }, { + Entity: "units", + Count: 3, + }}, + SLA: &jujuparams.ModelSLAInfo{ Level: "unsupported", }, - Machines: 1, - Cores: 2, - Units: 3, } - m.CloudRegion.Cloud = cl - - ms := m.ToJujuModelSummary() + ms := m.MergeModelSummaryFromController(&modelSummaryFromController, "", "writer") c.Check(ms, qt.DeepEquals, jujuparams.ModelSummary{ Name: "test-model", Type: "iaas", @@ -292,6 +248,7 @@ func TestToJujuModelSummary(t *testing.T) { CloudTag: "cloud-test-cloud", CloudRegion: "test-region", CloudCredentialTag: "cloudcred-test-cloud_bob@canonical.com_test-cred", + UserAccess: "writer", OwnerTag: "user-bob@canonical.com", Life: life.Value(state.Alive.String()), Status: jujuparams.EntityStatus{ @@ -425,20 +382,7 @@ func TestModelFromJujuModelInfo(t *testing.T) { Owner: *i, }, OwnerIdentityName: "bob@canonical.com", - Type: "iaas", - IsController: false, - DefaultSeries: "warty", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: now, - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, }) } @@ -463,15 +407,5 @@ func TestModelFromJujuModelUpdate(t *testing.T) { c.Assert(model, qt.DeepEquals, dbmodel.Model{ Name: "test-model", Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: now, - Valid: true, - }, - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, }) } diff --git a/internal/dbmodel/sql/postgres/1_14.sql b/internal/dbmodel/sql/postgres/1_14.sql index 5b782028a..b1d9a3dd0 100644 --- a/internal/dbmodel/sql/postgres/1_14.sql +++ b/internal/dbmodel/sql/postgres/1_14.sql @@ -1,4 +1,4 @@ --- 1_13.sql is a migration simplifies application offers +-- 1_14.sql is a migration simplifies application offers DROP INDEX IF EXISTS idx_application_offer_connections_deleted_at; DROP INDEX IF EXISTS idx_application_offer_remote_endpoints_deleted_at; DROP INDEX IF EXISTS idx_application_offer_remote_spaces_deleted_at; diff --git a/internal/dbmodel/sql/postgres/1_17.sql b/internal/dbmodel/sql/postgres/1_17.sql new file mode 100644 index 000000000..e0adc1eeb --- /dev/null +++ b/internal/dbmodel/sql/postgres/1_17.sql @@ -0,0 +1,6 @@ +-- 1_17.sql remove non essential fields from model. +ALTER TABLE models DROP COLUMN default_series, DROP COLUMN migration_controller_id, DROP COLUMN is_controller, DROP COLUMN cores, + DROP COLUMN machines, DROP COLUMN units, DROP COLUMN type, DROP COLUMN status_status, DROP COLUMN status_info, DROP COLUMN status_data, + DROP COLUMN status_since, DROP COLUMN status_version, DROP COLUMN sla_level, DROP COLUMN sla_owner; + +UPDATE versions SET major=1, minor=17 WHERE component='jimmdb'; diff --git a/internal/dbmodel/version.go b/internal/dbmodel/version.go index ce19910fa..72c39e0e4 100644 --- a/internal/dbmodel/version.go +++ b/internal/dbmodel/version.go @@ -20,7 +20,7 @@ const ( // Minor is the minor version of the model described in the dbmodel // package. It should be incremented for any change made to the // database model from database model in a released JIMM. - Minor = 16 + Minor = 17 ) type Version struct { diff --git a/internal/jimm/applicationoffer_test.go b/internal/jimm/applicationoffer_test.go index 39b2be394..a668e109c 100644 --- a/internal/jimm/applicationoffer_test.go +++ b/internal/jimm/applicationoffer_test.go @@ -2121,19 +2121,13 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: bob@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -2141,23 +2135,14 @@ models: access: admin - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 1.2.3 - name: model-2 - type: iaas uuid: 00000002-0000-0000-0000-000000000002 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -2165,9 +2150,6 @@ models: access: write - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 1.2.3 application-offers: - name: offer-1 url: test-offer-url diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go index a985e664d..feb54eeb3 100644 --- a/internal/jimm/controller.go +++ b/internal/jimm/controller.go @@ -629,52 +629,6 @@ func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerNa return errors.E(op, err) } - return importer.handleModelDeltas(ctx) -} - -func (m *modelImporter) handleModelDeltas(ctx context.Context) error { - const op = errors.Op("jimm.getModelDeltas") - - modelAPI, err := m.jimm.dialModel(ctx, &m.model.Controller, m.model.ResourceTag()) - if err != nil { - return errors.E(op, err) - } - defer modelAPI.Close() - - watcherID, err := modelAPI.WatchAll(ctx) - if err != nil { - return errors.E(op, err) - } - defer func() { - if err := modelAPI.ModelWatcherStop(ctx, watcherID); err != nil { - zapctx.Error(ctx, "failed to stop model watcher", zap.Error(err)) - } - }() - - deltas, err := modelAPI.ModelWatcherNext(ctx, watcherID) - if err != nil { - return errors.E(op, err) - } - - modelIDf := func(uuid string) *modelState { - if uuid == m.model.UUID.String { - return &modelState{ - id: m.model.ID, - machines: make(map[string]int64), - units: make(map[string]bool), - } - } - return nil - } - - w := &Watcher{ - Database: m.jimm.Database, - } - for _, d := range deltas { - if err := w.handleDelta(ctx, modelIDf, d); err != nil { - return errors.E(op, err) - } - } return nil } diff --git a/internal/jimm/controller_test.go b/internal/jimm/controller_test.go index d9dd21df0..474c92176 100644 --- a/internal/jimm/controller_test.go +++ b/internal/jimm/controller_test.go @@ -585,22 +585,7 @@ func TestImportModel(t *testing.T) { CloudCredential: dbmodel.CloudCredential{ Name: "test-credential", }, - Type: "test-type", - DefaultSeries: "test-series", - Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Info: "updated status message", - Since: sql.NullTime{ - Valid: true, - Time: now, - }, - Version: "1.2.3", - }, - SLA: dbmodel.SLA{ - Level: "1", - Owner: "me", - }, + Life: state.Alive.String(), }, }, { about: "model from local user imported", @@ -673,22 +658,7 @@ func TestImportModel(t *testing.T) { CloudCredential: dbmodel.CloudCredential{ Name: "test-credential", }, - Type: "test-type", - DefaultSeries: "test-series", - Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Info: "test-info", - Since: sql.NullTime{ - Valid: true, - Time: now, - }, - Version: "2.1.0", - }, - SLA: dbmodel.SLA{ - Level: "essential", - Owner: "local-user", - }, + Life: state.Alive.String(), }, }, { about: "new model owner is local user", @@ -909,31 +879,19 @@ func TestImportModel(t *testing.T) { CloudCredential: dbmodel.CloudCredential{ Name: "test-credential", }, - Type: "test-type", - DefaultSeries: "test-series", - Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "ok", - Info: "test-info", - Since: sql.NullTime{ - Valid: true, - Time: now, + Life: state.Alive.String(), + Offers: []dbmodel.ApplicationOffer{ + { + URL: "url1", + UUID: "00000001-0000-0000-0000-000000000001", + Name: "offer1", + }, + { + URL: "url2", + UUID: "00000001-0000-0000-0000-000000000002", + Name: "offer2", }, - Version: "2.1.0", - }, - SLA: dbmodel.SLA{ - Level: "essential", - Owner: "alice@canonical.com", }, - Offers: []dbmodel.ApplicationOffer{{ - URL: "url1", - UUID: "00000001-0000-0000-0000-000000000001", - Name: "offer1", - }, { - URL: "url2", - UUID: "00000001-0000-0000-0000-000000000002", - Name: "offer2", - }}, }, offers: []jujuparams.ApplicationOfferAdminDetailsV5{{ ApplicationOfferDetailsV5: jujuparams.ApplicationOfferDetailsV5{ @@ -956,21 +914,6 @@ func TestImportModel(t *testing.T) { c.Run(test.about, func(c *qt.C) { api := &jimmtest.API{ ModelInfo_: test.modelInfo, - ModelWatcherNext_: func(ctx context.Context, id string) ([]jujuparams.Delta, error) { - if id != test.about { - return nil, errors.E("incorrect id") - } - return test.deltas, nil - }, - ModelWatcherStop_: func(ctx context.Context, id string) error { - if id != test.about { - return errors.E("incorrect id") - } - return nil - }, - WatchAll_: func(context.Context) (string, error) { - return test.about, nil - }, ListApplicationOffers_: func(ctx context.Context, of []jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) { return test.offers, nil }, @@ -1244,19 +1187,12 @@ controllers: agent-version: 3.3 models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000003 controller: controller-1 - default-series: mantic cloud: test-cloud region: test-region-1 cloud-credential: test-cred owner: alice@canonical.com - life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -1264,19 +1200,13 @@ models: access: write - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 3.3 - name: model-2 - type: iaas uuid: 00000002-0000-0000-0000-000000000004 controller: controller-2 - default-series: mantic cloud: test-cloud region: test-region-1 cloud-credential: test-cred owner: alice@canonical.com - life: alive status: status: available info: "OK!" @@ -1288,9 +1218,6 @@ models: access: write - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 3.3 ` func TestInitiateMigration(t *testing.T) { diff --git a/internal/jimm/export_test.go b/internal/jimm/export_test.go index 1abcc4ef4..365b02cd0 100644 --- a/internal/jimm/export_test.go +++ b/internal/jimm/export_test.go @@ -24,10 +24,6 @@ var ( ResolveTag = resolveTag ) -func WatchController(w *Watcher, ctx context.Context, ctl *dbmodel.Controller) error { - return w.watchController(ctx, ctl) -} - func NewWatcherWithControllerUnavailableChan(db *db.Database, dialer Dialer, pubsub Publisher, testChannel chan error) *Watcher { return &Watcher{ Pubsub: pubsub, diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index a67e6ab80..07a4c12e3 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -349,13 +349,6 @@ type API interface { // AddCloud adds a new cloud. AddCloud(context.Context, names.CloudTag, jujuparams.Cloud, bool) error - // AllModelWatcherNext returns the next set of deltas from an - // all-model watcher. - AllModelWatcherNext(context.Context, string) ([]jujuparams.Delta, error) - - // AllModelWatcherStop stops an all-model watcher. - AllModelWatcherStop(context.Context, string) error - // ChangeModelCredential replaces cloud credential for a given model with the provided one. ChangeModelCredential(context.Context, names.ModelTag, names.CloudCredentialTag) error @@ -430,6 +423,9 @@ type API interface { // filter. ListApplicationOffers(context.Context, []jujuparams.OfferFilter) ([]jujuparams.ApplicationOfferAdminDetailsV5, error) + // ListModelSummaries lists models summaries + ListModelSummaries(context.Context, jujuparams.ModelSummariesRequest) (jujuparams.ModelSummaryResults, error) + // ModelInfo fetches a model's ModelInfo. ModelInfo(context.Context, *jujuparams.ModelInfo) error @@ -443,13 +439,6 @@ type API interface { // ModelSummaryWatcherStop stops a model summary watcher. ModelSummaryWatcherStop(context.Context, string) error - // ModelWatcherNext receives the next set of results from the model - // watcher with the given id. - ModelWatcherNext(ctx context.Context, id string) ([]jujuparams.Delta, error) - - // ModelWatcherStop stops the model watcher with the given id. - ModelWatcherStop(ctx context.Context, id string) error - // Offer creates a new application-offer. Offer(context.Context, crossmodel.OfferURL, jujuparams.AddApplicationOffer) error @@ -492,15 +481,9 @@ type API interface { // ValidateModelUpgrade validates that a model can be upgraded. ValidateModelUpgrade(context.Context, names.ModelTag, bool) error - // WatchAll creates a watcher that reports deltas for a specific model. - WatchAll(context.Context) (string, error) - // WatchAllModelSummaries creates a ModelSummaryWatcher. WatchAllModelSummaries(context.Context) (string, error) - // WatchAllModels creates a megawatcher. - WatchAllModels(context.Context) (string, error) - // ListFilesystems lists filesystems for desired machines. // If no machines provided, a list of all filesystems is returned. ListFilesystems(ctx context.Context, machines []string) ([]jujuparams.FilesystemDetailsListResult, error) diff --git a/internal/jimm/jimm_test.go b/internal/jimm/jimm_test.go index f57572b2b..478a29b07 100644 --- a/internal/jimm/jimm_test.go +++ b/internal/jimm/jimm_test.go @@ -361,19 +361,13 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -381,9 +375,6 @@ models: access: write - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 1.2.3 ` func TestRemoveController(t *testing.T) { @@ -489,19 +480,13 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -509,9 +494,6 @@ models: access: write - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 1.2.3 ` func TestRemoveAndAddController(t *testing.T) { @@ -566,10 +548,8 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 @@ -775,10 +755,8 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: myController - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 diff --git a/internal/jimm/model.go b/internal/jimm/model.go index e9ad44f99..e3c56e08d 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -9,6 +9,7 @@ import ( "math/rand" "sort" "strings" + "sync" "time" jujupermission "github.com/juju/juju/core/permission" @@ -742,16 +743,101 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model return j.mergeModelInfo(ctx, user, mi, m) } +// modelSummariesMap is a safe map to add records concurrently because the access is guarded by a Mutex. +// The read operations are not guarded because only inserts are done concurrently. +type modelSummariesMap struct { + mu sync.Mutex + modelSummaries map[string]jujuparams.ModelSummaryResult +} + +func (m *modelSummariesMap) addModelSummary(summary jujuparams.ModelSummaryResult) { + m.mu.Lock() + defer m.mu.Unlock() + if m.modelSummaries == nil { + m.modelSummaries = make(map[string]jujuparams.ModelSummaryResult) + } + m.modelSummaries[summary.Result.UUID] = summary +} + +// ModelSummaries returns the list of modelsummary the user has access to. +// It queries the controllers and then merge the info from the JIMM db. +func (j *JIMM) ModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) { + const op = errors.Op("jimm.ModelSummaries") + + modelSummariesSafeMap := modelSummariesMap{} + modelSummaryResults := []jujuparams.ModelSummaryResult{} + + var models []struct { + model *dbmodel.Model + userAccess jujuparams.UserAccessPermission + } + // we collect models belonging to the user and we extract the unique controllers. + var uniqueControllers []dbmodel.Controller + uniqueControllerMap := make(map[string]struct{}, 0) + err := j.ForEachUserModel(ctx, user, func(m *dbmodel.Model, uap jujuparams.UserAccessPermission) error { + models = append(models, struct { + model *dbmodel.Model + userAccess jujuparams.UserAccessPermission + }{model: m, userAccess: uap}) + if _, ok := uniqueControllerMap[m.Controller.UUID]; !ok { + uniqueControllers = append(uniqueControllers, m.Controller) + uniqueControllerMap[m.Controller.UUID] = struct{}{} + } + return nil + }) + if err != nil { + return jujuparams.ModelSummaryResults{}, errors.E(op, err) + } + + // we query the model summaries for each controller + err = j.forEachController(ctx, uniqueControllers, func(c *dbmodel.Controller, a API) error { + results, err := a.ListModelSummaries(ctx, jujuparams.ModelSummariesRequest{All: true}) + if err != nil { + return err + } + for _, res := range results.Results { + modelSummariesSafeMap.addModelSummary(res) + } + return nil + }) + if err != nil { + // we log the error and continue, because even if one controller is not reachable we are still able to fill the response. + zapctx.Error(ctx, "Error querying the controllers for model summaries", zap.Error(err)) + } + + // we map models to modelsummaries + for _, m := range models { + modelSummaryFromController, ok := modelSummariesSafeMap.modelSummaries[m.model.UUID.String] + modelSummaryResult := m.model.MergeModelSummaryFromController(modelSummaryFromController.Result, maskingControllerUUID, m.userAccess) + if modelSummaryFromController.Error != nil { + modelSummaryResults = append(modelSummaryResults, jujuparams.ModelSummaryResult{ + Result: &modelSummaryResult, + Error: modelSummaryFromController.Error, + }) + continue + } + if !ok { + // if model was not found in any controller we mark it as anavailable + modelSummaryResult.Status.Status = "unavailable" + } + modelSummaryResults = append(modelSummaryResults, jujuparams.ModelSummaryResult{ + Result: &modelSummaryResult, + }) + } + return jujuparams.ModelSummaryResults{ + Results: modelSummaryResults, + }, nil +} + // mergeModelInfo replaces fields on the juju model info object with // information from JIMM where JIMM specific information should be used. func (j *JIMM) mergeModelInfo(ctx context.Context, user *openfga.User, modelInfo *jujuparams.ModelInfo, jimmModel dbmodel.Model) (*jujuparams.ModelInfo, error) { const op = errors.Op("jimm.mergeModelInfo") zapctx.Info(ctx, string(op)) - jimmSummary := jimmModel.ToJujuModelSummary() - modelInfo.CloudCredentialTag = jimmSummary.CloudCredentialTag - modelInfo.ControllerUUID = jimmSummary.ControllerUUID - modelInfo.OwnerTag = jimmSummary.OwnerTag + modelInfo.CloudCredentialTag = jimmModel.CloudCredential.Tag().String() + modelInfo.ControllerUUID = jimmModel.Controller.UUID + modelInfo.OwnerTag = jimmModel.Owner.Tag().String() userAccess := make(map[string]string) @@ -1074,15 +1160,21 @@ func (j *JIMM) DestroyModel(ctx context.Context, user *openfga.User, mt names.Mo zapctx.Info(ctx, string(op)) err := j.doModelAdmin(ctx, user, mt, func(m *dbmodel.Model, api API) error { - if err := api.DestroyModel(ctx, mt, destroyStorage, force, maxWait, timeout); err != nil { - return err - } m.Life = state.Dying.String() if err := j.Database.UpdateModel(ctx, m); err != nil { - // If the database fails to update don't worry too much the - // monitor should catch it. zapctx.Error(ctx, "failed to store model change", zaputil.Error(err)) + return err + } + if err := api.DestroyModel(ctx, mt, destroyStorage, force, maxWait, timeout); err != nil { + zapctx.Error(ctx, "failed to call DestroyModel juju api", zaputil.Error(err)) + // this is a manual way of restoring the life state to alive if the JUJU api fails. + m.Life = state.Alive.String() + if uerr := j.Database.UpdateModel(ctx, m); uerr != nil { + zapctx.Error(ctx, "failed to store model change", zaputil.Error(uerr)) + } + return err } + return nil }) if err != nil { diff --git a/internal/jimm/model_cleanup.go b/internal/jimm/model_cleanup.go new file mode 100644 index 000000000..f4c040b07 --- /dev/null +++ b/internal/jimm/model_cleanup.go @@ -0,0 +1,53 @@ +// Copyright 2024 Canonical. + +package jimm + +import ( + "context" + "fmt" + + jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" + "github.com/juju/zaputil/zapctx" + + "github.com/canonical/jimm/v3/internal/dbmodel" + "github.com/canonical/jimm/v3/internal/errors" +) + +// CleanupDyingModels loops over dying models, contacting the respective controller. +// And deleting the model from our database if the error is `NotFound` which means the model was successfully deleted. +func (j *JIMM) CleanupDyingModels(ctx context.Context) error { + const op = errors.Op("jimm.CleanupDyingModels") + zapctx.Info(ctx, string(op)) + + err := j.DB().ForEachModel(ctx, func(m *dbmodel.Model) error { + if m.Life != state.Dying.String() { + return nil + } + // if the model is dying and not found by querying the controller we can assume it is dead. + // And safely delete the reference from our db. + api, err := j.dialController(ctx, &m.Controller) + if err != nil { + zapctx.Error(ctx, fmt.Sprintf("cannot dial controller %s: %s\n", m.Controller.UUID, err)) + return nil + } + if err := api.ModelInfo(ctx, &jujuparams.ModelInfo{UUID: m.UUID.String}); err != nil { + // Some versions of juju return unauthorized for models that cannot be found. + if errors.ErrorCode(err) == errors.CodeNotFound || errors.ErrorCode(err) == errors.CodeUnauthorized { + if err := j.DB().DeleteModel(ctx, m); err != nil { + zapctx.Error(ctx, fmt.Sprintf("cannot delete model %s: %s\n", m.UUID.String, err)) + } else { + return nil + } + } else { + zapctx.Error(ctx, fmt.Sprintf("cannot get ModelInfo for model %s: %s\n", m.UUID.String, err)) + return nil + } + } + return nil + }) + if err != nil { + return errors.E(op, err) + } + return nil +} diff --git a/internal/jimm/model_cleanup_test.go b/internal/jimm/model_cleanup_test.go new file mode 100644 index 000000000..3241a9dbd --- /dev/null +++ b/internal/jimm/model_cleanup_test.go @@ -0,0 +1,184 @@ +// Copyright 2024 Canonical. + +package jimm_test + +import ( + "context" + "database/sql" + "testing" + "time" + + qt "github.com/frankban/quicktest" + "github.com/frankban/quicktest/qtsuite" + jujuparams "github.com/juju/juju/rpc/params" + "github.com/juju/juju/state" + "github.com/juju/names/v5" + + "github.com/canonical/jimm/v3/internal/dbmodel" + "github.com/canonical/jimm/v3/internal/errors" + "github.com/canonical/jimm/v3/internal/jimm" + "github.com/canonical/jimm/v3/internal/openfga" + "github.com/canonical/jimm/v3/internal/testutils/jimmtest" +) + +const modelPollerTestEnv = `clouds: +- name: test-cloud + type: test-provider + regions: + - name: test-cloud-region +cloud-credentials: +- owner: alice@canonical.com + name: cred-1 + cloud: test-cloud +controllers: +- name: controller-1 + uuid: 00000001-0000-0000-0000-000000000001 + cloud: test-cloud + region: test-cloud-region +models: +- name: model-1 + uuid: 00000002-0000-0000-0000-000000000001 + controller: controller-1 + cloud: test-cloud + region: test-cloud-region + cloud-credential: cred-1 + owner: alice@canonical.com + life: alive + users: + - user: alice@canonical.com + access: admin + - user: bob@canonical.com + access: admin +- name: model-2 + uuid: 00000002-0000-0000-0000-000000000002 + controller: controller-1 + cloud: test-cloud + region: test-cloud-region + cloud-credential: cred-1 + owner: alice@canonical.com + life: alive + users: + - user: alice@canonical.com + access: admin + - user: bob@canonical.com + access: write +- name: model-3 + uuid: 00000002-0000-0000-0000-000000000003 + controller: controller-1 + cloud: test-cloud + region: test-cloud-region + cloud-credential: cred-1 + owner: alice@canonical.com + life: alive + users: + - user: alice@canonical.com + access: admin + - user: bob@canonical.com + access: write +users: +- username: alice@canonical.com + controller-access: superuser +` + +type modelCleanupSuite struct { + jimm *jimm.JIMM + jimmAdmin *openfga.User + ofgaClient *openfga.OFGAClient + env *jimmtest.Environment +} + +func (s *modelCleanupSuite) Init(c *qt.C) { + ctx := context.Background() + var err error + s.ofgaClient, _, _, err = jimmtest.SetupTestOFGAClient(c.Name()) + c.Assert(err, qt.IsNil) + s.jimm = jimmtest.NewJIMM(c, nil) + err = s.jimm.Database.Migrate(ctx, false) + c.Assert(err, qt.IsNil) + s.jimmAdmin, err = s.jimm.GetUser(ctx, "alice@canonical.com") + c.Assert(err, qt.IsNil) + + s.env = jimmtest.ParseEnvironment(c, modelPollerTestEnv) + s.env.PopulateDBAndPermissions(c, s.jimm.ResourceTag(), s.jimm.Database, s.ofgaClient) +} + +func (s *modelCleanupSuite) TestPollModelsDying(c *qt.C) { + ctx := context.Background() + + s.jimm.Dialer = &jimmtest.Dialer{ + API: &jimmtest.API{ + ModelInfo_: func(ctx context.Context, mi *jujuparams.ModelInfo) error { + switch mi.UUID { + case s.env.Models[0].UUID: + return errors.E(errors.CodeNotFound) + case s.env.Models[1].UUID: + return nil + default: + return errors.E("new error") + } + }, + DestroyModel_: func(ctx context.Context, mt names.ModelTag, b1, b2 *bool, d1, d2 *time.Duration) error { + return nil + }, + }, + } + err := s.jimm.DestroyModel(ctx, s.jimmAdmin, names.NewModelTag(s.env.Models[0].UUID), nil, nil, nil, nil) + c.Assert(err, qt.IsNil) + + // test + err = s.jimm.CleanupDyingModels(ctx) + c.Assert(err, qt.IsNil) + + model := dbmodel.Model{ + UUID: sql.NullString{ + String: s.env.Models[0].UUID, + Valid: true, + }, + } + err = s.jimm.DB().GetModel(ctx, &model) + c.Assert(err, qt.ErrorMatches, "model not found") + + model = dbmodel.Model{ + UUID: sql.NullString{ + String: s.env.Models[1].UUID, + Valid: true, + }, + } + err = s.jimm.DB().GetModel(ctx, &model) + c.Assert(err, qt.IsNil) +} + +func (s *modelCleanupSuite) TestPollModelsDyingControllerErrors(c *qt.C) { + ctx := context.Background() + + s.jimm.Dialer = &jimmtest.Dialer{ + API: &jimmtest.API{ + ModelInfo_: func(ctx context.Context, mi *jujuparams.ModelInfo) error { + return errors.E("controller not available") + }, + DestroyModel_: func(ctx context.Context, mt names.ModelTag, b1, b2 *bool, d1, d2 *time.Duration) error { + return nil + }, + }, + } + err := s.jimm.DestroyModel(ctx, s.jimmAdmin, names.NewModelTag(s.env.Models[0].UUID), nil, nil, nil, nil) + c.Assert(err, qt.IsNil) + + // test + err = s.jimm.CleanupDyingModels(ctx) + c.Assert(err, qt.IsNil) + + model := dbmodel.Model{ + UUID: sql.NullString{ + String: s.env.Models[0].UUID, + Valid: true, + }, + } + err = s.jimm.DB().GetModel(ctx, &model) + c.Assert(err, qt.IsNil) + c.Assert(model.Life, qt.Equals, state.Dying.String()) +} + +func TestDyingModelsCleanup(t *testing.T) { + qtsuite.Run(qt.New(t), &modelCleanupSuite{}) +} diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index 39b82d886..cd33831bd 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -14,6 +14,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/juju/juju/core/life" + "github.com/juju/juju/rpc/params" jujuparams "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" "github.com/juju/names/v5" @@ -243,10 +244,6 @@ users: AuthType: "empty", }, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "started", - Info: "running a test", - }, }, }, { name: "CreateModelWithoutCloudRegion", @@ -352,10 +349,6 @@ users: AuthType: "empty", }, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "started", - Info: "running a test", - }, }, }, { name: "CreateModelWithCloud", @@ -445,10 +438,6 @@ users: AuthType: "empty", }, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "started", - Info: "running a test", - }, }, }, { name: "CreateModelInOtherNamespaceAsSuperUser", @@ -545,10 +534,6 @@ users: AuthType: "empty", }, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "started", - Info: "running a test", - }, }, }, { name: "CreateModelInOtherNamespace", @@ -963,10 +948,6 @@ users: AuthType: "empty", }, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "started", - Info: "running a test", - }, }, }, { name: "CreateModelWithImplicitCloudAndMultipleClouds", @@ -1165,22 +1146,13 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z - sla: - level: unsupported - agent-version: 1.2.3 ` func TestGetModel(t *testing.T) { @@ -1221,22 +1193,13 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z - sla: - level: unsupported - agent-version: 1.2.3 users: - user: alice@canonical.com access: admin @@ -1469,10 +1432,8 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 @@ -1593,44 +1554,26 @@ controllers: region: test-cloud-region models: - name: model-1 - type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin - user: bob@canonical.com access: admin - sla: - level: unsupported - agent-version: 1.2.3 - cores: 3 - machines: 2 - units: 4 - name: model-2 - type: iaas uuid: 00000002-0000-0000-0000-000000000002 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -1638,53 +1581,30 @@ models: access: write sla: level: unsupported - agent-version: 1.2.3 - cores: 3 - machines: 2 - name: model-3 - type: iaas uuid: 00000002-0000-0000-0000-000000000003 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin - sla: - level: unsupported - agent-version: 1.2.3 - cores: 3 - machines: 2 - name: model-4 - type: iaas uuid: 00000002-0000-0000-0000-000000000004 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin - user: bob@canonical.com access: read - sla: - level: unsupported - agent-version: 1.2.3 users: - username: alice@canonical.com controller-access: superuser @@ -1704,8 +1624,7 @@ func TestForEachUserModel(t *testing.T) { var res []jujuparams.ModelSummaryResult err := j.ForEachUserModel(ctx, user, func(m *dbmodel.Model, access jujuparams.UserAccessPermission) error { - s := m.ToJujuModelSummary() - s.UserAccess = access + s := m.MergeModelSummaryFromController(nil, "", access) res = append(res, jujuparams.ModelSummaryResult{Result: &s}) return nil }) @@ -1714,103 +1633,40 @@ func TestForEachUserModel(t *testing.T) { Result: &jujuparams.ModelSummary{ Name: "model-1", UUID: "00000002-0000-0000-0000-000000000001", - Type: "iaas", ControllerUUID: "00000001-0000-0000-0000-000000000001", ProviderType: "test-provider", - DefaultSeries: "warty", CloudTag: names.NewCloudTag("test-cloud").String(), CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), Life: life.Value(state.Alive.String()), - Status: jujuparams.EntityStatus{ - Status: "available", - Info: "OK!", - Since: newDate(2020, 02, 20, 20, 02, 20, 0, time.UTC), - }, - UserAccess: "admin", - Counts: []jujuparams.ModelEntityCount{{ - Entity: "machines", - Count: 2, - }, { - Entity: "cores", - Count: 3, - }, { - Entity: "units", - Count: 4, - }}, - SLA: &jujuparams.ModelSLAInfo{ - Level: "unsupported", - }, - AgentVersion: newVersion("1.2.3"), + UserAccess: "admin", }, }, { Result: &jujuparams.ModelSummary{ Name: "model-2", UUID: "00000002-0000-0000-0000-000000000002", - Type: "iaas", ControllerUUID: "00000001-0000-0000-0000-000000000001", ProviderType: "test-provider", - DefaultSeries: "warty", CloudTag: names.NewCloudTag("test-cloud").String(), CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), Life: life.Value(state.Alive.String()), - Status: jujuparams.EntityStatus{ - Status: "available", - Info: "OK!", - Since: newDate(2020, 02, 20, 20, 02, 20, 0, time.UTC), - }, - UserAccess: "write", - Counts: []jujuparams.ModelEntityCount{{ - Entity: "machines", - Count: 2, - }, { - Entity: "cores", - Count: 3, - }, { - Entity: "units", - Count: 0, - }}, - SLA: &jujuparams.ModelSLAInfo{ - Level: "unsupported", - }, - AgentVersion: newVersion("1.2.3"), + UserAccess: "write", }, }, { Result: &jujuparams.ModelSummary{ Name: "model-4", UUID: "00000002-0000-0000-0000-000000000004", - Type: "iaas", ControllerUUID: "00000001-0000-0000-0000-000000000001", ProviderType: "test-provider", - DefaultSeries: "warty", CloudTag: names.NewCloudTag("test-cloud").String(), CloudRegion: "test-cloud-region", CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), OwnerTag: names.NewUserTag("alice@canonical.com").String(), Life: life.Value(state.Alive.String()), - Status: jujuparams.EntityStatus{ - Status: "available", - Info: "OK!", - Since: newDate(2020, 02, 20, 20, 02, 20, 0, time.UTC), - }, - UserAccess: "read", - Counts: []jujuparams.ModelEntityCount{{ - Entity: "machines", - Count: 0, - }, { - Entity: "cores", - Count: 0, - }, { - Entity: "units", - Count: 0, - }}, - SLA: &jujuparams.ModelSLAInfo{ - Level: "unsupported", - }, - AgentVersion: newVersion("1.2.3"), + UserAccess: "read", }, }}) } @@ -1852,6 +1708,278 @@ func TestForEachModel(t *testing.T) { }) } +const modelSummariesTestEnv = `clouds: +- name: test-cloud + type: test-provider + regions: + - name: test-cloud-region +cloud-credentials: +- owner: alice@canonical.com + name: cred-1 + cloud: test-cloud +controllers: +- name: controller-1 + uuid: 00000001-0000-0000-0000-000000000001 + cloud: test-cloud + region: test-cloud-region +models: +- name: model-1 + uuid: 00000002-0000-0000-0000-000000000001 + controller: controller-1 + cloud: test-cloud + region: test-cloud-region + cloud-credential: cred-1 + owner: alice@canonical.com + life: alive + users: + - user: alice@canonical.com + access: admin + - user: bob@canonical.com + access: admin +- name: model-2 + uuid: 00000002-0000-0000-0000-000000000002 + controller: controller-1 + cloud: test-cloud + region: test-cloud-region + cloud-credential: cred-1 + owner: alice@canonical.com + life: alive + users: + - user: alice@canonical.com + access: admin + - user: bob@canonical.com + access: write +users: +- username: alice@canonical.com + controller-access: superuser +` + +func TestModelSummaries(t *testing.T) { + c := qt.New(t) + ctx := context.Background() + + j := jimmtest.NewJIMM(c, nil) + + err := j.Database.Migrate(ctx, false) + c.Assert(err, qt.IsNil) + + env := jimmtest.ParseEnvironment(c, modelSummariesTestEnv) + env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, j.OpenFGAClient) + + dbUser := env.User("alice@canonical.com").DBObject(c, j.Database) + alice := openfga.NewUser(&dbUser, j.OpenFGAClient) + + tests := []struct { + description string + controllerAPISummaries []params.ModelSummaryResult + expectedSummaries []params.ModelSummaryResult + expectedSummariesSize int + }{ + { + description: "info from controller, so all models available", + controllerAPISummaries: []params.ModelSummaryResult{ + { + Result: ¶ms.ModelSummary{ + Name: "model-1", + UUID: "00000002-0000-0000-0000-000000000001", + Type: "iaas", + ControllerUUID: "00000002-0000-0000-0000-000000000001", + IsController: false, + DefaultSeries: "series-1", + Life: "alive", + Status: params.EntityStatus{ + Status: "available", + }, + UserAccess: "testtest", + }, + }, + { + Result: ¶ms.ModelSummary{ + Name: "model-2", + UUID: "00000002-0000-0000-0000-000000000002", + Type: "iaas", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + DefaultSeries: "series-2", + Life: "alive", + Status: params.EntityStatus{ + Status: "available", + }, + UserAccess: "admin", + }, + }, + }, + expectedSummaries: []params.ModelSummaryResult{ + { + Result: ¶ms.ModelSummary{ + Name: "model-1", + UUID: "00000002-0000-0000-0000-000000000001", + Type: "iaas", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + ProviderType: "test-provider", + DefaultSeries: "series-1", + CloudTag: "cloud-test-cloud", + CloudRegion: "test-cloud-region", + CloudCredentialTag: "cloudcred-test-cloud_alice@canonical.com_cred-1", + OwnerTag: "user-alice@canonical.com", + Life: "alive", + Status: params.EntityStatus{ + Status: "available", + }, + UserAccess: "admin", + }, + Error: (*params.Error)(nil), + }, + { + Result: ¶ms.ModelSummary{ + Name: "model-2", + UUID: "00000002-0000-0000-0000-000000000002", + Type: "iaas", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + ProviderType: "test-provider", + DefaultSeries: "series-2", + CloudTag: "cloud-test-cloud", + CloudRegion: "test-cloud-region", + CloudCredentialTag: "cloudcred-test-cloud_alice@canonical.com_cred-1", + OwnerTag: "user-alice@canonical.com", + Life: "alive", + Status: params.EntityStatus{ + Status: "available", + }, + UserAccess: "admin", + }, + }, + }, + expectedSummariesSize: 2, + }, + { + description: "partial info from controller, so one model is not available and info are not filled in.", + controllerAPISummaries: []params.ModelSummaryResult{ + { + Result: ¶ms.ModelSummary{ + Name: "model-1", + UUID: "00000002-0000-0000-0000-000000000001", + Type: "iaas", + ControllerUUID: "00000002-0000-0000-0000-000000000001", + IsController: false, + DefaultSeries: "", + Life: "alive", + Status: params.EntityStatus{ + Status: "available", + }, + UserAccess: "testtest", + }, + }, + }, + expectedSummaries: []params.ModelSummaryResult{ + { + Result: ¶ms.ModelSummary{ + Name: "model-1", + UUID: "00000002-0000-0000-0000-000000000001", + Type: "iaas", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + ProviderType: "test-provider", + DefaultSeries: "", + CloudTag: "cloud-test-cloud", + CloudRegion: "test-cloud-region", + CloudCredentialTag: "cloudcred-test-cloud_alice@canonical.com_cred-1", + OwnerTag: "user-alice@canonical.com", + Life: "alive", + Status: params.EntityStatus{ + Status: "available", + }, + UserAccess: "admin", + }, + Error: (*params.Error)(nil), + }, + { + Result: ¶ms.ModelSummary{ + Name: "model-2", + UUID: "00000002-0000-0000-0000-000000000002", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + ProviderType: "test-provider", + CloudTag: "cloud-test-cloud", + CloudRegion: "test-cloud-region", + CloudCredentialTag: "cloudcred-test-cloud_alice@canonical.com_cred-1", + OwnerTag: "user-alice@canonical.com", + Life: "alive", + Status: params.EntityStatus{ + Status: "unavailable", + }, + UserAccess: "admin", + }, + }, + }, + expectedSummariesSize: 2, + }, + { + description: "no info from controller, so all models unavailable", + expectedSummaries: []params.ModelSummaryResult{ + { + Result: ¶ms.ModelSummary{ + Name: "model-1", + UUID: "00000002-0000-0000-0000-000000000001", + Type: "", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + ProviderType: "test-provider", + DefaultSeries: "", + CloudTag: "cloud-test-cloud", + CloudRegion: "test-cloud-region", + CloudCredentialTag: "cloudcred-test-cloud_alice@canonical.com_cred-1", + OwnerTag: "user-alice@canonical.com", + Life: "alive", + Status: params.EntityStatus{ + Status: "unavailable", + }, + UserAccess: "admin", + }, + }, + { + Result: ¶ms.ModelSummary{ + Name: "model-2", + UUID: "00000002-0000-0000-0000-000000000002", + Type: "", + ControllerUUID: "00000001-0000-0000-0000-000000000001", + IsController: false, + ProviderType: "test-provider", + DefaultSeries: "", + CloudTag: "cloud-test-cloud", + CloudRegion: "test-cloud-region", + CloudCredentialTag: "cloudcred-test-cloud_alice@canonical.com_cred-1", + OwnerTag: "user-alice@canonical.com", + Life: "alive", + Status: params.EntityStatus{ + Status: "unavailable", + }, + UserAccess: "admin", + }, + }, + }, + expectedSummariesSize: 2, + }, + } + for _, t := range tests { + c.Run(t.description, func(c *qt.C) { + j.Dialer = &jimmtest.Dialer{ + API: &jimmtest.API{ + ListModelSummaries_: func(ctx context.Context, msr jujuparams.ModelSummariesRequest) (jujuparams.ModelSummaryResults, error) { + return jujuparams.ModelSummaryResults{Results: t.controllerAPISummaries}, nil + }, + }, + } + summaries, err := j.ModelSummaries(ctx, alice, "") + c.Check(err, qt.IsNil) + c.Check(summaries.Results, qt.HasLen, t.expectedSummariesSize) + c.Check(summaries.Results, qt.DeepEquals, t.expectedSummaries) + }) + } +} + const grantModelAccessTestEnv = `clouds: - name: test-cloud type: test-provider @@ -2889,6 +3017,7 @@ var destroyModelTests = []struct { timeout *time.Duration expectError string expectErrorCode errors.Code + expectedLife string }{{ name: "NotFound", env: destroyModelTestEnv, @@ -2930,30 +3059,34 @@ var destroyModelTests = []struct { force: newBool(false), maxWait: newDuration(time.Second), timeout: newDuration(time.Second), + expectedLife: "dying", }, { name: "SuperuserSuccess", env: destroyModelTestEnv, destroyModel: func(_ context.Context, _ names.ModelTag, _, _ *bool, _, _ *time.Duration) error { return nil }, - username: "charlie@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", + username: "charlie@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + expectedLife: "dying", }, { - name: "DialError", - env: destroyModelTestEnv, - dialError: errors.E("dial error"), - username: "alice@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", - expectError: `dial error`, + name: "DialError", + env: destroyModelTestEnv, + dialError: errors.E("dial error"), + username: "alice@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + expectError: `dial error`, + expectedLife: "alive", }, { name: "APIError", env: destroyModelTestEnv, destroyModel: func(_ context.Context, _ names.ModelTag, _, _ *bool, _, _ *time.Duration) error { return errors.E("api error") }, - username: "charlie@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", - expectError: `api error`, + username: "charlie@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + expectError: `api error`, + expectedLife: "alive", }} func TestDestroyModel(t *testing.T) { @@ -2987,18 +3120,20 @@ func TestDestroyModel(t *testing.T) { if test.expectErrorCode != "" { c.Check(errors.ErrorCode(err), qt.Equals, test.expectErrorCode) } - return + } else { + c.Assert(err, qt.IsNil) } - c.Assert(err, qt.IsNil) - m := dbmodel.Model{ - UUID: sql.NullString{ - String: test.uuid, - Valid: true, - }, + if test.expectedLife != "" { + m := dbmodel.Model{ + UUID: sql.NullString{ + String: test.uuid, + Valid: true, + }, + } + err = j.Database.GetModel(ctx, &m) + c.Assert(err, qt.IsNil) + c.Assert(m.Life, qt.Equals, test.expectedLife) } - err = j.Database.GetModel(ctx, &m) - c.Assert(err, qt.IsNil) - c.Check(m.Life, qt.Equals, state.Dying.String()) }) } } diff --git a/internal/jimm/utils.go b/internal/jimm/utils.go index c97cdf225..1e8387178 100644 --- a/internal/jimm/utils.go +++ b/internal/jimm/utils.go @@ -68,13 +68,3 @@ func (j *JIMM) dialController(ctx context.Context, ctl *dbmodel.Controller) (API } return api, nil } - -// dialModel dials a model. -func (j *JIMM) dialModel(ctx context.Context, ctl *dbmodel.Controller, mt names.ModelTag) (API, error) { - api, err := j.dial(ctx, ctl, mt) - if err != nil { - zapctx.Error(ctx, "failed to dial the controller", zaputil.Error(err)) - return nil, err - } - return api, nil -} diff --git a/internal/jimm/watcher.go b/internal/jimm/watcher.go index e3a37b15c..e4f9397e7 100644 --- a/internal/jimm/watcher.go +++ b/internal/jimm/watcher.go @@ -7,8 +7,6 @@ import ( "database/sql" "time" - jujuparams "github.com/juju/juju/rpc/params" - "github.com/juju/juju/state" "github.com/juju/names/v5" "github.com/juju/zaputil/zapctx" "go.uber.org/zap" @@ -16,7 +14,6 @@ import ( "github.com/canonical/jimm/v3/internal/db" "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" - "github.com/canonical/jimm/v3/internal/servermon" ) // Publisher defines the interface used by the Watcher @@ -42,49 +39,6 @@ type Watcher struct { deltaProcessedChan chan bool } -// Watch starts the watcher which connects to all known controllers and -// monitors them for updates. Watch polls the database at the given -// interval to find any new controllers to watch. Watch blocks until either -// the given context is closed, or there is an error querying the database. -func (w *Watcher) Watch(ctx context.Context, interval time.Duration) error { - const op = errors.Op("jimm.Watch") - - r := newRunner() - // Ensure that all started goroutines are completed before we return. - defer r.wait() - - // Ensure that if the watcher stops because of a database error all - // the controller connections get closed. - ctx, cancel := context.WithCancel(ctx) - defer cancel() - - ticker := time.NewTicker(interval) - defer ticker.Stop() - for { - err := w.Database.ForEachController(ctx, func(ctl *dbmodel.Controller) error { - ctx := zapctx.WithFields(ctx, zap.String("controller", ctl.Name)) - r.run(ctl.Name, func() { - zapctx.Info(ctx, "starting controller watcher") - err := w.watchController(ctx, ctl) - zapctx.Error(ctx, "controller watcher stopped", zap.Error(err)) - }) - return nil - }) - if err != nil { - // Ignore temporary database errors. - if errors.ErrorCode(err) != errors.CodeDatabaseLocked { - return errors.E(op, err) - } - zapctx.Warn(ctx, "temporary error polling for controllers", zap.Error(err)) - } - select { - case <-ctx.Done(): - return ctx.Err() - case <-ticker.C: - } - } -} - // WatchAllModelSummaries starts the watcher which connects to all known // controllers and monitors them for model summary updates. // WatchAllModelSummaries polls the database at the given @@ -165,201 +119,6 @@ func (w *Watcher) dialController(ctx context.Context, ctl *dbmodel.Controller) ( return api, nil } -// A modelState holds the in-memory state of a model for the watcher. -type modelState struct { - // id is the database id of the model. - id uint - changed bool - - // machines maps the Id of all the machines that have been seen to - // the number of cores reported. - machines map[string]int64 - - // units stores the ids of all units that have been seen. - units map[string]bool -} - -func (w *Watcher) checkControllerModels(ctx context.Context, ctl *dbmodel.Controller, checks ...func(*dbmodel.Model) error) (map[string]*modelState, error) { - const op = errors.Op("jimm.checkControllerModels") - - // modelIDs contains the set of models running on the - // controller that JIMM is interested in. - modelStates := make(map[string]*modelState) - // find all the models we expect to get deltas from initially. - err := w.Database.ForEachControllerModel(ctx, ctl, func(m *dbmodel.Model) error { - // models without a UUID are currently being initialised - // and we don't want to check for those yet. - if !m.UUID.Valid { - return nil - } - - for _, check := range checks { - err := check(m) - if err != nil { - return errors.E(op, err) - } - } - modelStates[m.UUID.String] = &modelState{ - id: m.ID, - machines: make(map[string]int64), - units: make(map[string]bool), - } - return nil - }) - if err != nil { - return nil, errors.E(op, err) - } - return modelStates, nil -} - -func (w *Watcher) deltaProcessedNotification() { - if w.deltaProcessedChan != nil { - select { - case w.deltaProcessedChan <- true: - default: - } - } -} - -// watchController connects to the given controller and watches for model -// changes on the controller. -// -// nolint:gocognit // We ignore watch as watchers are removed in Juju 4.0. -func (w *Watcher) watchController(ctx context.Context, ctl *dbmodel.Controller) error { - const op = errors.Op("jimm.watchController") - - // connect to the controller - api, err := w.dialController(ctx, ctl) - if err != nil { - return errors.E(op, err) - } - defer api.Close() - // start the all watcher - id, err := api.WatchAllModels(ctx) - if err != nil { - return errors.E(op, err) - } - defer func() { - if err := api.AllModelWatcherStop(ctx, id); err != nil { - zapctx.Error(ctx, "failed to stop all model watcher", zap.Error(err)) - } - }() - - checkDyingModel := func(m *dbmodel.Model) error { - if m.Life == state.Dying.String() || m.Life == state.Dead.String() { - // models that were in the dying state may no - // longer be on the controller, check if it should - // be immediately deleted. - mi := jujuparams.ModelInfo{ - UUID: m.UUID.String, - } - if err := api.ModelInfo(ctx, &mi); err != nil { - // Some versions of juju return unauthorized for models that cannot be found. - if errors.ErrorCode(err) == errors.CodeNotFound || errors.ErrorCode(err) == errors.CodeUnauthorized { - if err := w.Database.DeleteModel(ctx, m); err != nil { - return errors.E(op, err) - } else { - return nil - } - } else { - return errors.E(op, err) - } - } - } - return nil - } - - // modelStates contains the set of models running on the - // controller that JIMM is interested in. The function also - // check for any dying models and deletes them where necessary. - modelStates, err := w.checkControllerModels(ctx, ctl, checkDyingModel) - if err != nil { - return errors.E(op, err) - } - - modelStatef := func(uuid string) *modelState { - state, ok := modelStates[uuid] - if ok { - return state - } - m := dbmodel.Model{ - UUID: sql.NullString{ - String: uuid, - Valid: true, - }, - ControllerID: ctl.ID, - } - err := w.Database.GetModel(ctx, &m) - switch { - case err == nil: - st := modelState{ - id: m.ID, - machines: make(map[string]int64), - units: make(map[string]bool), - } - modelStates[uuid] = &st - case errors.ErrorCode(err) == errors.CodeNotFound: - modelStates[uuid] = nil - default: - zapctx.Error(ctx, "cannot get model", zap.Error(err)) - } - return modelStates[uuid] - } - - for { - // wait for updates from the all watcher. - deltas, err := api.AllModelWatcherNext(ctx, id) - if err != nil { - return errors.E(op, err) - } - servermon.MonitorDeltasReceivedCount.WithLabelValues(ctl.UUID).Add(float64(len(deltas))) - for _, d := range deltas { - eid := d.Entity.EntityId() - ctx := zapctx.WithFields(ctx, zap.String("model-uuid", eid.ModelUUID), zap.String("kind", eid.Kind), zap.String("id", eid.Id)) - zapctx.Debug(ctx, "processing delta") - if err := w.handleDelta(ctx, modelStatef, d); err != nil { - return errors.E(op, err) - } - } - for k, v := range modelStates { - if v == nil { - // If we have cached not to process a model - // remove it so we check again next time. - delete(modelStates, k) - continue - } - if v.changed { - v.changed = false - // Update changed model. - err := w.Database.Transaction(func(tx *db.Database) error { - m := dbmodel.Model{ - ID: v.id, - } - if err := tx.GetModel(ctx, &m); err != nil { - return err - } - var machines, cores int64 - for _, n := range v.machines { - machines++ - cores += n - } - m.Cores = cores - m.Machines = machines - m.Units = int64(len(v.units)) - if err := tx.UpdateModel(ctx, &m); err != nil { - return err - } - return nil - }) - if err != nil { - zapctx.Error(ctx, "cannot get model for update", zap.Error(err)) - continue - } - } - } - } -} - // watchAllModelSummaries connects to the given controller and watches the // summary updates. func (w *Watcher) watchAllModelSummaries(ctx context.Context, ctl *dbmodel.Controller) error { @@ -387,36 +146,6 @@ func (w *Watcher) watchAllModelSummaries(ctx context.Context, ctl *dbmodel.Contr } }() - // modelIDs contains the set of models running on the - // controller that JIMM is interested in. - modelStates, err := w.checkControllerModels(ctx, ctl) - if err != nil { - return errors.E(op, err) - } - - modelIDf := func(uuid string) uint { - state, ok := modelStates[uuid] - if ok { - return state.id - } - m := dbmodel.Model{ - UUID: sql.NullString{ - String: uuid, - Valid: true, - }, - ControllerID: ctl.ID, - } - err := w.Database.GetModel(ctx, &m) - if err == nil || errors.ErrorCode(err) == errors.CodeNotFound { - modelStates[uuid] = &modelState{ - id: m.ID, - } - return m.ID - } - zapctx.Error(ctx, "cannot get model", zap.Error(err)) - return 0 - } - for { select { case <-ctx.Done(): @@ -430,12 +159,18 @@ func (w *Watcher) watchAllModelSummaries(ctx context.Context, ctl *dbmodel.Contr } // Sanitize the model abstracts. for _, summary := range modelSummaries { - modelID := modelIDf(summary.UUID) - if modelID == 0 { - // skip unknown models + m := dbmodel.Model{ + UUID: sql.NullString{ + String: summary.UUID, + Valid: true, + }, + ControllerID: ctl.ID, + } + err := w.Database.GetModel(ctx, &m) + if err != nil { + // skip summaries for model not present in JIMM's db continue } - summary := summary admins := make([]string, 0, len(summary.Admins)) for _, admin := range summary.Admins { if names.NewUserTag(admin).IsLocal() { @@ -449,90 +184,3 @@ func (w *Watcher) watchAllModelSummaries(ctx context.Context, ctl *dbmodel.Contr } } } - -func (w *Watcher) handleDelta(ctx context.Context, modelIDf func(string) *modelState, d jujuparams.Delta) error { - defer w.deltaProcessedNotification() - eid := d.Entity.EntityId() - state := modelIDf(eid.ModelUUID) - if state == nil { - return nil - } - switch eid.Kind { - case "machine": - if d.Removed { - state.changed = true - delete(state.machines, eid.Id) - return nil - } - var cores int64 - machine := d.Entity.(*jujuparams.MachineInfo) - if machine.HardwareCharacteristics != nil && machine.HardwareCharacteristics.CpuCores != nil { - //nolint:gosec // We expect cpu cores to fit into int64. - cores = int64(*machine.HardwareCharacteristics.CpuCores) - } - sCores, ok := state.machines[eid.Id] - if !ok || sCores != cores { - state.machines[eid.Id] = cores - state.changed = true - } - case "model": - model := dbmodel.Model{ - ID: state.id, - } - if d.Removed { - return w.deleteModel(ctx, &model) - } - return w.updateModel(ctx, &model, d.Entity.(*jujuparams.ModelUpdate)) - case "unit": - if d.Removed { - state.changed = true - delete(state.units, eid.Id) - return nil - } - if !state.units[eid.Id] { - state.changed = true - state.units[eid.Id] = true - } - default: - } - return nil -} - -func (w *Watcher) deleteModel(ctx context.Context, model *dbmodel.Model) error { - const op = errors.Op("watcher.deleteModel") - - err := w.Database.Transaction(func(db *db.Database) error { - if err := db.GetModel(ctx, model); err != nil { - if errors.ErrorCode(err) != errors.CodeNotFound { - return err - } - } - if !(model.Life == state.Dying.String() || model.Life == state.Dead.String()) { - // If the model hasn't been marked as dying, don't remove it. - return nil - } - return db.DeleteModel(ctx, model) - }) - if err != nil { - return errors.E(op, err) - } - return nil -} - -func (w *Watcher) updateModel(ctx context.Context, model *dbmodel.Model, info *jujuparams.ModelUpdate) error { - const op = errors.Op("watcher.updateModel") - - err := w.Database.Transaction(func(db *db.Database) error { - if err := db.GetModel(ctx, model); err != nil { - if errors.ErrorCode(err) != errors.CodeNotFound { - return err - } - } - model.FromJujuModelUpdate(*info) - return db.UpdateModel(ctx, model) - }) - if err != nil { - return errors.E(op, err) - } - return nil -} diff --git a/internal/jimm/watcher_test.go b/internal/jimm/watcher_test.go index a6cf2478d..80e24af81 100644 --- a/internal/jimm/watcher_test.go +++ b/internal/jimm/watcher_test.go @@ -11,10 +11,7 @@ import ( "time" qt "github.com/frankban/quicktest" - "github.com/juju/juju/core/instance" - "github.com/juju/juju/core/life" jujuparams "github.com/juju/juju/rpc/params" - "github.com/juju/juju/state" "github.com/canonical/jimm/v3/internal/db" "github.com/canonical/jimm/v3/internal/dbmodel" @@ -42,16 +39,11 @@ models: type: iaas uuid: 00000002-0000-0000-0000-000000000001 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 owner: alice@canonical.com life: alive - status: - status: available - info: "OK!" - since: 2020-02-20T20:02:20Z users: - user: alice@canonical.com access: admin @@ -59,14 +51,10 @@ models: access: write - user: charlie@canonical.com access: read - sla: - level: unsupported - agent-version: 1.2.3 - name: model-2 type: iaas uuid: 00000002-0000-0000-0000-000000000002 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 @@ -76,7 +64,6 @@ models: type: iaas uuid: 00000002-0000-0000-0000-000000000003 controller: controller-1 - default-series: warty cloud: test-cloud region: test-cloud-region cloud-credential: cred-1 @@ -84,499 +71,6 @@ models: life: dead ` -var watcherTests = []struct { - name string - initDB func(*qt.C, *db.Database) - deltas [][]jujuparams.Delta - checkDB func(*qt.C, *db.Database) -}{{ - name: "AddMachine", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.MachineInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Id: "2", - InstanceId: "machine-2", - HardwareCharacteristics: &instance.HardwareCharacteristics{ - CpuCores: newUint64(2), - }, - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - - c.Check(model.Machines, qt.Equals, int64(1)) - c.Check(model.Cores, qt.Equals, int64(2)) - }, -}, { - name: "UpdateMachine", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.MachineInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Id: "0", - InstanceId: "machine-0", - }, - }}, {{ - Entity: &jujuparams.MachineInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Id: "0", - InstanceId: "machine-0", - HardwareCharacteristics: &instance.HardwareCharacteristics{ - CpuCores: newUint64(4), - }, - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - - c.Check(model.Machines, qt.Equals, int64(1)) - c.Check(model.Cores, qt.Equals, int64(4)) - }, -}, { - name: "DeleteMachine", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.MachineInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Id: "0", - InstanceId: "machine-0", - HardwareCharacteristics: &instance.HardwareCharacteristics{ - CpuCores: newUint64(3), - }, - }, - }}, {{ - Removed: true, - Entity: &jujuparams.MachineInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Id: "0", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - - c.Check(model.Machines, qt.Equals, int64(0)) - c.Check(model.Cores, qt.Equals, int64(0)) - }, -}, { - name: "AddUnit", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.UnitInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1/2", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - - c.Check(model.Units, qt.Equals, int64(1)) - }, -}, { - name: "UpdateUnit", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.UnitInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1/2", - }, - }}, - {{ - Entity: &jujuparams.UnitInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1/2", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - - c.Check(model.Units, qt.Equals, int64(1)) - }, -}, { - name: "DeleteUnit", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.UnitInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1/0", - }, - }}, - {{ - Removed: true, - Entity: &jujuparams.UnitInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1/0", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - - c.Check(model.Units, qt.Equals, int64(0)) - }, -}, { - name: "UnknownModelsIgnored", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.ModelUpdate{ - ModelUUID: "00000002-0000-0000-0000-000000000004", - Name: "new-model", - Owner: "charlie@canonical.com", - Life: "starting", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000004", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Check(err, qt.ErrorMatches, `model not found`) - c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound) - }, -}, { - name: "UpdateModel", - deltas: [][]jujuparams.Delta{ - {{ - Entity: &jujuparams.ModelUpdate{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "model-1", - Owner: "alice@canonical.com", - Life: life.Value(state.Alive.String()), - ControllerUUID: "00000001-0000-0000-0000-000000000001", - Status: jujuparams.StatusInfo{ - Current: "available", - Message: "updated status message", - Version: "1.2.3", - }, - SLA: jujuparams.ModelSLAInfo{ - Level: "1", - Owner: "me", - }, - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - // zero any uninteresting associations - // TODO(mhilton) don't fetch these in the first place. - model.Owner = dbmodel.Identity{} - model.CloudCredential = dbmodel.CloudCredential{} - model.CloudRegion = dbmodel.CloudRegion{} - model.Controller = dbmodel.Controller{} - c.Check(model, jimmtest.DBObjectEquals, dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - Name: "model-1", - Type: "iaas", - DefaultSeries: "warty", - Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Info: "updated status message", - Version: "1.2.3", - }, - SLA: dbmodel.SLA{ - Level: "1", - Owner: "me", - }, - }) - }, -}, { - name: "DeleteDyingModel", - deltas: [][]jujuparams.Delta{ - {{ - Removed: true, - Entity: &jujuparams.ModelUpdate{ - ModelUUID: "00000002-0000-0000-0000-000000000002", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000002", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Check(err, qt.ErrorMatches, `model not found`) - c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound) - }, -}, { - name: "DeleteDeadModel", - deltas: [][]jujuparams.Delta{ - {{ - Removed: true, - Entity: &jujuparams.ModelUpdate{ - ModelUUID: "00000002-0000-0000-0000-000000000003", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000003", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Check(err, qt.ErrorMatches, `model not found`) - c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound) - }, -}, { - name: "DeleteLivingModelFails", - deltas: [][]jujuparams.Delta{ - {{ - Removed: true, - Entity: &jujuparams.ModelUpdate{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - }, - }}, - nil, - }, - checkDB: func(c *qt.C, db *db.Database) { - ctx := context.Background() - - model := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err := db.GetModel(ctx, &model) - c.Assert(err, qt.IsNil) - // zero any uninteresting associations - // TODO(mhilton) don't fetch these in the first place. - model.Owner = dbmodel.Identity{} - model.CloudCredential = dbmodel.CloudCredential{} - model.CloudRegion = dbmodel.CloudRegion{} - model.Controller = dbmodel.Controller{} - c.Check(model, jimmtest.DBObjectEquals, dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - Name: "model-1", - Type: "iaas", - DefaultSeries: "warty", - Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Info: "OK!", - Since: sql.NullTime{ - Time: time.Date(2020, 2, 20, 20, 2, 20, 0, time.UTC), - Valid: true, - }, - Version: "1.2.3", - }, - SLA: dbmodel.SLA{ - Level: "unsupported", - }, - }) - }, -}} - -//nolint:gocognit -func TestWatcher(t *testing.T) { - c := qt.New(t) - - for _, test := range watcherTests { - c.Run(test.name, func(c *qt.C) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - nextC := make(chan []jujuparams.Delta, len(test.deltas)) - var stopped uint32 - - deltaProcessedChannel := make(chan bool, len(test.deltas)) - - w := jimm.NewWatcherWithDeltaProcessedChannel( - &db.Database{ - DB: jimmtest.PostgresDB(c, nil), - }, - &jimmtest.Dialer{ - API: &jimmtest.API{ - AllModelWatcherNext_: func(ctx context.Context, id string) ([]jujuparams.Delta, error) { - if id != test.name { - return nil, errors.E("incorrect id") - } - - select { - case <-ctx.Done(): - return nil, ctx.Err() - case d, ok := <-nextC: - c.Logf("AllModelWatcherNext received %#v, %v", d, ok) - if ok { - return d, nil - } - cancel() - <-ctx.Done() - return nil, ctx.Err() - } - }, - AllModelWatcherStop_: func(ctx context.Context, id string) error { - if id != test.name { - return errors.E("incorrect id") - } - atomic.StoreUint32(&stopped, 1) - return nil - }, - WatchAllModels_: func(context.Context) (string, error) { - return test.name, nil - }, - ModelInfo_: func(_ context.Context, info *jujuparams.ModelInfo) error { - switch info.UUID { - case "00000002-0000-0000-0000-000000000002": - return errors.E(errors.CodeNotFound) - case "00000002-0000-0000-0000-000000000003": - return errors.E(errors.CodeUnauthorized) - default: - c.Errorf("unexpected model uuid: %s", info.UUID) - return errors.E("unexpected API call") - } - - }, - }, - }, - nil, - deltaProcessedChannel, - ) - - env := jimmtest.ParseEnvironment(c, testWatcherEnv) - err := w.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - env.PopulateDB(c, w.Database) - - if test.initDB != nil { - test.initDB(c, w.Database) - } - - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - err := w.Watch(ctx, time.Millisecond) - checkIfContextCanceled(c, ctx, err) - }() - - validDeltas := 0 - for _, d := range test.deltas { - select { - case nextC <- d: - if d != nil { - validDeltas++ - } - case <-ctx.Done(): - c.Fatal("context closed prematurely") - } - } - - for i := 0; i < validDeltas; i++ { - select { - case <-deltaProcessedChannel: - case <-ctx.Done(): - c.Fatal("context closed prematurely") - } - } - - close(nextC) - wg.Wait() - - test.checkDB(c, w.Database) - }) - } -} - var modelSummaryWatcherTests = []struct { name string summaries [][]jujuparams.ModelAbstract @@ -747,7 +241,7 @@ func TestWatcherSetsControllerUnavailable(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - err := w.Watch(ctx, time.Millisecond) + err := w.WatchAllModelSummaries(ctx, time.Millisecond) checkIfContextCanceled(c, ctx, err) }() @@ -779,7 +273,7 @@ func TestWatcherClearsControllerUnavailable(t *testing.T) { }, Dialer: &jimmtest.Dialer{ API: &jimmtest.API{ - AllModelWatcherNext_: func(_ context.Context, _ string) ([]jujuparams.Delta, error) { + ModelSummaryWatcherNext_: func(ctx context.Context, s string) ([]jujuparams.ModelAbstract, error) { cancel() <-ctx.Done() return nil, ctx.Err() @@ -793,9 +287,10 @@ func TestWatcherClearsControllerUnavailable(t *testing.T) { } return errors.E(errors.CodeNotFound) }, - WatchAllModels_: func(ctx context.Context) (string, error) { + WatchAllModelSummaries_: func(ctx context.Context) (string, error) { return "1234", nil }, + SupportsModelSummaryWatcher_: true, }, }, Pubsub: &testPublisher{}, @@ -824,7 +319,7 @@ func TestWatcherClearsControllerUnavailable(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - err := w.Watch(ctx, time.Millisecond) + err := w.WatchAllModelSummaries(ctx, time.Millisecond) checkIfContextCanceled(c, ctx, err) }() wg.Wait() @@ -838,206 +333,6 @@ func TestWatcherClearsControllerUnavailable(t *testing.T) { c.Assert(ctl.UnavailableSince.Valid, qt.IsFalse) } -func TestWatcherRemoveDyingModelsOnStartup(t *testing.T) { - c := qt.New(t) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - w := &jimm.Watcher{ - Pubsub: &testPublisher{}, - Database: &db.Database{ - DB: jimmtest.PostgresDB(c, nil), - }, - Dialer: &jimmtest.Dialer{ - API: &jimmtest.API{ - AllModelWatcherNext_: func(_ context.Context, _ string) ([]jujuparams.Delta, error) { - cancel() - <-ctx.Done() - return nil, ctx.Err() - }, - ModelInfo_: func(_ context.Context, info *jujuparams.ModelInfo) error { - switch info.UUID { - default: - c.Errorf("unexpected model uuid: %s", info.UUID) - case "00000002-0000-0000-0000-000000000002": - case "00000002-0000-0000-0000-000000000003": - } - return errors.E(errors.CodeNotFound) - }, - WatchAllModels_: func(ctx context.Context) (string, error) { - return "1234", nil - }, - }, - }, - } - env := jimmtest.ParseEnvironment(c, testWatcherEnv) - err := w.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - env.PopulateDB(c, w.Database) - - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - err := w.Watch(ctx, time.Millisecond) - checkIfContextCanceled(c, ctx, err) - }() - wg.Wait() - - m := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000002", - Valid: true, - }, - } - err = w.Database.GetModel(context.Background(), &m) - c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeNotFound) -} - -const testWatcherIgnoreDeltasForModelsFromIncorrectControllerEnv = `clouds: -- name: test-cloud - type: test-provider - regions: - - name: test-cloud-region -cloud-credentials: -- owner: alice@canonical.com - name: cred-1 - cloud: test-cloud -controllers: -- name: controller-1 - uuid: 00000001-0000-0000-0000-000000000001 - cloud: test-cloud - region: test-cloud-region -- name: controller-2 - uuid: 00000001-0000-0000-0000-000000000002 - cloud: test-cloud - region: test-cloud-region -models: -- name: model-1 - type: iaas - uuid: 00000002-0000-0000-0000-000000000001 - controller: controller-1 - default-series: warty - cloud: test-cloud - region: test-cloud-region - cloud-credential: cred-1 - owner: alice@canonical.com - life: alive -` - -func TestWatcherIgnoreDeltasForModelsFromIncorrectController(t *testing.T) { - c := qt.New(t) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - nextC := make(chan []jujuparams.Delta) - w := &jimm.Watcher{ - Pubsub: &testPublisher{}, - Database: &db.Database{ - DB: jimmtest.PostgresDB(c, nil), - }, - Dialer: jimmtest.DialerMap{ - "controller-1": &jimmtest.Dialer{ - API: &jimmtest.API{ - AllModelWatcherNext_: func(_ context.Context, _ string) ([]jujuparams.Delta, error) { - <-ctx.Done() - return nil, ctx.Err() - }, - WatchAllModels_: func(ctx context.Context) (string, error) { - return "1234", nil - }, - }, - }, - "controller-2": &jimmtest.Dialer{ - API: &jimmtest.API{ - AllModelWatcherNext_: func(_ context.Context, _ string) ([]jujuparams.Delta, error) { - select { - case <-ctx.Done(): - return nil, ctx.Err() - case d, ok := <-nextC: - if ok { - return d, nil - } - cancel() - <-ctx.Done() - return nil, ctx.Err() - } - - }, - WatchAllModels_: func(ctx context.Context) (string, error) { - return "1234", nil - }, - }, - }, - }, - } - env := jimmtest.ParseEnvironment(c, testWatcherIgnoreDeltasForModelsFromIncorrectControllerEnv) - err := w.Database.Migrate(ctx, false) - c.Assert(err, qt.IsNil) - env.PopulateDB(c, w.Database) - - m1 := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err = w.Database.GetModel(context.Background(), &m1) - c.Assert(err, qt.IsNil) - - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - err := w.Watch(ctx, time.Millisecond) - checkIfContextCanceled(c, ctx, err) - }() - - nextC <- []jujuparams.Delta{{ - Entity: &jujuparams.ModelUpdate{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "model-1", - Owner: "alice@canonical.com", - Life: life.Value(state.Alive.String()), - Status: jujuparams.StatusInfo{ - Current: "busy", - }, - }, - }} - nextC <- []jujuparams.Delta{{ - Entity: &jujuparams.MachineInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Id: "0", - }, - }} - nextC <- []jujuparams.Delta{{ - Entity: &jujuparams.ApplicationInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1", - }, - }} - nextC <- []jujuparams.Delta{{ - Entity: &jujuparams.UnitInfo{ - ModelUUID: "00000002-0000-0000-0000-000000000001", - Name: "app-1/0", - }, - }} - close(nextC) - - wg.Wait() - m2 := dbmodel.Model{ - UUID: sql.NullString{ - String: "00000002-0000-0000-0000-000000000001", - Valid: true, - }, - } - err = w.Database.GetModel(context.Background(), &m2) - c.Assert(err, qt.IsNil) - c.Check(m2, qt.DeepEquals, m1) -} - func checkIfContextCanceled(c *qt.C, ctx context.Context, err error) { errorToCheck := err if ctx.Err() != nil { @@ -1064,7 +359,3 @@ func (p *testPublisher) Publish(model string, content interface{}) <-chan struct close(done) return done } - -func newUint64(i uint64) *uint64 { - return &i -} diff --git a/internal/jujuapi/access_control_test.go b/internal/jujuapi/access_control_test.go index a7fc38e93..d8e5b8491 100644 --- a/internal/jujuapi/access_control_test.go +++ b/internal/jujuapi/access_control_test.go @@ -5,7 +5,6 @@ package jujuapi_test import ( "context" "database/sql" - "time" petname "github.com/dustinkirkland/golang-petname" "github.com/google/uuid" @@ -1517,13 +1516,6 @@ func createTestControllerEnvironment(ctx context.Context, c *gc.C, s *accessCont CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().UTC().Truncate(time.Millisecond), - Valid: true, - }, - }, } err = db.AddModel(ctx, &model) diff --git a/internal/jujuapi/modelmanager.go b/internal/jujuapi/modelmanager.go index 8eac81390..65b0ea68e 100644 --- a/internal/jujuapi/modelmanager.go +++ b/internal/jujuapi/modelmanager.go @@ -71,6 +71,7 @@ type ModelManager interface { ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error ModelDefaultsForCloud(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag) (jujuparams.ModelDefaultsResult, error) ModelInfo(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelInfo, error) + ModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) ModelStatus(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelStatus, error) QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error) SetModelDefaults(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, configs map[string]interface{}) error @@ -108,27 +109,16 @@ func (r *controllerRoot) DumpModels(ctx context.Context, args jujuparams.DumpMod // authenticated user has access to. The request parameter is ignored. func (r *controllerRoot) ListModelSummaries(ctx context.Context, _ jujuparams.ModelSummariesRequest) (jujuparams.ModelSummaryResults, error) { const op = errors.Op("jujuapi.ListModelSummaries") - - var results []jujuparams.ModelSummaryResult - err := r.jimm.ForEachUserModel(ctx, r.user, func(m *dbmodel.Model, access jujuparams.UserAccessPermission) error { - // TODO(Kian) CSS-6040 Refactor the below to use a better abstraction for Postgres/OpenFGA to Juju types. - ms := m.ToJujuModelSummary() - ms.UserAccess = access - if r.controllerUUIDMasking { - ms.ControllerUUID = r.params.ControllerUUID - } - result := jujuparams.ModelSummaryResult{ - Result: &ms, - } - results = append(results, result) - return nil - }) + maskingControllerUUID := "" + if r.controllerUUIDMasking { + maskingControllerUUID = r.params.ControllerUUID + } + res, err := r.jimm.ModelSummaries(ctx, r.user, maskingControllerUUID) if err != nil { return jujuparams.ModelSummaryResults{}, errors.E(op, err) } - return jujuparams.ModelSummaryResults{ - Results: results, - }, nil + + return res, nil } // ListModels returns the models that the authenticated user diff --git a/internal/jujuapi/modelmanager_test.go b/internal/jujuapi/modelmanager_test.go index a0b4ba367..989e7db7b 100644 --- a/internal/jujuapi/modelmanager_test.go +++ b/internal/jujuapi/modelmanager_test.go @@ -44,17 +44,17 @@ var _ = gc.Suite(&modelManagerSuite{}) func (s *modelManagerSuite) TestListModelSummaries(c *gc.C) { conn := s.open(c, nil, "bob") defer conn.Close() - // Add some machines and units to test the counts. - s.Model.Machines = 1 - s.Model.Cores = 2 - s.Model.Units = 1 ctx := context.Background() err := s.JIMM.Database.UpdateModel(ctx, s.Model) c.Assert(err, gc.Equals, nil) - + stateMachine1, err := s.StatePool.Get(s.Model.Tag().Id()) + c.Assert(err, gc.Equals, nil) + f := factory.NewFactory(stateMachine1.State, s.StatePool) + f.MakeMachine(c, &factory.MachineParams{}) + f.MakeUnit(c, &factory.UnitParams{}) client := modelmanager.NewClient(conn) - models, err := client.ListModelSummaries("bob@canonical.com", false) + models, err := client.ListModelSummaries("bob@canonical.com", true) c.Assert(err, gc.Equals, nil) c.Assert(models, jimmtest.CmpEquals( cmpopts.IgnoreTypes(&time.Time{}), @@ -79,9 +79,6 @@ func (s *modelManagerSuite) TestListModelSummaries(c *gc.C) { ModelUserAccess: "admin", Counts: []base.EntityCount{{ Entity: "machines", - Count: 1, - }, { - Entity: "cores", Count: 2, }, { Entity: "units", @@ -90,7 +87,8 @@ func (s *modelManagerSuite) TestListModelSummaries(c *gc.C) { AgentVersion: &jujuversion.Current, Type: "iaas", SLA: &base.SLASummary{ - Level: "unsupported", + Level: "", + Owner: "bob@canonical.com", }, }, { Name: "model-3", @@ -108,20 +106,12 @@ func (s *modelManagerSuite) TestListModelSummaries(c *gc.C) { Data: map[string]interface{}{}, }, ModelUserAccess: "read", - Counts: []base.EntityCount{{ - Entity: "machines", - Count: 0, - }, { - Entity: "cores", - Count: 0, - }, { - Entity: "units", - Count: 0, - }}, - AgentVersion: &jujuversion.Current, - Type: "iaas", + Counts: []base.EntityCount{}, + AgentVersion: &jujuversion.Current, + Type: "iaas", SLA: &base.SLASummary{ - Level: "unsupported", + Level: "", + Owner: "charlie@canonical.com", }, }}) } @@ -131,7 +121,14 @@ func (s *modelManagerSuite) TestListModelSummariesWithoutControllerUUIDMasking(c defer conn1.Close() err := conn1.APICall("JIMM", 4, "", "DisableControllerUUIDMasking", nil, nil) c.Assert(err, gc.ErrorMatches, `unauthorized \(unauthorized access\)`) - + stateMachine3, err := s.StatePool.Get(s.Model3.Tag().Id()) + c.Assert(err, gc.Equals, nil) + f := factory.NewFactory(stateMachine3.State, s.StatePool) + f.MakeMachine(c, &factory.MachineParams{ + Characteristics: &instance.HardwareCharacteristics{ + Arch: newString("bbc-micro"), + }, + }) s.AddAdminUser(c, "adam@canonical.com") // we need to make bob jimm admin to disable controller UUID masking @@ -162,7 +159,7 @@ func (s *modelManagerSuite) TestListModelSummariesWithoutControllerUUIDMasking(c c.Assert(err, gc.Equals, nil) client := modelmanager.NewClient(conn) - models, err := client.ListModelSummaries("bob", false) + models, err := client.ListModelSummaries("bob@canonical.com", false) c.Assert(err, gc.Equals, nil) c.Assert(models, jimmtest.CmpEquals( cmpopts.IgnoreTypes(&time.Time{}), @@ -185,20 +182,12 @@ func (s *modelManagerSuite) TestListModelSummariesWithoutControllerUUIDMasking(c Data: map[string]interface{}{}, }, ModelUserAccess: "admin", - Counts: []base.EntityCount{{ - Entity: "machines", - Count: 0, - }, { - Entity: "cores", - Count: 0, - }, { - Entity: "units", - Count: 0, - }}, - AgentVersion: &jujuversion.Current, - Type: "iaas", + Counts: []base.EntityCount{}, + AgentVersion: &jujuversion.Current, + Type: "iaas", SLA: &base.SLASummary{ - Level: "unsupported", + Level: "", + Owner: "bob@canonical.com", }, }, { Name: "model-3", @@ -216,20 +205,17 @@ func (s *modelManagerSuite) TestListModelSummariesWithoutControllerUUIDMasking(c Data: map[string]interface{}{}, }, ModelUserAccess: "read", - Counts: []base.EntityCount{{ - Entity: "machines", - Count: 0, - }, { - Entity: "cores", - Count: 0, - }, { - Entity: "units", - Count: 0, - }}, + Counts: []base.EntityCount{ + { + Entity: "machines", + Count: 1, + }, + }, AgentVersion: &jujuversion.Current, Type: "iaas", SLA: &base.SLASummary{ - Level: "unsupported", + Level: "", + Owner: "charlie@canonical.com", }, }}) } @@ -303,7 +289,6 @@ func (s *modelManagerSuite) TestModelInfo(c *gc.C) { conn := s.open(c, nil, "bob") defer conn.Close() client := modelmanager.NewClient(conn) - models, err := client.ModelInfo([]names.ModelTag{ s.Model.ResourceTag(), s.Model2.ResourceTag(), diff --git a/internal/jujuclient/allwatcher.go b/internal/jujuclient/allwatcher.go deleted file mode 100644 index b1ecc55db..000000000 --- a/internal/jujuclient/allwatcher.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2024 Canonical. - -package jujuclient - -import ( - "context" - - jujuerrors "github.com/juju/errors" - jujuparams "github.com/juju/juju/rpc/params" - - "github.com/canonical/jimm/v3/internal/errors" -) - -// WatchAllModels initialises a new AllModelWatcher. On success the watcher -// ID is returned. This uses the WatchAllModels method on the Controller -// facade. -func (c Connection) WatchAllModels(ctx context.Context) (string, error) { - const op = errors.Op("jujuclient.WatchAllModels") - var resp jujuparams.SummaryWatcherID - if err := c.CallHighestFacadeVersion(ctx, "Controller", []int{11, 7}, "", "WatchAllModels", nil, &resp); err != nil { - return "", errors.E(op, jujuerrors.Cause(err)) - } - return resp.WatcherID, nil -} - -// AllModelWatcherNext receives the next set of results from the all-model -// watcher with the given id. This uses the Next method on the -// AllModelWatcher facade. -func (c Connection) AllModelWatcherNext(ctx context.Context, id string) ([]jujuparams.Delta, error) { - const op = errors.Op("jujuclient.AllModelWatcherNext") - var resp jujuparams.AllWatcherNextResults - if err := c.CallHighestFacadeVersion(ctx, "AllModelWatcher", []int{4, 2}, id, "Next", nil, &resp); err != nil { - return nil, errors.E(op, jujuerrors.Cause(err)) - } - return resp.Deltas, nil -} - -// AllModelWatcherStop stops the all-model watcher with the given id. This -// uses the Stop method on the AllModelWatcher facade. -func (c Connection) AllModelWatcherStop(ctx context.Context, id string) error { - const op = errors.Op("jujuclient.AllModelWatcherStop") - if err := c.CallHighestFacadeVersion(ctx, "AllModelWatcher", []int{4, 2}, id, "Stop", nil, nil); err != nil { - return errors.E(op, jujuerrors.Cause(err)) - } - return nil -} diff --git a/internal/jujuclient/allwatcher_test.go b/internal/jujuclient/allwatcher_test.go deleted file mode 100644 index b04b28d5b..000000000 --- a/internal/jujuclient/allwatcher_test.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2024 Canonical. - -package jujuclient_test - -import ( - "context" - - jujuparams "github.com/juju/juju/rpc/params" - gc "gopkg.in/check.v1" -) - -type allModelWatcherSuite struct { - jujuclientSuite -} - -var _ = gc.Suite(&allModelWatcherSuite{}) - -func (s *allModelWatcherSuite) TestWatchAllModels(c *gc.C) { - ctx := context.Background() - - id, err := s.API.WatchAllModels(ctx) - c.Assert(err, gc.Equals, nil) - c.Assert(id, gc.Not(gc.Equals), "") - - err = s.API.AllModelWatcherStop(ctx, id) - c.Assert(err, gc.Equals, nil) -} - -func (s *allModelWatcherSuite) TestAllModelWatcherNext(c *gc.C) { - ctx := context.Background() - - id, err := s.API.WatchAllModels(ctx) - c.Assert(err, gc.Equals, nil) - - _, err = s.API.AllModelWatcherNext(ctx, id) - c.Assert(err, gc.Equals, nil) - - err = s.API.AllModelWatcherStop(ctx, id) - c.Assert(err, gc.Equals, nil) -} - -func (s *allModelWatcherSuite) TestAllModelWatcherNextError(c *gc.C) { - _, err := s.API.AllModelWatcherNext(context.Background(), "invalid-watcher") - c.Check(jujuparams.ErrCode(err), gc.Equals, jujuparams.CodeNotFound) - c.Check(err, gc.ErrorMatches, `unknown watcher id \(not found\)`) -} - -func (s *allModelWatcherSuite) TestAllModelWatcherStopError(c *gc.C) { - err := s.API.AllModelWatcherStop(context.Background(), "invalid-watcher") - c.Check(jujuparams.ErrCode(err), gc.Equals, jujuparams.CodeNotFound) - c.Check(err, gc.ErrorMatches, `unknown watcher id \(not found\)`) -} diff --git a/internal/jujuclient/modelmanager.go b/internal/jujuclient/modelmanager.go index 0fcf3160f..b19808df6 100644 --- a/internal/jujuclient/modelmanager.go +++ b/internal/jujuclient/modelmanager.go @@ -208,6 +208,22 @@ func (c Connection) ControllerModelSummary(ctx context.Context, ms *jujuparams.M return errors.E(op, "controller model not found", errors.CodeNotFound) } +// ListModelSummaries retrieves the list of model summaries from the controler +func (c Connection) ListModelSummaries(ctx context.Context, ms jujuparams.ModelSummariesRequest) (jujuparams.ModelSummaryResults, error) { + const op = errors.Op("jujuclient.ControllerModelSummary") + args := jujuparams.ModelSummariesRequest{ + UserTag: c.userTag, + All: ms.All, + } + var resp jujuparams.ModelSummaryResults + err := c.Call(ctx, "ModelManager", 9, "", "ListModelSummaries", &args, &resp) + if err != nil { + return jujuparams.ModelSummaryResults{}, errors.E(op, jujuerrors.Cause(err)) + } + + return resp, nil +} + // ValidateModelUpgrade validates if a model is allowed to perform an upgrade. It // uses ValidateModelUpgrades on the ModelManager facade. func (c Connection) ValidateModelUpgrade(ctx context.Context, model names.ModelTag, force bool) error { diff --git a/internal/jujuclient/modelmanager_test.go b/internal/jujuclient/modelmanager_test.go index abeb6e6e7..3d7c7d964 100644 --- a/internal/jujuclient/modelmanager_test.go +++ b/internal/jujuclient/modelmanager_test.go @@ -246,6 +246,15 @@ func (s *modelmanagerSuite) TestModelStatus(c *gc.C) { }) } +func (s *modelmanagerSuite) TestListModelSummaries(c *gc.C) { + ctx := context.Background() + + res, err := s.API.ListModelSummaries(ctx, jujuparams.ModelSummariesRequest{}) + c.Assert(err, gc.Equals, nil) + c.Assert(len(res.Results), gc.Equals, 1) + c.Assert(res.Results[0].Result.Name, gc.Equals, s.Model.Name()) +} + func (s *modelmanagerSuite) TestModelStatusError(c *gc.C) { ctx := context.Background() diff --git a/internal/jujuclient/modelwatcher.go b/internal/jujuclient/modelwatcher.go deleted file mode 100644 index 00137b572..000000000 --- a/internal/jujuclient/modelwatcher.go +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2024 Canonical. - -package jujuclient - -import ( - "context" - - jujuerrors "github.com/juju/errors" - jujuparams "github.com/juju/juju/rpc/params" - - "github.com/canonical/jimm/v3/internal/errors" -) - -// WatchAll initialises a new ModelWatcher. On success the watcher -// ID is returned. This uses the WatchAll method on the Client. -func (c Connection) WatchAll(ctx context.Context) (string, error) { - const op = errors.Op("jujuclient.WatchAll") - var resp jujuparams.AllWatcherId - if err := c.CallHighestFacadeVersion(ctx, "Client", []int{6, 1}, "", "WatchAll", nil, &resp); err != nil { - return "", errors.E(op, jujuerrors.Cause(err)) - } - return resp.AllWatcherId, nil -} - -// ModelWatcherNext receives the next set of results from the model -// watcher with the given id. This uses the Next method on the -// AllWatcher facade version 1. -func (c Connection) ModelWatcherNext(ctx context.Context, id string) ([]jujuparams.Delta, error) { - const op = errors.Op("jujuclient.ModelWatcherNext") - var resp jujuparams.AllWatcherNextResults - if err := c.CallHighestFacadeVersion(ctx, "AllWatcher", []int{3, 2, 1}, id, "Next", nil, &resp); err != nil { - return nil, errors.E(op, jujuerrors.Cause(err)) - } - return resp.Deltas, nil -} - -// ModelWatcherStop stops the model watcher with the given id. This -// uses the Stop method on the AllWatcher facade version 1. -func (c Connection) ModelWatcherStop(ctx context.Context, id string) error { - const op = errors.Op("jujuclient.ModelWatcherStop") - if err := c.CallHighestFacadeVersion(ctx, "AllWatcher", []int{3, 2, 1}, id, "Stop", nil, nil); err != nil { - return errors.E(op, jujuerrors.Cause(err)) - } - return nil -} diff --git a/internal/jujuclient/modelwatcher_test.go b/internal/jujuclient/modelwatcher_test.go deleted file mode 100644 index bb12a8f77..000000000 --- a/internal/jujuclient/modelwatcher_test.go +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright 2024 Canonical. - -package jujuclient_test - -import ( - "context" - - "github.com/juju/juju/core/network" - jujuparams "github.com/juju/juju/rpc/params" - gc "gopkg.in/check.v1" - - "github.com/canonical/jimm/v3/internal/dbmodel" - "github.com/canonical/jimm/v3/internal/jimm" - "github.com/canonical/jimm/v3/internal/jujuclient" - "github.com/canonical/jimm/v3/internal/testutils/jimmtest" -) - -type modelWatcherSuite struct { - jimmtest.JujuSuite - - Dialer jimm.Dialer - API jimm.API -} - -func (s *modelWatcherSuite) SetUpTest(c *gc.C) { - s.JujuSuite.SetUpTest(c) - - s.Dialer = &jujuclient.Dialer{ - JWTService: s.JIMM.JWTService, - } - var err error - info := s.APIInfo(c) - hpss := make(dbmodel.HostPorts, 0, len(info.Addrs)) - for _, addr := range info.Addrs { - hp, err := network.ParseMachineHostPort(addr) - if err != nil { - continue - } - hpss = append(hpss, []jujuparams.HostPort{{ - Address: jujuparams.FromMachineAddress(hp.MachineAddress), - Port: hp.Port(), - }}) - } - ctl := dbmodel.Controller{ - UUID: s.ControllerConfig.ControllerUUID(), - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - Addresses: hpss, - } - - s.API, err = s.Dialer.Dial(context.Background(), &ctl, s.Model.ModelTag(), nil) - c.Assert(err, gc.Equals, nil) -} - -func (s *modelWatcherSuite) TearDownTest(c *gc.C) { - if s.API != nil { - err := s.API.Close() - s.API = nil - c.Assert(err, gc.Equals, nil) - } - s.JujuSuite.TearDownTest(c) -} - -var _ = gc.Suite(&modelWatcherSuite{}) - -func (s *modelWatcherSuite) TestWatchAll(c *gc.C) { - ctx := context.Background() - - id, err := s.API.WatchAll(ctx) - c.Assert(err, gc.Equals, nil) - c.Assert(id, gc.Not(gc.Equals), "") - - err = s.API.ModelWatcherStop(ctx, id) - c.Assert(err, gc.Equals, nil) -} - -func (s *modelWatcherSuite) TestModelWatcherNext(c *gc.C) { - ctx := context.Background() - - id, err := s.API.WatchAll(ctx) - c.Assert(err, gc.Equals, nil) - - _, err = s.API.ModelWatcherNext(ctx, id) - c.Assert(err, gc.Equals, nil) - - err = s.API.ModelWatcherStop(ctx, id) - c.Assert(err, gc.Equals, nil) -} - -func (s *modelWatcherSuite) TestModelWatcherNextError(c *gc.C) { - _, err := s.API.ModelWatcherNext(context.Background(), "invalid-watcher") - c.Check(jujuparams.ErrCode(err), gc.Equals, jujuparams.CodeNotFound) - c.Check(err, gc.ErrorMatches, `unknown watcher id \(not found\)`) -} - -func (s *modelWatcherSuite) TestModelWatcherStopError(c *gc.C) { - err := s.API.ModelWatcherStop(context.Background(), "invalid-watcher") - c.Check(jujuparams.ErrCode(err), gc.Equals, jujuparams.CodeNotFound) - c.Check(err, gc.ErrorMatches, `unknown watcher id \(not found\)`) -} diff --git a/internal/testutils/jimmtest/api.go b/internal/testutils/jimmtest/api.go index e68847c77..d2851027c 100644 --- a/internal/testutils/jimmtest/api.go +++ b/internal/testutils/jimmtest/api.go @@ -122,8 +122,6 @@ type API struct { base.APICaller AddCloud_ func(context.Context, names.CloudTag, jujuparams.Cloud, bool) error - AllModelWatcherNext_ func(context.Context, string) ([]jujuparams.Delta, error) - AllModelWatcherStop_ func(context.Context, string) error ChangeModelCredential_ func(context.Context, names.ModelTag, names.CloudCredentialTag) error CheckCredentialModels_ func(context.Context, jujuparams.TaggedCredential) ([]jujuparams.UpdateCredentialModelResult, error) Close_ func() error @@ -149,8 +147,7 @@ type API struct { ModelStatus_ func(context.Context, *jujuparams.ModelStatus) error ModelSummaryWatcherNext_ func(context.Context, string) ([]jujuparams.ModelAbstract, error) ModelSummaryWatcherStop_ func(context.Context, string) error - ModelWatcherNext_ func(ctx context.Context, id string) ([]jujuparams.Delta, error) - ModelWatcherStop_ func(ctx context.Context, id string) error + ListModelSummaries_ func(context.Context, jujuparams.ModelSummariesRequest) (jujuparams.ModelSummaryResults, error) Offer_ func(context.Context, crossmodel.OfferURL, jujuparams.AddApplicationOffer) error Ping_ func(context.Context) error RemoveCloud_ func(context.Context, names.CloudTag) error @@ -164,9 +161,7 @@ type API struct { UpdateCloud_ func(context.Context, names.CloudTag, jujuparams.Cloud) error UpdateCredential_ func(context.Context, jujuparams.TaggedCredential) ([]jujuparams.UpdateCredentialModelResult, error) ValidateModelUpgrade_ func(context.Context, names.ModelTag, bool) error - WatchAll_ func(context.Context) (string, error) WatchAllModelSummaries_ func(context.Context) (string, error) - WatchAllModels_ func(context.Context) (string, error) ListFilesystems_ func(ctx context.Context, machines []string) ([]jujuparams.FilesystemDetailsListResult, error) ListVolumes_ func(ctx context.Context, machines []string) ([]jujuparams.VolumeDetailsListResult, error) ListStorageDetails_ func(ctx context.Context) ([]jujuparams.StorageDetails, error) @@ -179,20 +174,6 @@ func (a *API) AddCloud(ctx context.Context, tag names.CloudTag, cld jujuparams.C return a.AddCloud_(ctx, tag, cld, force) } -func (a *API) AllModelWatcherNext(ctx context.Context, id string) ([]jujuparams.Delta, error) { - if a.AllModelWatcherNext_ == nil { - return nil, errors.E(errors.CodeNotImplemented) - } - return a.AllModelWatcherNext_(ctx, id) -} - -func (a *API) AllModelWatcherStop(ctx context.Context, id string) error { - if a.AllModelWatcherStop_ == nil { - return errors.E(errors.CodeNotImplemented) - } - return a.AllModelWatcherStop_(ctx, id) -} - func (a *API) CheckCredentialModels(ctx context.Context, cred jujuparams.TaggedCredential) ([]jujuparams.UpdateCredentialModelResult, error) { if a.CheckCredentialModels_ == nil { return nil, errors.E(errors.CodeNotImplemented) @@ -358,6 +339,13 @@ func (a *API) ModelSummaryWatcherStop(ctx context.Context, id string) error { return a.ModelSummaryWatcherStop_(ctx, id) } +func (a *API) ListModelSummaries(ctx context.Context, req jujuparams.ModelSummariesRequest) (jujuparams.ModelSummaryResults, error) { + if a.ListModelSummaries_ == nil { + return jujuparams.ModelSummaryResults{}, errors.E(errors.CodeNotImplemented) + } + return a.ListModelSummaries_(ctx, req) +} + func (a *API) Offer(ctx context.Context, offerURL crossmodel.OfferURL, aao jujuparams.AddApplicationOffer) error { if a.Offer_ == nil { return errors.E(errors.CodeNotImplemented) @@ -450,13 +438,6 @@ func (a *API) WatchAllModelSummaries(ctx context.Context) (string, error) { return a.WatchAllModelSummaries_(ctx) } -func (a *API) WatchAllModels(ctx context.Context) (string, error) { - if a.WatchAllModels_ == nil { - return "", errors.E(errors.CodeNotImplemented) - } - return a.WatchAllModels_(ctx) -} - func (a *API) ChangeModelCredential(ctx context.Context, model names.ModelTag, cred names.CloudCredentialTag) error { if a.ChangeModelCredential_ == nil { return errors.E(errors.CodeNotImplemented) @@ -464,27 +445,6 @@ func (a *API) ChangeModelCredential(ctx context.Context, model names.ModelTag, c return a.ChangeModelCredential_(ctx, model, cred) } -func (a *API) ModelWatcherNext(ctx context.Context, id string) ([]jujuparams.Delta, error) { - if a.ModelWatcherNext_ == nil { - return nil, errors.E(errors.CodeNotImplemented) - } - return a.ModelWatcherNext_(ctx, id) -} - -func (a *API) ModelWatcherStop(ctx context.Context, id string) error { - if a.ModelWatcherStop_ == nil { - return errors.E(errors.CodeNotImplemented) - } - return a.ModelWatcherStop_(ctx, id) -} - -func (a *API) WatchAll(ctx context.Context) (string, error) { - if a.WatchAll_ == nil { - return "", errors.E(errors.CodeNotImplemented) - } - return a.WatchAll_(ctx) -} - func (a *API) ListFilesystems(ctx context.Context, machines []string) ([]jujuparams.FilesystemDetailsListResult, error) { if a.ListFilesystems_ == nil { return nil, errors.E(errors.CodeNotImplemented) diff --git a/internal/testutils/jimmtest/env.go b/internal/testutils/jimmtest/env.go index 594528cb6..4556fe402 100644 --- a/internal/testutils/jimmtest/env.go +++ b/internal/testutils/jimmtest/env.go @@ -484,21 +484,10 @@ func (m *Model) DBObject(c Tester, db *db.Database) dbmodel.Model { migrationControllerID.Int32 = int32(m.env.Controller(m.MigrationController).dbo.ID) migrationControllerID.Valid = true } - m.dbo.MigrationControllerID = migrationControllerID m.dbo.CloudRegion = m.env.Cloud(m.Cloud).DBObject(c, db).Region(m.CloudRegion) m.dbo.CloudCredential = m.env.CloudCredential(m.Owner, m.Cloud, m.CloudCredential).DBObject(c, db) - m.dbo.Type = m.Type - m.dbo.DefaultSeries = m.DefaultSeries m.dbo.Life = m.Life - m.dbo.Status.FromJujuEntityStatus(m.Status) - m.dbo.Status.Version = m.AgentVersion - if m.SLA != nil { - m.dbo.SLA.FromJujuModelSLAInfo(*m.SLA) - } - m.dbo.Cores = m.Cores - m.dbo.Machines = m.Machines - m.dbo.Units = m.Units err := db.AddModel(context.Background(), &m.dbo) if err != nil { diff --git a/internal/testutils/jimmtest/fixture.go b/internal/testutils/jimmtest/fixture.go index b9458b534..3aa9b674c 100644 --- a/internal/testutils/jimmtest/fixture.go +++ b/internal/testutils/jimmtest/fixture.go @@ -5,7 +5,6 @@ package jimmtest import ( "context" "database/sql" - "time" petname "github.com/dustinkirkland/golang-petname" qt "github.com/frankban/quicktest" @@ -94,13 +93,6 @@ func CreateTestControllerEnvironment(ctx context.Context, c *qt.C, db *db.Databa CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().UTC().Truncate(time.Millisecond), - Valid: true, - }, - }, } err = db.AddModel(ctx, &model) diff --git a/internal/testutils/jimmtest/mocks/model.go b/internal/testutils/jimmtest/mocks/model.go index 61fb244e8..397d6c917 100644 --- a/internal/testutils/jimmtest/mocks/model.go +++ b/internal/testutils/jimmtest/mocks/model.go @@ -25,6 +25,7 @@ type ModelManager struct { ForEachModel_ func(ctx context.Context, u *openfga.User, f func(*dbmodel.Model, jujuparams.UserAccessPermission) error) error ForEachUserModel_ func(ctx context.Context, u *openfga.User, f func(*dbmodel.Model, jujuparams.UserAccessPermission) error) error FullModelStatus_ func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, patterns []string) (*jujuparams.FullStatus, error) + ModelSummaries_ func(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) GetModel_ func(ctx context.Context, uuid string) (dbmodel.Model, error) ImportModel_ func(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error IdentityModelDefaults_ func(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error) @@ -128,6 +129,13 @@ func (j *ModelManager) ModelStatus(ctx context.Context, u *openfga.User, mt name return j.ModelStatus_(ctx, u, mt) } +func (j *ModelManager) ModelSummaries(ctx context.Context, u *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) { + if j.ModelSummaries_ == nil { + return jujuparams.ModelSummaryResults{}, errors.E(errors.CodeNotImplemented) + } + return j.ModelSummaries_(ctx, u, maskingControllerUUID) +} + func (j *ModelManager) QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error) { if j.QueryModelsJq_ == nil { return params.CrossModelQueryResponse{}, errors.E(errors.CodeNotImplemented) diff --git a/local/seed_db/main.go b/local/seed_db/main.go index 964ae4196..cebfe4fee 100644 --- a/local/seed_db/main.go +++ b/local/seed_db/main.go @@ -6,7 +6,6 @@ import ( "database/sql" "fmt" "os" - "time" petname "github.com/dustinkirkland/golang-petname" "github.com/google/uuid" @@ -105,13 +104,6 @@ func main() { CloudRegionID: cloud.Regions[0].ID, CloudCredentialID: cred.ID, Life: state.Alive.String(), - Status: dbmodel.Status{ - Status: "available", - Since: sql.NullTime{ - Time: time.Now().UTC().Truncate(time.Millisecond), - Valid: true, - }, - }, } if err = db.AddModel(ctx, &model); err != nil { fmt.Println("failed to add model to db ", err)