Skip to content

Commit 88879e7

Browse files
axwaxw
committed
Add volume attachments to StartInstanceParams
Add VolumeAttachments to the StartInstanceParams struct, which contains the params for attaching existing volumes to the instance being created. The provisioner is not required to attach the volume; it will still be attached by the storage provisioner as we do today. The new field is added so that volume attachments may influence how instances are created, e.g. by constraining the instance to a specific availability zone.
1 parent f0c89e9 commit 88879e7

File tree

20 files changed

+361
-167
lines changed

20 files changed

+361
-167
lines changed

‎apiserver/params/internal.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -606,16 +606,17 @@ type AgentVersionResult struct {
606606

607607
// ProvisioningInfo holds machine provisioning info.
608608
type ProvisioningInfo struct {
609-
Constraints constraints.Value `json:"constraints"`
610-
Series string `json:"series"`
611-
Placement string `json:"placement"`
612-
Jobs []multiwatcher.MachineJob `json:"jobs"`
613-
Volumes []VolumeParams `json:"volumes,omitempty"`
614-
Tags map[string]string `json:"tags,omitempty"`
615-
SubnetsToZones map[string][]string `json:"subnets-to-zones,omitempty"`
616-
ImageMetadata []CloudImageMetadata `json:"image-metadata,omitempty"`
617-
EndpointBindings map[string]string `json:"endpoint-bindings,omitempty"`
618-
ControllerConfig map[string]interface{} `json:"controller-config,omitempty"`
609+
Constraints constraints.Value `json:"constraints"`
610+
Series string `json:"series"`
611+
Placement string `json:"placement"`
612+
Jobs []multiwatcher.MachineJob `json:"jobs"`
613+
Volumes []VolumeParams `json:"volumes,omitempty"`
614+
VolumeAttachments []VolumeAttachmentParams `json:"volume-attachments,omitempty"`
615+
Tags map[string]string `json:"tags,omitempty"`
616+
SubnetsToZones map[string][]string `json:"subnets-to-zones,omitempty"`
617+
ImageMetadata []CloudImageMetadata `json:"image-metadata,omitempty"`
618+
EndpointBindings map[string]string `json:"endpoint-bindings,omitempty"`
619+
ControllerConfig map[string]interface{} `json:"controller-config,omitempty"`
619620
}
620621

621622
// ProvisioningInfoResult holds machine provisioning info or an error.

‎apiserver/provisioner/provisioner_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ import (
2828
"github.com/juju/juju/state"
2929
statetesting "github.com/juju/juju/state/testing"
3030
"github.com/juju/juju/status"
31+
"github.com/juju/juju/storage"
3132
"github.com/juju/juju/storage/poolmanager"
33+
"github.com/juju/juju/storage/provider"
3234
coretesting "github.com/juju/juju/testing"
3335
)
3436

@@ -978,7 +980,10 @@ func (s *withoutControllerSuite) TestConstraints(c *gc.C) {
978980
}
979981

980982
func (s *withoutControllerSuite) TestSetInstanceInfo(c *gc.C) {
981-
pm := poolmanager.New(state.NewStateSettings(s.State), dummy.StorageProviders())
983+
pm := poolmanager.New(state.NewStateSettings(s.State), storage.ChainedProviderRegistry{
984+
dummy.StorageProviders(),
985+
provider.CommonStorageProviders(),
986+
})
982987
_, err := pm.Create("static-pool", "static", map[string]interface{}{"foo": "bar"})
983988
c.Assert(err, jc.ErrorIsNil)
984989
err = s.State.UpdateModelConfig(map[string]interface{}{

‎apiserver/provisioner/provisioninginfo.go

Lines changed: 78 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ func (p *ProvisionerAPI) ProvisioningInfo(args params.Entities) (params.Provisio
3636
if err != nil {
3737
return result, errors.Trace(err)
3838
}
39+
env, err := environs.GetEnviron(p.configGetter, environs.New)
40+
if err != nil {
41+
return result, errors.Annotate(err, "could not get environ")
42+
}
3943
for i, entity := range args.Entities {
4044
tag, err := names.ParseMachineTag(entity.Tag)
4145
if err != nil {
@@ -44,20 +48,20 @@ func (p *ProvisionerAPI) ProvisioningInfo(args params.Entities) (params.Provisio
4448
}
4549
machine, err := p.getMachine(canAccess, tag)
4650
if err == nil {
47-
result.Results[i].Result, err = p.getProvisioningInfo(machine)
51+
result.Results[i].Result, err = p.getProvisioningInfo(machine, env)
4852
}
4953
result.Results[i].Error = common.ServerError(err)
5054
}
5155
return result, nil
5256
}
5357

54-
func (p *ProvisionerAPI) getProvisioningInfo(m *state.Machine) (*params.ProvisioningInfo, error) {
58+
func (p *ProvisionerAPI) getProvisioningInfo(m *state.Machine, env environs.Environ) (*params.ProvisioningInfo, error) {
5559
cons, err := m.Constraints()
5660
if err != nil {
5761
return nil, errors.Trace(err)
5862
}
5963

60-
volumes, err := p.machineVolumeParams(m)
64+
volumes, volumeAttachments, err := p.machineVolumeParams(m, env)
6165
if err != nil {
6266
return nil, errors.Trace(err)
6367
}
@@ -81,97 +85,117 @@ func (p *ProvisionerAPI) getProvisioningInfo(m *state.Machine) (*params.Provisio
8185
if err != nil {
8286
return nil, errors.Annotate(err, "cannot determine machine endpoint bindings")
8387
}
84-
imageMetadata, err := p.availableImageMetadata(m)
88+
89+
imageMetadata, err := p.availableImageMetadata(m, env)
8590
if err != nil {
8691
return nil, errors.Annotate(err, "cannot get available image metadata")
8792
}
93+
8894
controllerCfg, err := p.st.ControllerConfig()
8995
if err != nil {
9096
return nil, errors.Annotate(err, "cannot get controller configuration")
9197
}
9298

9399
return &params.ProvisioningInfo{
94-
Constraints: cons,
95-
Series: m.Series(),
96-
Placement: m.Placement(),
97-
Jobs: jobs,
98-
Volumes: volumes,
99-
Tags: tags,
100-
SubnetsToZones: subnetsToZones,
101-
EndpointBindings: endpointBindings,
102-
ImageMetadata: imageMetadata,
103-
ControllerConfig: controllerCfg,
100+
Constraints: cons,
101+
Series: m.Series(),
102+
Placement: m.Placement(),
103+
Jobs: jobs,
104+
Volumes: volumes,
105+
VolumeAttachments: volumeAttachments,
106+
Tags: tags,
107+
SubnetsToZones: subnetsToZones,
108+
EndpointBindings: endpointBindings,
109+
ImageMetadata: imageMetadata,
110+
ControllerConfig: controllerCfg,
104111
}, nil
105112
}
106113

107114
// machineVolumeParams retrieves VolumeParams for the volumes that should be
108115
// provisioned with, and attached to, the machine. The client should ignore
109116
// parameters that it does not know how to handle.
110-
func (p *ProvisionerAPI) machineVolumeParams(m *state.Machine) ([]params.VolumeParams, error) {
117+
func (p *ProvisionerAPI) machineVolumeParams(
118+
m *state.Machine,
119+
env environs.Environ,
120+
) ([]params.VolumeParams, []params.VolumeAttachmentParams, error) {
111121
volumeAttachments, err := m.VolumeAttachments()
112122
if err != nil {
113-
return nil, errors.Trace(err)
123+
return nil, nil, errors.Trace(err)
114124
}
115125
if len(volumeAttachments) == 0 {
116-
return nil, nil
126+
return nil, nil, nil
117127
}
118128
modelConfig, err := p.st.ModelConfig()
119129
if err != nil {
120-
return nil, errors.Trace(err)
130+
return nil, nil, errors.Trace(err)
121131
}
122132
controllerCfg, err := p.st.ControllerConfig()
123133
if err != nil {
124-
return nil, errors.Trace(err)
134+
return nil, nil, errors.Trace(err)
125135
}
126136
allVolumeParams := make([]params.VolumeParams, 0, len(volumeAttachments))
137+
var allVolumeAttachmentParams []params.VolumeAttachmentParams
127138
for _, volumeAttachment := range volumeAttachments {
128139
volumeTag := volumeAttachment.Volume()
129140
volume, err := p.st.Volume(volumeTag)
130141
if err != nil {
131-
return nil, errors.Annotatef(err, "getting volume %q", volumeTag.Id())
142+
return nil, nil, errors.Annotatef(err, "getting volume %q", volumeTag.Id())
132143
}
133144
storageInstance, err := storagecommon.MaybeAssignedStorageInstance(
134145
volume.StorageInstance, p.st.StorageInstance,
135146
)
136147
if err != nil {
137-
return nil, errors.Annotatef(err, "getting volume %q storage instance", volumeTag.Id())
148+
return nil, nil, errors.Annotatef(err, "getting volume %q storage instance", volumeTag.Id())
138149
}
139150
volumeParams, err := storagecommon.VolumeParams(
140151
volume, storageInstance, modelConfig.UUID(), controllerCfg.ControllerUUID(),
141152
modelConfig, p.storagePoolManager, p.storageProviderRegistry,
142153
)
143154
if err != nil {
144-
return nil, errors.Annotatef(err, "getting volume %q parameters", volumeTag.Id())
145-
}
146-
provider, err := p.storageProviderRegistry.StorageProvider(storage.ProviderType(volumeParams.Provider))
147-
if err != nil {
148-
return nil, errors.Annotate(err, "getting storage provider")
155+
return nil, nil, errors.Annotatef(err, "getting volume %q parameters", volumeTag.Id())
149156
}
150-
if provider.Dynamic() {
151-
// Leave dynamic storage to the storage provisioner.
157+
if _, err := env.StorageProvider(storage.ProviderType(volumeParams.Provider)); errors.IsNotFound(err) {
158+
// This storage type is not managed by the environ
159+
// provider, so ignore it. It'll be managed by one
160+
// of the storage provisioners.
152161
continue
162+
} else if err != nil {
163+
return nil, nil, errors.Annotate(err, "getting storage provider")
153164
}
154-
volumeAttachmentParams, ok := volumeAttachment.Params()
155-
if !ok {
156-
// Attachment is already provisioned; this is an insane
157-
// state, so we should not proceed with the volume.
158-
return nil, errors.Errorf(
159-
"volume %s already attached to machine %s",
160-
volumeTag.Id(), m.Id(),
161-
)
165+
166+
var volumeProvisioned bool
167+
volumeInfo, err := volume.Info()
168+
if err == nil {
169+
volumeProvisioned = true
170+
} else if !errors.IsNotProvisioned(err) {
171+
return nil, nil, errors.Annotate(err, "getting volume info")
162172
}
163-
// Not provisioned yet, so ask the cloud provisioner do it.
164-
volumeParams.Attachment = &params.VolumeAttachmentParams{
173+
stateVolumeAttachmentParams, volumeDetached := volumeAttachment.Params()
174+
if !volumeDetached {
175+
// Volume is already attached to the machine, so
176+
// there's nothing more to do for it.
177+
continue
178+
}
179+
volumeAttachmentParams := params.VolumeAttachmentParams{
165180
volumeTag.String(),
166181
m.Tag().String(),
167-
"", // we're creating the volume, so it has no volume ID.
182+
volumeInfo.VolumeId,
168183
"", // we're creating the machine, so it has no instance ID.
169184
volumeParams.Provider,
170-
volumeAttachmentParams.ReadOnly,
185+
stateVolumeAttachmentParams.ReadOnly,
186+
}
187+
if volumeProvisioned {
188+
// Volume is already provisioned, so we just need to attach it.
189+
allVolumeAttachmentParams = append(
190+
allVolumeAttachmentParams, volumeAttachmentParams,
191+
)
192+
} else {
193+
// Not provisioned yet, so ask the cloud provisioner do it.
194+
volumeParams.Attachment = &volumeAttachmentParams
195+
allVolumeParams = append(allVolumeParams, volumeParams)
171196
}
172-
allVolumeParams = append(allVolumeParams, volumeParams)
173197
}
174-
return allVolumeParams, nil
198+
return allVolumeParams, allVolumeAttachmentParams, nil
175199
}
176200

177201
// machineTags returns machine-specific tags to set on the instance.
@@ -354,8 +378,8 @@ func (p *ProvisionerAPI) allSpaceNamesToProviderIds() (map[string]string, error)
354378

355379
// availableImageMetadata returns all image metadata available to this machine
356380
// or an error fetching them.
357-
func (p *ProvisionerAPI) availableImageMetadata(m *state.Machine) ([]params.CloudImageMetadata, error) {
358-
imageConstraint, env, err := p.constructImageConstraint(m)
381+
func (p *ProvisionerAPI) availableImageMetadata(m *state.Machine, env environs.Environ) ([]params.CloudImageMetadata, error) {
382+
imageConstraint, err := p.constructImageConstraint(m, env)
359383
if err != nil {
360384
return nil, errors.Annotate(err, "could not construct image constraint")
361385
}
@@ -371,52 +395,34 @@ func (p *ProvisionerAPI) availableImageMetadata(m *state.Machine) ([]params.Clou
371395
}
372396

373397
// constructImageConstraint returns model-specific criteria used to look for image metadata.
374-
func (p *ProvisionerAPI) constructImageConstraint(m *state.Machine) (*imagemetadata.ImageConstraint, environs.Environ, error) {
375-
// If we can determine current region,
376-
// we want only metadata specific to this region.
377-
cloud, env, err := p.obtainEnvCloudConfig()
378-
if err != nil {
379-
return nil, nil, errors.Trace(err)
380-
}
381-
398+
func (p *ProvisionerAPI) constructImageConstraint(m *state.Machine, env environs.Environ) (*imagemetadata.ImageConstraint, error) {
382399
lookup := simplestreams.LookupParams{
383400
Series: []string{m.Series()},
384401
Stream: env.Config().ImageStream(),
385402
}
386403

387404
mcons, err := m.Constraints()
388405
if err != nil {
389-
return nil, nil, errors.Annotatef(err, "cannot get machine constraints for machine %v", m.MachineTag().Id())
406+
return nil, errors.Annotatef(err, "cannot get machine constraints for machine %v", m.MachineTag().Id())
390407
}
391408

392409
if mcons.Arch != nil {
393410
lookup.Arches = []string{*mcons.Arch}
394411
}
395-
if cloud != nil {
396-
lookup.CloudSpec = *cloud
397-
}
398-
399-
return imagemetadata.NewImageConstraint(lookup), env, nil
400-
}
401-
402-
// obtainEnvCloudConfig returns environment specific cloud information
403-
// to be used in search for compatible images and their metadata.
404-
func (p *ProvisionerAPI) obtainEnvCloudConfig() (*simplestreams.CloudSpec, environs.Environ, error) {
405-
env, err := environs.GetEnviron(p.configGetter, environs.New)
406-
if err != nil {
407-
return nil, nil, errors.Annotate(err, "could not get model")
408-
}
409412

410-
if inst, ok := env.(simplestreams.HasRegion); ok {
411-
cloud, err := inst.Region()
413+
if hasRegion, ok := env.(simplestreams.HasRegion); ok {
414+
// We can determine current region; we want only
415+
// metadata specific to this region.
416+
spec, err := hasRegion.Region()
412417
if err != nil {
413418
// can't really find images if we cannot determine cloud region
414419
// TODO (anastasiamac 2015-12-03) or can we?
415-
return nil, nil, errors.Annotate(err, "getting provider region information (cloud spec)")
420+
return nil, errors.Annotate(err, "getting provider region information (cloud spec)")
416421
}
417-
return &cloud, env, nil
422+
lookup.CloudSpec = spec
418423
}
419-
return nil, env, nil
424+
425+
return imagemetadata.NewImageConstraint(lookup), nil
420426
}
421427

422428
// findImageMetadata returns all image metadata or an error fetching them.

0 commit comments

Comments
 (0)