Skip to content

Commit

Permalink
Merge pull request juju#17056 from nvinuesa/remove-remote-spaces
Browse files Browse the repository at this point in the history
juju#17056

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.



## Checklist



- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps


Bootstrap and create a model:
```
juju bootstrap localhost c
juju add-model m1
```
deploy grafana, create a second model and deploy influx on it:
```
juju deploy grafana
juju add-model m2
juju deploy influxdb
```
offer the `grafana-source` endpoint of influxdb and integrate it with grafana:
```
juju offer influxdb:grafana-source
juju switch m1
juju integrate grafana admin/m2.influxdb
```
Also, try a CMR with a controller on 3.5 and another with this branch:
```
# Bootstrap a controller using juju 3.5 and deploy influxdb:
juju bootstrap localhost ctrl-src1
juju add-model model-src1
juju deploy influxdb
juju offer influxdb:grafana-source 

# Bootstrap a second controller using this branch (4.0) and deploy grafana:
juju bootstrap localhost ctrl-src2
juju add-model model-src2
juju deploy grafana

# Integration between grafana and influxdb should work:
juju integrate grafana ctrl-src1:admin/model-src1.influxdb

juju status --relations
Model Controller Cloud/Region Version SLA Timestamp
model-src2 ctrl-src2 localhost/localhost 4.0-beta3.1 unsupported 12:09:18+01:00

SAAS Status Store URL
influxdb active ctrl-src1 admin/model-src1.influxdb

App Version Status Scale Charm Channel Rev Exposed Message
grafana active 1 grafana latest/stable 69 no Ready

Unit Workload Agent Machine Public address Ports Message
grafana/0* active idle 0 10.170.236.239 3000/tcp Ready

Machine State Address Inst id Base AZ Message
0 started 10.170.236.239 juju-3f7181-0 [email protected] Running

Integration provider Requirer Interface Type Message
influxdb:grafana-source grafana:grafana-source grafana-source regular
```
## Links


**Jira card:** JUJU-5663
  • Loading branch information
jujubot authored Mar 22, 2024
2 parents 3fff7c9 + 2a8cc8e commit e6fd53b
Show file tree
Hide file tree
Showing 23 changed files with 5 additions and 836 deletions.
4 changes: 2 additions & 2 deletions api/facadeversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
Expand Down
2 changes: 1 addition & 1 deletion apiserver/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
Expand Down
17 changes: 0 additions & 17 deletions apiserver/facades/client/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions apiserver/facades/client/application/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 0 additions & 28 deletions apiserver/facades/client/application/mocks/application_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 0 additions & 25 deletions apiserver/facades/client/applicationoffers/applicationoffers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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{
Expand Down
10 changes: 0 additions & 10 deletions apiserver/facades/client/applicationoffers/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
14 changes: 0 additions & 14 deletions apiserver/facades/client/applicationoffers/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 0 additions & 15 deletions apiserver/facades/controller/crossmodelrelations/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down
10 changes: 0 additions & 10 deletions apiserver/restrict_anonymous_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion apiserver/restrict_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit e6fd53b

Please sign in to comment.