Skip to content

Commit

Permalink
Merge pull request juju#16979 from nvinuesa/juju-5575
Browse files Browse the repository at this point in the history
juju#16979

There is only one call done to the legacy state, `AllSpaceInfos()` which was replaced by `GetAllSpaces()` from the new network domain. Inject the service on the provisioner API as well.

Bonus: move the go generate mocks to package_test.go
## 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

We need to bootstrap on aws and deploy to a new container:
```
juju bootstrap aws/eu-west-3 c 
juju add-model m
juju deploy ubuntu --to lxd

juju status
Model Controller Cloud/Region Version Timestamp
m c aws/eu-west-3 4.0-beta3.1 17:59:10+01:00

App Version Status Scale Charm Channel Rev Exposed Message
ubuntu 22.04 active 1 ubuntu stable 24 no

Unit Workload Agent Machine Public address Ports Message
ubuntu/0* active idle 0/lxd/0 252.41.208.66

Machine State Address Inst id Base AZ Message
0 started 13.38.121.76 i-03e4cb7487e12da4c [email protected] eu-west-3c running
0/lxd/0 started 252.41.208.66 juju-1a25f0-0-lxd-0 [email protected] eu-west-3c Container started
```
Also you can double check that the assigned address is indeed in one of the subnets:
```
juju spaces
Name Space ID Subnets
alpha 0 172.31.0.0/20
 172.31.16.0/20
 172.31.32.0/20
 252.0.0.0/12
 252.16.0.0/12
 252.32.0.0/12
```

## Links


**Jira card:** JUJU-5575
  • Loading branch information
jujubot authored Mar 20, 2024
2 parents 2ea1a23 + d9bf9a6 commit 7b17a76
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 8 deletions.
12 changes: 11 additions & 1 deletion apiserver/facades/agent/provisioner/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package provisioner

import (
"context"
stdcontext "context"
"sync"

Expand All @@ -26,6 +27,7 @@ import (
"github.com/juju/juju/core/instance"
"github.com/juju/juju/core/life"
"github.com/juju/juju/core/lxdprofile"
"github.com/juju/juju/core/network"
"github.com/juju/juju/core/status"
"github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
Expand All @@ -51,6 +53,12 @@ type StoragePoolGetter interface {
GetStoragePoolByName(ctx stdcontext.Context, name string) (*storage.Config, error)
}

// SpaceService is the interface that is used to interact with the
// network spaces.
type SpaceService interface {
GetAllSpaces(ctx context.Context) (network.SpaceInfos, error)
}

// ProvisionerAPI provides access to the Provisioner API facade.
type ProvisionerAPI struct {
*common.ControllerConfigAPI
Expand All @@ -67,6 +75,7 @@ type ProvisionerAPI struct {
*common.ToolsGetter
*networkingcommon.NetworkConfigAPI

spaceService SpaceService
st *state.State
m *state.Model
controllerConfigService ControllerConfigService
Expand Down Expand Up @@ -191,6 +200,7 @@ func NewProvisionerAPI(stdCtx stdcontext.Context, ctx facade.ModelContext) (*Pro
serviceFactory.ExternalController(),
),
NetworkConfigAPI: netConfigAPI,
spaceService: ctx.ServiceFactory().Space(),
st: st,
m: model,
controllerConfigService: serviceFactory.ControllerConfig(),
Expand Down Expand Up @@ -887,7 +897,7 @@ func (api *ProvisionerAPI) processEachContainer(ctx stdcontext.Context, args par
return errors.Trace(err)
}

policy, err := containerizer.NewBridgePolicy(env, api.st)
policy, err := containerizer.NewBridgePolicy(ctx, env, api.spaceService)
if err != nil {
return errors.Trace(err)
}
Expand Down
5 changes: 3 additions & 2 deletions internal/network/containerizer/bridgepolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package containerizer

import (
"context"
"fmt"
"hash/crc32"
"sort"
Expand Down Expand Up @@ -53,10 +54,10 @@ type BridgePolicy struct {

// NewBridgePolicy returns a new BridgePolicy for the input environ config
// getter and state indirection.
func NewBridgePolicy(cfgGetter environs.ConfigGetter, st SpaceBacking) (*BridgePolicy, error) {
func NewBridgePolicy(ctx context.Context, cfgGetter environs.ConfigGetter, spaceService SpaceService) (*BridgePolicy, error) {
cfg := cfgGetter.Config()

spaces, err := st.AllSpaceInfos()
spaces, err := spaceService.GetAllSpaces(ctx)
if err != nil {
return nil, errors.Annotate(err, "getting space infos")
}
Expand Down
1 change: 1 addition & 0 deletions internal/network/containerizer/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
gc "gopkg.in/check.v1"
)

//go:generate go run go.uber.org/mock/mockgen -package containerizer -destination bridgepolicy_mock_test.go github.com/juju/juju/internal/network/containerizer Container,Address,Subnet,LinkLayerDevice
func TestAll(t *stdtesting.T) {
gc.TestingT(t)
}
11 changes: 6 additions & 5 deletions internal/network/containerizer/shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package containerizer

import (
"context"

"github.com/juju/collections/set"
"github.com/juju/errors"

Expand All @@ -13,11 +15,10 @@ import (
"github.com/juju/juju/state"
)

//go:generate go run go.uber.org/mock/mockgen -package containerizer -destination bridgepolicy_mock_test.go github.com/juju/juju/internal/network/containerizer Container,Address,Subnet,LinkLayerDevice

// SpaceBacking describes the retrieval of all spaces from the DB.
type SpaceBacking interface {
AllSpaceInfos() (network.SpaceInfos, error)
// SpaceService is the interface that is used to interact with the
// network spaces.
type SpaceService interface {
GetAllSpaces(ctx context.Context) (network.SpaceInfos, error)
}

// LinkLayerDevice is an indirection for state.LinkLayerDevice.
Expand Down

0 comments on commit 7b17a76

Please sign in to comment.