Skip to content

Commit b4777c2

Browse files
axwaxw
committed
storage: remove persistent as pool configuration
It no longer makes sense to have "persistent" as a pool configuration attribute. Instead, providers create persistent storage if they support it, or otherwise don't (there *could* be provider-specific config for this, but so far it has been unnecessary). Later we'll have the ability to manage the lifecycle binding of storage, which will cause persistent storage to live beyond attachments.
1 parent 2c18df5 commit b4777c2

File tree

8 files changed

+45
-81
lines changed

8 files changed

+45
-81
lines changed

‎apiserver/storage/storage.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -234,17 +234,10 @@ func (api *API) isPersistent(si state.StorageInstance) (bool, error) {
234234
if err != nil {
235235
return false, err
236236
}
237-
// If the volume is not provisioned, we read its config attributes.
238-
if params, ok := volume.Params(); ok {
239-
_, cfg, err := common.StoragePoolConfig(params.Pool, api.poolManager)
240-
if err != nil {
241-
return false, err
242-
}
243-
return cfg.IsPersistent(), nil
244-
}
245-
// If the volume is provisioned, we look at its provisioning info.
246237
info, err := volume.Info()
247-
if err != nil {
238+
if errors.IsNotProvisioned(err) {
239+
return false, nil
240+
} else if err != nil {
248241
return false, err
249242
}
250243
return info.Persistent, nil

‎featuretests/storage_test.go

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,14 @@ import (
2424
)
2525

2626
const (
27-
testPool = "block"
28-
testPersistentPool = "block-persistent"
27+
testPool = "block"
2928
)
3029

3130
func setupTestStorageSupport(c *gc.C, s *state.State) {
3231
stsetts := state.NewStateSettings(s)
3332
poolManager := poolmanager.New(stsetts)
3433
_, err := poolManager.Create(testPool, provider.LoopProviderType, map[string]interface{}{"it": "works"})
3534
c.Assert(err, jc.ErrorIsNil)
36-
_, err = poolManager.Create(testPersistentPool, ec2.EBS_ProviderType, map[string]interface{}{"persistent": true})
37-
c.Assert(err, jc.ErrorIsNil)
3835

3936
registry.RegisterEnvironStorageProviders("dummy", ec2.EBS_ProviderType)
4037
registry.RegisterEnvironStorageProviders("dummyenv", ec2.EBS_ProviderType)
@@ -144,13 +141,13 @@ storage-block/0 data/0 pending false
144141
}
145142

146143
func (s *cmdStorageSuite) TestStorageListPersistent(c *gc.C) {
147-
createUnitWithStorage(c, &s.JujuConnSuite, testPersistentPool)
144+
createUnitWithStorage(c, &s.JujuConnSuite, testPool)
148145

149146
context := runList(c)
150147
expected := `
151148
[Storage]
152149
UNIT ID LOCATION STATUS PERSISTENT
153-
storage-block/0 data/0 pending true
150+
storage-block/0 data/0 pending false
154151
155152
`[1:]
156153
c.Assert(testing.Stdout(context), gc.Equals, expected)
@@ -180,7 +177,7 @@ storage-block/0:
180177
}
181178

182179
func (s *cmdStorageSuite) TestStoragePersistentUnprovisioned(c *gc.C) {
183-
createUnitWithStorage(c, &s.JujuConnSuite, testPersistentPool)
180+
createUnitWithStorage(c, &s.JujuConnSuite, testPool)
184181

185182
context := runShow(c, "data/0")
186183
expected := `
@@ -189,7 +186,7 @@ storage-block/0:
189186
storage: data
190187
kind: block
191188
status: pending
192-
persistent: true
189+
persistent: false
193190
`[1:]
194191
c.Assert(testing.Stdout(context), gc.Equals, expected)
195192
}
@@ -207,10 +204,6 @@ block:
207204
provider: loop
208205
attrs:
209206
it: works
210-
block-persistent:
211-
provider: ebs
212-
attrs:
213-
persistent: true
214207
ebs:
215208
provider: ebs
216209
loop:
@@ -226,13 +219,12 @@ tmpfs:
226219
func (s *cmdStorageSuite) TestListPoolsTabular(c *gc.C) {
227220
context := runPoolList(c, "--format", "tabular")
228221
expected := `
229-
NAME PROVIDER ATTRS
230-
block loop it=works
231-
block-persistent ebs persistent=true
232-
ebs ebs
233-
loop loop
234-
rootfs rootfs
235-
tmpfs tmpfs
222+
NAME PROVIDER ATTRS
223+
block loop it=works
224+
ebs ebs
225+
loop loop
226+
rootfs rootfs
227+
tmpfs tmpfs
236228
237229
`[1:]
238230
c.Assert(testing.Stdout(context), gc.Equals, expected)
@@ -260,14 +252,14 @@ func (s *cmdStorageSuite) TestListPoolsNameInvalid(c *gc.C) {
260252
}
261253

262254
func (s *cmdStorageSuite) TestListPoolsProvider(c *gc.C) {
263-
context := runPoolList(c, "--provider", "ebs")
255+
context := runPoolList(c, "--provider", "loop")
264256
expected := `
265-
block-persistent:
266-
provider: ebs
257+
block:
258+
provider: loop
267259
attrs:
268-
persistent: true
269-
ebs:
270-
provider: ebs
260+
it: works
261+
loop:
262+
provider: loop
271263
`[1:]
272264
c.Assert(testing.Stdout(context), gc.Equals, expected)
273265
}
@@ -401,11 +393,11 @@ func (s *cmdStorageSuite) TestListVolumeInvalidMachine(c *gc.C) {
401393
}
402394

403395
func (s *cmdStorageSuite) TestListVolumeTabularFilterMatch(c *gc.C) {
404-
createUnitWithStorage(c, &s.JujuConnSuite, testPersistentPool)
396+
createUnitWithStorage(c, &s.JujuConnSuite, testPool)
405397
context := runVolumeList(c, "0")
406398
expected := `
407399
MACHINE UNIT STORAGE DEVICE VOLUME ID SIZE STATE MESSAGE
408-
0 storage-block/0 data/0 0 pending
400+
0 storage-block/0 data/0 0/0 pending
409401
410402
`[1:]
411403
c.Assert(testing.Stdout(context), gc.Equals, expected)

‎provider/dummy/environs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ func (e *environ) StartInstance(args environs.StartInstanceParams) (*environs.St
974974
// Simulate creating volumes when requested.
975975
volumes := make([]storage.Volume, len(args.Volumes))
976976
for iv, v := range args.Volumes {
977-
persistent, _ := v.Attributes[storage.Persistent].(bool)
977+
persistent, _ := v.Attributes["persistent"].(bool)
978978
volumes[iv] = storage.Volume{
979979
Tag: names.NewVolumeTag(strconv.Itoa(iv + 1)),
980980
VolumeInfo: storage.VolumeInfo{

‎provider/ec2/ebs.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ type ebsProvider struct{}
112112
var _ storage.Provider = (*ebsProvider)(nil)
113113

114114
var ebsConfigFields = schema.Fields{
115-
storage.Persistent: schema.Bool(),
116115
EBS_VolumeType: schema.OneOf(
117116
schema.Const(volumeTypeMagnetic),
118117
schema.Const(volumeTypeSsd),
@@ -128,15 +127,13 @@ var ebsConfigFields = schema.Fields{
128127
var ebsConfigChecker = schema.FieldMap(
129128
ebsConfigFields,
130129
schema.Defaults{
131-
storage.Persistent: false,
132-
EBS_VolumeType: volumeTypeMagnetic,
133-
EBS_IOPS: schema.Omit,
134-
EBS_Encrypted: false,
130+
EBS_VolumeType: volumeTypeMagnetic,
131+
EBS_IOPS: schema.Omit,
132+
EBS_Encrypted: false,
135133
},
136134
)
137135

138136
type ebsConfig struct {
139-
persistent bool
140137
volumeType string
141138
iops int
142139
encrypted bool
@@ -151,7 +148,6 @@ func newEbsConfig(attrs map[string]interface{}) (*ebsConfig, error) {
151148
iops, _ := coerced[EBS_IOPS].(int)
152149
volumeType := coerced[EBS_VolumeType].(string)
153150
ebsConfig := &ebsConfig{
154-
persistent: coerced[storage.Persistent].(bool),
155151
volumeType: volumeType,
156152
iops: iops,
157153
encrypted: coerced[EBS_Encrypted].(bool),

‎storage/config.go

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,43 +19,32 @@ const (
1919
// should not be relied upon until a storage source is
2020
// constructed.
2121
ConfigStorageDir = "storage-dir"
22-
23-
// Persistent is true if storage survives the lifecycle of the
24-
// machine to which it is attached.
25-
Persistent = "persistent"
2622
)
2723

2824
// Config defines the configuration for a storage source.
2925
type Config struct {
30-
name string
31-
provider ProviderType
32-
attrs map[string]interface{}
33-
persistent bool
26+
name string
27+
provider ProviderType
28+
attrs map[string]interface{}
3429
}
3530

36-
var fields = schema.Fields{
37-
Persistent: schema.Bool(),
38-
}
31+
var fields = schema.Fields{}
3932

4033
var configChecker = schema.FieldMap(
4134
fields,
42-
schema.Defaults{
43-
Persistent: false,
44-
},
35+
schema.Defaults{},
4536
)
4637

4738
// NewConfig creates a new Config for instantiating a storage source.
4839
func NewConfig(name string, provider ProviderType, attrs map[string]interface{}) (*Config, error) {
49-
out, err := configChecker.Coerce(attrs, nil)
40+
_, err := configChecker.Coerce(attrs, nil)
5041
if err != nil {
5142
return nil, errors.Annotate(err, "validating common storage config")
5243
}
53-
coerced := out.(map[string]interface{})
5444
return &Config{
55-
name: name,
56-
provider: provider,
57-
attrs: attrs,
58-
persistent: coerced[Persistent].(bool),
45+
name: name,
46+
provider: provider,
47+
attrs: attrs,
5948
}, nil
6049
}
6150

@@ -87,8 +76,3 @@ func (c *Config) ValueString(name string) (string, bool) {
8776
v, ok := c.attrs[name].(string)
8877
return v, ok
8978
}
90-
91-
// IsPersistent returns true if config has persistent set to true.
92-
func (c *Config) IsPersistent() bool {
93-
return c.persistent
94-
}

‎storage/interface.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,6 @@ type VolumeParams struct {
177177
Attachment *VolumeAttachmentParams
178178
}
179179

180-
// IsPersistent returns true if the params has persistent set to true.
181-
func (p *VolumeParams) IsPersistent() bool {
182-
v, _ := p.Attributes[Persistent].(bool)
183-
return v
184-
}
185-
186180
// VolumeAttachmentParams is a set of parameters for volume attachment or
187181
// detachment.
188182
type VolumeAttachmentParams struct {

‎storage/poolmanager/poolmanager_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
statetesting "github.com/juju/juju/state/testing"
1313
"github.com/juju/juju/storage"
1414
"github.com/juju/juju/storage/poolmanager"
15+
"github.com/juju/juju/storage/provider/dummy"
16+
"github.com/juju/juju/storage/provider/registry"
1517
)
1618

1719
type poolSuite struct {
@@ -114,8 +116,14 @@ func (s *poolSuite) TestCreateMissingType(c *gc.C) {
114116
}
115117

116118
func (s *poolSuite) TestCreateInvalidConfig(c *gc.C) {
117-
_, err := s.poolManager.Create("testpool", storage.ProviderType("loop"), map[string]interface{}{"persistent": true})
118-
c.Assert(err, gc.ErrorMatches, `validating storage provider config: machine scoped storage provider "testpool" does not support persistent storage`)
119+
registry.RegisterProvider("invalid", &dummy.StorageProvider{
120+
ValidateConfigFunc: func(*storage.Config) error {
121+
return errors.New("no good")
122+
},
123+
})
124+
defer registry.RegisterProvider("invalid", nil)
125+
_, err := s.poolManager.Create("testpool", "invalid", nil)
126+
c.Assert(err, gc.ErrorMatches, "validating storage provider config: no good")
119127
}
120128

121129
func (s *poolSuite) TestDelete(c *gc.C) {

‎storage/provider/common.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,5 @@ func CommonProviders() map[storage.ProviderType]storage.Provider {
2323
// ValidateConfig performs storage provider config validation, including
2424
// any common validation.
2525
func ValidateConfig(p storage.Provider, cfg *storage.Config) error {
26-
if p.Scope() == storage.ScopeMachine && cfg.IsPersistent() {
27-
return errors.Errorf("machine scoped storage provider %q does not support persistent storage", cfg.Name())
28-
}
2926
return p.ValidateConfig(cfg)
3027
}

0 commit comments

Comments
 (0)