From 5942540148f23e1f79de06d6c0c44f10efb0f6d1 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Mon, 18 Mar 2024 23:34:49 +0100 Subject: [PATCH 1/4] Remove remote spaces This patch completely removes the concept of remote spaces in Juju. We only had this for binding offers on different models, but it is not useful since none of the providers support `ProviderSpaceInfo` and we have removed the `AreSpacesRoutable` on the providers. The cleanup includes the checks on the application offers API. --- .../facades/client/application/backend.go | 2 - .../application/mocks/application_mock.go | 14 -- .../facades/client/applicationoffers/base.go | 10 -- state/migration_export.go | 26 ---- state/migration_export_test.go | 115 --------------- state/migration_import.go | 25 ---- state/migration_import_tasks_test.go | 6 - state/migration_import_test.go | 48 ------ state/migrations/remoteapplications.go | 34 ----- .../remoteapplications_mock_test.go | 14 -- state/migrations/remoteapplications_test.go | 39 ----- state/remoteapplication.go | 139 ------------------ state/remoteapplication_test.go | 138 ----------------- 13 files changed, 610 deletions(-) diff --git a/apiserver/facades/client/application/backend.go b/apiserver/facades/client/application/backend.go index cff45d6358d..d04e7fb7424 100644 --- a/apiserver/facades/client/application/backend.go +++ b/apiserver/facades/client/application/backend.go @@ -380,8 +380,6 @@ type RemoteApplication interface { SourceModel() names.ModelTag Endpoints() ([]state.Endpoint, error) AddEndpoints(eps []charm.Relation) error - Bindings() map[string]string - Spaces() []state.RemoteSpace Destroy() error DestroyOperation(force bool) *state.DestroyRemoteApplicationOperation Status() (status.StatusInfo, error) diff --git a/apiserver/facades/client/application/mocks/application_mock.go b/apiserver/facades/client/application/mocks/application_mock.go index 241ce5bae5d..b5669f92f7b 100644 --- a/apiserver/facades/client/application/mocks/application_mock.go +++ b/apiserver/facades/client/application/mocks/application_mock.go @@ -1589,20 +1589,6 @@ func (mr *MockRemoteApplicationMockRecorder) SourceModel() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SourceModel", reflect.TypeOf((*MockRemoteApplication)(nil).SourceModel)) } -// Spaces mocks base method. -func (m *MockRemoteApplication) Spaces() []state.RemoteSpace { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Spaces") - ret0, _ := ret[0].([]state.RemoteSpace) - return ret0 -} - -// Spaces indicates an expected call of Spaces. -func (mr *MockRemoteApplicationMockRecorder) Spaces() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Spaces", reflect.TypeOf((*MockRemoteApplication)(nil).Spaces)) -} - // Status mocks base method. func (m *MockRemoteApplication) Status() (status.StatusInfo, error) { m.ctrl.T.Helper() diff --git a/apiserver/facades/client/applicationoffers/base.go b/apiserver/facades/client/applicationoffers/base.go index 3dcdd0c8ede..d8ed1321170 100644 --- a/apiserver/facades/client/applicationoffers/base.go +++ b/apiserver/facades/client/applicationoffers/base.go @@ -19,7 +19,6 @@ import ( "github.com/juju/juju/core/permission" envcontext "github.com/juju/juju/environs/envcontext" "github.com/juju/juju/rpc/params" - "github.com/juju/juju/state" ) // BaseAPI provides various boilerplate methods used by the facade business logic. @@ -443,14 +442,5 @@ func (api *BaseAPI) makeOfferParams( } - // CAAS models don't have spaces. - model, err := backend.Model() - if err != nil { - return nil, nil, errors.Trace(err) - } - if model.Type() == state.ModelTypeCAAS { - return &result, app, nil - } - return &result, app, nil } diff --git a/state/migration_export.go b/state/migration_export.go index 485fabd4745..fdd38475d6e 100644 --- a/state/migration_export.go +++ b/state/migration_export.go @@ -2319,32 +2319,6 @@ func (s remoteApplicationShim) Endpoints() ([]migrations.MigrationRemoteEndpoint return result, nil } -func (s remoteApplicationShim) Spaces() []migrations.MigrationRemoteSpace { - spaces := s.RemoteApplication.Spaces() - result := make([]migrations.MigrationRemoteSpace, len(spaces)) - for k, v := range spaces { - subnets := make([]migrations.MigrationRemoteSubnet, len(v.Subnets)) - for k, v := range v.Subnets { - subnets[k] = migrations.MigrationRemoteSubnet{ - CIDR: v.CIDR, - ProviderId: v.ProviderId, - VLANTag: v.VLANTag, - AvailabilityZones: v.AvailabilityZones, - ProviderSpaceId: v.ProviderSpaceId, - ProviderNetworkId: v.ProviderNetworkId, - } - } - result[k] = migrations.MigrationRemoteSpace{ - CloudType: v.CloudType, - Name: v.Name, - ProviderId: v.ProviderId, - ProviderAttributes: v.ProviderAttributes, - Subnets: subnets, - } - } - return result -} - func (s remoteApplicationShim) GlobalKey() string { return s.RemoteApplication.globalKey() } diff --git a/state/migration_export_test.go b/state/migration_export_test.go index faa385da735..25fd78ba7ed 100644 --- a/state/migration_export_test.go +++ b/state/migration_export_test.go @@ -34,7 +34,6 @@ import ( "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/status" domainstorage "github.com/juju/juju/domain/storage" - "github.com/juju/juju/environs" "github.com/juju/juju/environs/config" "github.com/juju/juju/internal/feature" internalpassword "github.com/juju/juju/internal/password" @@ -2159,77 +2158,6 @@ func (s *MigrationExportSuite) newResource(c *gc.C, appName, name string, revisi } func (s *MigrationExportSuite) TestRemoteApplications(c *gc.C) { - mac, err := newMacaroon("apimac") - c.Assert(err, gc.IsNil) - dbApp, err := s.State.AddRemoteApplication(state.AddRemoteApplicationParams{ - Name: "gravy-rainbow", - URL: "me/model.rainbow", - SourceModel: s.Model.ModelTag(), - Token: "charisma", - OfferUUID: "offer-uuid", - Endpoints: []charm.Relation{{ - Interface: "mysql", - Name: "db", - Role: charm.RoleProvider, - Scope: charm.ScopeGlobal, - }, { - Interface: "mysql-root", - Name: "db-admin", - Limit: 5, - Role: charm.RoleProvider, - Scope: charm.ScopeGlobal, - }, { - Interface: "logging", - Name: "logging", - Role: charm.RoleProvider, - Scope: charm.ScopeGlobal, - }}, - Spaces: []*environs.ProviderSpaceInfo{{ - CloudType: "ec2", - ProviderAttributes: map[string]interface{}{ - "thing1": 23, - "thing2": "halberd", - "network": "network-1", - }, - SpaceInfo: network.SpaceInfo{ - Name: "public", - ProviderId: "juju-space-public", - Subnets: []network.SubnetInfo{{ - ProviderId: "juju-subnet-12", - CIDR: "1.2.3.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-public", - ProviderNetworkId: "network-1", - }}, - }, - }, { - CloudType: "ec2", - ProviderAttributes: map[string]interface{}{ - "thing1": 24, - "thing2": "bardiche", - "network": "network-1", - }, - SpaceInfo: network.SpaceInfo{ - Name: "private", - ProviderId: "juju-space-private", - Subnets: []network.SubnetInfo{{ - ProviderId: "juju-subnet-24", - CIDR: "1.2.4.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-private", - ProviderNetworkId: "network-1", - }}, - }, - }}, - Bindings: map[string]string{ - "db": "private", - "db-admin": "private", - "logging": "public", - }, - // Macaroon not exported. - Macaroon: mac, - }) - c.Assert(err, jc.ErrorIsNil) state.AddTestingApplication(c, s.State, s.objectStore, "wordpress", state.AddTestingCharm(c, s.State, "wordpress")) eps, err := s.State.InferEndpoints("gravy-rainbow", "wordpress") c.Assert(err, jc.ErrorIsNil) @@ -2267,39 +2195,14 @@ func (s *MigrationExportSuite) TestRemoteApplications(c *gc.C) { c.Check(ep.Interface(), gc.Equals, "logging") c.Check(ep.Role(), gc.Equals, "provider") - originalSpaces := dbApp.Spaces() actualSpaces := app.Spaces() c.Assert(actualSpaces, gc.HasLen, 2) - checkSpaceMatches(c, actualSpaces[0], originalSpaces[0]) - checkSpaceMatches(c, actualSpaces[1], originalSpaces[1]) c.Assert(model.Relations(), gc.HasLen, 1) rel := model.Relations()[0] c.Assert(rel.Key(), gc.Equals, "wordpress:db gravy-rainbow:db") } -func checkSpaceMatches(c *gc.C, actual description.RemoteSpace, original state.RemoteSpace) { - c.Check(actual.CloudType(), gc.Equals, original.CloudType) - c.Check(actual.Name(), gc.Equals, original.Name) - c.Check(actual.ProviderId(), gc.Equals, original.ProviderId) - c.Check(actual.ProviderAttributes(), gc.DeepEquals, map[string]interface{}(original.ProviderAttributes)) - subnets := actual.Subnets() - c.Assert(subnets, gc.HasLen, len(original.Subnets)) - for i, subnet := range subnets { - c.Logf("subnet %d", i) - checkSubnetMatches(c, subnet, original.Subnets[i]) - } -} - -func checkSubnetMatches(c *gc.C, actual description.Subnet, original state.RemoteSubnet) { - c.Check(actual.CIDR(), gc.Equals, original.CIDR) - c.Check(actual.ProviderId(), gc.Equals, original.ProviderId) - c.Check(actual.VLANTag(), gc.Equals, original.VLANTag) - c.Check(actual.AvailabilityZones(), gc.DeepEquals, original.AvailabilityZones) - c.Check(actual.ProviderSpaceId(), gc.Equals, original.ProviderSpaceId) - c.Check(actual.ProviderNetworkId(), gc.Equals, original.ProviderNetworkId) -} - func (s *MigrationExportSuite) TestModelStatus(c *gc.C) { model, err := s.State.Export(map[string]string{}, state.NewObjectStore(c, s.State.ModelUUID())) c.Assert(err, jc.ErrorIsNil) @@ -2378,24 +2281,6 @@ func (s *MigrationExportSuite) TestRemoteRelationSettingsForUnitsInCMR(c *gc.C) Role: charm.RoleProvider, Scope: charm.ScopeGlobal, }}, - Spaces: []*environs.ProviderSpaceInfo{{ - CloudType: "ec2", - ProviderAttributes: map[string]interface{}{"network": "network-1"}, - SpaceInfo: network.SpaceInfo{ - Name: "private", - ProviderId: "juju-space-private", - Subnets: []network.SubnetInfo{{ - ProviderId: "juju-subnet-24", - CIDR: "1.2.4.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-private", - ProviderNetworkId: "network-1", - }}, - }, - }}, - Bindings: map[string]string{ - "db": "private", - }, // Macaroon not exported. Macaroon: mac, }) diff --git a/state/migration_import.go b/state/migration_import.go index 0f45dcc464e..a180c9e8612 100644 --- a/state/migration_import.go +++ b/state/migration_import.go @@ -1557,7 +1557,6 @@ func (i *importer) makeRemoteApplicationDoc(app description.RemoteApplication) * URL: app.URL(), SourceModelUUID: app.SourceModelTag().Id(), IsConsumerProxy: app.IsConsumerProxy(), - Bindings: app.Bindings(), Macaroon: app.Macaroon(), Version: app.ConsumeVersion(), } @@ -1575,30 +1574,6 @@ func (i *importer) makeRemoteApplicationDoc(app description.RemoteApplication) * } } doc.Endpoints = eps - descSpaces := app.Spaces() - spaces := make([]remoteSpaceDoc, len(descSpaces)) - for i, space := range descSpaces { - spaces[i] = remoteSpaceDoc{ - CloudType: space.CloudType(), - Name: space.Name(), - ProviderId: space.ProviderId(), - ProviderAttributes: space.ProviderAttributes(), - } - descSubnets := space.Subnets() - subnets := make([]remoteSubnetDoc, len(descSubnets)) - for i, subnet := range descSubnets { - subnets[i] = remoteSubnetDoc{ - CIDR: subnet.CIDR(), - ProviderId: subnet.ProviderId(), - VLANTag: subnet.VLANTag(), - AvailabilityZones: subnet.AvailabilityZones(), - ProviderSpaceId: subnet.ProviderSpaceId(), - ProviderNetworkId: subnet.ProviderNetworkId(), - } - } - spaces[i].Subnets = subnets - } - doc.Spaces = spaces return doc } diff --git a/state/migration_import_tasks_test.go b/state/migration_import_tasks_test.go index 1a5413bc032..e9883f465d1 100644 --- a/state/migration_import_tasks_test.go +++ b/state/migration_import_tasks_test.go @@ -213,9 +213,6 @@ func (s *MigrationImportTasksSuite) TestImportRemoteApplications(c *gc.C) { {Name: "db", Interface: "mysql"}, {Name: "db-admin", Interface: "mysql-root"}, }, - Spaces: []remoteSpaceDoc{ - {CloudType: "ec2"}, - }, } statusDoc := statusDoc{} statusOp := txn.Op{} @@ -271,9 +268,6 @@ func (s *MigrationImportTasksSuite) TestImportRemoteApplicationsWithMissingStatu {Name: "db", Interface: "mysql"}, {Name: "db-admin", Interface: "mysql-root"}, }, - Spaces: []remoteSpaceDoc{ - {CloudType: "ec2"}, - }, } model := NewMockRemoteApplicationsInput(ctrl) diff --git a/state/migration_import_test.go b/state/migration_import_test.go index cd8da051368..410ead06d4d 100644 --- a/state/migration_import_test.go +++ b/state/migration_import_test.go @@ -33,7 +33,6 @@ import ( "github.com/juju/juju/core/permission" "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/status" - "github.com/juju/juju/environs" "github.com/juju/juju/environs/config" internalpassword "github.com/juju/juju/internal/password" "github.com/juju/juju/internal/storage" @@ -2468,17 +2467,6 @@ func (s *MigrationImportSuite) TestRemoteApplications(c *gc.C) { Role: charm.RoleProvider, Scope: charm.ScopeGlobal, }}, - Spaces: []*environs.ProviderSpaceInfo{{ - SpaceInfo: network.SpaceInfo{ - Name: "unicorns", - ProviderId: "space-provider-id", - Subnets: []network.SubnetInfo{{ - CIDR: "10.0.1.0/24", - ProviderId: "subnet-provider-id", - AvailabilityZones: []string{"eu-west-1"}, - }}, - }, - }}, }) c.Assert(err, jc.ErrorIsNil) err = remoteApp.SetStatus(status.StatusInfo{Status: status.Active}) @@ -2514,7 +2502,6 @@ func (s *MigrationImportSuite) TestRemoteApplications(c *gc.C) { c.Assert(token, gc.Equals, "charisma") s.assertRemoteApplicationEndpoints(c, remoteApp, remoteApplication) - s.assertRemoteApplicationSpaces(c, remoteApp, remoteApplication) } func (s *MigrationImportSuite) TestRemoteApplicationsConsumerProxy(c *gc.C) { @@ -2542,17 +2529,6 @@ func (s *MigrationImportSuite) TestRemoteApplicationsConsumerProxy(c *gc.C) { Role: charm.RoleProvider, Scope: charm.ScopeGlobal, }}, - Spaces: []*environs.ProviderSpaceInfo{{ - SpaceInfo: network.SpaceInfo{ - Name: "unicorns", - ProviderId: "space-provider-id", - Subnets: []network.SubnetInfo{{ - CIDR: "10.0.1.0/24", - ProviderId: "subnet-provider-id", - AvailabilityZones: []string{"eu-west-1"}, - }}, - }, - }}, }) c.Assert(err, jc.ErrorIsNil) @@ -2586,7 +2562,6 @@ func (s *MigrationImportSuite) TestRemoteApplicationsConsumerProxy(c *gc.C) { c.Assert(token, gc.Equals, "charisma") s.assertRemoteApplicationEndpoints(c, remoteApp, remoteApplication) - s.assertRemoteApplicationSpaces(c, remoteApp, remoteApplication) } func (s *MigrationImportSuite) assertRemoteApplicationEndpoints(c *gc.C, expected, received *state.RemoteApplication) { @@ -2605,29 +2580,6 @@ func (s *MigrationImportSuite) assertRemoteApplicationEndpoints(c *gc.C, expecte } } -func (s *MigrationImportSuite) assertRemoteApplicationSpaces(c *gc.C, expected, received *state.RemoteApplication) { - receivedSpaces := received.Spaces() - c.Assert(receivedSpaces, gc.HasLen, 1) - - expectedSpaces := expected.Spaces() - c.Assert(expectedSpaces, gc.HasLen, 1) - for k, expectedSpace := range expectedSpaces { - receivedSpace := receivedSpaces[k] - c.Assert(receivedSpace.Name, gc.Equals, expectedSpace.Name) - c.Assert(receivedSpace.ProviderId, gc.Equals, expectedSpace.ProviderId) - - c.Assert(receivedSpace.Subnets, gc.HasLen, 1) - receivedSubnet := receivedSpace.Subnets[0] - - c.Assert(expectedSpace.Subnets, gc.HasLen, 1) - expectedSubnet := expectedSpace.Subnets[0] - - c.Assert(receivedSubnet.CIDR, gc.Equals, expectedSubnet.CIDR) - c.Assert(receivedSubnet.ProviderId, gc.Equals, expectedSubnet.ProviderId) - c.Assert(receivedSubnet.AvailabilityZones, gc.DeepEquals, expectedSubnet.AvailabilityZones) - } -} - func (s *MigrationImportSuite) TestApplicationsWithNilConfigValues(c *gc.C) { application := s.Factory.MakeApplication(c, &factory.ApplicationParams{ CharmConfig: map[string]interface{}{ diff --git a/state/migrations/remoteapplications.go b/state/migrations/remoteapplications.go index ae37ece5cca..ef4bccb6989 100644 --- a/state/migrations/remoteapplications.go +++ b/state/migrations/remoteapplications.go @@ -19,8 +19,6 @@ type MigrationRemoteApplication interface { SourceModel() names.ModelTag IsConsumerProxy() bool Endpoints() ([]MigrationRemoteEndpoint, error) - Bindings() map[string]string - Spaces() []MigrationRemoteSpace GlobalKey() string Macaroon() string ConsumeVersion() int @@ -33,15 +31,6 @@ type MigrationRemoteEndpoint struct { Interface string } -// MigrationRemoteSpace is an in-place representation of the state.RemoteSpace -type MigrationRemoteSpace struct { - CloudType string - Name string - ProviderId string - ProviderAttributes map[string]interface{} - Subnets []MigrationRemoteSubnet -} - // MigrationRemoteSubnet is an in-place representation of the state.RemoteSubnet type MigrationRemoteSubnet struct { CIDR string @@ -110,7 +99,6 @@ func (m ExportRemoteApplications) addRemoteApplication(src RemoteApplicationSour URL: url, SourceModel: app.SourceModel(), IsConsumerProxy: app.IsConsumerProxy(), - Bindings: app.Bindings(), Macaroon: app.Macaroon(), ConsumeVersion: app.ConsumeVersion(), } @@ -136,27 +124,5 @@ func (m ExportRemoteApplications) addRemoteApplication(src RemoteApplicationSour Interface: ep.Interface, }) } - for _, space := range app.Spaces() { - m.addRemoteSpace(descApp, space) - } return nil } - -func (m ExportRemoteApplications) addRemoteSpace(descApp description.RemoteApplication, space MigrationRemoteSpace) { - descSpace := descApp.AddSpace(description.RemoteSpaceArgs{ - CloudType: space.CloudType, - Name: space.Name, - ProviderId: space.ProviderId, - ProviderAttributes: space.ProviderAttributes, - }) - for _, subnet := range space.Subnets { - descSpace.AddSubnet(description.SubnetArgs{ - CIDR: subnet.CIDR, - ProviderId: subnet.ProviderId, - VLANTag: subnet.VLANTag, - AvailabilityZones: subnet.AvailabilityZones, - ProviderSpaceId: subnet.ProviderSpaceId, - ProviderNetworkId: subnet.ProviderNetworkId, - }) - } -} diff --git a/state/migrations/remoteapplications_mock_test.go b/state/migrations/remoteapplications_mock_test.go index 0f72185e469..e79ed184e93 100644 --- a/state/migrations/remoteapplications_mock_test.go +++ b/state/migrations/remoteapplications_mock_test.go @@ -153,20 +153,6 @@ func (mr *MockMigrationRemoteApplicationMockRecorder) SourceModel() *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SourceModel", reflect.TypeOf((*MockMigrationRemoteApplication)(nil).SourceModel)) } -// Spaces mocks base method. -func (m *MockMigrationRemoteApplication) Spaces() []MigrationRemoteSpace { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Spaces") - ret0, _ := ret[0].([]MigrationRemoteSpace) - return ret0 -} - -// Spaces indicates an expected call of Spaces. -func (mr *MockMigrationRemoteApplicationMockRecorder) Spaces() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Spaces", reflect.TypeOf((*MockMigrationRemoteApplication)(nil).Spaces)) -} - // Tag mocks base method. func (m *MockMigrationRemoteApplication) Tag() names.Tag { m.ctrl.T.Helper() diff --git a/state/migrations/remoteapplications_test.go b/state/migrations/remoteapplications_test.go index 0d1bf9d61f6..110ff3290f3 100644 --- a/state/migrations/remoteapplications_test.go +++ b/state/migrations/remoteapplications_test.go @@ -40,42 +40,11 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplication(c *gc.C) { Interface: "db", }, }, nil) - // Return the spaces mocks - expect.Spaces().Return([]MigrationRemoteSpace{ - { - Name: "app-uuid-1-spaces-1", - CloudType: "aws", - ProviderId: "provider-id-1", - ProviderAttributes: map[string]interface{}{ - "attr-1": "value-1", - }, - Subnets: []MigrationRemoteSubnet{ - { - CIDR: "10.0.0.1/24", - ProviderId: "provider-id-2", - VLANTag: 1, - AvailabilityZones: []string{"eu-west-1"}, - ProviderSpaceId: "provider-space-id", - ProviderNetworkId: "provider-network-id", - }, - }, - }, - }) expect.GlobalKey().Return("c#app-uuid-1") }), } - remoteSpace := NewMockRemoteSpace(ctrl) - remoteSpace.EXPECT().AddSubnet(description.SubnetArgs{ - CIDR: "10.0.0.1/24", - ProviderId: "provider-id-2", - VLANTag: 1, - AvailabilityZones: []string{"eu-west-1"}, - ProviderSpaceId: "provider-space-id", - ProviderNetworkId: "provider-network-id", - }) - remoteApplication := NewMockRemoteApplication(ctrl) remoteApplication.EXPECT().SetStatus(description.StatusArgs{ Value: "status-value", @@ -85,14 +54,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplication(c *gc.C) { Role: "role", Interface: "db", }) - remoteApplication.EXPECT().AddSpace(description.RemoteSpaceArgs{ - Name: "app-uuid-1-spaces-1", - CloudType: "aws", - ProviderId: "provider-id-1", - ProviderAttributes: map[string]interface{}{ - "attr-1": "value-1", - }, - }).Return(remoteSpace) source := NewMockRemoteApplicationSource(ctrl) source.EXPECT().AllRemoteApplications().Return(entities, nil) diff --git a/state/remoteapplication.go b/state/remoteapplication.go index c437b68b7da..8b80c71645c 100644 --- a/state/remoteapplication.go +++ b/state/remoteapplication.go @@ -21,7 +21,6 @@ import ( "github.com/juju/juju/core/crossmodel" "github.com/juju/juju/core/status" - "github.com/juju/juju/environs" ) // RemoteApplication represents the state of an application hosted @@ -40,8 +39,6 @@ type remoteApplicationDoc struct { SourceControllerUUID string `bson:"source-controller-uuid"` SourceModelUUID string `bson:"source-model-uuid"` Endpoints []remoteEndpointDoc `bson:"endpoints"` - Spaces []remoteSpaceDoc `bson:"spaces"` - Bindings map[string]string `bson:"bindings"` Life Life `bson:"life"` RelationCount int `bson:"relationcount"` IsConsumerProxy bool `bson:"is-consumer-proxy"` @@ -60,46 +57,6 @@ type remoteEndpointDoc struct { type attributeMap map[string]interface{} -// remoteSpaceDoc represents the internal state of a space in another -// model in the DB. -type remoteSpaceDoc struct { - CloudType string `bson:"cloud-type"` - Name string `bson:"name"` - ProviderId string `bson:"provider-id"` - ProviderAttributes attributeMap `bson:"provider-attributes"` - Subnets []remoteSubnetDoc `bson:"subnets"` -} - -// RemoteSpace represents a space in another model that endpoints are -// bound to. -type RemoteSpace struct { - CloudType string - Name string - ProviderId string - ProviderAttributes attributeMap - Subnets []RemoteSubnet -} - -// remoteSubnetDoc represents a subnet in another model in the DB. -type remoteSubnetDoc struct { - CIDR string `bson:"cidr"` - ProviderId string `bson:"provider-id"` - VLANTag int `bson:"vlan-tag"` - AvailabilityZones []string `bson:"availability-zones"` - ProviderSpaceId string `bson:"provider-space-id"` - ProviderNetworkId string `bson:"provider-network-id"` -} - -// RemoteSubnet represents a subnet in another model. -type RemoteSubnet struct { - CIDR string - ProviderId string - VLANTag int - AvailabilityZones []string - ProviderSpaceId string - ProviderNetworkId string -} - func newRemoteApplication(st *State, doc *remoteApplicationDoc) *RemoteApplication { app := &RemoteApplication{ st: st, @@ -207,63 +164,6 @@ func (a *RemoteApplication) StatusHistory(filter status.StatusHistoryFilter) ([] return statusHistory(args) } -// Spaces returns the remote spaces this application is connected to. -func (a *RemoteApplication) Spaces() []RemoteSpace { - var result []RemoteSpace - for _, space := range a.doc.Spaces { - result = append(result, remoteSpaceFromDoc(space)) - } - return result -} - -// Bindings returns the endpoint->space bindings for the application. -func (a *RemoteApplication) Bindings() map[string]string { - result := make(map[string]string) - for epName, spName := range a.doc.Bindings { - result[epName] = spName - } - return result -} - -// SpaceForEndpoint returns the remote space an endpoint is bound to, -// if one is found. -func (a *RemoteApplication) SpaceForEndpoint(endpointName string) (RemoteSpace, bool) { - spaceName, ok := a.doc.Bindings[endpointName] - if !ok { - return RemoteSpace{}, false - } - for _, space := range a.doc.Spaces { - if space.Name == spaceName { - return remoteSpaceFromDoc(space), true - } - } - return RemoteSpace{}, false -} - -func remoteSpaceFromDoc(space remoteSpaceDoc) RemoteSpace { - result := RemoteSpace{ - CloudType: space.CloudType, - Name: space.Name, - ProviderId: space.ProviderId, - ProviderAttributes: copyAttributes(space.ProviderAttributes), - } - for _, subnet := range space.Subnets { - result.Subnets = append(result.Subnets, remoteSubnetFromDoc(subnet)) - } - return result -} - -func remoteSubnetFromDoc(subnet remoteSubnetDoc) RemoteSubnet { - return RemoteSubnet{ - CIDR: subnet.CIDR, - ProviderId: subnet.ProviderId, - VLANTag: subnet.VLANTag, - AvailabilityZones: copyStrings(subnet.AvailabilityZones), - ProviderSpaceId: subnet.ProviderSpaceId, - ProviderNetworkId: subnet.ProviderNetworkId, - } -} - func copyStrings(values []string) []string { if values == nil { return nil @@ -873,13 +773,6 @@ type AddRemoteApplicationParams struct { // Endpoints describes the endpoints that the remote application implements. Endpoints []charm.Relation - // Spaces describes the network spaces that the remote - // application's endpoints inhabit in the remote model. - Spaces []*environs.ProviderSpaceInfo - - // Bindings maps each endpoint name to the remote space it is bound to. - Bindings map[string]string - // IsConsumerProxy is true when a remote application is created as a result // of a registration operation from a remote model. IsConsumerProxy bool @@ -908,15 +801,6 @@ func (p AddRemoteApplicationParams) Validate() error { if p.SourceModel == (names.ModelTag{}) { return errors.NotValidf("empty source model tag") } - spaceNames := set.NewStrings() - for _, space := range p.Spaces { - spaceNames.Add(string(space.Name)) - } - for endpoint, space := range p.Bindings { - if !spaceNames.Contains(space) { - return errors.NotValidf("endpoint %q bound to missing space %q", endpoint, space) - } - } return nil } @@ -953,7 +837,6 @@ func (st *State) AddRemoteApplication(args AddRemoteApplicationParams) (_ *Remot SourceControllerUUID: args.ExternalControllerUUID, SourceModelUUID: args.SourceModel.Id(), URL: args.URL, - Bindings: args.Bindings, Life: Alive, IsConsumerProxy: args.IsConsumerProxy, Version: args.ConsumeVersion, @@ -976,28 +859,6 @@ func (st *State) AddRemoteApplication(args AddRemoteApplicationParams) (_ *Remot } } appDoc.Endpoints = eps - spaces := make([]remoteSpaceDoc, len(args.Spaces)) - for i, space := range args.Spaces { - spaces[i] = remoteSpaceDoc{ - CloudType: space.CloudType, - Name: string(space.Name), - ProviderId: string(space.ProviderId), - ProviderAttributes: space.ProviderAttributes, - } - subnets := make([]remoteSubnetDoc, len(space.Subnets)) - for i, subnet := range space.Subnets { - subnets[i] = remoteSubnetDoc{ - CIDR: subnet.CIDR, - ProviderId: string(subnet.ProviderId), - VLANTag: subnet.VLANTag, - AvailabilityZones: copyStrings(subnet.AvailabilityZones), - ProviderSpaceId: string(subnet.ProviderSpaceId), - ProviderNetworkId: string(subnet.ProviderNetworkId), - } - } - spaces[i].Subnets = subnets - } - appDoc.Spaces = spaces app := newRemoteApplication(st, appDoc) buildTxn := func(attempt int) ([]txn.Op, error) { diff --git a/state/remoteapplication_test.go b/state/remoteapplication_test.go index 2e665721bd8..ac8d1e9c7bd 100644 --- a/state/remoteapplication_test.go +++ b/state/remoteapplication_test.go @@ -14,10 +14,8 @@ import ( "github.com/juju/worker/v4/workertest" gc "gopkg.in/check.v1" - "github.com/juju/juju/core/network" "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/status" - "github.com/juju/juju/environs" "github.com/juju/juju/internal/uuid" "github.com/juju/juju/state" "github.com/juju/juju/state/testing" @@ -61,48 +59,6 @@ func (s *remoteApplicationSuite) makeRemoteApplication(c *gc.C, name, url string }, } - spaces := []*environs.ProviderSpaceInfo{{ - CloudType: "ec2", - ProviderAttributes: map[string]interface{}{ - "thing1": 23, - "thing2": "halberd", - "network": "network-1", - }, - SpaceInfo: network.SpaceInfo{ - Name: "public", - ProviderId: "juju-space-public", - Subnets: []network.SubnetInfo{{ - ProviderId: "juju-subnet-12", - CIDR: "1.2.3.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-public", - ProviderNetworkId: "network-1", - }}, - }, - }, { - CloudType: "ec2", - ProviderAttributes: map[string]interface{}{ - "thing1": 24, - "thing2": "bardiche", - "network": "network-1", - }, - SpaceInfo: network.SpaceInfo{ - Name: "private", - ProviderId: "juju-space-private", - Subnets: []network.SubnetInfo{{ - ProviderId: "juju-subnet-24", - CIDR: "1.2.4.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-private", - ProviderNetworkId: "network-1", - }}, - }, - }} - bindings := map[string]string{ - "db": "private", - "db-admin": "private", - "logging": "public", - } mac, err := newMacaroon("test") c.Assert(err, jc.ErrorIsNil) s.application, err = s.State.AddRemoteApplication(state.AddRemoteApplicationParams{ @@ -112,8 +68,6 @@ func (s *remoteApplicationSuite) makeRemoteApplication(c *gc.C, name, url string SourceModel: s.Model.ModelTag(), Token: "app-token", Endpoints: eps, - Spaces: spaces, - Bindings: bindings, Macaroon: mac, }) c.Assert(err, jc.ErrorIsNil) @@ -257,62 +211,6 @@ func (s *remoteApplicationSuite) TestURL(c *gc.C) { c.Assert(url, gc.Equals, "") } -func (s *remoteApplicationSuite) TestSpaces(c *gc.C) { - spaces := s.application.Spaces() - c.Assert(spaces, gc.DeepEquals, []state.RemoteSpace{{ - CloudType: "ec2", - Name: "public", - ProviderId: "juju-space-public", - ProviderAttributes: map[string]interface{}{ - "thing1": 23, - "thing2": "halberd", - "network": "network-1", - }, - Subnets: []state.RemoteSubnet{{ - ProviderId: "juju-subnet-12", - CIDR: "1.2.3.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-public", - ProviderNetworkId: "network-1", - }}, - }, { - CloudType: "ec2", - Name: "private", - ProviderId: "juju-space-private", - ProviderAttributes: map[string]interface{}{ - "thing1": 24, - "thing2": "bardiche", - "network": "network-1", - }, - Subnets: []state.RemoteSubnet{{ - ProviderId: "juju-subnet-24", - CIDR: "1.2.4.0/24", - AvailabilityZones: []string{"az1", "az2"}, - ProviderSpaceId: "juju-space-private", - ProviderNetworkId: "network-1", - }}, - }}) -} - -func (s *remoteApplicationSuite) TestSpaceForEndpoint(c *gc.C) { - space, ok := s.application.SpaceForEndpoint("db") - c.Assert(ok, jc.IsTrue) - c.Assert(space.Name, gc.Equals, "private") - space, ok = s.application.SpaceForEndpoint("logging") - c.Assert(ok, jc.IsTrue) - c.Assert(space.Name, gc.Equals, "public") - space, ok = s.application.SpaceForEndpoint("something else") - c.Assert(ok, jc.IsFalse) -} - -func (s *remoteApplicationSuite) TestBindings(c *gc.C) { - c.Assert(s.application.Bindings(), gc.DeepEquals, map[string]string{ - "db": "private", - "db-admin": "private", - "logging": "public", - }) -} - func (s *remoteApplicationSuite) TestMysqlEndpoints(c *gc.C) { _, err := s.application.Endpoint("foo") c.Assert(err, gc.ErrorMatches, `saas application "mysql" has no "foo" relation`) @@ -412,42 +310,6 @@ func (s *remoteApplicationSuite) TestAddRemoteApplicationErrors(c *gc.C) { c.Assert(err, gc.ErrorMatches, `saas application "borken" not found`) } -func (s *remoteApplicationSuite) TestParamsValidateChecksBindings(c *gc.C) { - eps := []charm.Relation{ - { - Interface: "mysql", - Name: "db", - Role: charm.RoleProvider, - Scope: charm.ScopeGlobal, - }, - } - - spaces := []*environs.ProviderSpaceInfo{{ - SpaceInfo: network.SpaceInfo{ - Name: "public", - }, - }} - bindings := map[string]string{ - "db": "private", - } - args := state.AddRemoteApplicationParams{ - Name: "mysql", - URL: "me/model.mysql", - SourceModel: s.Model.ModelTag(), - Token: "app-token", - Endpoints: eps, - Spaces: spaces, - Bindings: bindings, - } - err := args.Validate() - c.Assert(err, gc.ErrorMatches, `endpoint "db" bound to missing space "private" not valid`) - bindings["db"] = "public" - // Tolerates bindings for non-existent endpoints. - bindings["gidget"] = "public" - err = args.Validate() - c.Assert(err, jc.ErrorIsNil) -} - func (s *remoteApplicationSuite) TestAddRemoteApplication(c *gc.C) { foo, err := s.State.AddRemoteApplication(state.AddRemoteApplicationParams{ Name: "foo", OfferUUID: "offer-uuid", URL: "me/model.foo", SourceModel: s.Model.ModelTag()}) From 0d47fb884cfbfa45163416effcd49781e2b97a36 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Mon, 18 Mar 2024 23:58:04 +0100 Subject: [PATCH 2/4] Rebuild schema, generate mocks --- .../application/mocks/application_mock.go | 14 ---------- apiserver/facades/schema.json | 27 ++++++++----------- .../remoteapplications_mock_test.go | 14 ---------- state/migrations/remoteapplications_test.go | 10 ------- state/remoteapplication.go | 22 --------------- 5 files changed, 11 insertions(+), 76 deletions(-) diff --git a/apiserver/facades/client/application/mocks/application_mock.go b/apiserver/facades/client/application/mocks/application_mock.go index b5669f92f7b..819959b844a 100644 --- a/apiserver/facades/client/application/mocks/application_mock.go +++ b/apiserver/facades/client/application/mocks/application_mock.go @@ -1490,20 +1490,6 @@ func (mr *MockRemoteApplicationMockRecorder) AddEndpoints(arg0 any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddEndpoints", reflect.TypeOf((*MockRemoteApplication)(nil).AddEndpoints), arg0) } -// Bindings mocks base method. -func (m *MockRemoteApplication) Bindings() map[string]string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Bindings") - ret0, _ := ret[0].(map[string]string) - return ret0 -} - -// Bindings indicates an expected call of Bindings. -func (mr *MockRemoteApplicationMockRecorder) Bindings() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Bindings", reflect.TypeOf((*MockRemoteApplication)(nil).Bindings)) -} - // Destroy mocks base method. func (m *MockRemoteApplication) Destroy() error { m.ctrl.T.Helper() diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 022c53fa61b..d05fb719e55 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -3181,33 +3181,28 @@ "Constraints": { "type": "object", "properties": { - "Attributes": { - "type": "object", - "patternProperties": { - ".*": { - "type": "string" - } - } - }, "Count": { "type": "integer" }, - "Type": { + "Pool": { "type": "string" + }, + "Size": { + "type": "integer" } }, "additionalProperties": false, "required": [ - "Type", - "Count", - "Attributes" + "Pool", + "Size", + "Count" ] }, - "ConsumeApplicationArgV5": { + "ConsumeApplicationArg": { "type": "object", "properties": { - "ApplicationOfferDetailsV5": { - "$ref": "#/definitions/ApplicationOfferDetailsV5" + "ApplicationOfferDetails": { + "$ref": "#/definitions/ApplicationOfferDetails" }, "application-alias": { "type": "string" @@ -47348,4 +47343,4 @@ } } } -] \ No newline at end of file +] diff --git a/state/migrations/remoteapplications_mock_test.go b/state/migrations/remoteapplications_mock_test.go index e79ed184e93..88710f503ea 100644 --- a/state/migrations/remoteapplications_mock_test.go +++ b/state/migrations/remoteapplications_mock_test.go @@ -40,20 +40,6 @@ func (m *MockMigrationRemoteApplication) EXPECT() *MockMigrationRemoteApplicatio return m.recorder } -// Bindings mocks base method. -func (m *MockMigrationRemoteApplication) Bindings() map[string]string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Bindings") - ret0, _ := ret[0].(map[string]string) - return ret0 -} - -// Bindings indicates an expected call of Bindings. -func (mr *MockMigrationRemoteApplicationMockRecorder) Bindings() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Bindings", reflect.TypeOf((*MockMigrationRemoteApplication)(nil).Bindings)) -} - // ConsumeVersion mocks base method. func (m *MockMigrationRemoteApplication) ConsumeVersion() int { m.ctrl.T.Helper() diff --git a/state/migrations/remoteapplications_test.go b/state/migrations/remoteapplications_test.go index 110ff3290f3..9e42192dcc5 100644 --- a/state/migrations/remoteapplications_test.go +++ b/state/migrations/remoteapplications_test.go @@ -29,9 +29,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplication(c *gc.C) { expect.IsConsumerProxy().Return(false) expect.Macaroon().Return("mac") expect.ConsumeVersion().Return(1) - expect.Bindings().Return(map[string]string{ - "binding-key": "binding-value", - }) // Return the endpoint mocks expect.Endpoints().Return([]MigrationRemoteEndpoint{ { @@ -107,9 +104,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplicationWithEndpoints expect.IsConsumerProxy().Return(false) expect.Macaroon().Return("mac") expect.ConsumeVersion().Return(1) - expect.Bindings().Return(map[string]string{ - "binding-key": "binding-value", - }) // Return the endpoint mocks expect.Endpoints().Return(nil, errors.New("fail")) expect.GlobalKey().Return("c#app-uuid-1") @@ -159,10 +153,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplicationWithStatusArg expect.IsConsumerProxy().Return(false) expect.Macaroon().Return("mac") expect.ConsumeVersion().Return(1) - expect.Bindings().Return(map[string]string{ - "binding-key": "binding-value", - }) - expect.GlobalKey().Return("c#app-uuid-1") }), } diff --git a/state/remoteapplication.go b/state/remoteapplication.go index 8b80c71645c..2b67daeb713 100644 --- a/state/remoteapplication.go +++ b/state/remoteapplication.go @@ -55,8 +55,6 @@ type remoteEndpointDoc struct { Scope charm.RelationScope `bson:"scope"` } -type attributeMap map[string]interface{} - func newRemoteApplication(st *State, doc *remoteApplicationDoc) *RemoteApplication { app := &RemoteApplication{ st: st, @@ -164,26 +162,6 @@ func (a *RemoteApplication) StatusHistory(filter status.StatusHistoryFilter) ([] return statusHistory(args) } -func copyStrings(values []string) []string { - if values == nil { - return nil - } - result := make([]string, len(values)) - copy(result, values) - return result -} - -func copyAttributes(values attributeMap) attributeMap { - if values == nil { - return nil - } - result := make(attributeMap) - for key, value := range values { - result[key] = value - } - return result -} - // DestroyOperation returns a model operation to destroy remote application. func (a *RemoteApplication) DestroyOperation(force bool) *DestroyRemoteApplicationOperation { return &DestroyRemoteApplicationOperation{ From 14d6a6fe331ac8be68f58b06d8da21aca07fec83 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Tue, 19 Mar 2024 10:49:15 +0100 Subject: [PATCH 3/4] Fix state tests after removal of remote spaces --- apiserver/facades/schema.json | 27 +++++++++------ state/migration_export_test.go | 37 ++++++++++++++++----- state/migrations/remoteapplications_test.go | 9 ----- 3 files changed, 45 insertions(+), 28 deletions(-) diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index d05fb719e55..022c53fa61b 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -3181,28 +3181,33 @@ "Constraints": { "type": "object", "properties": { + "Attributes": { + "type": "object", + "patternProperties": { + ".*": { + "type": "string" + } + } + }, "Count": { "type": "integer" }, - "Pool": { + "Type": { "type": "string" - }, - "Size": { - "type": "integer" } }, "additionalProperties": false, "required": [ - "Pool", - "Size", - "Count" + "Type", + "Count", + "Attributes" ] }, - "ConsumeApplicationArg": { + "ConsumeApplicationArgV5": { "type": "object", "properties": { - "ApplicationOfferDetails": { - "$ref": "#/definitions/ApplicationOfferDetails" + "ApplicationOfferDetailsV5": { + "$ref": "#/definitions/ApplicationOfferDetailsV5" }, "application-alias": { "type": "string" @@ -47343,4 +47348,4 @@ } } } -] +] \ No newline at end of file diff --git a/state/migration_export_test.go b/state/migration_export_test.go index 25fd78ba7ed..02e7fd540a6 100644 --- a/state/migration_export_test.go +++ b/state/migration_export_test.go @@ -2158,6 +2158,35 @@ func (s *MigrationExportSuite) newResource(c *gc.C, appName, name string, revisi } func (s *MigrationExportSuite) TestRemoteApplications(c *gc.C) { + mac, err := newMacaroon("apimac") + c.Assert(err, gc.IsNil) + _, err = s.State.AddRemoteApplication(state.AddRemoteApplicationParams{ + Name: "gravy-rainbow", + URL: "me/model.rainbow", + SourceModel: s.Model.ModelTag(), + Token: "charisma", + OfferUUID: "offer-uuid", + Endpoints: []charm.Relation{{ + Interface: "mysql", + Name: "db", + Role: charm.RoleProvider, + Scope: charm.ScopeGlobal, + }, { + Interface: "mysql-root", + Name: "db-admin", + Limit: 5, + Role: charm.RoleProvider, + Scope: charm.ScopeGlobal, + }, { + Interface: "logging", + Name: "logging", + Role: charm.RoleProvider, + Scope: charm.ScopeGlobal, + }}, + // Macaroon not exported. + Macaroon: mac, + }) + c.Assert(err, jc.ErrorIsNil) state.AddTestingApplication(c, s.State, s.objectStore, "wordpress", state.AddTestingCharm(c, s.State, "wordpress")) eps, err := s.State.InferEndpoints("gravy-rainbow", "wordpress") c.Assert(err, jc.ErrorIsNil) @@ -2175,11 +2204,6 @@ func (s *MigrationExportSuite) TestRemoteApplications(c *gc.C) { c.Check(app.URL(), gc.Equals, "me/model.rainbow") c.Check(app.SourceModelTag(), gc.Equals, s.Model.ModelTag()) c.Check(app.IsConsumerProxy(), jc.IsFalse) - c.Check(app.Bindings(), gc.DeepEquals, map[string]string{ - "db": "private", - "db-admin": "private", - "logging": "public", - }) c.Assert(app.Endpoints(), gc.HasLen, 3) ep := app.Endpoints()[0] @@ -2195,9 +2219,6 @@ func (s *MigrationExportSuite) TestRemoteApplications(c *gc.C) { c.Check(ep.Interface(), gc.Equals, "logging") c.Check(ep.Role(), gc.Equals, "provider") - actualSpaces := app.Spaces() - c.Assert(actualSpaces, gc.HasLen, 2) - c.Assert(model.Relations(), gc.HasLen, 1) rel := model.Relations()[0] c.Assert(rel.Key(), gc.Equals, "wordpress:db gravy-rainbow:db") diff --git a/state/migrations/remoteapplications_test.go b/state/migrations/remoteapplications_test.go index 9e42192dcc5..66b1058332c 100644 --- a/state/migrations/remoteapplications_test.go +++ b/state/migrations/remoteapplications_test.go @@ -67,9 +67,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplication(c *gc.C) { IsConsumerProxy: false, Macaroon: "mac", ConsumeVersion: 1, - Bindings: map[string]string{ - "binding-key": "binding-value", - }, }).Return(remoteApplication) migration := ExportRemoteApplications{} @@ -130,9 +127,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplicationWithEndpoints IsConsumerProxy: false, Macaroon: "mac", ConsumeVersion: 1, - Bindings: map[string]string{ - "binding-key": "binding-value", - }, }).Return(remoteApplication) migration := ExportRemoteApplications{} @@ -174,9 +168,6 @@ func (s *RemoteApplicationsExportSuite) TestExportRemoteApplicationWithStatusArg IsConsumerProxy: false, Macaroon: "mac", ConsumeVersion: 1, - Bindings: map[string]string{ - "binding-key": "binding-value", - }, }).Return(remoteApplication) migration := ExportRemoteApplications{} From 2a8cc8e1d36978debed19a27d4a091740bfbdf09 Mon Sep 17 00:00:00 2001 From: Nicolas Vinuesa Date: Fri, 22 Mar 2024 11:06:29 +0100 Subject: [PATCH 4/4] Remove older API versions supporting remote spaces --- api/facadeversions.go | 4 +- apiserver/admin_test.go | 2 +- .../facades/client/application/application.go | 17 ------ .../applicationoffers/applicationoffers.go | 25 --------- .../client/applicationoffers/register.go | 14 ----- .../crossmodelrelations.go | 36 ------------ .../crossmodelrelations/register.go | 15 ----- apiserver/restrict_anonymous_test.go | 10 ---- apiserver/restrict_controller_test.go | 2 +- rpc/params/crossmodel.go | 56 ------------------- 10 files changed, 4 insertions(+), 177 deletions(-) diff --git a/api/facadeversions.go b/api/facadeversions.go index fab5a80c650..1da0ecbf7cd 100644 --- a/api/facadeversions.go +++ b/api/facadeversions.go @@ -30,7 +30,7 @@ var facadeVersions = facades.FacadeVersions{ "AllWatcher": {3}, "Annotations": {2}, "Application": {19, 20}, - "ApplicationOffers": {4, 5}, + "ApplicationOffers": {5}, "ApplicationScaler": {1}, "Backups": {3}, "Block": {2}, @@ -54,7 +54,7 @@ var facadeVersions = facades.FacadeVersions{ "CredentialManager": {1}, "CredentialValidator": {2}, "CrossController": {1}, - "CrossModelRelations": {2, 3}, + "CrossModelRelations": {3}, "CrossModelSecrets": {1}, "Deployer": {1}, "DiskManager": {2}, diff --git a/apiserver/admin_test.go b/apiserver/admin_test.go index 8262eed1f7b..db48347f498 100644 --- a/apiserver/admin_test.go +++ b/apiserver/admin_test.go @@ -588,7 +588,7 @@ func (s *loginSuite) TestAnonymousModelLogin(c *gc.C) { c.Assert(result.ControllerTag, gc.Equals, s.ControllerModel(c).State().ControllerTag().String()) c.Assert(result.ModelTag, gc.Equals, names.NewModelTag(s.ControllerModelUUID()).String()) c.Assert(result.Facades, jc.DeepEquals, []params.FacadeVersions{ - {Name: "CrossModelRelations", Versions: []int{2, 3}}, + {Name: "CrossModelRelations", Versions: []int{3}}, {Name: "CrossModelSecrets", Versions: []int{1}}, {Name: "NotifyWatcher", Versions: []int{1}}, {Name: "OfferStatusWatcher", Versions: []int{1}}, diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go index 514694f858e..bc780d428df 100644 --- a/apiserver/facades/client/application/application.go +++ b/apiserver/facades/client/application/application.go @@ -2071,23 +2071,6 @@ func (api *APIBase) consumeOne(ctx context.Context, arg params.ConsumeApplicatio return err } -// Consume adds remote applications to the model without creating any -// relations. -func (api *APIv19) Consume(ctx context.Context, args params.ConsumeApplicationArgsV4) (params.ErrorResults, error) { - var consumeApplicationArgs []params.ConsumeApplicationArgV5 - for _, arg := range args.Args { - consumeApplicationArgs = append(consumeApplicationArgs, params.ConsumeApplicationArgV5{ - Macaroon: arg.Macaroon, - ControllerInfo: arg.ControllerInfo, - ApplicationAlias: arg.ApplicationAlias, - ApplicationOfferDetailsV5: arg.ApplicationOfferDetailsV5, - }) - } - return api.APIv20.Consume(ctx, params.ConsumeApplicationArgsV5{ - Args: consumeApplicationArgs, - }) -} - // saveRemoteApplication saves the details of the specified remote application and its endpoints // to the state model so relations to the remote application can be created. func (api *APIBase) saveRemoteApplication( diff --git a/apiserver/facades/client/applicationoffers/applicationoffers.go b/apiserver/facades/client/applicationoffers/applicationoffers.go index 952033b8f18..100b2fbe687 100644 --- a/apiserver/facades/client/applicationoffers/applicationoffers.go +++ b/apiserver/facades/client/applicationoffers/applicationoffers.go @@ -34,12 +34,6 @@ type OffersAPIv5 struct { authContext *commoncrossmodel.AuthContext } -// OffersAPIv4 implements the cross model interface and is the concrete -// implementation of the api end point. -type OffersAPIv4 struct { - OffersAPIv5 -} - // createAPI returns a new application offers OffersAPI facade. func createOffersAPI( getApplicationOffers func(interface{}) jujucrossmodel.ApplicationOffers, @@ -164,25 +158,6 @@ func (api *OffersAPIv5) ListApplicationOffers(ctx context.Context, filters param return result, nil } -// ListApplicationOffers gets deployed details about application offers that match given filter. -// The results contain details about the deployed applications such as connection count. -func (api *OffersAPIv4) ListApplicationOffers(ctx context.Context, filters params.OfferFilters) (params.QueryApplicationOffersResultsV4, error) { - res, err := api.OffersAPIv5.ListApplicationOffers(ctx, filters) - if err != nil { - return params.QueryApplicationOffersResultsV4{}, errors.Trace(err) - } - resultsV4 := make([]params.ApplicationOfferAdminDetailsV4, len(res.Results)) - for i, result := range res.Results { - resultsV4[i] = params.ApplicationOfferAdminDetailsV4{ - ApplicationOfferAdminDetailsV5: result, - } - } - - return params.QueryApplicationOffersResultsV4{ - Results: resultsV4, - }, nil -} - // ModifyOfferAccess changes the application offer access granted to users. func (api *OffersAPIv5) ModifyOfferAccess(ctx context.Context, args params.ModifyOfferAccessRequest) (result params.ErrorResults, _ error) { result = params.ErrorResults{ diff --git a/apiserver/facades/client/applicationoffers/register.go b/apiserver/facades/client/applicationoffers/register.go index a43320cf077..3dd72cc25a5 100644 --- a/apiserver/facades/client/applicationoffers/register.go +++ b/apiserver/facades/client/applicationoffers/register.go @@ -19,25 +19,11 @@ import ( // Register is called to expose a package of facades onto a given registry. func Register(registry facade.FacadeRegistry) { - registry.MustRegister("ApplicationOffers", 4, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) { - return newOffersAPIV4(ctx) - }, reflect.TypeOf((*OffersAPIv4)(nil))) registry.MustRegister("ApplicationOffers", 5, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) { return newOffersAPI(ctx) }, reflect.TypeOf((*OffersAPIv5)(nil))) } -// newOffersAPIV4 returns a new application offers OffersAPIV4 facade. -func newOffersAPIV4(ctx facade.ModelContext) (*OffersAPIv4, error) { - offersAPI, err := newOffersAPI(ctx) - if err != nil { - return nil, errors.Trace(err) - } - return &OffersAPIv4{ - *offersAPI, - }, nil -} - // newOffersAPI returns a new application offers OffersAPI facade. func newOffersAPI(facadeContext facade.ModelContext) (*OffersAPIv5, error) { serviceFactory := facadeContext.ServiceFactory() diff --git a/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go b/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go index ee31b0b457f..01c23a7e6da 100644 --- a/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go +++ b/apiserver/facades/controller/crossmodelrelations/crossmodelrelations.go @@ -54,11 +54,6 @@ type CrossModelRelationsAPIv3 struct { logger loggo.Logger } -// CrossModelRelationsAPIv2 provides access to the CrossModelRelations API facade. -type CrossModelRelationsAPIv2 struct { - *CrossModelRelationsAPIv3 -} - // NewCrossModelRelationsAPI returns a new server-side CrossModelRelationsAPI facade. func NewCrossModelRelationsAPI( st CrossModelRelationsState, @@ -169,37 +164,6 @@ func (api *CrossModelRelationsAPIv3) PublishRelationChanges( return results, nil } -// RegisterRemoteRelations sets up the model to participate -// in the specified relations. This operation is idempotent. -func (api *CrossModelRelationsAPIv2) RegisterRemoteRelations( - ctx context.Context, - relations params.RegisterRemoteRelationArgsV2, -) (params.RegisterRemoteRelationResults, error) { - results := params.RegisterRemoteRelationResults{ - Results: make([]params.RegisterRemoteRelationResult, len(relations.Relations)), - } - for i, relation := range relations.Relations { - id, err := api.registerRemoteRelation(ctx, - params.RegisterRemoteRelationArg{ - ApplicationToken: relation.ApplicationToken, - SourceModelTag: relation.SourceModelTag, - RelationToken: relation.RelationToken, - RemoteEndpoint: relation.RemoteEndpoint, - // RemoteSpace isn't used so we can simply - // ignore it. - OfferUUID: relation.OfferUUID, - LocalEndpointName: relation.LocalEndpointName, - ConsumeVersion: relation.ConsumeVersion, - Macaroons: relation.Macaroons, - BakeryVersion: relation.BakeryVersion, - }, - ) - results.Results[i].Result = id - results.Results[i].Error = apiservererrors.ServerError(err) - } - return results, nil -} - // RegisterRemoteRelations sets up the model to participate // in the specified relations. This operation is idempotent. func (api *CrossModelRelationsAPIv3) RegisterRemoteRelations( diff --git a/apiserver/facades/controller/crossmodelrelations/register.go b/apiserver/facades/controller/crossmodelrelations/register.go index 0d52674c5f1..8dcebf1103b 100644 --- a/apiserver/facades/controller/crossmodelrelations/register.go +++ b/apiserver/facades/controller/crossmodelrelations/register.go @@ -7,8 +7,6 @@ import ( "context" "reflect" - "github.com/juju/errors" - "github.com/juju/juju/apiserver/common" commoncrossmodel "github.com/juju/juju/apiserver/common/crossmodel" "github.com/juju/juju/apiserver/common/firewall" @@ -18,24 +16,11 @@ import ( // Register is called to expose a package of facades onto a given registry. func Register(registry facade.FacadeRegistry) { - registry.MustRegister("CrossModelRelations", 2, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) { - return newStateCrossModelRelationsAPIV2(ctx) // Adds WatchRelationChanges, removes WatchRelationUnits - }, reflect.TypeOf((*CrossModelRelationsAPIv2)(nil))) registry.MustRegister("CrossModelRelations", 3, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) { return newStateCrossModelRelationsAPI(ctx) // Removes remote spaces }, reflect.TypeOf((*CrossModelRelationsAPIv3)(nil))) } -// newStateCrossModelRelationsAPIV2 creates a new server-side CrossModelRelations API facade -// backed by global state. -func newStateCrossModelRelationsAPIV2(ctx facade.ModelContext) (*CrossModelRelationsAPIv2, error) { - api, err := newStateCrossModelRelationsAPI(ctx) - if err != nil { - return nil, errors.Trace(err) - } - return &CrossModelRelationsAPIv2{api}, nil -} - // newStateCrossModelRelationsAPI creates a new server-side CrossModelRelations API facade // backed by global state. func newStateCrossModelRelationsAPI(ctx facade.ModelContext) (*CrossModelRelationsAPIv3, error) { diff --git a/apiserver/restrict_anonymous_test.go b/apiserver/restrict_anonymous_test.go index 0be0833f8f6..f7728d5a91e 100644 --- a/apiserver/restrict_anonymous_test.go +++ b/apiserver/restrict_anonymous_test.go @@ -25,19 +25,9 @@ func (s *restrictAnonymousSuite) SetUpSuite(c *gc.C) { s.root = apiserver.TestingAnonymousRoot() } -func (s *restrictAnonymousSuite) TestAllowed(c *gc.C) { - s.assertMethod(c, "CrossModelRelations", 2, "RegisterRemoteRelations") -} - func (s *restrictAnonymousSuite) TestNotAllowed(c *gc.C) { caller, err := s.root.FindMethod("Client", clientFacadeVersion, "FullStatus") c.Assert(err, gc.ErrorMatches, `facade "Client" not supported for anonymous API connections`) c.Assert(err, jc.ErrorIs, errors.NotSupported) c.Assert(caller, gc.IsNil) } - -func (s *restrictAnonymousSuite) assertMethod(c *gc.C, facadeName string, version int, method string) { - caller, err := s.root.FindMethod(facadeName, version, method) - c.Check(err, jc.ErrorIsNil) - c.Check(caller, gc.NotNil) -} diff --git a/apiserver/restrict_controller_test.go b/apiserver/restrict_controller_test.go index 606e7defef8..fef1ce5fc04 100644 --- a/apiserver/restrict_controller_test.go +++ b/apiserver/restrict_controller_test.go @@ -33,7 +33,7 @@ func (s *restrictControllerSuite) TestAllowed(c *gc.C) { s.assertMethod(c, "Pinger", pingerFacadeVersion, "Ping") s.assertMethod(c, "Bundle", 8, "GetChangesMapArgs") s.assertMethod(c, "HighAvailability", highAvailabilityFacadeVersion, "EnableHA") - s.assertMethod(c, "ApplicationOffers", 4, "ApplicationOffers") + s.assertMethod(c, "ApplicationOffers", 5, "ApplicationOffers") } func (s *restrictControllerSuite) TestNotAllowed(c *gc.C) { diff --git a/rpc/params/crossmodel.go b/rpc/params/crossmodel.go index f7c17206dcd..73a86b04f71 100644 --- a/rpc/params/crossmodel.go +++ b/rpc/params/crossmodel.go @@ -79,13 +79,6 @@ type ApplicationOfferDetailsV5 struct { Users []OfferUserDetails `json:"users,omitempty"` } -// ApplicationOfferDetailsV4 represents an application offering from an external model. -type ApplicationOfferDetailsV4 struct { - ApplicationOfferDetailsV5 - Spaces []RemoteSpace `json:"spaces,omitempty"` - Bindings map[string]string `json:"bindings,omitempty"` -} - // OfferUserDetails represents an offer consumer and their permission on the offer. type OfferUserDetails struct { UserName string `json:"user"` @@ -102,14 +95,6 @@ type ApplicationOfferAdminDetailsV5 struct { Connections []OfferConnection `json:"connections,omitempty"` } -// ApplicationOfferAdminDetailsV4 represents an application offering, -// including details about how it has been deployed. -type ApplicationOfferAdminDetailsV4 struct { - ApplicationOfferAdminDetailsV5 - Spaces []RemoteSpace `json:"spaces,omitempty"` - Bindings map[string]string `json:"bindings,omitempty"` -} - // OfferConnection holds details about a connection to an offer. type OfferConnection struct { SourceModelTag string `json:"source-model-tag"` @@ -126,12 +111,6 @@ type QueryApplicationOffersResultsV5 struct { Results []ApplicationOfferAdminDetailsV5 `json:"results"` } -// QueryApplicationOffersResultsV4 is a result of searching application offers. -type QueryApplicationOffersResultsV4 struct { - // Results contains application offers matching each filter. - Results []ApplicationOfferAdminDetailsV4 `json:"results"` -} - // AddApplicationOffers is used when adding offers to an application directory. type AddApplicationOffers struct { Offers []AddApplicationOffer @@ -161,15 +140,6 @@ type RemoteEndpoint struct { Limit int `json:"limit"` } -// RemoteSpace represents a space in some remote model. -type RemoteSpace struct { - CloudType string `json:"cloud-type"` - Name string `json:"name"` - ProviderId string `json:"provider-id"` - ProviderAttributes map[string]interface{} `json:"provider-attributes"` - Subnets []Subnet `json:"subnets"` -} - // ApplicationOfferResult is a result of querying a remote // application offer based on its URL. type ApplicationOfferResult struct { @@ -211,23 +181,11 @@ type ConsumeApplicationArgV5 struct { ApplicationAlias string `json:"application-alias,omitempty"` } -// ConsumeApplicationArgV4 holds the arguments for consuming a remote application. -type ConsumeApplicationArgV4 struct { - ConsumeApplicationArgV5 - Spaces []RemoteSpace `json:"spaces,omitempty"` - Bindings map[string]string `json:"bindings,omitempty"` -} - // ConsumeApplicationArgsV5 is a collection of arg for consuming applications. type ConsumeApplicationArgsV5 struct { Args []ConsumeApplicationArgV5 `json:"args,omitempty"` } -// ConsumeApplicationArgsV4 is a collection of arg for consuming applications. -type ConsumeApplicationArgsV4 struct { - Args []ConsumeApplicationArgV4 `json:"args,omitempty"` -} - // TokenResult holds a token and an error. type TokenResult struct { Token string `json:"token,omitempty"` @@ -572,25 +530,11 @@ type RegisterRemoteRelationArg struct { BakeryVersion bakery.Version `json:"bakery-version,omitempty"` } -// RegisterRemoteRelationArg holds attributes used to register a remote relation. -type RegisterRemoteRelationArgV2 struct { - RegisterRemoteRelationArg - - // RemoteSpace contains provider-level info about the space the - // endpoint is bound to in the remote model. - RemoteSpace RemoteSpace `json:"remote-space"` -} - // RegisterRemoteRelationArgs holds args used to add remote relations. type RegisterRemoteRelationArgs struct { Relations []RegisterRemoteRelationArg `json:"relations"` } -// RegisterRemoteRelationArgsV2 holds args used to add remote relations. -type RegisterRemoteRelationArgsV2 struct { - Relations []RegisterRemoteRelationArgV2 `json:"relations"` -} - // RegisterRemoteRelationResult holds a remote relation details and an error. type RegisterRemoteRelationResult struct { Result *RemoteRelationDetails `json:"result,omitempty"`