Merge pull request juju#18951 from wallyworld/storage-ddl-tweaks

juju#18951

Add missing unique indices for generated id columns.
Rename size column in storage_instance table. It is really the requested size and the rename helps disambiguate it from the actual size columns in the filesystem and volume tables.
Remove unnecessary pool columns from filesystem and volume tables - it doesn't need to be denormalised there since it's recorded on storage_instance.
On storage instance and the storage directive tables, introduce 2 separate (nullable) columns for storage pool and storage type - one must be set.
Also make volume name column nullable as it's not known till provisioning.

Also add consts for storage provisioning status and storage kind to match the DB lookup values.

## QA steps

Just unit tests.

## Links

**Jira card:** [JUJU-7593](https://warthogs.atlassian.net/browse/JUJU-7593)



[JUJU-7593]: https://warthogs.atlassian.net/browse/JUJU-7593?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

domain/annotation/state/state_test.go

@@ -438,7 +438,7 @@ VALUES (?, ?, ?, ?, ?)
 func (s *stateSuite) ensureStorage(c *gc.C, name, uuid, charmUUID string) {
 	err := s.TxnRunner().StdTxn(context.Background(), func(ctx context.Context, tx *sql.Tx) error {
 		_, err := tx.ExecContext(ctx, `
-INSERT INTO storage_instance (uuid, storage_id, storage_pool, size_mib, charm_uuid, storage_name, life_id)
+INSERT INTO storage_instance (uuid, storage_id, storage_type, requested_size_mib, charm_uuid, storage_name, life_id)
 VALUES (?, ?, ?, ?, ?, ?, ?)
 		`, uuid, name+"/0", "loop", 100, charmUUID, name, 0)
 		return err

domain/application/service/application.go

@@ -783,10 +783,10 @@ func makeStorageArgs(storage map[string]storage.Directive) []application.AddAppl
 	var result []application.AddApplicationStorageArg
 	for name, stor := range storage {
 		result = append(result, application.AddApplicationStorageArg{
-			Name:  name,
-			Pool:  stor.Pool,
-			Size:  stor.Size,
-			Count: stor.Count,
+			Name:           name,
+			PoolNameOrType: stor.Pool,
+			Size:           stor.Size,
+			Count:          stor.Count,
 		})
 	}
 	return result

domain/application/service/application_test.go

@@ -683,10 +683,10 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlock(c *gc.C) {
 		},
 		Platform: platform,
 		Storage: []application.AddApplicationStorageArg{{
-			Name:  "data",
-			Pool:  "loop",
-			Size:  10,
-			Count: 1,
+			Name:           "data",
+			PoolNameOrType: "loop",
+			Size:           10,
+			Count:          1,
 		}},
 		Scale: 1,
 	}
@@ -792,10 +792,10 @@ func (s *applicationServiceSuite) TestCreateWithStorageBlockDefaultSource(c *gc.
 		},
 		Platform: platform,
 		Storage: []application.AddApplicationStorageArg{{
-			Name:  "data",
-			Pool:  "fast",
-			Size:  10,
-			Count: 2,
+			Name:           "data",
+			PoolNameOrType: "fast",
+			Size:           10,
+			Count:          2,
 		}},
 		Scale: 1,
 	}
@@ -905,10 +905,10 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystem(c *gc.C) {
 		},
 		Platform: platform,
 		Storage: []application.AddApplicationStorageArg{{
-			Name:  "data",
-			Pool:  "rootfs",
-			Size:  10,
-			Count: 1,
+			Name:           "data",
+			PoolNameOrType: "rootfs",
+			Size:           10,
+			Count:          1,
 		}},
 		Scale: 1,
 	}
@@ -1015,10 +1015,10 @@ func (s *applicationServiceSuite) TestCreateWithStorageFilesystemDefaultSource(c
 		},
 		Platform: platform,
 		Storage: []application.AddApplicationStorageArg{{
-			Name:  "data",
-			Pool:  "fast",
-			Size:  10,
-			Count: 2,
+			Name:           "data",
+			PoolNameOrType: "fast",
+			Size:           10,
+			Count:          2,
 		}},
 		Scale: 1,
 	}

domain/application/state/storage.go

@@ -18,6 +18,28 @@ import (
 	"github.com/juju/juju/internal/storage"
 )
 
+func (st *State) loadStoragePoolUUIDByName(ctx context.Context, tx *sqlair.TX, poolNames []string) (map[string]string, error) {
+	type poolnames []string
+	storageQuery, err := st.Prepare(`
+SELECT &storagePool.*
+FROM   storage_pool
+WHERE  name IN ($poolnames[:])
+`, storagePool{}, poolnames{})
+	if err != nil {
+		return nil, errors.Capture(err)
+	}
+	var dbPools []storagePool
+	err = tx.Query(ctx, storageQuery, poolnames(poolNames)).GetAll(&dbPools)
+	if err != nil && !errors.Is(err, sql.ErrNoRows) {
+		return nil, errors.Errorf("querying storage pools: %w", err)
+	}
+	poolsByName := make(map[string]string)
+	for _, p := range dbPools {
+		poolsByName[p.Name] = p.UUID
+	}
+	return poolsByName, nil
+}
+
 // insertStorage constructs inserts storage directive records for the application.
 func (st *State) insertStorage(ctx context.Context, tx *sqlair.TX, appDetails applicationDetails, appStorage []application.AddApplicationStorageArg) error {
 	if len(appStorage) == 0 {
@@ -52,16 +74,34 @@ WHERE  charm_uuid = $applicationDetails.charm_uuid
 		return errors.Errorf("storage %q is not supported", unsupportedStorage.SortedValues())
 	}
 
+	// Storage is either a storage type or a pool name.
+	// Get a mapping of pool name to pool UUID for any
+	// pools specified in the app storage directives.
+	poolNames := make([]string, len(appStorage))
+	for i, stor := range appStorage {
+		poolNames[i] = stor.PoolNameOrType
+	}
+	poolsByName, err := st.loadStoragePoolUUIDByName(ctx, tx, poolNames)
+	if err != nil {
+		return errors.Errorf("loading storage pool UUIDs: %w", err)
+	}
+
 	storage := make([]storageToAdd, len(appStorage))
 	for i, stor := range appStorage {
 		storage[i] = storageToAdd{
 			ApplicationUUID: appDetails.UUID.String(),
 			CharmUUID:       appDetails.CharmID,
 			StorageName:     stor.Name,
-			StoragePool:     stor.Pool,
 			Size:            uint(stor.Size),
 			Count:           uint(stor.Count),
 		}
+		// PoolNameOrType has already been validated to either be
+		// a pool name or a valid storage type for the relevant cloud.
+		if uuid, ok := poolsByName[stor.PoolNameOrType]; ok {
+			storage[i].StoragePoolUUID = &uuid
+		} else {
+			storage[i].StorageType = &stor.PoolNameOrType
+		}
 	}
 
 	insertStmt, err := st.Prepare(`

domain/application/state/storage_test.go

@@ -16,35 +16,56 @@ import (
 	"github.com/juju/juju/domain/application/charm"
 	storageerrors "github.com/juju/juju/domain/storage/errors"
 	"github.com/juju/juju/internal/errors"
+	"github.com/juju/juju/internal/uuid"
 )
 
 // TestCreateApplicationWithResources tests creation of an application with
 // specified resources.
 // It verifies that the charm_resource table is populated, alongside the
 // resource and application_resource table with datas from charm and arguments.
 func (s *applicationStateSuite) TestCreateApplicationWithStorage(c *gc.C) {
+	ctx := context.Background()
+	uuid := uuid.MustNewUUID().String()
+	err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
+		_, err := tx.ExecContext(ctx, `
+INSERT INTO storage_pool (uuid, name, type) VALUES (?, ?, ?)`,
+			uuid, "fast", "ebs")
+		if err != nil {
+			return errors.Capture(err)
+		}
+		return nil
+	})
 	chStorage := []charm.Storage{{
 		Name: "database",
 		Type: "block",
 	}, {
 		Name: "logs",
 		Type: "filesystem",
+	}, {
+		Name: "cache",
+		Type: "block",
 	}}
 	addStorageArgs := []application.AddApplicationStorageArg{
 		{
-			Name:  "database",
-			Pool:  "ebs",
-			Size:  10,
-			Count: 2,
+			Name:           "database",
+			PoolNameOrType: "ebs",
+			Size:           10,
+			Count:          2,
+		},
+		{
+			Name:           "logs",
+			PoolNameOrType: "rootfs",
+			Size:           20,
+			Count:          1,
 		},
 		{
-			Name:  "logs",
-			Pool:  "rootfs",
-			Size:  20,
-			Count: 1,
+			Name:           "cache",
+			PoolNameOrType: "fast",
+			Size:           30,
+			Count:          1,
 		},
 	}
-	ctx := context.Background()
+	c.Assert(err, jc.ErrorIsNil)
 
 	appUUID, err := s.state.CreateApplication(ctx, "666", s.addApplicationArgForStorage(c, "666",
 		chStorage, addStorageArgs), nil)
@@ -61,6 +82,7 @@ WHERE name=?`, "666").Scan(&charmUUID)
 	var (
 		foundCharmStorage []charm.Storage
 		foundAppStorage   []application.AddApplicationStorageArg
+		poolUUID          string
 	)
 
 	err = s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
@@ -87,24 +109,38 @@ WHERE charm_uuid=?`, charmUUID)
 	err = s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
 		rows, err := tx.QueryContext(ctx, `
 SELECT storage_name, storage_pool, size_mib, count
-FROM application_storage_directive
+FROM v_application_storage_directive
 WHERE application_uuid = ? AND charm_uuid = ?`, appUUID, charmUUID)
 		if err != nil {
 			return errors.Capture(err)
 		}
 		defer func() { _ = rows.Close() }()
 		for rows.Next() {
 			var stor application.AddApplicationStorageArg
-			if err := rows.Scan(&stor.Name, &stor.Pool, &stor.Size, &stor.Count); err != nil {
+			if err := rows.Scan(&stor.Name, &stor.PoolNameOrType, &stor.Size, &stor.Count); err != nil {
 				return errors.Capture(err)
 			}
 			foundAppStorage = append(foundAppStorage, stor)
 		}
+		rows, err = tx.QueryContext(ctx, `
+SELECT storage_pool_uuid
+FROM application_storage_directive
+WHERE storage_type IS NULL AND application_uuid = ? AND charm_uuid = ?`, appUUID, charmUUID)
+		if err != nil {
+			return errors.Capture(err)
+		}
+		defer func() { _ = rows.Close() }()
+		for rows.Next() {
+			if err := rows.Scan(&poolUUID); err != nil {
+				return errors.Capture(err)
+			}
+		}
 		return nil
 	})
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(foundCharmStorage, jc.SameContents, chStorage)
-	c.Assert(foundAppStorage, jc.SameContents, addStorageArgs)
+	c.Check(foundCharmStorage, jc.SameContents, chStorage)
+	c.Check(foundAppStorage, jc.SameContents, addStorageArgs)
+	c.Assert(poolUUID, gc.Equals, uuid)
 }
 
 func (s *applicationStateSuite) TestCreateApplicationWithUnrecognisedStorage(c *gc.C) {
@@ -113,10 +149,10 @@ func (s *applicationStateSuite) TestCreateApplicationWithUnrecognisedStorage(c *
 		Type: "block",
 	}}
 	addStorageArgs := []application.AddApplicationStorageArg{{
-		Name:  "foo",
-		Pool:  "rootfs",
-		Size:  20,
-		Count: 1,
+		Name:           "foo",
+		PoolNameOrType: "rootfs",
+		Size:           20,
+		Count:          1,
 	}}
 	ctx := context.Background()
 
@@ -127,10 +163,10 @@ func (s *applicationStateSuite) TestCreateApplicationWithUnrecognisedStorage(c *
 
 func (s *applicationStateSuite) TestCreateApplicationWithStorageButCharmHasNone(c *gc.C) {
 	addStorageArgs := []application.AddApplicationStorageArg{{
-		Name:  "foo",
-		Pool:  "rootfs",
-		Size:  20,
-		Count: 1,
+		Name:           "foo",
+		PoolNameOrType: "rootfs",
+		Size:           20,
+		Count:          1,
 	}}
 	ctx := context.Background()
 
@@ -198,8 +234,8 @@ func (s *applicationStateSuite) TestGetStorageUUIDByID(c *gc.C) {
 
 	err := s.TxnRunner().StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error {
 		_, err := tx.ExecContext(ctx, `
-INSERT INTO storage_instance(uuid, charm_uuid, storage_name, storage_id, life_id, storage_pool, size_mib)
-VALUES (?, ?, ?, ?, ?, ?, ?)`, uuid, charmUUID, "pgdata", "pgdata/0", 0, "pool", 666)
+INSERT INTO storage_instance(uuid, charm_uuid, storage_name, storage_id, life_id, storage_type, requested_size_mib)
+VALUES (?, ?, ?, ?, ?, ?, ?)`, uuid, charmUUID, "pgdata", "pgdata/0", 0, "rootfs", 666)
 		return err
 	})
 	c.Assert(err, jc.ErrorIsNil)

domain/application/state/types.go

@@ -640,13 +640,19 @@ type resourceToAdd struct {
 	CreatedAt time.Time `db:"created_at"`
 }
 
+type storagePool struct {
+	UUID string `db:"uuid"`
+	Name string `db:"name"`
+}
+
 type storageToAdd struct {
-	ApplicationUUID string `db:"application_uuid"`
-	CharmUUID       string `db:"charm_uuid"`
-	StorageName     string `db:"storage_name"`
-	StoragePool     string `db:"storage_pool"`
-	Size            uint   `db:"size_mib"`
-	Count           uint   `db:"count"`
+	ApplicationUUID string  `db:"application_uuid"`
+	CharmUUID       string  `db:"charm_uuid"`
+	StorageName     string  `db:"storage_name"`
+	StoragePoolUUID *string `db:"storage_pool_uuid"`
+	StorageType     *string `db:"storage_type"`
+	Size            uint    `db:"size_mib"`
+	Count           uint    `db:"count"`
 }
 
 type linkResourceApplication struct {

domain/application/types.go

@@ -62,10 +62,10 @@ type AddApplicationResourceArg struct {
 
 // AddApplicationStorageArg defines the arguments required to add storage to an application.
 type AddApplicationStorageArg struct {
-	Name  string
-	Pool  string
-	Size  uint64
-	Count uint64
+	Name           string
+	PoolNameOrType string
+	Size           uint64
+	Count          uint64
 }
 
 // Channel represents the channel of a application charm.