Skip to content

Commit

Permalink
Merge pull request juju#18735 from jack-w-shaw/JUJU-7148_remove_port_…
Browse files Browse the repository at this point in the history
…run_atomic

juju#18735

RunAtomic is an anti-pattern. We recently decided to remove it.

One of the places RunAtomic was used was in the unit ports domain. We had the admirable aim of isolating the business logic, specifically how juju interacts with the wildcard endpoint, away from persistence logic.

So removing RunAtomic meant moving all this logic out of the service layer and into the state layer. UpdateUnitPorts is now very complex.

Move all the logic surrounding UpdateUnitPorts into it's own file. Do the same for tests.

This was pretty much a 1-to-1 migration. However, there was one marginal change. I discovered a bug with GetEndpointOpenedPorts, a method designed to get the opened ports for a specific endpoint. The wildcard is a special case endpoint, which broke this method. But it turned out, now the method is private, we only need this method to query the wildcard endpoint.

Rewire this method slightly to be specialised for the wildcard endpoint.

## QA steps

### Prepare



```
$ juju bootstrap lxd lxd
$ juju add-model m
```

Download and modify the `ubuntu` charm archive such that it's metadata.yaml reads:
```
...
summary: A pristine Ubuntu Server
provides:
 ep1:
 interface: ep1
 ep2:
 interface: ep2
 ep3:
 interface: ep3
```

And continue:
```
$ juju deploy ./ubuntu.charm
(wait until active)
```
In another terminal:
```
$ juju ssh -m controller 0
$ sudo /var/lib/juju/tools/machine-0/jujud db-repl --machine-id 0
repl (controller)> .switch model-m
```

### Tests

```
$ juju exec --unit ubuntu/0 "open-port --endpoints ep1 1000/tcp"
repl (model-m)> SELECT prdesc FROM v_port_range
uuid from_port to_port unit_uuid unit_name protocol endpoint 
bda33dda-852e-45ee-8505-1481d509f8a2 1000 1000 7e7414c1-71a7-4350-89d3-081118304afc ubuntu/0 tcp ep1
```

```
$ juju exec --unit ubuntu/0 "open-port --endpoints ep2 1000/tcp"
repl (model-m)> SELECT prdesc FROM v_port_range
uuid from_port to_port unit_uuid unit_name protocol endpoint 
bda33dda-852e-45ee-8505-1481d509f8a2 1000 1000 7e7414c1-71a7-4350-89d3-081118304afc ubuntu/0 tcp ep1 
1fb45aee-16fb-42d9-80f0-cf889af2e7bc 1000 1000 7e7414c1-71a7-4350-89d3-081118304afc ubuntu/0 tcp ep2 
```

```
$ juju exec --unit ubuntu/0 "close-port 1000/tcp"
repl (model-m)> SELECT prdesc FROM v_port_range
uuid from_port to_port unit_uuid unit_name protocol endpoint 
```

```
$ juju exec --unit ubuntu/0 "open-port 1000/tcp"
$ juju exec --unit ubuntu/0 "close-port --endpoints ep2 1000/tcp"
> SELECT prdesc FROM v_port_range
uuid from_port to_port unit_uuid unit_name protocol endpoint 
82936455-979d-4e15-8395-6ad68005493b 1000 1000 7e7414c1-71a7-4350-89d3-081118304afc ubuntu/0 tcp ep3 
2c051f61-310b-4eea-802f-c30563ee6e36 1000 1000 7e7414c1-71a7-4350-89d3-081118304afc ubuntu/0 tcp ep1
```
  • Loading branch information
jujubot authored Jan 29, 2025
2 parents e6c0c3e + 8e7712f commit aa725c2
Show file tree
Hide file tree
Showing 8 changed files with 1,406 additions and 1,532 deletions.
20 changes: 9 additions & 11 deletions domain/application/state/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -946,17 +946,15 @@ func (s *applicationStateSuite) TestDeleteUnit(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)

portSt := portstate.NewState(s.TxnRunnerFactory())
err = portSt.RunAtomic(context.Background(), func(ctx domain.AtomicContext) error {
return portSt.UpdateUnitPorts(ctx, unitUUID, network.GroupedPortRanges{
"endpoint": {
{Protocol: "tcp", FromPort: 80, ToPort: 80},
{Protocol: "udp", FromPort: 1000, ToPort: 1500},
},
"misc": {
{Protocol: "tcp", FromPort: 8080, ToPort: 8080},
},
}, network.GroupedPortRanges{})
})
err = portSt.UpdateUnitPorts(context.Background(), unitUUID, network.GroupedPortRanges{
"endpoint": {
{Protocol: "tcp", FromPort: 80, ToPort: 80},
{Protocol: "udp", FromPort: 1000, ToPort: 1500},
},
"misc": {
{Protocol: "tcp", FromPort: 8080, ToPort: 8080},
},
}, network.GroupedPortRanges{})
c.Assert(err, jc.ErrorIsNil)

var gotIsLast bool
Expand Down
162 changes: 3 additions & 159 deletions domain/port/service/package_mock_test.go

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

Loading

0 comments on commit aa725c2

Please sign in to comment.