Skip to content

Commit

Permalink
Merge pull request juju#16125 from SimonRichardson/pass-in-controller…
Browse files Browse the repository at this point in the history 
…-config

juju#16125

This is another follow up PR that removes controller config from being called directly inside of state. This should nearly clean up the last of the controller config call sites from inside state.

Once this is done, and after the peergrouper has removed it controller config dependency, then we should be able to remove controller config from state.

## 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

## QA steps

```sh
$ juju bootstrap lxd test --build-agent
$ juju enable-ha
$ juju add-model default
$ juju deploy ubuntu -n 3
```

## Links

JUJU-4511
  • Loading branch information
@jujubot
jujubot authored Aug 21, 2023
2 parents 080df81 + 477298d commit 9473545
Show file tree
Hide file tree
Showing 19 changed files with 386 additions and 134 deletions.
13 changes: 8 additions & 5 deletions apiserver/facades/agent/deployer/deployer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,17 +334,20 @@ func (s *deployerSuite) TestRemove(c *gc.C) {
}

func (s *deployerSuite) TestConnectionInfo(c *gc.C) {
err := s.machine0.SetProviderAddresses(network.NewSpaceAddress("0.1.2.3", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)))
st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

err = s.machine0.SetProviderAddresses(controllerConfig,
network.NewSpaceAddress("0.1.2.3", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)

// Default host port scope is public, so change the cloud-local one
hostPorts := network.NewSpaceHostPorts(1234, "0.1.2.3", "1.2.3.4")
hostPorts[1].Scope = network.ScopeCloudLocal

st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)
err = st.SetAPIHostPorts(controllerConfig, []network.SpaceHostPorts{hostPorts})
c.Assert(err, jc.ErrorIsNil)

Expand Down
6 changes: 5 additions & 1 deletion apiserver/facades/agent/machine/machiner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ func (api *MachinerAPI) SetMachineAddresses(ctx context.Context, args params.Set
if err != nil {
return results, err
}
controllerConfig, err := api.st.ControllerConfig()
if err != nil {
return results, err
}
for i, arg := range args.MachineAddresses {
m, err := api.getMachine(arg.Tag, canModify)
if err != nil {
Expand All @@ -96,7 +100,7 @@ func (api *MachinerAPI) SetMachineAddresses(ctx context.Context, args params.Set
results.Results[i].Error = apiservererrors.ServerError(err)
continue
}
if err := m.SetMachineAddresses(addresses...); err != nil {
if err := m.SetMachineAddresses(controllerConfig, addresses...); err != nil {
results.Results[i].Error = apiservererrors.ServerError(err)
}
}
Expand Down
99 changes: 81 additions & 18 deletions apiserver/facades/agent/uniter/networkinfo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,16 @@ func (s *networkInfoSuite) TestNetworksForRelation(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
id, err := prr.pu0.AssignedMachineId()
c.Assert(err, jc.ErrorIsNil)
machine, err := s.ControllerModel(c).State().Machine(id)

st := s.ControllerModel(c).State()
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

err = machine.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("10.2.3.4", network.WithScope(network.ScopeCloudLocal)),
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopePublic)),
)
Expand Down Expand Up @@ -135,10 +141,13 @@ func (s *networkInfoSuite) TestProcessAPIRequestForBinding(c *gc.C) {
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

// We need at least one address on the machine itself, because these are
// retrieved up-front to use as a fallback when we fail to locate addresses
// on link-layer devices.
err = machine.SetProviderAddresses(network.NewSpaceAddress("10.2.3.4/16"))
err = machine.SetProviderAddresses(controllerConfig, network.NewSpaceAddress("10.2.3.4/16"))
c.Assert(err, jc.ErrorIsNil)

s.addDevicesWithAddresses(c, machine, "10.2.3.4/16", "100.2.3.4/24")
Expand Down Expand Up @@ -194,10 +203,13 @@ func (s *networkInfoSuite) TestProcessAPIRequestBridgeWithSameIPOverNIC(c *gc.C)

ip := "10.2.3.4/16"

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

// We need at least one address on the machine itself, because these are
// retrieved up-front to use as a fallback when we fail to locate addresses
// on link-layer devices.
err = machine.SetProviderAddresses(network.NewSpaceAddress(ip))
err = machine.SetProviderAddresses(controllerConfig, network.NewSpaceAddress(ip))
c.Assert(err, jc.ErrorIsNil)

// Create a NIC and bridge, but also add the IP to the NIC to simulate
Expand Down Expand Up @@ -244,8 +256,13 @@ func (s *networkInfoSuite) TestAPIRequestForRelationIAASHostNameIngressNoEgress(
host := "host.at.somewhere"
ip := "100.2.3.4"

st := s.ControllerModel(c).State()

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

addr := network.NewSpaceAddress(host)
err = machine.SetProviderAddresses(addr)
err = machine.SetProviderAddresses(controllerConfig, addr)
c.Assert(err, jc.ErrorIsNil)

lookup := func(addr string) ([]string, error) {
Expand Down Expand Up @@ -356,7 +373,12 @@ func (s *networkInfoSuite) TestNetworksForRelationWithSpaces(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
id, err := prr.pu0.AssignedMachineId()
c.Assert(err, jc.ErrorIsNil)
machine, err := s.ControllerModel(c).State().Machine(id)

st := s.ControllerModel(c).State()
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

addresses := []network.SpaceAddress{
Expand All @@ -365,7 +387,7 @@ func (s *networkInfoSuite) TestNetworksForRelationWithSpaces(c *gc.C) {
network.NewSpaceAddress("10.2.3.4", network.WithScope(network.ScopeCloudLocal)),
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopePublic)),
}
err = machine.SetProviderAddresses(addresses...)
err = machine.SetProviderAddresses(controllerConfig, addresses...)
c.Assert(err, jc.ErrorIsNil)

s.addDevicesWithAddresses(c, machine, "1.2.3.4/16", "2.2.3.4/16", "10.2.3.4/16", "4.3.2.1/16")
Expand Down Expand Up @@ -393,10 +415,16 @@ func (s *networkInfoSuite) TestNetworksForRelationRemoteRelation(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
id, err := prr.ru0.AssignedMachineId()
c.Assert(err, jc.ErrorIsNil)
machine, err := s.ControllerModel(c).State().Machine(id)

st := s.ControllerModel(c).State()
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

err = machine.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopePublic)),
)
Expand All @@ -418,10 +446,16 @@ func (s *networkInfoSuite) TestNetworksForRelationRemoteRelationNoPublicAddr(c *
c.Assert(err, jc.ErrorIsNil)
id, err := prr.ru0.AssignedMachineId()
c.Assert(err, jc.ErrorIsNil)
machine, err := s.ControllerModel(c).State().Machine(id)

st := s.ControllerModel(c).State()
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

err = machine.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -442,7 +476,12 @@ func (s *networkInfoSuite) TestNetworksForRelationRemoteRelationDelayedPublicAdd
c.Assert(err, jc.ErrorIsNil)
id, err := prr.ru0.AssignedMachineId()
c.Assert(err, jc.ErrorIsNil)
machine, err := s.ControllerModel(c).State().Machine(id)

st := s.ControllerModel(c).State()
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

retryFactory := func() retry.CallArgs {
Expand All @@ -454,7 +493,9 @@ func (s *networkInfoSuite) TestNetworksForRelationRemoteRelationDelayedPublicAdd
// Set the address after one failed retrieval attempt.
if attempt == 1 {
err := machine.SetProviderAddresses(
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopePublic)))
controllerConfig,
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopePublic)),
)
c.Assert(err, jc.ErrorIsNil)
}
},
Expand All @@ -477,7 +518,12 @@ func (s *networkInfoSuite) TestNetworksForRelationRemoteRelationDelayedPrivateAd
c.Assert(err, jc.ErrorIsNil)
id, err := prr.ru0.AssignedMachineId()
c.Assert(err, jc.ErrorIsNil)
machine, err := s.ControllerModel(c).State().Machine(id)

st := s.ControllerModel(c).State()
machine, err := st.Machine(id)
c.Assert(err, jc.ErrorIsNil)

controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

// The first attempt is for the public address.
Expand All @@ -502,7 +548,10 @@ func (s *networkInfoSuite) TestNetworksForRelationRemoteRelationDelayedPrivateAd
NotifyFunc: func(lastError error, attempt int) {
// Set the private address after one failed retrieval attempt.
if attempt == 1 {
err := machine.SetProviderAddresses(network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopeCloudLocal)))
err := machine.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)
}
},
Expand Down Expand Up @@ -712,7 +761,8 @@ func (s *networkInfoSuite) TestMachineNetworkInfos(c *gc.C) {
unit, err := app.AddUnit(state.AddUnitParams{})
c.Assert(err, jc.ErrorIsNil)

machine, err := s.ControllerModel(c).State().AddOneMachine(state.MachineTemplate{
st := s.ControllerModel(c).State()
machine, err := st.AddOneMachine(state.MachineTemplate{
Base: state.UbuntuBase("12.10"),
Jobs: []state.MachineJob{state.JobHostUnits},
})
Expand All @@ -725,10 +775,16 @@ func (s *networkInfoSuite) TestMachineNetworkInfos(c *gc.C) {
s.createNICWithIP(c, machine, network.EthernetDevice, "eth1", "10.10.0.20/24")
s.createNICWithIP(c, machine, network.EthernetDevice, "eth2", "10.20.0.20/24")

err = machine.SetMachineAddresses(network.NewSpaceAddress("10.0.0.20", network.WithScope(network.ScopePublic)),
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

err = machine.SetMachineAddresses(
controllerConfig,
network.NewSpaceAddress("10.0.0.20", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("10.10.0.20", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("10.10.0.30", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("10.20.0.20", network.WithScope(network.ScopeCloudLocal)))
network.NewSpaceAddress("10.20.0.20", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)

ni := s.newNetworkInfo(c, unit.UnitTag(), nil, nil)
Expand Down Expand Up @@ -777,7 +833,8 @@ func (s *networkInfoSuite) TestMachineNetworkInfosAlphaNoSubnets(c *gc.C) {
unit, err := app.AddUnit(state.AddUnitParams{})
c.Assert(err, jc.ErrorIsNil)

machine, err := s.ControllerModel(c).State().AddOneMachine(state.MachineTemplate{
st := s.ControllerModel(c).State()
machine, err := st.AddOneMachine(state.MachineTemplate{
Base: state.UbuntuBase("12.10"),
Jobs: []state.MachineJob{state.JobHostUnits},
})
Expand All @@ -790,10 +847,16 @@ func (s *networkInfoSuite) TestMachineNetworkInfosAlphaNoSubnets(c *gc.C) {
s.createNICWithIP(c, machine, network.EthernetDevice, "eth1", "10.10.0.20/24")
s.createNICWithIP(c, machine, network.EthernetDevice, "eth2", "10.20.0.20/24")

err = machine.SetMachineAddresses(network.NewSpaceAddress("10.0.0.20", network.WithScope(network.ScopePublic)),
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

err = machine.SetMachineAddresses(
controllerConfig,
network.NewSpaceAddress("10.0.0.20", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("10.10.0.20", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("10.10.0.30", network.WithScope(network.ScopePublic)),
network.NewSpaceAddress("10.20.0.20", network.WithScope(network.ScopeCloudLocal)))
network.NewSpaceAddress("10.20.0.20", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)

ni := s.newNetworkInfo(c, unit.UnitTag(), nil, nil)
Expand Down
41 changes: 35 additions & 6 deletions apiserver/facades/agent/uniter/uniter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,11 @@ func (s *uniterSuite) TestPublicAddress(c *gc.C) {
})

// Now set it an try again.
st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)
err = s.machine0.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopePublic)),
)
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -607,7 +611,11 @@ func (s *uniterSuite) TestPrivateAddress(c *gc.C) {
})

// Now set it and try again.
st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)
err = s.machine0.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -629,7 +637,11 @@ func (s *uniterSuite) TestPrivateAddress(c *gc.C) {
// TestNetworkInfoSpaceless is in uniterSuite and not uniterNetworkInfoSuite since we don't want
// all the spaces set up.
func (s *uniterSuite) TestNetworkInfoSpaceless(c *gc.C) {
err := s.machine0.SetProviderAddresses(
st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)
err = s.machine0.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -1971,7 +1983,11 @@ func (s *uniterSuite) TestProviderType(c *gc.C) {

func (s *uniterSuite) TestEnterScope(c *gc.C) {
// Set wordpressUnit's private address first.
err := s.machine0.SetProviderAddresses(
st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)
err = s.machine0.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -3376,14 +3392,18 @@ func (s *uniterSuite) TestSLALevel(c *gc.C) {
func (s *uniterSuite) setupRemoteRelationScenario(c *gc.C) (names.Tag, *state.RelationUnit) {
s.makeRemoteWordpress(c)

st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

// Set mysql's addresses first.
err := s.machine1.SetProviderAddresses(
err = s.machine1.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
network.NewSpaceAddress("4.3.2.1", network.WithScope(network.ScopePublic)),
)
c.Assert(err, jc.ErrorIsNil)

st := s.ControllerModel(c).State()
eps, err := st.InferEndpoints("mysql", "remote-wordpress")
c.Assert(err, jc.ErrorIsNil)
rel, err := st.AddRelation(eps...)
Expand Down Expand Up @@ -3423,8 +3443,14 @@ func (s *uniterSuite) TestPrivateAddressWithRemoteRelationNoPublic(c *gc.C) {
relTag, relUnit := s.setupRemoteRelationScenario(c)

thisUniter := s.makeMysqlUniter(c)

st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)

// Set mysql's addresses - no public address.
err := s.machine1.SetProviderAddresses(
err = s.machine1.SetProviderAddresses(
controllerConfig,
network.NewSpaceAddress("1.2.3.4", network.WithScope(network.ScopeCloudLocal)),
)
c.Assert(err, jc.ErrorIsNil)
Expand Down Expand Up @@ -3914,7 +3940,10 @@ func (s *uniterNetworkInfoSuite) addProvisionedMachineWithDevicesAndAddresses(c
for i, addr := range machineAddrs {
netAddrs[i] = network.NewSpaceAddress(addr.Value())
}
err = machine.SetProviderAddresses(netAddrs...)
st := s.ControllerModel(c).State()
controllerConfig, err := st.ControllerConfig()
c.Assert(err, jc.ErrorIsNil)
err = machine.SetProviderAddresses(controllerConfig, netAddrs...)
c.Assert(err, jc.ErrorIsNil)

return machine
Expand Down
Loading

0 comments on commit 9473545

Please sign in to comment.