Merge pull request juju#16711 from nvinuesa/juju-5105-3

juju#16711

This patch brings the second set of methods implemented on the network domain at the service layer for the spaces (sub-domain).

Note that SaveProviderSubnets is already present from a previous patch and the methods now implemented are much simpler, basically proxies for the state methods.


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

Again, this is not wired-up yet so unit tests are enough:
```
 go test github.com/juju/juju/domain/network/... -gocheck.v
 ```

## Links

**Jira card:** JUJU-5105

domain/network/service/space.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/juju/errors"
+	"github.com/juju/names/v4"
 	"github.com/juju/utils/v3"
 
 	"github.com/juju/juju/core/network"
@@ -19,7 +20,7 @@ import (
 // domain service.
 type State interface {
 	// AddSpace creates and returns a new space.
-	AddSpace(ctx context.Context, uuid utils.UUID, name string, providerID network.Id, subnetIDs []string) error
+	AddSpace(ctx context.Context, uuid string, name string, providerID network.Id, subnetIDs []string) error
 	// GetSpace returns the space by UUID.
 	GetSpace(ctx context.Context, uuid string) (*network.SpaceInfo, error)
 	// GetSpaceByName returns the space by name.
@@ -61,12 +62,54 @@ func NewSpaceService(st State, logger Logger) *SpaceService {
 	}
 }
 
+// AddSpace creates and returns a new space.
+func (s *SpaceService) AddSpace(ctx context.Context, name string, providerID network.Id, subnetIDs []string) (network.Id, error) {
+	if !names.IsValidSpace(name) {
+		return "", errors.NotValidf("space name %q", name)
+	}
+
+	uuid, err := utils.NewUUID()
+	if err != nil {
+		return "", errors.Annotatef(err, "creating uuid for new space %q", name)
+	}
+	spaceID := network.Id(uuid.String())
+
+	if err := s.st.AddSpace(ctx, spaceID.String(), name, providerID, subnetIDs); err != nil {
+		return "", errors.Trace(err)
+	}
+	return spaceID, nil
+}
+
+// Space returns a space from state that matches the input ID.
+// An error is returned if the space does not exist or if there was a problem
+// accessing its information.
+func (s *SpaceService) Space(ctx context.Context, uuid string) (*network.SpaceInfo, error) {
+	return s.st.GetSpace(ctx, uuid)
+}
+
+// SpaceByName returns a space from state that matches the input name.
+// An error is returned that satisfied errors.NotFound if the space was not found
+// or an error static any problems fetching the given space.
+func (s *SpaceService) SpaceByName(ctx context.Context, name string) (*network.SpaceInfo, error) {
+	return s.st.GetSpaceByName(ctx, name)
+}
+
+// GetAllSpaces returns all spaces for the model.
+func (s *SpaceService) GetAllSpaces(ctx context.Context) (network.SpaceInfos, error) {
+	return s.st.GetAllSpaces(ctx)
+}
+
+// Remove deletes a space identified by its uuid.
+func (s *SpaceService) Remove(ctx context.Context, uuid string) error {
+	return s.st.DeleteSpace(ctx, uuid)
+}
+
 // SaveProviderSubnets loads subnets into state.
 // Currently it does not delete removed subnets.
 func (s *SpaceService) SaveProviderSubnets(
 	ctx context.Context,
 	subnets []network.SubnetInfo,
-	spaceUUID string,
+	spaceUUID network.Id,
 	fans network.FanConfig,
 ) error {
 
@@ -83,7 +126,7 @@ func (s *SpaceService) SaveProviderSubnets(
 
 		// Add the subnet with the provided space UUID to the upsert list.
 		subnetToUpsert := subnet
-		subnetToUpsert.SpaceID = spaceUUID
+		subnetToUpsert.SpaceID = spaceUUID.String()
 		subnetsToUpsert = append(subnetsToUpsert, subnetToUpsert)
 
 		// Iterate over fan configs.
@@ -112,7 +155,7 @@ func (s *SpaceService) SaveProviderSubnets(
 				fanSubnetToUpsert := subnet
 				fanSubnetToUpsert.ProviderId = network.Id(fanSubnetID)
 				fanSubnetToUpsert.SetFan(fanSubnetToUpsert.CIDR, fan.Overlay.String())
-				fanSubnetToUpsert.SpaceID = spaceUUID
+				fanSubnetToUpsert.SpaceID = spaceUUID.String()
 
 				fanInfo := &network.FanCIDRs{
 					FanLocalUnderlay: fanSubnetToUpsert.CIDR,

domain/network/service/space_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/juju/errors"
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
 	gomock "go.uber.org/mock/gomock"
@@ -41,6 +42,109 @@ func (s *spaceSuite) TestGenerateFanSubnetID(c *gc.C) {
 	c.Check(obtained, gc.Equals, "-INFAN-192-168-0-0-16")
 }
 
+func (s *spaceSuite) TestAddSpaceInvalidNameEmpty(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	// Make sure no calls to state are done
+	s.st.EXPECT().AddSpace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
+
+	_, err := NewSpaceService(s.st, s.logger).AddSpace(context.Background(), "", "", []string{})
+	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("space name \"\" not valid"))
+}
+
+func (s *spaceSuite) TestAddSpaceInvalidName(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	// Make sure no calls to state are done
+	s.st.EXPECT().AddSpace(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
+
+	_, err := NewSpaceService(s.st, s.logger).AddSpace(context.Background(), "-bad name-", "provider-id", []string{})
+	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("space name \"-bad name-\" not valid"))
+}
+
+func (s *spaceSuite) TestAddSpaceErrorAdding(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().AddSpace(gomock.Any(), gomock.Any(), "0", network.Id("provider-id"), []string{"0"}).
+		Return(errors.Errorf("updating subnet %q using space uuid \"space0\"", "0"))
+
+	_, err := NewSpaceService(s.st, s.logger).AddSpace(context.Background(), "0", network.Id("provider-id"), []string{"0"})
+	c.Assert(err, gc.ErrorMatches, "updating subnet \"0\" using space uuid \"space0\"")
+}
+
+func (s *spaceSuite) TestAddSpace(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	var expectedUUID string
+	// Verify that the passed UUID is also returned.
+	s.st.EXPECT().AddSpace(gomock.Any(), gomock.Any(), "space0", network.Id("provider-id"), []string{}).
+		Do(
+			func(
+				ctx context.Context,
+				uuid string,
+				name string,
+				providerID network.Id,
+				subnetIDs []string,
+			) error {
+				expectedUUID = uuid
+				return nil
+			})
+
+	returnedUUID, err := NewSpaceService(s.st, s.logger).AddSpace(context.Background(), "space0", network.Id("provider-id"), []string{})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(returnedUUID.String(), gc.Equals, expectedUUID)
+}
+
+func (s *spaceSuite) TestRetrieveSpaceByID(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().GetSpace(gomock.Any(), network.AlphaSpaceId)
+	_, err := NewSpaceService(s.st, s.logger).Space(context.Background(), network.AlphaSpaceId)
+	c.Assert(err, jc.ErrorIsNil)
+}
+
+func (s *spaceSuite) TestRetrieveSpaceByIDNotFound(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().GetSpace(gomock.Any(), "unknown-space").
+		Return(nil, errors.NotFoundf("space %q", "unknown-space"))
+	_, err := NewSpaceService(s.st, s.logger).Space(context.Background(), "unknown-space")
+	c.Assert(err, gc.ErrorMatches, "space \"unknown-space\" not found")
+}
+
+func (s *spaceSuite) TestRetrieveSpaceByName(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().GetSpaceByName(gomock.Any(), network.AlphaSpaceName)
+	_, err := NewSpaceService(s.st, s.logger).SpaceByName(context.Background(), network.AlphaSpaceName)
+	c.Assert(err, jc.ErrorIsNil)
+}
+
+func (s *spaceSuite) TestRetrieveSpaceByNameNotFound(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().GetSpaceByName(gomock.Any(), "unknown-space-name").
+		Return(nil, errors.NotFoundf("space with name %q", "unknown-space-name"))
+	_, err := NewSpaceService(s.st, s.logger).SpaceByName(context.Background(), "unknown-space-name")
+	c.Assert(err, gc.ErrorMatches, "space with name \"unknown-space-name\" not found")
+}
+
+func (s *spaceSuite) TestRetrieveAllSpaces(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().GetAllSpaces(gomock.Any())
+	_, err := NewSpaceService(s.st, s.logger).GetAllSpaces(context.Background())
+	c.Assert(err, jc.ErrorIsNil)
+}
+
+func (s *spaceSuite) TestRemoveSpace(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	s.st.EXPECT().DeleteSpace(gomock.Any(), "space0")
+	err := NewSpaceService(s.st, s.logger).Remove(context.Background(), "space0")
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 func (s *spaceSuite) TestSaveProviderSubnetsWithoutSpaceUUID(c *gc.C) {
 	defer s.setupMocks(c).Finish()
 

domain/network/state/space.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/canonical/sqlair"
 	"github.com/juju/errors"
-	"github.com/juju/utils/v3"
 
 	coreDB "github.com/juju/juju/core/database"
 	"github.com/juju/juju/core/network"
@@ -33,7 +32,7 @@ func NewState(factory coreDB.TxnRunnerFactory) *State {
 // AddSpace creates and returns a new space.
 func (st *State) AddSpace(
 	ctx context.Context,
-	uuid utils.UUID,
+	uuid string,
 	name string,
 	providerID network.Id,
 	subnetIDs []string,
@@ -69,7 +68,7 @@ WHERE  subnet_type.is_space_settable = FALSE AND subnet.uuid IN (%s)`, subnetBin
 		// that are of a type on which the space can be set.
 		nonSettableSubnets, err := tx.QueryContext(ctx, checkInputSubnetsStmt, subnetVals...)
 		if err != nil {
-			return errors.Annotatef(err, "checking if there are fan subnets for space %q", uuid.String())
+			return errors.Annotatef(err, "checking if there are fan subnets for space %q", uuid)
 		}
 		defer func() { _ = nonSettableSubnets.Close() }()
 		// If any row is returned we must fail with the returned fan
@@ -92,19 +91,19 @@ WHERE  subnet_type.is_space_settable = FALSE AND subnet.uuid IN (%s)`, subnetBin
 				"cannot set space for FAN subnet UUIDs %q - it is always inherited from underlay", nonSettableUUIDs)
 		}
 
-		if _, err := tx.ExecContext(ctx, insertSpaceStmt, uuid.String(), name); err != nil {
-			return errors.Annotatef(err, "inserting space uuid %q into space table", uuid.String())
+		if _, err := tx.ExecContext(ctx, insertSpaceStmt, uuid, name); err != nil {
+			return errors.Annotatef(err, "inserting space uuid %q into space table", uuid)
 		}
 		if providerID != "" {
-			if _, err := tx.ExecContext(ctx, insertProviderStmt, providerID, uuid.String()); err != nil {
+			if _, err := tx.ExecContext(ctx, insertProviderStmt, providerID, uuid); err != nil {
 				return errors.Annotatef(err, "inserting provider id %q into provider_space table", providerID)
 			}
 		}
 
 		// Retrieve the fan overlays (if any) of the passed subnet ids.
 		rows, err := tx.QueryContext(ctx, findFanSubnetsStmt, subnetVals...)
 		if err != nil {
-			return errors.Annotatef(err, "retrieving the fan subnets for space %q", uuid.String())
+			return errors.Annotatef(err, "retrieving the fan subnets for space %q", uuid)
 		}
 		defer func() { _ = rows.Close() }()
 		// Append the fan subnet (unique) ids (if any) to the provided
@@ -127,8 +126,8 @@ WHERE  subnet_type.is_space_settable = FALSE AND subnet.uuid IN (%s)`, subnetBin
 		// Update all subnets (including their fan overlays) to include
 		// the space uuid.
 		for _, subnetID := range subnetIDs {
-			if err := updateSubnetSpaceID(ctx, tx, subnetID, uuid.String()); err != nil {
-				return errors.Annotatef(err, "updating subnet %q using space uuid %q", subnetID, uuid.String())
+			if err := updateSubnetSpaceID(ctx, tx, subnetID, uuid); err != nil {
+				return errors.Annotatef(err, "updating subnet %q using space uuid %q", subnetID, uuid)
 			}
 		}
 		return nil

domain/network/state/space_test.go

@@ -47,7 +47,7 @@ func (s *stateSuite) TestAddSpace(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	subnets := []string{subnetUUID.String()}
-	err = st.AddSpace(ctx.Background(), uuid, "space0", "foo", subnets)
+	err = st.AddSpace(ctx.Background(), uuid.String(), "space0", "foo", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 
 	// Check the space entity.
@@ -103,7 +103,7 @@ func (s *stateSuite) TestAddSpaceFailDuplicateName(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	subnets := []string{subnetUUID.String()}
-	err = st.AddSpace(ctx.Background(), uuid, "space0", "foo", subnets)
+	err = st.AddSpace(ctx.Background(), uuid.String(), "space0", "foo", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 
 	// Check the space entity.
@@ -114,7 +114,7 @@ func (s *stateSuite) TestAddSpaceFailDuplicateName(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Check(name, gc.Equals, "space0")
 	// Fails when trying to add a new space with the same name.
-	err = st.AddSpace(ctx.Background(), uuid, "space0", "bar", subnets)
+	err = st.AddSpace(ctx.Background(), uuid.String(), "space0", "bar", subnets)
 	c.Assert(err, gc.ErrorMatches, "inserting space (.*) into space table: UNIQUE constraint failed: space.name")
 
 }
@@ -145,7 +145,7 @@ func (s *stateSuite) TestAddSpaceEmptyProviderID(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	subnets := []string{subnetUUID.String()}
-	err = st.AddSpace(ctx.Background(), uuid, "space0", "", subnets)
+	err = st.AddSpace(ctx.Background(), uuid.String(), "space0", "", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 
 	sp, err := st.GetSpace(ctx.Background(), uuid.String())
@@ -205,7 +205,7 @@ func (s *stateSuite) TestAddSpaceFailFanOverlay(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	subnets := []string{subnetUUID.String(), subnetFanUUID.String()}
-	err = st.AddSpace(ctx.Background(), uuid, "space0", "foo", subnets)
+	err = st.AddSpace(ctx.Background(), uuid.String(), "space0", "foo", subnets)
 
 	// Should fail with error indicating we cannot set the space for a
 	// FAN subnet.
@@ -273,7 +273,7 @@ func (s *stateSuite) TestRetrieveSpaceByUUID(c *gc.C) {
 	subnets := []string{subnetUUID0.String(), subnetUUID1.String()}
 	spaceUUID, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID, "space0", "foo", subnets)
+	err = st.AddSpace(ctx.Background(), spaceUUID.String(), "space0", "foo", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 
 	sp, err := st.GetSpace(ctx.Background(), spaceUUID.String())
@@ -333,11 +333,11 @@ func (s *stateSuite) TestRetrieveSpaceByName(c *gc.C) {
 
 	spaceUUID0, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID0, "space0", "provider0", []string{})
+	err = st.AddSpace(ctx.Background(), spaceUUID0.String(), "space0", "provider0", []string{})
 	c.Assert(err, jc.ErrorIsNil)
 	spaceUUID1, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID1, "space1", "provider1", []string{})
+	err = st.AddSpace(ctx.Background(), spaceUUID1.String(), "space1", "provider1", []string{})
 	c.Assert(err, jc.ErrorIsNil)
 
 	sp0, err := st.GetSpaceByName(ctx.Background(), "space0")
@@ -362,7 +362,7 @@ func (s *stateSuite) TestRetrieveSpaceByUUIDWithoutSubnet(c *gc.C) {
 
 	spaceUUID, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID, "space0", "foo", []string{})
+	err = st.AddSpace(ctx.Background(), spaceUUID.String(), "space0", "foo", []string{})
 	c.Assert(err, jc.ErrorIsNil)
 
 	sp, err := st.GetSpace(ctx.Background(), spaceUUID.String())
@@ -428,17 +428,17 @@ func (s *stateSuite) TestRetrieveAllSpaces(c *gc.C) {
 	subnets := []string{subnetUUID0.String()}
 	spaceUUID0, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID0, "space0", "foo0", subnets)
+	err = st.AddSpace(ctx.Background(), spaceUUID0.String(), "space0", "foo0", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 	subnets = []string{subnetUUID1.String()}
 	spaceUUID1, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID1, "space1", "foo1", subnets)
+	err = st.AddSpace(ctx.Background(), spaceUUID1.String(), "space1", "foo1", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 	subnets = []string{subnetUUID2.String()}
 	spaceUUID2, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spaceUUID2, "space2", "foo2", subnets)
+	err = st.AddSpace(ctx.Background(), spaceUUID2.String(), "space2", "foo2", subnets)
 	c.Assert(err, jc.ErrorIsNil)
 
 	sp, err := st.GetAllSpaces(ctx.Background())
@@ -451,7 +451,7 @@ func (s *stateSuite) TestUpdateSpace(c *gc.C) {
 
 	uuid, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), uuid, "space0", "foo", []string{})
+	err = st.AddSpace(ctx.Background(), uuid.String(), "space0", "foo", []string{})
 	c.Assert(err, jc.ErrorIsNil)
 
 	err = st.UpdateSpace(ctx.Background(), uuid.String(), "newSpaceName0")
@@ -494,7 +494,7 @@ func (s *stateSuite) TestDeleteSpace(c *gc.C) {
 	// Create a space containing the newly created subnet.
 	spUUID, err := utils.NewUUID()
 	c.Assert(err, jc.ErrorIsNil)
-	err = st.AddSpace(ctx.Background(), spUUID, "space0", "foo", []string{subnetUUID0.String()})
+	err = st.AddSpace(ctx.Background(), spUUID.String(), "space0", "foo", []string{subnetUUID0.String()})
 	c.Assert(err, jc.ErrorIsNil)
 
 	// Check the subnet entity.