Replicate k8s checking logic into storage package.

bnhassin · Jun 7, 2022 · 38f2b69 · 38f2b69
1 parent 113892a
commit 38f2b69
Show file tree

Hide file tree

Showing 17 changed files with 165 additions and 58 deletions.
diff --git a/caas/kubernetes/provider/storage.go b/caas/kubernetes/provider/storage.go
@@ -37,11 +37,7 @@ var _ jujustorage.Provider = (*storageProvider)(nil)
 
 //ValidateStorageProvider returns an error if the storage type and config is not valid
 // for a Kubernetes deployment.
-func (g *storageProvider) ValidateStorageProvider(isCaas bool, attributes map[string]any) error {
-
-	if !isCaas {
-		return errors.NotValidf("storage provider type %q", constants.StorageProviderType)
-	}
+func (g *storageProvider) ValidateForK8s(attributes map[string]any) error {
 
 	if attributes == nil {
 		return nil

diff --git a/caas/kubernetes/provider/storage_test.go b/caas/kubernetes/provider/storage_test.go
@@ -15,7 +15,6 @@ import (
 	"github.com/juju/juju/caas/kubernetes/provider/constants"
 	"github.com/juju/juju/environs/context"
 	"github.com/juju/juju/storage"
-	storageprovider "github.com/juju/juju/storage/provider"
 )
 
 var _ = gc.Suite(&storageSuite{})
@@ -180,25 +179,18 @@ func (s *storageSuite) TestValidateStorageProvider(c *gc.C) {
 	prov := s.k8sProvider(c, ctrl)
 
 	for _, t := range []struct {
-		providerType storage.ProviderType
-		attrs        map[string]interface{}
-		err          string
+		attrs map[string]interface{}
+		err   string
 	}{
 		{
-			providerType: storageprovider.RootfsProviderType,
-		}, {
-			providerType: storageprovider.TmpfsProviderType,
-		}, {
-			providerType: storageprovider.LoopProviderType,
-			err:          `storage provider type "loop" not valid`,
-		}, {
-			providerType: storageprovider.TmpfsProviderType,
-			attrs:        map[string]interface{}{"storage-medium": "foo"},
-			err:          `storage medium "foo" not valid`,
+			attrs: map[string]interface{}{"storage-medium": "foo"},
+			err:   `storage medium "foo" not valid`,
+		},
+		{
+			attrs: nil,
 		},
 	} {
-		err := prov.ValidateStorageProvider(t.providerType, t.attrs)
-		// err := provider.ValidateStorageProvider(t.providerType, t.attrs)
+		err := prov.ValidateForK8s(t.attrs)
 		if t.err == "" {
 			c.Check(err, jc.ErrorIsNil)
 		} else {

diff --git a/provider/azure/storage.go b/provider/azure/storage.go
@@ -83,7 +83,7 @@ func newAzureStorageConfig(attrs map[string]interface{}) (*azureStorageConfig, e
 	return azureStorageConfig, nil
 }
 
-func (e *azureStorageProvider) ValidateStorageProvider(bool, map[string]any) error {
+func (e *azureStorageProvider) ValidateForK8s(map[string]any) error {
 	// no validation required
 	return nil
 }

diff --git a/provider/ec2/ebs.go b/provider/ec2/ebs.go
@@ -271,7 +271,7 @@ func newEbsConfig(attrs map[string]interface{}) (*ebsConfig, error) {
 	return ebsConfig, nil
 }
 
-func (e *ebsProvider) ValidateStorageProvider(bool, map[string]any) error {
+func (e *ebsProvider) ValidateForK8s(map[string]any) error {
 	// no validation required
 	return nil
 }

diff --git a/provider/gce/disks.go b/provider/gce/disks.go
@@ -41,7 +41,7 @@ type storageProvider struct {
 
 var _ storage.Provider = (*storageProvider)(nil)
 
-func (g *storageProvider) ValidateStorageProvider(bool, map[string]any) error {
+func (g *storageProvider) ValidateForK8s(map[string]any) error {
 	// no validation required
 	return nil
 }

diff --git a/provider/lxd/storage.go b/provider/lxd/storage.go
@@ -130,9 +130,8 @@ func newLXDStorageConfig(attrs map[string]interface{}) (*lxdStorageConfig, error
 	return lxdStorageConfig, nil
 }
 
-func (e *lxdStorageProvider) ValidateStorageProvider(bool, map[string]any) error {
-	// no validation required
-	return nil
+func (e *lxdStorageProvider) ValidateForK8s(map[string]any) error {
+	return errors.NotValidf("storage provider type %q", lxdStorageProviderType)
 }
 
 // ValidateConfig is part of the Provider interface.

diff --git a/provider/maas/volumes.go b/provider/maas/volumes.go
@@ -93,7 +93,7 @@ func newStorageConfig(attrs map[string]interface{}) (*storageConfig, error) {
 	return &storageConfig{tags: tags}, nil
 }
 
-func (maasStorageProvider) ValidateStorageProvider(bool, map[string]any) error {
+func (maasStorageProvider) ValidateForK8s(map[string]any) error {
 	// no validation required
 	return nil
 }

diff --git a/provider/oci/storage.go b/provider/oci/storage.go
@@ -83,9 +83,8 @@ func (s *storageProvider) DefaultPools() []*storage.Config {
 	return []*storage.Config{pool}
 }
 
-func (s *storageProvider) ValidateStorageProvider(bool, map[string]any) error {
-	// no validation required
-	return nil
+func (s *storageProvider) ValidateForK8s(map[string]any) error {
+	return errors.NotValidf("storage provider type %q", ociStorageProviderType)
 }
 
 func (s *storageProvider) ValidateConfig(cfg *storage.Config) error {

diff --git a/provider/openstack/cinder.go b/provider/openstack/cinder.go
@@ -188,9 +188,8 @@ func (s *cinderProvider) Scope() storage.Scope {
 	return storage.ScopeEnviron
 }
 
-func (p *cinderProvider) ValidateStorageProvider(bool, map[string]any) error {
-	// no validation required
-	return nil
+func (p *cinderProvider) ValidateForK8s(map[string]any) error {
+	return errors.NotValidf("storage provider type %q", CinderProviderType)
 }
 
 // ValidateConfig implements storage.Provider.

diff --git a/state/storage.go b/state/storage.go
@@ -1875,9 +1875,11 @@ func validateStoragePool(
 			*machineId = ""
 		}
 	}
-	isCaas := sb.modelType == ModelTypeCAAS
-	if err := aProvider.ValidateStorageProvider(isCaas, poolConfig); err != nil {
-		return errors.Annotatef(err, "invalid storage config")
+	//
+	if sb.modelType == ModelTypeCAAS {
+		if err := aProvider.ValidateForK8s(poolConfig); err != nil {
+			return errors.Annotatef(err, "invalid storage config")
+		}
 	}
 
 	return nil

diff --git a/storage/constants.go b/storage/constants.go
@@ -0,0 +1,21 @@
+// Copyright 2022 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package storage
+
+const (
+	// K8sStorageMediumConst is the constant key
+	K8sStorageMediumConst = "storage-medium"
+	// K8sStorageMediumMemory is the value use tmpfs in K8s
+	K8sStorageMediumMemory = "Memory"
+	// K8sStorageMediumHugePages is a K8s storage for volumes
+	K8sStorageMediumHugePages = "HugePages"
+)
+
+const (
+	// StorageClass is the name of a storage class resource.
+	K8sStorageClass       = "storage-class"
+	K8sStorageProvisioner = "storage-provisioner"
+	K8sStorageMedium      = "storage-medium"
+	K8sStorageMode        = "storage-mode"
+)
diff --git a/storage/interface.go b/storage/interface.go
@@ -85,9 +85,9 @@ type Provider interface {
 	// returning an error if it is invalid.
 	ValidateConfig(*Config) error
 
-	// ValidateStorageProvider validates if for a CaaS model the
-	// current provider is valid with the given attributes.
-	ValidateStorageProvider(bool, map[string]any) error
+	// ValidateForkK8s validates if a storage provider can be set for
+	// a given K8s configuration.
+	ValidateForK8s(map[string]any) error
 }
 
 // VolumeSource provides an interface for creating, destroying, describing,

diff --git a/storage/provider/dummy/provider.go b/storage/provider/dummy/provider.go
@@ -44,6 +44,10 @@ type StorageProvider struct {
 	// otherwise ValidateConfig returns nil.
 	ValidateConfigFunc func(*storage.Config) error
 
+	// ValidateForK8sFunc will be called by ValidateForK8s, if non-nil;
+	// otherwise ValidateForK8s returns nil.
+	ValidateForK8sFunc func(map[string]any) error
+
 	// SupportsFunc will be called by Supports, if non-nil; otherwise,
 	// Supports returns true.
 	SupportsFunc func(kind storage.StorageKind) bool
@@ -76,9 +80,9 @@ func (p *StorageProvider) ValidateConfig(providerConfig *storage.Config) error {
 	return nil
 }
 
-func (p *StorageProvider) ValidateStorageProvider(bool, map[string]any) error {
-	// no validation required
-	return nil
+func (p *StorageProvider) ValidateForK8s(attributes map[string]any) error {
+	p.MethodCall(p, "ValidateForK8s", attributes)
+	return p.ValidateForK8sFunc(attributes)
 }
 
 // Supports is defined on storage.Provider.

diff --git a/storage/provider/k8s_validation.go b/storage/provider/k8s_validation.go
@@ -0,0 +1,98 @@
+// Copyright 2022 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package provider
+
+import (
+	"fmt"
+
+	"github.com/juju/errors"
+	"github.com/juju/juju/storage"
+	"github.com/juju/schema"
+)
+
+// checkK8sConfig checks that the attributes in a configuration
+// are valid for a K8s deployment.
+func checkK8sConfig(attributes map[string]any) error {
+	if mediumValue, ok := attributes[storage.K8sStorageMediumConst]; ok {
+		medium := fmt.Sprintf("%v", mediumValue)
+		if medium != storage.K8sStorageMediumMemory && medium != storage.K8sStorageMediumHugePages {
+			return errors.NotValidf("storage medium %q", mediumValue)
+		}
+	}
+
+	if err := validateStorageAttributes(attributes); err != nil {
+		return errors.Trace(err)
+	}
+
+	return nil
+}
+
+func validateStorageAttributes(attributes map[string]any) error {
+	if err := validateStorageConfig(attributes); err != nil {
+		return errors.Trace(err)
+	}
+	if err := validateStorageMode(attributes); err != nil {
+		return errors.Trace(err)
+	}
+	return nil
+}
+
+var storageConfigFields = schema.Fields{
+	storage.K8sStorageClass:       schema.String(),
+	storage.K8sStorageProvisioner: schema.String(),
+}
+
+var storageConfigChecker = schema.FieldMap(
+	storageConfigFields,
+	schema.Defaults{
+		storage.K8sStorageClass:       schema.Omit,
+		storage.K8sStorageProvisioner: schema.Omit,
+	},
+)
+
+// validateStorageConfig returns issues in the configuration if any.
+func validateStorageConfig(attrs map[string]any) error {
+	out, err := storageConfigChecker.Coerce(attrs, nil)
+	if err != nil {
+		return errors.Annotate(err, "validating storage config")
+	}
+	coerced := out.(map[string]any)
+
+	if coerced[storage.K8sStorageProvisioner] != "" && coerced[storage.K8sStorageClass] == "" {
+		return errors.New("storage-class must be specified if storage-provisioner is specified")
+	}
+
+	return nil
+}
+
+var storageModeFields = schema.Fields{
+	storage.K8sStorageMode: schema.String(),
+}
+
+var storageModeChecker = schema.FieldMap(
+	storageModeFields,
+	schema.Defaults{
+		storage.K8sStorageMode: "ReadWriteOnce",
+	},
+)
+
+// validateStorageMode returns an error if the K8s persistent
+// volume is not configured correctly.
+func validateStorageMode(attrs map[string]any) error {
+	out, err := storageModeChecker.Coerce(attrs, nil)
+	if err != nil {
+		return errors.Annotate(err, "validating storage mode")
+	}
+	coerced := out.(map[string]any)
+	mode := coerced[storage.K8sStorageMode]
+	switch coerced[storage.K8sStorageMode] {
+	case "ReadOnlyMany", "ROX":
+	case "ReadWriteMany", "RWX":
+	case "ReadWriteOnce", "RWO":
+	default:
+		return errors.NotSupportedf("storage mode %q", mode)
+	}
+
+	return nil
+}
diff --git a/storage/provider/loop.go b/storage/provider/loop.go
@@ -31,12 +31,8 @@ type loopProvider struct {
 
 var _ storage.Provider = (*loopProvider)(nil)
 
-func (*loopProvider) ValidateStorageProvider(isCaas bool, _ map[string]any) error {
-	// loop does not work with CaaS
-	if isCaas {
-		return errors.NotValidf("storage provider type %q", LoopProviderType)
-	}
-	return nil
+func (*loopProvider) ValidateForK8s(map[string]any) error {
+	return errors.NotValidf("storage provider type %q", LoopProviderType)
 }
 
 // ValidateConfig is defined on the Provider interface.

diff --git a/storage/provider/rootfs.go b/storage/provider/rootfs.go
@@ -28,12 +28,12 @@ var (
 	_ storage.Provider = (*rootfsProvider)(nil)
 )
 
-func (p *rootfsProvider) ValidateStorageProvider(isCaas bool, config map[string]any) error {
-	if isCaas {
-		return errors.NotValidf("storage provider type %q", RootfsProviderType)
+func (p *rootfsProvider) ValidateForK8s(attributes map[string]any) error {
+	if attributes == nil {
+		return nil
 	}
-
-	return nil
+	// check the configuration
+	return checkK8sConfig(attributes)
 }
 
 // ValidateConfig is defined on the Provider interface.

diff --git a/storage/provider/tmpfs.go b/storage/provider/tmpfs.go
@@ -30,12 +30,13 @@ var (
 	_ storage.Provider = (*tmpfsProvider)(nil)
 )
 
-func (p *tmpfsProvider) ValidateStorageProvider(isCaas bool, config map[string]any) error {
-	if !isCaas {
-		return errors.NotValidf("storage provider type %q", TmpfsProviderType)
+func (p *tmpfsProvider) ValidateForK8s(attributes map[string]any) error {
+	if attributes == nil {
+		return nil
 	}
 
-	return nil
+	// check the configuration
+	return checkK8sConfig(attributes)
 }
 
 // ValidateConfig is defined on the Provider interface.