Skip to content

Commit

Permalink
Merge pull request #8935 from wallyworld/create-cass-storageclass
Browse files Browse the repository at this point in the history
#8935

## Description of change

If the juju storage pool nominated to be used to create charm storage specifies a k8s provisioner, then we now create that storage class. Previously, the storage class needed to be set up manually by the user.

We also move all of the code to construct the storage class from k8s.go to storage.go

## QA steps

On an AWS k8s setup, create a caas model.

$ juju create-storage-pool k8s-ebs kubernetes storage-class=juju-ebs storage-provisioner=kubernetes.io/aws-ebs parameters.type=gp2
$ juju deploy ./mariadb --storage database=10M,k8s-ebs
  • Loading branch information
jujubot committed Jul 16, 2018
2 parents cf542df + ec65d9c commit 96ce458
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 38 deletions.
13 changes: 13 additions & 0 deletions caas/kubernetes/provider/export_test.go
Expand Up @@ -17,6 +17,7 @@ var (
OperatorPod = operatorPod
ExtractRegistryURL = extractRegistryURL
CreateDockerConfigJSON = createDockerConfigJSON
NewStorageConfig = newStorageConfig
)

func PodSpec(u *unitSpec) core.PodSpec {
Expand All @@ -30,3 +31,15 @@ func NewProvider() caas.ContainerEnvironProvider {
func StorageProvider(k8sClient kubernetes.Interface, namespace string) storage.Provider {
return &storageProvider{&kubernetesClient{Interface: k8sClient, namespace: namespace}}
}

func StorageClass(cfg *storageConfig) string {
return cfg.storageClass
}

func StorageProvisioner(cfg *storageConfig) string {
return cfg.storageProvisioner
}

func StorageParameters(cfg *storageConfig) map[string]string {
return cfg.parameters
}
71 changes: 47 additions & 24 deletions caas/kubernetes/provider/k8s.go
Expand Up @@ -49,12 +49,7 @@ const (
labelVersion = "juju-version"
labelApplication = "juju-application"

// Well known storage pool attributes.
jujuStorageClassKey = "juju-storage-class"
jujuStorageLabelKey = "juju-storage-label"

jujuDefaultStorageClassName = "juju-unit-storage"
operatorStorageClassName = "juju-operator-storage"
operatorStorageClassName = "juju-operator-storage"
// TODO(caas) - make this configurable using application config
operatorStorageSize = "10Mi"
)
Expand Down Expand Up @@ -216,7 +211,7 @@ func (k *kubernetesClient) EnsureOperator(appName, agentPath string, config *caa
// If there are none, that's ok, we'll just use ephemeral storage.
volStorageLabel := fmt.Sprintf("%s-operator-storage", appName)
params := volumeParams{
storageClassName: operatorStorageClassName,
storageConfig: &storageConfig{storageClass: operatorStorageClassName},
storageLabels: []string{volStorageLabel, k.namespace, "default"},
pvcName: operatorVolumeClaim(appName),
requestedVolumeSize: operatorStorageSize,
Expand Down Expand Up @@ -308,13 +303,12 @@ func operatorVolumeClaim(appName string) string {
}

type volumeParams struct {
storageLabels []string
storageClassName string
fallbackStorageClassName string
pvcName string
requestedVolumeSize string
labels map[string]string
accessMode core.PersistentVolumeAccessMode
storageLabels []string
storageConfig *storageConfig
pvcName string
requestedVolumeSize string
labels map[string]string
accessMode core.PersistentVolumeAccessMode
}

// maybeGetVolumeClaimSpec returns a persistent volume claim spec, and a bool indicating
Expand All @@ -335,12 +329,12 @@ func (k *kubernetesClient) maybeGetVolumeClaimSpec(params volumeParams) (*core.P
// We need to create a new claim.
logger.Debugf("creating new persistent volume claim for %v", params.pvcName)

storageClassName := params.storageClassName
storageClassName := params.storageConfig.storageClass
if storageClassName != "" {
// If a specific storage class has been requested, make sure it exists.
_, err := k.StorageV1().StorageClasses().Get(storageClassName, v1.GetOptions{})
err := k.ensureStorageClass(params.storageConfig)
if err != nil {
if !k8serrors.IsNotFound(err) {
if !errors.IsNotFound(err) {
return nil, false, errors.Trace(err)
}
storageClassName = ""
Expand All @@ -353,7 +347,7 @@ func (k *kubernetesClient) maybeGetVolumeClaimSpec(params volumeParams) (*core.P
if err == nil {
storageClassName = sc.Name
} else {
storageClassName = params.fallbackStorageClassName
storageClassName = defaultStorageClass
}
}
if storageClassName == "" {
Expand All @@ -380,6 +374,36 @@ func (k *kubernetesClient) maybeGetVolumeClaimSpec(params volumeParams) (*core.P
}, false, nil
}

func (k *kubernetesClient) ensureStorageClass(cfg *storageConfig) error {
// First see if the named storage class exists.
storageClasses := k.StorageV1().StorageClasses()
_, err := storageClasses.Get(cfg.storageClass, v1.GetOptions{})
if err == nil {
return nil
}

if !k8serrors.IsNotFound(err) {
return errors.Trace(err)
}
// If it's not found but there's no provisioner specified, we can't
// create it so just return not found.
if err != nil && cfg.storageProvisioner == "" {
return errors.NewNotFound(nil,
fmt.Sprintf("storage class %q doesn't exist, but no storage provisioner has been specified",
cfg.storageClass))
}

// Create the storage class with the specified provisioner.
_, err = storageClasses.Create(&k8sstorage.StorageClass{
ObjectMeta: v1.ObjectMeta{
Name: cfg.storageClass,
},
Provisioner: cfg.storageProvisioner,
Parameters: cfg.parameters,
})
return errors.Trace(err)
}

// DeleteOperator deletes the specified operator.
func (k *kubernetesClient) DeleteOperator(appName string) (err error) {
logger.Debugf("deleting %s operator", appName)
Expand Down Expand Up @@ -572,14 +596,13 @@ func (k *kubernetesClient) configureStorage(
requestedVolumeSize: fmt.Sprintf("%dMi", fs.Size),
labels: map[string]string{labelApplication: appName},
}
if storageClassName, ok := fs.Attributes[jujuStorageClassKey]; ok {
params.storageClassName = fmt.Sprintf("%v", storageClassName)
} else {
params.fallbackStorageClassName = jujuDefaultStorageClassName
}
if storageLabel, ok := fs.Attributes[jujuStorageLabelKey]; ok {
if storageLabel, ok := fs.Attributes[storageLabel]; ok {
params.storageLabels = append([]string{fmt.Sprintf("%v", storageLabel)}, params.storageLabels...)
}
params.storageConfig, err = newStorageConfig(fs.Attributes)
if err != nil {
return errors.Annotatef(err, "invalid storage configuration for %v", fs.StorageName)
}

pvcSpec, _, err := k.maybeGetVolumeClaimSpec(params)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions caas/kubernetes/provider/k8s_test.go
Expand Up @@ -258,7 +258,7 @@ func (s *K8sBrokerSuite) TestEnsureServiceWithStorage(c *gc.C) {
MountPath: "path/to/here",
}}

scName := "sc"
scName := "juju-unit-storage"
statefulSetArg := &appsv1.StatefulSet{
ObjectMeta: v1.ObjectMeta{
Name: "juju-test",
Expand Down Expand Up @@ -309,8 +309,8 @@ func (s *K8sBrokerSuite) TestEnsureServiceWithStorage(c *gc.C) {
gomock.InOrder(
s.mockPersistentVolumeClaims.EXPECT().Get("juju-database-0", v1.GetOptions{}).
Return(nil, s.k8sNotFoundError()),
s.mockStorageClass.EXPECT().List(v1.ListOptions{LabelSelector: "juju-storage in (test-unit-storage, test, default)"}).
Return(&storagev1.StorageClassList{Items: []storagev1.StorageClass{{ObjectMeta: v1.ObjectMeta{Name: "sc"}}}}, nil),
s.mockStorageClass.EXPECT().Get("juju-unit-storage", v1.GetOptions{IncludeUninitialized: false}).Times(1).
Return(&storagev1.StorageClass{ObjectMeta: v1.ObjectMeta{Name: "juju-unit-storage"}}, nil),
s.mockStatefulSets.EXPECT().Update(statefulSetArg).Times(1).
Return(nil, s.k8sNotFoundError()),
s.mockStatefulSets.EXPECT().Create(statefulSetArg).Times(1).
Expand Down
41 changes: 34 additions & 7 deletions caas/kubernetes/provider/storage.go
Expand Up @@ -4,6 +4,9 @@
package provider

import (
"fmt"
"strings"

"github.com/juju/errors"
"github.com/juju/juju/caas"
"github.com/juju/schema"
Expand All @@ -17,10 +20,12 @@ const (
K8s_ProviderType = storage.ProviderType("kubernetes")

// K8s storage pool attributes.
storageClass = "storage-class"
storageClass = "storage-class"
storageProvisioner = "storage-provisioner"
storageLabel = "storage-label"

// K8s storage pool attribute default values.
defaultStorageClass = "<default>"
defaultStorageClass = "juju-unit-storage"
)

// StorageProviderTypes is defined on the storage.ProviderRegistry interface.
Expand All @@ -43,18 +48,25 @@ type storageProvider struct {
var _ storage.Provider = (*storageProvider)(nil)

var storageConfigFields = schema.Fields{
storageClass: schema.String(),
storageClass: schema.String(),
storageLabel: schema.String(),
storageProvisioner: schema.String(),
}

var storageConfigChecker = schema.FieldMap(
storageConfigFields,
schema.Defaults{
storageClass: defaultStorageClass,
storageClass: defaultStorageClass,
storageLabel: schema.Omit,
storageProvisioner: schema.Omit,
},
)

type storageConfig struct {
storageClass string
storageClass string
storageProvisioner string
storageLabels []string
parameters map[string]string
}

func newStorageConfig(attrs map[string]interface{}) (*storageConfig, error) {
Expand All @@ -63,10 +75,25 @@ func newStorageConfig(attrs map[string]interface{}) (*storageConfig, error) {
return nil, errors.Annotate(err, "validating storage config")
}
coerced := out.(map[string]interface{})
storageClass := coerced[storageClass].(string)
storageClassName := coerced[storageClass].(string)
storageConfig := &storageConfig{
storageClass: storageClass,
storageClass: storageClassName,
}
if storageProvisioner, ok := coerced[storageProvisioner].(string); ok {
storageConfig.storageProvisioner = storageProvisioner
}
if storageConfig.storageProvisioner != "" && storageConfig.storageClass == "" {
return nil, errors.New("storage-class must be specified if storage-provisioner is specified")
}
storageConfig.parameters = make(map[string]string)
for k, v := range attrs {
k = strings.TrimPrefix(k, "parameters.")
storageConfig.parameters[k] = fmt.Sprintf("%v", v)
}
delete(storageConfig.parameters, storageClass)
delete(storageConfig.parameters, storageLabel)
delete(storageConfig.parameters, storageProvisioner)

return storageConfig, nil
}

Expand Down
39 changes: 35 additions & 4 deletions caas/kubernetes/provider/storage_test.go
Expand Up @@ -33,25 +33,56 @@ func (s *storageSuite) TestValidateConfig(c *gc.C) {

p := s.k8sProvider(c, ctrl)
cfg, err := storage.NewConfig("name", provider.K8s_ProviderType, map[string]interface{}{
"storage-class": "my-storage",
"storage-class": "my-storage",
"storage-provisioner": "aws-storage",
"storage-label": "storage-fred",
})
c.Assert(err, jc.ErrorIsNil)
err = p.ValidateConfig(cfg)
c.Assert(err, jc.ErrorIsNil)
c.Assert(cfg.Attrs(), jc.DeepEquals, map[string]interface{}{
"storage-class": "my-storage",
"storage-class": "my-storage",
"storage-provisioner": "aws-storage",
"storage-label": "storage-fred",
})
}

func (s *storageSuite) TestValidateConfigDefaultStorageClass(c *gc.C) {
func (s *storageSuite) TestValidateConfigError(c *gc.C) {
ctrl := s.setupBroker(c)
defer ctrl.Finish()

p := s.k8sProvider(c, ctrl)
cfg, err := storage.NewConfig("name", provider.K8s_ProviderType, map[string]interface{}{})
cfg, err := storage.NewConfig("name", provider.K8s_ProviderType, map[string]interface{}{
"storage-class": "",
"storage-provisioner": "aws-storage",
})
c.Assert(err, jc.ErrorIsNil)
err = p.ValidateConfig(cfg)
c.Assert(err, gc.ErrorMatches, "storage-class must be specified if storage-provisioner is specified")
}

func (s *storageSuite) TestValidateConfigDefaultStorageClass(c *gc.C) {
ctrl := s.setupBroker(c)
defer ctrl.Finish()

cfg, err := provider.NewStorageConfig(map[string]interface{}{"storage-provisioner": "ebs"})
c.Assert(err, jc.ErrorIsNil)
c.Assert(provider.StorageClass(cfg), gc.Equals, "juju-unit-storage")
}

func (s *storageSuite) TestNewStorageConfig(c *gc.C) {
ctrl := s.setupBroker(c)
defer ctrl.Finish()

cfg, err := provider.NewStorageConfig(map[string]interface{}{
"storage-class": "juju-ebs",
"storage-provisioner": "ebs",
"parameters.type": "gp2",
})
c.Assert(err, jc.ErrorIsNil)
c.Assert(provider.StorageClass(cfg), gc.Equals, "juju-ebs")
c.Assert(provider.StorageProvisioner(cfg), gc.Equals, "ebs")
c.Assert(provider.StorageParameters(cfg), jc.DeepEquals, map[string]string{"type": "gp2"})
}

func (s *storageSuite) TestSupports(c *gc.C) {
Expand Down

0 comments on commit 96ce458

Please sign in to comment.