Merge pull request juju#18402 from jack-w-shaw/JUJU-6869_RI_ports_proper

juju#18402

At the time, remove the unit_endpoint table. This table wasn't providing
us with anything but pain. Instead, attach the foreign keys (unit and
charm_relation) directly to the port range

This then required a fairly substantial refactor of the state layer.

Introducing RI from ports to relation resulted in an additional
complexity with the wildcard endpoint, as this does not have a
corresponding charm relation. I have dealt with this by using the null
value. So a null value for relation_uuid represents the wildcard
endpoint. This turned out to be a fairly natural solution.

This refactor also meant that the watcher now emits unit uuids, rather
than unit_endpoint uuids. This meant the watcher code needed a
refactor/simplification as well.

## QA steps

Download a charm and reconfigure it's relations
```
$ mkdir ubuntu-lite
$ cd ubuntu-lite
$ juju download ubuntu-lite
$ unzip *
[edit metadata.yaml such that]
$ cat metadata.yaml
...
provides:
 foo:
 interface: foo
 bar:
 interface: bar
 baz:
 interface: baz
...

$ zip -r ubunty-lite.charm .
```

Bootstrap
```
$ juju bootstrap lxd lxd
$ juju add-model m
$ juju show-model m
...
 model-uuid: 1fad9711-72de-4479-8f9a-939808932230
...
$ juju deploy ./ubuntu-lite.charm
```

Run tests
```
$ juju exec --unit ubuntu-lite/0 "open-port --endpoint foo 1000/tcp"
$ juju sql 1fad9711-72de-4479-8f9a-939808932230
> SELECT prdesc FROM port_range
uuid protocol_id from_port to_port relation_uuid unit_uuid 
ff980f34-4943-47a6-8499-a33d19117cc8 1 1000 1000 43748109-9769-4018-8443-c7abd8728dfa 57921041-0f1e-44fb-83af-cc5a70c1050a 

$ juju exec --unit ubuntu-lite/0 "open-port --endpoints bar 1000/tcp"
$ juju sql 1fad9711-72de-4479-8f9a-939808932230
> SELECT prdesc FROM port_range
uuid protocol_id from_port to_port relation_uuid unit_uuid 
ff980f34-4943-47a6-8499-a33d19117cc8 1 1000 1000 43748109-9769-4018-8443-c7abd8728dfa 57921041-0f1e-44fb-83af-cc5a70c1050a 
96f38b5a-32ec-4df8-88dc-599355fe4254 1 1000 1000 af92f025-5f77-4d9a-8627-416d93090cfc 57921041-0f1e-44fb-83af-cc5a70c1050a

$ juju exec --unit ubuntu-lite/0 "open-port 1000/tcp"
$ juju sql 1fad9711-72de-4479-8f9a-939808932230
> SELECT prdesc FROM port_range
uuid protocol_id from_port to_port relation_uuid unit_uuid 
6a89d03b-4da8-41ee-8b00-6861900a8121 1 1000 1000 <nil> 57921041-0f1e-44fb-83af-cc5a70c1050a
```

### Test watchers

IAAS
```
$ juju bootstrap aws aws
$ juju add-model m
$ juju deploy ubuntu
$ juju expose ubuntu
$ juju exec --unit ubuntu/0 open-port 1000/tcp
$ juju debug-log -m controller
....
machine-0: 11:41:43 INFO juju.provider.ec2 opened ports in security group juju-7df12cac-717b-424a-8b27-153d5e7bc8e2-0: [1000/tcp from 0.0.0.0/0,::/0]
```


CAAS
```
$ make microk8s-operator-update
$ juju bootstrap microk8s mk8s
$ juju add-model m
$ juju model-config workload-storage="microk8s-hostpath"
$ juju exec --unit hello-kubecon/0 open-port 1000/tcp
$ microk8s.kubectl get -n m services
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
hello-kubecon ClusterIP 10.152.183.69 <none> 1000/TCP 90s
```

apiserver/facades/client/client/api_test.go

@@ -228,6 +228,7 @@ var scenarioStatus = &params.FullStatus{
 				"":               network.AlphaSpaceName,
 				"server":         network.AlphaSpaceName,
 				"server-admin":   network.AlphaSpaceName,
+				"db":             network.AlphaSpaceName,
 				"db-router":      network.AlphaSpaceName,
 				"metrics-client": network.AlphaSpaceName,
 			},

apiserver/facades/client/client/filtering_test.go

@@ -240,14 +240,14 @@ func (s *filteringStatusSuite) TestFilterByPortRange(c *gc.C) {
 
 	portService := s.ControllerDomainServices(c).Port()
 	err = portService.UpdateUnitPorts(context.Background(), unit0UUID, network.GroupedPortRanges{
-		"":    []network.PortRange{network.MustParsePortRange("1000/tcp")},
-		"foo": []network.PortRange{network.MustParsePortRange("2000/tcp")},
+		"":   []network.PortRange{network.MustParsePortRange("1000/tcp")},
+		"db": []network.PortRange{network.MustParsePortRange("2000/tcp")},
 	}, network.GroupedPortRanges{})
 	c.Assert(err, jc.ErrorIsNil)
 
 	err = portService.UpdateUnitPorts(context.Background(), unit1UUID, network.GroupedPortRanges{
-		"":    []network.PortRange{network.MustParsePortRange("2000/tcp")},
-		"bar": []network.PortRange{network.MustParsePortRange("3000/tcp")},
+		"":      []network.PortRange{network.MustParsePortRange("2000/tcp")},
+		"cache": []network.PortRange{network.MustParsePortRange("3000/tcp")},
 	}, network.GroupedPortRanges{})
 	c.Assert(err, jc.ErrorIsNil)
 

apiserver/facades/client/client/status_test.go

@@ -851,8 +851,8 @@ func (s *statusUnitTestSuite) TestUnitsWithOpenedPortsSent(c *gc.C) {
 
 	portService := s.ControllerDomainServices(c).Port()
 	err = portService.UpdateUnitPorts(context.Background(), unitUUID, network.GroupedPortRanges{
-		"":    []network.PortRange{network.MustParsePortRange("1000/tcp")},
-		"foo": []network.PortRange{network.MustParsePortRange("2000/tcp")},
+		"":   []network.PortRange{network.MustParsePortRange("1000/tcp")},
+		"db": []network.PortRange{network.MustParsePortRange("2000/tcp")},
 	}, network.GroupedPortRanges{})
 	c.Assert(err, jc.ErrorIsNil)
 

domain/application/state/application.go

@@ -1023,10 +1023,7 @@ func (st *ApplicationState) deletePorts(ctx context.Context, tx *sqlair.TX, unit
 
 	deletePortRange := `
 DELETE FROM port_range
-WHERE unit_endpoint_uuid IN (
-    SELECT uuid FROM unit_endpoint ue
-    WHERE ue.unit_uuid = $minimalUnit.uuid
-)
+WHERE unit_uuid = $minimalUnit.uuid
 `
 	deletePortRangeStmt, err := st.Prepare(deletePortRange, unit)
 	if err != nil {
@@ -1037,15 +1034,6 @@ WHERE unit_endpoint_uuid IN (
 		return errors.Annotate(err, "cannot delete port range records")
 	}
 
-	deleteEndpoint := `DELETE FROM unit_endpoint WHERE unit_uuid = $minimalUnit.uuid`
-	deleteEndpointStmt, err := st.Prepare(deleteEndpoint, unit)
-	if err != nil {
-		return errors.Annotate(err, "cannot delete endpoint records")
-	}
-
-	if err := tx.Query(ctx, deleteEndpointStmt, unit).Run(); err != nil {
-		return errors.Trace(err)
-	}
 	return nil
 }
 
@@ -1063,7 +1051,6 @@ func (st *ApplicationState) deleteSimpleUnitReferences(ctx context.Context, tx *
 		"unit_workload_status",
 		"cloud_container_status_data",
 		"cloud_container_status",
-		"unit_endpoint",
 	} {
 		deleteUnitReference := fmt.Sprintf(`DELETE FROM %s WHERE unit_uuid = $minimalUnit.uuid`, table)
 		deleteUnitReferenceStmt, err := st.Prepare(deleteUnitReference, unit)

domain/application/state/application_test.go

@@ -680,7 +680,6 @@ func (s *applicationStateSuite) TestDeleteUnit(c *gc.C) {
 		deviceCount                   int
 		addressCount                  int
 		portCount                     int
-		endpointCount                 int
 		agentStatusCount              int
 		agentStatusDataCount          int
 		workloadStatusCount           int
@@ -704,9 +703,6 @@ func (s *applicationStateSuite) TestDeleteUnit(c *gc.C) {
 		if err := tx.QueryRowContext(ctx, "SELECT count(*) FROM cloud_container_port WHERE cloud_container_uuid=?", netNodeUUID).Scan(&portCount); err != nil {
 			return err
 		}
-		if err := tx.QueryRowContext(ctx, "SELECT count(*) FROM unit_endpoint WHERE unit_uuid=?", unitUUID).Scan(&endpointCount); err != nil {
-			return err
-		}
 		if err := tx.QueryRowContext(ctx, "SELECT count(*) FROM unit_agent_status WHERE unit_uuid=?", unitUUID).Scan(&agentStatusCount); err != nil {
 			return err
 		}
@@ -730,7 +726,6 @@ func (s *applicationStateSuite) TestDeleteUnit(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(addressCount, gc.Equals, 0)
 	c.Assert(portCount, gc.Equals, 0)
-	c.Assert(endpointCount, gc.Equals, 0)
 	c.Assert(deviceCount, gc.Equals, 0)
 	c.Assert(containerCount, gc.Equals, 0)
 	c.Assert(agentStatusCount, gc.Equals, 0)
@@ -1945,6 +1940,20 @@ func (s *applicationStateSuite) createApplication(c *gc.C, name string, l life.L
 			Charm: charm.Charm{
 				Metadata: charm.Metadata{
 					Name: name,
+					Provides: map[string]charm.Relation{
+						"endpoint": {
+							Name:  "endpoint",
+							Key:   "endpoint",
+							Role:  charm.RoleProvider,
+							Scope: charm.ScopeGlobal,
+						},
+						"misc": {
+							Name:  "misc",
+							Key:   "misc",
+							Role:  charm.RoleProvider,
+							Scope: charm.ScopeGlobal,
+						},
+					},
 				},
 			},
 			Origin: charm.CharmOrigin{

domain/port/errors/errors.go

@@ -15,4 +15,8 @@ const (
 	// PortRangeConflict describes an error that occurs when a user tries to open
 	// or close a port range overlaps with another.
 	PortRangeConflict = errors.ConstError("port range conflict")
+
+	// InvalidEndpoint describes an error that occurs when a user trying to open
+	// or close a port range with an endpoint which does not exist on the unit.
+	InvalidEndpoint = errors.ConstError("invalid endpoint(s)")
 )

domain/port/service/watcher.go

@@ -14,6 +14,7 @@ import (
 	"github.com/juju/juju/core/database"
 	"github.com/juju/juju/core/logger"
 	coremachine "github.com/juju/juju/core/machine"
+	"github.com/juju/juju/core/unit"
 	"github.com/juju/juju/core/watcher"
 	"github.com/juju/juju/core/watcher/eventsource"
 )
@@ -61,13 +62,13 @@ type WatcherState interface {
 	// event for the WatchMachineOpenedPorts watcher
 	InitialWatchMachineOpenedPortsStatement() string
 
-	// GetMachineNamesForUnitEndpoints returns map from endpoint uuids to the uuids of
+	// GetMachineNamesForUnits returns map from endpoint uuids to the uuids of
 	// the machines which host that endpoint for each provided endpoint uuid.
-	GetMachineNamesForUnitEndpoints(ctx context.Context, endpointUUIDs []string) ([]coremachine.Name, error)
+	GetMachineNamesForUnits(context.Context, []unit.UUID) ([]coremachine.Name, error)
 
 	// FilterEndpointsForApplication returns the subset of provided endpoint uuids
 	// that are associated with the provided application.
-	FilterEndpointsForApplication(ctx context.Context, eps []string, app coreapplication.ID) (set.Strings, error)
+	FilterUnitUUIDsForApplication(context.Context, []unit.UUID, coreapplication.ID) (set.Strings, error)
 }
 
 // WatchMachineOpenedPorts returns a strings watcher for opened ports. This watcher
@@ -101,11 +102,14 @@ func (s *WatchableService) endpointToMachineMapper(
 	ctx context.Context, db database.TxnRunner, events []changestream.ChangeEvent,
 ) ([]changestream.ChangeEvent, error) {
 
-	endpointUUIDs := transform.Slice(events, func(e changestream.ChangeEvent) string {
-		return e.Changed()
+	unitUUIDs, err := transform.SliceOrErr(events, func(e changestream.ChangeEvent) (unit.UUID, error) {
+		return unit.ParseID(e.Changed())
 	})
+	if err != nil {
+		return nil, err
+	}
 
-	machineNames, err := s.st.GetMachineNamesForUnitEndpoints(ctx, endpointUUIDs)
+	machineNames, err := s.st.GetMachineNamesForUnits(ctx, unitUUIDs)
 	if err != nil {
 		return nil, err
 	}
@@ -129,16 +133,20 @@ func (s *WatchableService) filterForApplication(applicationUUID coreapplication.
 	return func(
 		ctx context.Context, db database.TxnRunner, events []changestream.ChangeEvent,
 	) ([]changestream.ChangeEvent, error) {
-		endpointUUIDs := transform.Slice(events, func(e changestream.ChangeEvent) string {
-			return e.Changed()
+		unitUUIDs, err := transform.SliceOrErr(events, func(e changestream.ChangeEvent) (unit.UUID, error) {
+			return unit.ParseID(e.Changed())
 		})
-		endpointUUIDsForApplication, err := s.st.FilterEndpointsForApplication(ctx, endpointUUIDs, applicationUUID)
+		if err != nil {
+			return nil, err
+		}
+
+		unitUUIDsForApplication, err := s.st.FilterUnitUUIDsForApplication(ctx, unitUUIDs, applicationUUID)
 		if err != nil {
 			return nil, err
 		}
 		results := make([]changestream.ChangeEvent, 0, len(events))
 		for _, event := range events {
-			if endpointUUIDsForApplication.Contains(event.Changed()) {
+			if unitUUIDsForApplication.Contains(event.Changed()) {
 				results = append(results, event)
 			}
 		}