Skip to content

Commit

Permalink
Update storage provider API
Browse files Browse the repository at this point in the history 
This is an overhaul of the storage provider API,
necessary to address several issues. We introduce
a new storage.ProviderRegistry interface, which
can be consulted for available/supported storage
providers. The environs.Environ interace embeds
storage.ProviderRegistry, giving a way to acquire

An Environ is now the only way to acquire an
environ-scoped storage provider, thus there is
no global registry for storage providers except
for the common storage providers (tmpfs, loop,
etc.)  Similarly, it no longer makes sense to
globally register storage pools. Instead, we
extend the storage.Provider interface with a
method to acquire the default pool definitions.

We were not registering default storage pools
for hosted models. Now when creating a model in
state, we consult all storage providers supported
by the model and store the default pools then.

A minor bug in storage filtering has been fixed,
where we were passing in the model *name* rather
than *type* when determining whether a storage
provider was supported. This is no longer necessary,
since an Environ now only reports storage providers
that it supports.

Fixes https://bugs.launchpad.net/juju-core/+bug/1608818
Fixes https://bugs.launchpad.net/juju-core/+bug/1608821
  • Loading branch information
@axw
axw committed Aug 7, 2016
1 parent 6d3df82 commit dde838f
Show file tree
Hide file tree
Showing 137 changed files with 1,466 additions and 1,435 deletions.
31 changes: 19 additions & 12 deletions agent/agentbootstrap/bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/juju/juju/network"
"github.com/juju/juju/state"
"github.com/juju/juju/state/multiwatcher"
"github.com/juju/juju/storage"
)

var logger = loggo.GetLogger("juju.agent.agentbootstrap")
Expand All @@ -43,6 +44,10 @@ type InitializeStateParams struct {

// Provider is called to obtain an EnvironProvider.
Provider func(string) (environs.EnvironProvider, error)

// StorageProviderRegistry is used to determine and store the
// details of the default storage pools.
StorageProviderRegistry storage.ProviderRegistry
}

// InitializeState should be called on the bootstrap machine's agent
Expand Down Expand Up @@ -93,12 +98,13 @@ func InitializeState(
logger.Debugf("initializing address %v", info.Addrs)
st, err := state.Initialize(state.InitializeParams{
ControllerModelArgs: state.ModelArgs{
Owner: adminUser,
Config: args.ControllerModelConfig,
Constraints: args.ModelConstraints,
CloudName: args.ControllerCloudName,
CloudRegion: args.ControllerCloudRegion,
CloudCredential: args.ControllerCloudCredentialName,
Owner: adminUser,
Config: args.ControllerModelConfig,
Constraints: args.ModelConstraints,
CloudName: args.ControllerCloudName,
CloudRegion: args.ControllerCloudRegion,
CloudCredential: args.ControllerCloudCredentialName,
StorageProviderRegistry: args.StorageProviderRegistry,
},
CloudName: args.ControllerCloudName,
Cloud: args.ControllerCloud,
Expand Down Expand Up @@ -189,12 +195,13 @@ func InitializeState(
}

_, hostedModelState, err := st.NewModel(state.ModelArgs{
Owner: adminUser,
Config: hostedModelConfig,
Constraints: args.ModelConstraints,
CloudName: args.ControllerCloudName,
CloudRegion: args.ControllerCloudRegion,
CloudCredential: args.ControllerCloudCredentialName,
Owner: adminUser,
Config: hostedModelConfig,
Constraints: args.ModelConstraints,
CloudName: args.ControllerCloudName,
CloudRegion: args.ControllerCloudRegion,
CloudCredential: args.ControllerCloudCredentialName,
StorageProviderRegistry: args.StorageProviderRegistry,
})
if err != nil {
return nil, nil, errors.Annotate(err, "creating hosted model")
Expand Down
13 changes: 8 additions & 5 deletions agent/agentbootstrap/bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/juju/juju/provider/dummy"
"github.com/juju/juju/state"
"github.com/juju/juju/state/multiwatcher"
"github.com/juju/juju/storage/provider"
"github.com/juju/juju/testing"
jujuversion "github.com/juju/juju/version"
)
Expand Down Expand Up @@ -149,7 +150,7 @@ LXC_BRIDGE="ignored"`[1:])
"apt-mirror": "http://mirror",
}

var provider fakeProvider
var envProvider fakeProvider
args := agentbootstrap.InitializeStateParams{
StateInitializationParams: instancecfg.StateInitializationParams{
BootstrapMachineConstraints: expectBootstrapConstraints,
Expand All @@ -173,8 +174,9 @@ LXC_BRIDGE="ignored"`[1:])
SharedSecret: "abc123",
Provider: func(t string) (environs.EnvironProvider, error) {
c.Assert(t, gc.Equals, "dummy")
return &provider, nil
return &envProvider, nil
},
StorageProviderRegistry: provider.CommonStorageProviders(),
}

adminUser := names.NewLocalUserTag("agent-admin")
Expand Down Expand Up @@ -292,22 +294,22 @@ LXC_BRIDGE="ignored"`[1:])
defer st1.Close()

// Make sure that the hosted model Environ's Create method is called.
provider.CheckCallNames(c,
envProvider.CheckCallNames(c,
"RestrictedConfigAttributes",
"PrepareConfig",
"Validate",
"Open",
"Create",
)
provider.CheckCall(c, 3, "Open", environs.OpenParams{
envProvider.CheckCall(c, 3, "Open", environs.OpenParams{
Cloud: environs.CloudSpec{
Type: "dummy",
Name: "dummy",
Region: "some-region",
},
Config: hostedCfg,
})
provider.CheckCall(c, 4, "Create", environs.CreateParams{
envProvider.CheckCall(c, 4, "Create", environs.CreateParams{
ControllerUUID: controllerCfg.ControllerUUID(),
})
}
Expand Down Expand Up @@ -386,6 +388,7 @@ func (s *bootstrapSuite) TestInitializeStateFailsSecondTime(c *gc.C) {
Provider: func(t string) (environs.EnvironProvider, error) {
return &fakeProvider{}, nil
},
StorageProviderRegistry: provider.CommonStorageProviders(),
}

adminUser := names.NewLocalUserTag("agent-admin")
Expand Down
4 changes: 2 additions & 2 deletions api/facadeversions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ var facadeVersions = map[string]int{
"Spaces": 2,
"SSHClient": 1,
"StatusHistory": 2,
"Storage": 2,
"StorageProvisioner": 2,
"Storage": 3,
"StorageProvisioner": 3,
"StringsWatcher": 1,
"Subnets": 2,
"Undertaker": 1,
Expand Down
2 changes: 1 addition & 1 deletion api/provisioner/provisioner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (s *provisionerSuite) TestRefreshAndLife(c *gc.C) {
}

func (s *provisionerSuite) TestSetInstanceInfo(c *gc.C) {
pm := poolmanager.New(state.NewStateSettings(s.State))
pm := poolmanager.New(state.NewStateSettings(s.State), provider.CommonStorageProviders())
_, err := pm.Create("loop-pool", provider.LoopProviderType, map[string]interface{}{"foo": "bar"})
c.Assert(err, jc.ErrorIsNil)

Expand Down
14 changes: 1 addition & 13 deletions api/watcher/watcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import (
"github.com/juju/juju/core/migration"
"github.com/juju/juju/juju/testing"
"github.com/juju/juju/state"
"github.com/juju/juju/storage"
"github.com/juju/juju/storage/provider/dummy"
"github.com/juju/juju/storage/provider/registry"
coretesting "github.com/juju/juju/testing"
"github.com/juju/juju/testing/factory"
corewatcher "github.com/juju/juju/watcher"
Expand Down Expand Up @@ -182,20 +179,11 @@ func (s *watcherSuite) TestStringsWatcherStopsWithPendingSend(c *gc.C) {

// TODO(fwereade): 2015-11-18 lp:1517391
func (s *watcherSuite) TestWatchMachineStorage(c *gc.C) {
registry.RegisterProvider(
"envscoped",
&dummy.StorageProvider{
StorageScope: storage.ScopeEnviron,
},
)
registry.RegisterEnvironStorageProviders("dummy", "envscoped")
defer registry.RegisterProvider("envscoped", nil)

f := factory.NewFactory(s.BackingState)
f.MakeMachine(c, &factory.MachineParams{
Volumes: []state.MachineVolumeParams{{
Volume: state.VolumeParams{
Pool: "envscoped",
Pool: "environscoped",
Size: 1024,
},
}},
Expand Down
56 changes: 2 additions & 54 deletions apiserver/application/application_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ import (
statestorage "github.com/juju/juju/state/storage"
"github.com/juju/juju/status"
"github.com/juju/juju/storage"
"github.com/juju/juju/storage/poolmanager"
"github.com/juju/juju/storage/provider"
"github.com/juju/juju/storage/provider/registry"
"github.com/juju/juju/testcharms"
"github.com/juju/juju/testing/factory"
jujuversion "github.com/juju/juju/version"
Expand Down Expand Up @@ -188,18 +185,7 @@ func (s *serviceSuite) TestCompatibleSettingsParsing(c *gc.C) {
c.Assert(err, gc.ErrorMatches, `unknown option "yummy"`)
}

func setupStoragePool(c *gc.C, st *state.State) {
pm := poolmanager.New(state.NewStateSettings(st))
_, err := pm.Create("loop-pool", provider.LoopProviderType, map[string]interface{}{})
c.Assert(err, jc.ErrorIsNil)
err = st.UpdateModelConfig(map[string]interface{}{
"storage-default-block-source": "loop-pool",
}, nil, nil)
c.Assert(err, jc.ErrorIsNil)
}

func (s *serviceSuite) TestServiceDeployWithStorage(c *gc.C) {
setupStoragePool(c, s.State)
curl, ch := s.UploadCharm(c, "utopic/storage-block-10", "storage-block")
err := application.AddCharmWithAuthorization(s.State, params.AddCharmWithAuthorization{
URL: curl.String(),
Expand All @@ -209,7 +195,7 @@ func (s *serviceSuite) TestServiceDeployWithStorage(c *gc.C) {
"data": {
Count: 1,
Size: 1024,
Pool: "loop-pool",
Pool: "environscoped-block",
},
}

Expand All @@ -235,7 +221,7 @@ func (s *serviceSuite) TestServiceDeployWithStorage(c *gc.C) {
"data": {
Count: 1,
Size: 1024,
Pool: "loop-pool",
Pool: "environscoped-block",
},
"allecto": {
Count: 0,
Expand All @@ -255,7 +241,6 @@ func (s *serviceSuite) TestMinJujuVersionTooHigh(c *gc.C) {
}

func (s *serviceSuite) TestServiceDeployWithInvalidStoragePool(c *gc.C) {
setupStoragePool(c, s.State)
curl, _ := s.UploadCharm(c, "utopic/storage-block-0", "storage-block")
err := application.AddCharmWithAuthorization(s.State, params.AddCharmWithAuthorization{
URL: curl.String(),
Expand Down Expand Up @@ -285,44 +270,7 @@ func (s *serviceSuite) TestServiceDeployWithInvalidStoragePool(c *gc.C) {
c.Assert(results.Results[0].Error, gc.ErrorMatches, `.* pool "foo" not found`)
}

func (s *serviceSuite) TestServiceDeployWithUnsupportedStoragePool(c *gc.C) {
registry.RegisterProvider("hostloop", &mockStorageProvider{kind: storage.StorageKindBlock})
pm := poolmanager.New(state.NewStateSettings(s.State))
_, err := pm.Create("host-loop-pool", provider.HostLoopProviderType, map[string]interface{}{})
c.Assert(err, jc.ErrorIsNil)

curl, _ := s.UploadCharm(c, "utopic/storage-block-0", "storage-block")
err = application.AddCharmWithAuthorization(s.State, params.AddCharmWithAuthorization{
URL: curl.String(),
})
c.Assert(err, jc.ErrorIsNil)
storageConstraints := map[string]storage.Constraints{
"data": storage.Constraints{
Pool: "host-loop-pool",
Count: 1,
Size: 1024,
},
}

var cons constraints.Value
args := params.ApplicationDeploy{
ApplicationName: "application",
CharmUrl: curl.String(),
NumUnits: 1,
Constraints: cons,
Storage: storageConstraints,
}
results, err := s.applicationApi.Deploy(params.ApplicationsDeploy{
Applications: []params.ApplicationDeploy{args}},
)
c.Assert(err, jc.ErrorIsNil)
c.Assert(results.Results, gc.HasLen, 1)
c.Assert(results.Results[0].Error, gc.ErrorMatches,
`.*pool "host-loop-pool" uses storage provider "hostloop" which is not supported for models of type "dummy"`)
}

func (s *serviceSuite) TestServiceDeployDefaultFilesystemStorage(c *gc.C) {
setupStoragePool(c, s.State)
curl, ch := s.UploadCharm(c, "trusty/storage-filesystem-1", "storage-filesystem")
err := application.AddCharmWithAuthorization(s.State, params.AddCharmWithAuthorization{
URL: curl.String(),
Expand Down
12 changes: 0 additions & 12 deletions apiserver/client/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import (
"github.com/juju/juju/state"
"github.com/juju/juju/state/multiwatcher"
"github.com/juju/juju/status"
"github.com/juju/juju/storage/poolmanager"
"github.com/juju/juju/storage/provider"
coretesting "github.com/juju/juju/testing"
"github.com/juju/juju/testing/factory"
"github.com/juju/juju/worker"
Expand Down Expand Up @@ -476,16 +474,6 @@ func (s *baseSuite) setUpScenario(c *gc.C) (entities []names.Tag) {
return
}

func (s *baseSuite) setupStoragePool(c *gc.C) {
pm := poolmanager.New(state.NewStateSettings(s.State))
_, err := pm.Create("loop-pool", provider.LoopProviderType, map[string]interface{}{})
c.Assert(err, jc.ErrorIsNil)
err = s.State.UpdateModelConfig(map[string]interface{}{
"storage-default-block-source": "loop-pool",
}, nil, nil)
c.Assert(err, jc.ErrorIsNil)
}

func (s *baseSuite) setAgentPresence(c *gc.C, u *state.Unit) {
pinger, err := u.SetAgentPresence()
c.Assert(err, jc.ErrorIsNil)
Expand Down
4 changes: 1 addition & 3 deletions apiserver/common/modelmanagerinterface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"gopkg.in/juju/names.v2"

"github.com/juju/juju/apiserver/metricsender"
"github.com/juju/juju/cloud"
"github.com/juju/juju/controller"
"github.com/juju/juju/core/description"
"github.com/juju/juju/environs/config"
Expand All @@ -24,9 +23,8 @@ type ModelManagerBackend interface {
ToolsStorageGetter
BlockGetter
metricsender.MetricsSenderBackend
state.CloudAccessor

Cloud(name string) (cloud.Cloud, error)
CloudCredentials(user names.UserTag, cloudName string) (map[string]cloud.Credential, error)
ModelUUID() string
ModelsForUser(names.UserTag) ([]*state.UserModel, error)
IsControllerAdministrator(user names.UserTag) (bool, error)
Expand Down
4 changes: 3 additions & 1 deletion apiserver/common/storagecommon/filesystems.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/juju/juju/apiserver/params"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/state"
"github.com/juju/juju/storage"
"github.com/juju/juju/storage/poolmanager"
)

Expand All @@ -21,6 +22,7 @@ func FilesystemParams(
modelUUID, controllerUUID string,
environConfig *config.Config,
poolManager poolmanager.PoolManager,
registry storage.ProviderRegistry,
) (params.FilesystemParams, error) {

var pool string
Expand All @@ -42,7 +44,7 @@ func FilesystemParams(
return params.FilesystemParams{}, errors.Annotate(err, "computing storage tags")
}

providerType, cfg, err := StoragePoolConfig(pool, poolManager)
providerType, cfg, err := StoragePoolConfig(pool, poolManager, registry)
if err != nil {
return params.FilesystemParams{}, errors.Trace(err)
}
Expand Down
6 changes: 3 additions & 3 deletions apiserver/common/storagecommon/volumes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/juju/juju/state"
"github.com/juju/juju/storage"
"github.com/juju/juju/storage/poolmanager"
"github.com/juju/juju/storage/provider/registry"
)

type volumeAlreadyProvisionedError struct {
Expand All @@ -34,6 +33,7 @@ func VolumeParams(
modelUUID, controllerUUID string,
environConfig *config.Config,
poolManager poolmanager.PoolManager,
registry storage.ProviderRegistry,
) (params.VolumeParams, error) {

var pool string
Expand All @@ -55,7 +55,7 @@ func VolumeParams(
return params.VolumeParams{}, errors.Annotate(err, "computing storage tags")
}

providerType, cfg, err := StoragePoolConfig(pool, poolManager)
providerType, cfg, err := StoragePoolConfig(pool, poolManager, registry)
if err != nil {
return params.VolumeParams{}, errors.Trace(err)
}
Expand All @@ -74,7 +74,7 @@ func VolumeParams(
// such pool with the specified name, but it identifies a
// storage provider, then that type will be returned with a
// nil configuration.
func StoragePoolConfig(name string, poolManager poolmanager.PoolManager) (storage.ProviderType, *storage.Config, error) {
func StoragePoolConfig(name string, poolManager poolmanager.PoolManager, registry storage.ProviderRegistry) (storage.ProviderType, *storage.Config, error) {
pool, err := poolManager.Get(name)
if errors.IsNotFound(err) {
// If not a storage pool, then maybe a provider type.
Expand Down
Loading

0 comments on commit dde838f

Please sign in to comment.