Merge lp:~axwalk/juju-core/addmachine-placement into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Introduce instance.Placement

This change introduces a new Placement
structure in the instance package that
encapsulates machine-id, container, and
(new) environment-specific placement
directives.

The add-machine subcommand is modified
to pass this new Placement structure
through the AddMachine client API, which
calls a new state method AddMachineWithPlacement.

Machine-id and container placement do the
same thing as before, while new environment-
specific placement structures are stored in
state. A new state policy is added to allow
an Environ to validate placement.

The provisioner API and worker are updated
to pass Placement through to StartInstance.

https://codereview.appspot.com/85040046/

Introduce instance.Placement

This change introduces a new Placement
structure in the instance package that
encapsulates machine-id, container, and
(new) environment-specific placement
directives.

The add-machine subcommand is modified
to pass this new Placement structure
through the AddMachine client API, which
calls a new state method AddMachineWithPlacement.

Machine-id and container placement do the
same thing as before, while new environment-
specific placement structures are stored in
state. A new state policy is added to allow
an Environ to validate placement.

The provisioner API and worker are updated
to pass Placement through to StartInstance.

Follow-ups to come:
 - update MAAS provider to pick maas-name out
   the Placement.
 - update ec2, OpenStack (and MAAS?) to pick
   out availbility-zone from Placement.
 - update add-unit and deploy subcommands to
   use Placement; this will involve modifications
   to state unit assignment to choose existing
   matching machines (e.g. for maas-name).

https://codereview.appspot.com/85040046/

[Andrew Wilkins (axwalk)]

Reviewers: mp+214761_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce instance.Placement

This change introduces a new Placement
structure in the instance package that
encapsulates machine-id, container, and
(new) environment-specific placement
directives.

The add-machine subcommand is modified
to pass this new Placement structure
through the AddMachine client API, which
calls a new state method AddMachineWithPlacement.

Machine-id and container placement do the
same thing as before, while new environment-
specific placement structures are stored in
state. A new state policy is added to allow
an Environ to validate placement.

The provisioner API and worker are updated
to pass Placement through to StartInstance.

Follow-ups to come:
  - update MAAS provider to pick maas-name out
    the Placement.
  - update ec2, OpenStack (and MAAS?) to pick
    out availbility-zone from Placement.
  - update add-unit and deploy subcommands to
    use Placement; this will involve modifications
    to state unit assignment to choose existing
    matching machines (e.g. for maas-name).

https://code.launchpad.net/~axwalk/juju-core/addmachine-placement/+merge/214761

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/85040046/

Affected files (+622, -63 lines):
   A [revision details]
   M cmd/juju/addmachine.go
   M cmd/juju/addmachine_test.go
   M environs/broker.go
   M environs/interface.go
   M environs/statepolicy.go
   A instance/placement.go
   A instance/placement_test.go
   M provider/azure/environ.go
   M provider/dummy/environs.go
   M provider/ec2/ec2.go
   M provider/joyent/environ.go
   M provider/local/environ.go
   M provider/maas/environ.go
   M provider/manual/environ.go
   M provider/openstack/provider.go
   M state/addmachine.go
   M state/api/params/params.go
   M state/api/provisioner/machine.go
   M state/api/provisioner/provisioner_test.go
   M state/apiserver/client/client.go
   M state/apiserver/client/client_test.go
   M state/apiserver/provisioner/provisioner.go
   M state/apiserver/provisioner/provisioner_test.go
   M state/conn_test.go
   M state/machine.go
   A state/placementvalidator_test.go
   M state/policy.go
   M worker/provisioner/provisioner_task.go

[Ian Booth (wallyworld)]

Some initial thoughts added. I assume William is ok with the design?
Seems ok to me. Maybe we can ultimately remove container constraints? My
main concern is about backwards compatibility of clients invoking add
machine. Do we support 1.18 add-machine commands?

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode127
cmd/juju/addmachine.go:127: results, err :=
client.AddMachines([]params.AddMachineParams{machineParams})
What about backwards compatibility?

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode137
cmd/juju/addmachine.go:137: // TODO(axw) figure out how to convey
whether it's a container?
Can't we inspect the scope to see if it's a valid container type?

https://codereview.appspot.com/85040046/diff/1/instance/placement_test.go
File instance/placement_test.go (right):

https://codereview.appspot.com/85040046/diff/1/instance/placement_test.go#newcode25
instance/placement_test.go:25: expect: &instance.Placement{Scope:
instance.MachineScope, Value: "0"},
Perhaps to reduce the verbosity, the test struct can have expectScope
and expectDirective and the test would use those individual values to
construct a Placement to compare with?

https://codereview.appspot.com/85040046/diff/1/provider/dummy/environs.go
File provider/dummy/environs.go (right):

https://codereview.appspot.com/85040046/diff/1/provider/dummy/environs.go#newcode546
provider/dummy/environs.go:546: func (*environ) ValidatePlacement(p
*instance.Placement) error {
I know we check for nil elsewhere, but maybe we should have it here as
well to be defensive and avoid a npe?

https://codereview.appspot.com/85040046/diff/1/state/addmachine.go
File state/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/1/state/addmachine.go#newcode85
state/addmachine.go:85: func (st *State)
AddMachineWithPlacement(template MachineTemplate, placement
*instance.Placement) (*Machine, error) {
We need a test for this method? I can see it's tested elsewhere outside
this package so maybe that's sufficient, but in general it's nice to
have at least one local test to cover all the corner cases, and the
external tests can be a little more general to test how things are wired
up.

https://codereview.appspot.com/85040046/diff/1/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/85040046/diff/1/state/api/params/params.go#newcode107
state/api/params/params.go:107: ContainerType instance.ContainerType
So we are retaining ContainerType and ParentId but add-machine does not
set them anymore?

https://codereview.appspot.com/85040046/diff/1/state/api/provisioner/provisioner_test.go
File state/api/provisioner/provisioner_test.go (right):

https://codereview.appspot.com/85040046/diff/1/state/api/provisioner/provisioner_test.go#newcode330
state/api/provisioner/provisioner_test.go:330: c.Assert(err,
gc.ErrorMatches, "machine 1 not found")
We have started returning perm denied if a machine is not found.
We should have tests for genuine perm denied cases as well?

https://codereview.appspot.com/85...

[Andrew Wilkins (axwalk)]

> I assume William is ok with the design?

I ran the basic idea by William, but not all the details. I asked for a
review last night, but I think he was busy reviewing other things.

> Maybe we can ultimately remove container constraints? My main concern
is about backwards compatibility of clients invoking add machine. Do we
support 1.18 add-machine commands?

Yeah I think we need to keep it for backwards compatibility. The client
code should continue to pass ContainerType and ParentId to the API
server; when we get to 1.22, then we can use Placement exclusively.

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode127
cmd/juju/addmachine.go:127: results, err :=
client.AddMachines([]params.AddMachineParams{machineParams})
On 2014/04/09 04:52:18, wallyworld wrote:
> What about backwards compatibility?

Yeah, I suppose we will need to continue setting ContainerType and
ParentId for backwards compatibility.

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode137
cmd/juju/addmachine.go:137: // TODO(axw) figure out how to convey
whether it's a container?
On 2014/04/09 04:52:18, wallyworld wrote:
> Can't we inspect the scope to see if it's a valid container type?

Yes indeed. I had previously implemented things differently, but with
the current implementation it's trivial to do so.

https://codereview.appspot.com/85040046/diff/1/instance/placement_test.go
File instance/placement_test.go (right):

https://codereview.appspot.com/85040046/diff/1/instance/placement_test.go#newcode25
instance/placement_test.go:25: expect: &instance.Placement{Scope:
instance.MachineScope, Value: "0"},
On 2014/04/09 04:52:18, wallyworld wrote:
> Perhaps to reduce the verbosity, the test struct can have expectScope
and
> expectDirective and the test would use those individual values to
construct a
> Placement to compare with?

Sure. There's a special case: empty scope and value == nil placement.

https://codereview.appspot.com/85040046/diff/1/provider/dummy/environs.go
File provider/dummy/environs.go (right):

https://codereview.appspot.com/85040046/diff/1/provider/dummy/environs.go#newcode546
provider/dummy/environs.go:546: func (*environ) ValidatePlacement(p
*instance.Placement) error {
On 2014/04/09 04:52:18, wallyworld wrote:
> I know we check for nil elsewhere, but maybe we should have it here as
well to
> be defensive and avoid a npe?

I suppose so. Alternatively, make it a non-pointer.

https://codereview.appspot.com/85040046/diff/1/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/85040046/diff/1/state/api/params/params.go#newcode107
state/api/params/params.go:107: ContainerType instance.ContainerType
On 2014/04/09 04:52:18, wallyworld wrote:
> So we are retaining ContainerType and ParentId but add-machine does
not set them
> anymore?

Yes, to support old clients. This deserves a DEPRECATED comment, due for
removal in 1.22.

https://codereview.appspot.com/85040046/diff/1/state/apiserver/provisioner/provisioner_test.go
File state/apiserver/provisioner/provisio...

[John A Meinel (jameinel)]

I'm a little concerned that we're changing an API.

How will we handle trying to send Placement to and older server?
(Remember that newer client should play nice when connecting to older
servers, just as newer servers should play nice when older clients talk
to them.)

https://codereview.appspot.com/85040046/

[Andrew Wilkins (axwalk)]

On 2014/04/09 09:58:21, jameinel wrote:
> I'm a little concerned that we're changing an API.

> How will we handle trying to send Placement to and older server?
(Remember that
> newer client should play nice when connecting to older servers, just
as newer
> servers should play nice when older clients talk to them.)

Yes, we will have to continue passing ContainerType/ParentId.

Actually we can't really reuse the API, because if we connect to an old
API server it'll just ignore the Placement field. I'll create a new API
method.

https://codereview.appspot.com/85040046/

[William Reade (fwereade)]

Coming along nicely -- let me know what you think about the following.

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode137
cmd/juju/addmachine.go:137: // TODO(axw) figure out how to convey
whether it's a container?
On 2014/04/09 05:20:22, axw wrote:
> On 2014/04/09 04:52:18, wallyworld wrote:
> > Can't we inspect the scope to see if it's a valid container type?

> Yes indeed. I had previously implemented things differently, but with
the
> current implementation it's trivial to do so.

And the machine id encodes this info anyway, doesn't it

https://codereview.appspot.com/85040046/diff/1/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/85040046/diff/1/environs/interface.go#newcode160
environs/interface.go:160: state.PlacementValidator
I'm not clear on how this is distinct from Prechecker -- isn't placement
just another StartInstance parameter?

Food for thought and maybe future implementation: should the prechecker
perhaps return something that implements InstanceBroker? that'd keep
what-we-ask-for and what-we-check in sync...

https://codereview.appspot.com/85040046/diff/1/instance/placement.go
File instance/placement.go (right):

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode54
instance/placement.go:54: if (scope == MachineScope ||
isContainerType(scope)) && !names.IsMachine(value) {
Hmm. Problem coming: environments named after a container type will act
in a surprising way. Can you think of any way around this?

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode63
instance/placement.go:63: return &Placement{Scope: directive}, nil
I'm not following this case. `add-machine --to kvm`? I think we need a
target machine for this to make sense.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode66
instance/placement.go:66: return &Placement{Value: directive}, nil
I think I'd prefer an explict ErrScopeUnclear, and explicit handling
thereof in the client.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode69
instance/placement.go:69: // ParsePlacement attempts to parse the
specified string and create a
s/Parse/MustParse/

https://codereview.appspot.com/85040046/diff/1/provider/azure/environ.go
File provider/azure/environ.go (right):

https://codereview.appspot.com/85040046/diff/1/provider/azure/environ.go#newcode1179
provider/azure/environ.go:1179: return fmt.Errorf("unknown placement
directive: %s", p)
Do we want a whole Placement here, or should we maybe just handle the
directive alone?

https://codereview.appspot.com/85040046/diff/1/provider/dummy/environs.go
File provider/dummy/environs.go (right):

https://codereview.appspot.com/85040046/diff/1/provider/dummy/environs.go#newcode548
provider/dummy/environs.go:548: case "valid":
"valid" scope would surely not be valid, unless the dummy environ's name
were itself "valid"? And I'm not sure that it's worth passing down a
value that always has to be a particular value... this STM to indicate
that it's just ValidatePla...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode127
cmd/juju/addmachine.go:127: results, err :=
client.AddMachines([]params.AddMachineParams{machineParams})
On 2014/04/09 05:20:22, axw wrote:
> On 2014/04/09 04:52:18, wallyworld wrote:
> > What about backwards compatibility?

> Yeah, I suppose we will need to continue setting ContainerType and
ParentId for
> backwards compatibility.

Done.

https://codereview.appspot.com/85040046/diff/1/cmd/juju/addmachine.go#newcode137
cmd/juju/addmachine.go:137: // TODO(axw) figure out how to convey
whether it's a container?
On 2014/04/09 05:20:22, axw wrote:
> On 2014/04/09 04:52:18, wallyworld wrote:
> > Can't we inspect the scope to see if it's a valid container type?

> Yes indeed. I had previously implemented things differently, but with
the
> current implementation it's trivial to do so.

Done.

https://codereview.appspot.com/85040046/diff/1/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/85040046/diff/1/environs/interface.go#newcode160
environs/interface.go:160: state.PlacementValidator
On 2014/04/11 12:27:11, fwereade wrote:
> I'm not clear on how this is distinct from Prechecker -- isn't
placement just
> another StartInstance parameter?

My intention was for PlacementValidator to be used for validating
add-unit/deploy placement too. I suppose those can just be handled by a
unit-placement policy.

> Food for thought and maybe future implementation: should the
prechecker perhaps
> return something that implements InstanceBroker? that'd keep
what-we-ask-for and
> what-we-check in sync...

Sounds like a decent idea, but I'd rather keep this CL to a minimum.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go
File instance/placement.go (right):

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode54
instance/placement.go:54: if (scope == MachineScope ||
isContainerType(scope)) && !names.IsMachine(value) {
On 2014/04/11 12:27:11, fwereade wrote:
> Hmm. Problem coming: environments named after a container type will
act in a
> surprising way. Can you think of any way around this?

Our current placement directives currently define the lxc, kvm, ssh and
machine-id special cases. Can't do anything about this without breaking
that existing format.

In the initial implementation we won't be able to handle environment
placement directives for an environment named "lxc" or "kvm". We *could*
add special casing so that it looks for "env:lxc:" or something, but
really I don't think it's worth it.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode63
instance/placement.go:63: return &Placement{Scope: directive}, nil
On 2014/04/11 12:27:11, fwereade wrote:
> I'm not following this case. `add-machine --to kvm`? I think we need a
target
> machine for this to make sense.

It means "add a kvm container on a new machine"; see the bottom of
state/apiserver.Client.addOneMachine.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode66
ins...

[William Reade (fwereade)]

Looking very nice, only a couple of details that maybe demand
thought/discussion.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go
File instance/placement.go (right):

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode54
instance/placement.go:54: if (scope == MachineScope ||
isContainerType(scope)) && !names.IsMachine(value) {
On 2014/04/17 06:13:46, axw wrote:
> On 2014/04/11 12:27:11, fwereade wrote:
> > Hmm. Problem coming: environments named after a container type will
act in a
> > surprising way. Can you think of any way around this?

> Our current placement directives currently define the lxc, kvm, ssh
and
> machine-id special cases. Can't do anything about this without
breaking that
> existing format.

> In the initial implementation we won't be able to handle environment
placement
> directives for an environment named "lxc" or "kvm". We *could* add
special
> casing so that it looks for "env:lxc:" or something, but really I
don't think
> it's worth it.

Yeah, I was more thinking that we ought to disallow "lxc" and "kvm" as
env names, but can't see how we can get there from here.

https://codereview.appspot.com/85040046/diff/1/instance/placement.go#newcode63
instance/placement.go:63: return &Placement{Scope: directive}, nil
On 2014/04/17 06:13:46, axw wrote:
> On 2014/04/11 12:27:11, fwereade wrote:
> > I'm not following this case. `add-machine --to kvm`? I think we need
a target
> > machine for this to make sense.

> It means "add a kvm container on a new machine"; see the bottom of
> state/apiserver.Client.addOneMachine.

Huh. I guess it was always like this, and I completely missed it.

Still not quite sure I like it, but let's not change things completely
arbitrarily :).

https://codereview.appspot.com/85040046/diff/20001/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/20001/cmd/juju/addmachine.go#newcode123
cmd/juju/addmachine.go:123: // message.
This feels a little bit off -- if we're not incompatible, we should
surely send the info in placement form? I think we should do this
tweaking *after* getting a NotImplemented...

https://codereview.appspot.com/85040046/diff/20001/environs/jujutest/livetests.go
File environs/jujutest/livetests.go (right):

https://codereview.appspot.com/85040046/diff/20001/environs/jujutest/livetests.go#newcode166
environs/jujutest/livetests.go:166: err :=
prechecker.PrecheckInstance("precise", constraints.Value{}, placement)
<3

https://codereview.appspot.com/85040046/diff/20001/instance/placement_test.go
File instance/placement_test.go (right):

https://codereview.appspot.com/85040046/diff/20001/instance/placement_test.go#newcode16
instance/placement_test.go:16: var parsePlacementTests = []struct {
FWIW, I really like it when test tables are defined inside the funcs
that use them, instead of cluttering up the package globals ;). It's a
bit surprising at first, but the structure is actually quite clean:

for i, test := range []struct{
     // fields
}{{
     // test
}, {
     // test
}} {
     // code
}

https://codereview.appspot.com/85040046/diff/20001/provider/maas/environ.go
File provider/maas/env...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/85040046/diff/20001/cmd/juju/addmachine.go
File cmd/juju/addmachine.go (right):

https://codereview.appspot.com/85040046/diff/20001/cmd/juju/addmachine.go#newcode123
cmd/juju/addmachine.go:123: // message.
On 2014/04/18 09:15:52, fwereade wrote:
> This feels a little bit off -- if we're not incompatible, we should
surely send
> the info in placement form? I think we should do this tweaking *after*
getting a
> NotImplemented...

Done. Sorry, misunderstood a previous comment about maintaining use of
ContainerType/ParentId. I now think you were referring to machine
templates.

https://codereview.appspot.com/85040046/diff/20001/instance/placement_test.go
File instance/placement_test.go (right):

https://codereview.appspot.com/85040046/diff/20001/instance/placement_test.go#newcode16
instance/placement_test.go:16: var parsePlacementTests = []struct {
On 2014/04/18 09:15:52, fwereade wrote:
> FWIW, I really like it when test tables are defined inside the funcs
that use
> them, instead of cluttering up the package globals ;). It's a bit
surprising at
> first, but the structure is actually quite clean:

> for i, test := range []struct{
> // fields
> }{{
> // test
> }, {
> // test
> }} {
> // code
> }

Sure, I just copied the structure from instance_test.go.

Updated. I don't like munging the slice literal into the range, as it
puts the assigned variables too far away from the loop body.

https://codereview.appspot.com/85040046/diff/20001/provider/maas/environ.go
File provider/maas/environ.go (right):

https://codereview.appspot.com/85040046/diff/20001/provider/maas/environ.go#newcode173
provider/maas/environ.go:173: // TODO(axw) handle maas-name placement
directive
On 2014/04/18 09:15:52, fwereade wrote:
> deserves a bug#

Done.

https://codereview.appspot.com/85040046/diff/20001/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85040046/diff/20001/state/api/client.go#newcode230
state/api/client.go:230: // This exists for backwards compatibility;
remove in 1.21 (client only).
On 2014/04/18 09:15:52, fwereade wrote:
> are we sure we can drop this? I fear we'll have to deal with the
possibility of
> a 1.18 client *or* server pretty-much-forever

I'm talking about the client code only, and then only from 1.21 when we
no longer require new-client-to-1.18-server compat.

https://codereview.appspot.com/85040046/diff/20001/state/api/client.go#newcode246
state/api/client.go:246: err := c.call("AddMachinesV2", args, results)
On 2014/04/18 09:15:52, fwereade wrote:
> grump, rage. jam, *please* get me some API versioning.

I didn't want to do this either.

https://codereview.appspot.com/85040046/diff/20001/state/api/params/params.go
File state/api/params/params.go (right):

https://codereview.appspot.com/85040046/diff/20001/state/api/params/params.go#newcode714
state/api/params/params.go:714: Results []PlacementResult
On 2014/04/18 09:15:52, fwereade wrote:
> This feels like it should be all together in a single call that
extracts all the
> info we need for a given machine -- ie we should be able to grab
> series+constraints+placement (and I guess instanc...

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/85040046/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/85040046/

[Ian Booth (wallyworld)]

LGTM with the error tweak as discussed

https://codereview.appspot.com/85040046/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/85040046/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/85040046/

=== modified file 'cmd/juju/addmachine.go'
--- cmd/juju/addmachine.go	2014-04-09 22:01:03 +0000
+++ cmd/juju/addmachine.go	2014-04-23 06:25:18 +0000
@@ -5,7 +5,6 @@
 
 import (
 	"fmt"
-	"strings"
 
 	"launchpad.net/gnuflag"
 
@@ -60,10 +59,9 @@
 	// If specified, use this series, else use the environment default-series
 	Series string
 	// If specified, these constraints are merged with those already in the environment.
-	Constraints   constraints.Value
-	MachineId     string
-	ContainerType instance.ContainerType
-	SSHHost       string
+	Constraints constraints.Value
+	// Placement is passed verbatim to the API, to be parsed and evaluated server-side.
+	Placement *instance.Placement
 }
 
 func (c *AddMachineCommand) Info() *cmd.Info {
@@ -89,33 +87,25 @@
 	if c.Constraints.Container != nil {
 		return fmt.Errorf("container constraint %q not allowed when adding a machine", *c.Constraints.Container)
 	}
-	containerSpec, err := cmd.ZeroOrOneArgs(args)
-	if err != nil {
-		return err
-	}
-	if containerSpec == "" {
-		return nil
-	}
-	if strings.HasPrefix(containerSpec, sshHostPrefix) {
-		c.SSHHost = containerSpec[len(sshHostPrefix):]
-	} else {
-		// container arg can either be 'type:machine' or 'type'
-		if c.ContainerType, err = instance.ParseContainerType(containerSpec); err != nil {
-			if names.IsMachine(containerSpec) || !cmd.IsMachineOrNewContainer(containerSpec) {
-				return fmt.Errorf("malformed container argument %q", containerSpec)
-			}
-			sep := strings.Index(containerSpec, ":")
-			c.MachineId = containerSpec[sep+1:]
-			c.ContainerType, err = instance.ParseContainerType(containerSpec[:sep])
-		}
-	}
-	return err
+	placement, err := cmd.ZeroOrOneArgs(args)
+	if err != nil {
+		return err
+	}
+	c.Placement, err = instance.ParsePlacement(placement)
+	if err == instance.ErrPlacementScopeMissing {
+		placement = c.EnvironName() + ":" + placement
+		c.Placement, err = instance.ParsePlacement(placement)
+	}
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 func (c *AddMachineCommand) Run(ctx *cmd.Context) error {
-	if c.SSHHost != "" {
+	if c.Placement != nil && c.Placement.Scope == "ssh" {
 		args := manual.ProvisionMachineArgs{
-			Host:    c.SSHHost,
+			Host:    c.Placement.Directive,
 			EnvName: c.EnvName,
 			Stdin:   ctx.Stdin,
 			Stdout:  ctx.Stdout,
@@ -131,27 +121,51 @@
 	}
 	defer client.Close()
 
+	if c.Placement != nil && c.Placement.Scope == instance.MachineScope {
+		// It does not make sense to add-machine <id>.
+		return fmt.Errorf("machine-id cannot be specified when adding machines")
+	}
+
 	machineParams := params.AddMachineParams{
-		ParentId:      c.MachineId,
-		ContainerType: c.ContainerType,
-		Series:        c.Series,
-		Constraints:   c.Constraints,
-		Jobs:          []params.MachineJob{params.JobHostUnits},
+		Placement:   c.Placement,
+		Series:      c.Series,
+		Constraints: c.Constraints,
+		Jobs:        []params.MachineJob{params.JobHostUnits},
 	}
 	results, err := client.AddMachines([]params.AddMachineParams{machineParams})
+	if params.IsCodeNotImplemented(err) {
+		if c.Placement != nil {
+			containerType, parseErr := instance.ParseContainerType(c.Placement.Scope)
+			if parseErr != nil {
+				// The user specified a non-container placement directive:
+				// return original API not implemented error.
+				return err
+			}
+			machineParams.ContainerType = containerType
+			machineParams.ParentId = c.Placement.Directive
+			machineParams.Placement = nil
+		}
+		logger.Infof(
+			"AddMachinesWithPlacement not supported by the API server, " +
+				"falling back to 1.18 compatibility mode",
+		)
+		results, err = client.AddMachines1dot18([]params.AddMachineParams{machineParams})
+	}
 	if err != nil {
 		return err
 	}
+
 	// Currently, only one machine is added, but in future there may be several added in one call.
 	machineInfo := results[0]
 	if machineInfo.Error != nil {
 		return machineInfo.Error
 	}
 	machineId := machineInfo.Machine
-	if c.ContainerType == "" {
-		logger.Infof("created machine %v", machineId)
+
+	if names.IsContainerMachine(machineId) {
+		ctx.Infof("created container %v", machineId)
 	} else {
-		logger.Infof("created %q container on machine %v", c.ContainerType, machineId)
+		ctx.Infof("created machine %v", machineId)
 	}
 	return nil
 }

=== modified file 'cmd/juju/addmachine_test.go'
--- cmd/juju/addmachine_test.go	2014-03-13 07:54:56 +0000
+++ cmd/juju/addmachine_test.go	2014-04-23 06:25:18 +0000
@@ -107,13 +107,17 @@
 
 func (s *AddMachineSuite) TestAddMachineErrors(c *gc.C) {
 	err := runAddMachine(c, ":lxc")
-	c.Assert(err, gc.ErrorMatches, `malformed container argument ":lxc"`)
+	c.Check(err, gc.ErrorMatches, `cannot add a new machine: :lxc placement is invalid`)
 	err = runAddMachine(c, "lxc:")
-	c.Assert(err, gc.ErrorMatches, `malformed container argument "lxc:"`)
+	c.Check(err, gc.ErrorMatches, `invalid value "" for "lxc" scope: expected machine-id`)
 	err = runAddMachine(c, "2")
-	c.Assert(err, gc.ErrorMatches, `malformed container argument "2"`)
+	c.Check(err, gc.ErrorMatches, `machine-id cannot be specified when adding machines`)
 	err = runAddMachine(c, "foo")
-	c.Assert(err, gc.ErrorMatches, `malformed container argument "foo"`)
+	c.Check(err, gc.ErrorMatches, `cannot add a new machine: foo placement is invalid`)
+	err = runAddMachine(c, "foo:bar")
+	c.Check(err, gc.ErrorMatches, `invalid environment name "foo"`)
+	err = runAddMachine(c, "dummyenv:invalid")
+	c.Check(err, gc.ErrorMatches, `cannot add a new machine: invalid placement is invalid`)
 	err = runAddMachine(c, "lxc", "--constraints", "container=lxc")
-	c.Assert(err, gc.ErrorMatches, `container constraint "lxc" not allowed when adding a machine`)
+	c.Check(err, gc.ErrorMatches, `container constraint "lxc" not allowed when adding a machine`)
 }

=== modified file 'environs/broker.go'
--- environs/broker.go	2014-04-18 16:37:28 +0000
+++ environs/broker.go	2014-04-23 06:25:18 +0000
@@ -25,6 +25,11 @@
 	// MachineConfig describes the machine's configuration.
 	MachineConfig *cloudinit.MachineConfig
 
+	// Placement, if non-empty, contains an environment-specific
+	// placement directive that may be used to decide how the
+	// instance should be started.
+	Placement string
+
 	// DistributionGroup, if non-nil, is a function
 	// that returns a slice of instance.Ids that belong
 	// to the same distribution group as the machine

=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2014-04-21 23:10:05 +0000
+++ environs/jujutest/livetests.go	2014-04-23 06:25:18 +0000
@@ -162,7 +162,8 @@
 	if !ok {
 		return
 	}
-	err := prechecker.PrecheckInstance("precise", constraints.Value{})
+	const placement = ""
+	err := prechecker.PrecheckInstance("precise", constraints.Value{}, placement)
 	c.Assert(err, gc.IsNil)
 }
 

=== added file 'instance/placement.go'
--- instance/placement.go	1970-01-01 00:00:00 +0000
+++ instance/placement.go	2014-04-23 06:25:18 +0000
@@ -0,0 +1,85 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package instance
+
+import (
+	"fmt"
+	"strings"
+
+	"launchpad.net/juju-core/names"
+)
+
+const (
+	// MachineScope is a special scope name that is used
+	// for machine placement directives (e.g. --to 0).
+	MachineScope = "#"
+)
+
+var ErrPlacementScopeMissing = fmt.Errorf("placement scope missing")
+
+// Placement defines a placement directive, which has a scope
+// and a value that is scope-specific.
+type Placement struct {
+	// Scope is the scope of the placement directive. Scope may
+	// be a container type (lxc, kvm), instance.MachineScope, or
+	// an environment name.
+	//
+	// If Scope is empty, then it must be inferred from the context.
+	Scope string
+
+	// Directive is a scope-specific placement idrective.
+	//
+	// For MachineScope or a container scope, this may be empty or
+	// the ID of an existing machine.
+	Directive string
+}
+
+func (p *Placement) String() string {
+	return fmt.Sprintf("%s:%s", p.Scope, p.Directive)
+}
+
+func isContainerType(s string) bool {
+	_, err := ParseContainerType(s)
+	return err == nil
+}
+
+// ParsePlacement attempts to parse the specified string and create a
+// corresponding Placement structure.
+//
+// If the placement directive is non-empty and missing a scope,
+// ErrPlacementScopeMissing will be returned as well as a Placement
+// with an empty Scope field.
+func ParsePlacement(directive string) (*Placement, error) {
+	if directive == "" {
+		return nil, nil
+	}
+	if colon := strings.IndexRune(directive, ':'); colon != -1 {
+		scope, directive := directive[:colon], directive[colon+1:]
+		if scope == "" {
+			return nil, ErrPlacementScopeMissing
+		}
+		// Sanity check: machine/container scopes require a machine ID as the value.
+		if (scope == MachineScope || isContainerType(scope)) && !names.IsMachine(directive) {
+			return nil, fmt.Errorf("invalid value %q for %q scope: expected machine-id", directive, scope)
+		}
+		return &Placement{Scope: scope, Directive: directive}, nil
+	}
+	if names.IsMachine(directive) {
+		return &Placement{Scope: MachineScope, Directive: directive}, nil
+	}
+	if isContainerType(directive) {
+		return &Placement{Scope: directive}, nil
+	}
+	return nil, ErrPlacementScopeMissing
+}
+
+// MustParsePlacement attempts to parse the specified string and create
+// a corresponding Placement structure, panicking if an error occurs.
+func MustParsePlacement(directive string) *Placement {
+	placement, err := ParsePlacement(directive)
+	if err != nil {
+		panic(err)
+	}
+	return placement
+}

=== added file 'instance/placement_test.go'
--- instance/placement_test.go	1970-01-01 00:00:00 +0000
+++ instance/placement_test.go	2014-04-23 06:25:18 +0000
@@ -0,0 +1,76 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package instance_test
+
+import (
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/instance"
+)
+
+type PlacementSuite struct{}
+
+var _ = gc.Suite(&PlacementSuite{})
+
+func (s *PlacementSuite) TestParsePlacement(c *gc.C) {
+	parsePlacementTests := []struct {
+		arg                          string
+		expectScope, expectDirective string
+		err                          string
+	}{{
+		arg: "",
+	}, {
+		arg:             "0",
+		expectScope:     instance.MachineScope,
+		expectDirective: "0",
+	}, {
+		arg:             "0/lxc/0",
+		expectScope:     instance.MachineScope,
+		expectDirective: "0/lxc/0",
+	}, {
+		arg: "#:x",
+		err: `invalid value "x" for "#" scope: expected machine-id`,
+	}, {
+		arg: "lxc:x",
+		err: `invalid value "x" for "lxc" scope: expected machine-id`,
+	}, {
+		arg: "kvm:x",
+		err: `invalid value "x" for "kvm" scope: expected machine-id`,
+	}, {
+		arg:             "kvm:123",
+		expectScope:     string(instance.KVM),
+		expectDirective: "123",
+	}, {
+		arg:         "lxc",
+		expectScope: string(instance.LXC),
+	}, {
+		arg: "non-standard",
+		err: "placement scope missing",
+	}, {
+		arg: ":non-standard",
+		err: "placement scope missing",
+	}, {
+		arg:             "non:standard",
+		expectScope:     "non",
+		expectDirective: "standard",
+	}}
+
+	for i, t := range parsePlacementTests {
+		c.Logf("test %d: %s", i, t.arg)
+		p, err := instance.ParsePlacement(t.arg)
+		if t.err != "" {
+			c.Assert(err, gc.ErrorMatches, t.err)
+		} else {
+			c.Assert(err, gc.IsNil)
+		}
+		if t.expectScope == "" && t.expectDirective == "" {
+			c.Assert(p, gc.IsNil)
+		} else {
+			c.Assert(p, gc.DeepEquals, &instance.Placement{
+				Scope:     t.expectScope,
+				Directive: t.expectDirective,
+			})
+		}
+	}
+}

=== modified file 'names/machine.go'
--- names/machine.go	2013-11-25 05:03:44 +0000
+++ names/machine.go	2014-04-23 06:25:18 +0000
@@ -21,6 +21,11 @@
 	return validMachine.MatchString(id)
 }
 
+// IsMachine returns whether id is a valid container machine id.
+func IsContainerMachine(id string) bool {
+	return validMachine.MatchString(id) && strings.Contains(id, "/")
+}
+
 // MachineTag returns the tag for the machine with the given id.
 func MachineTag(id string) string {
 	tag := makeTag(MachineTagKind, id)

=== modified file 'names/machine_test.go'
--- names/machine_test.go	2013-08-06 09:20:36 +0000
+++ names/machine_test.go	2014-04-23 06:25:18 +0000
@@ -26,8 +26,9 @@
 }
 
 var machineIdTests = []struct {
-	pattern string
-	valid   bool
+	pattern   string
+	valid     bool
+	container bool
 }{
 	{pattern: "42", valid: true},
 	{pattern: "042", valid: false},
@@ -37,7 +38,7 @@
 	{pattern: "55/", valid: false},
 	{pattern: "1/foo", valid: false},
 	{pattern: "2/foo/", valid: false},
-	{pattern: "3/lxc/42", valid: true},
+	{pattern: "3/lxc/42", valid: true, container: true},
 	{pattern: "3/lxc-nodash/42", valid: false},
 	{pattern: "0/lxc/00", valid: false},
 	{pattern: "0/lxc/0/", valid: false},
@@ -45,7 +46,7 @@
 	{pattern: "3/lxc/042", valid: false},
 	{pattern: "4/foo/bar", valid: false},
 	{pattern: "5/lxc/42/foo", valid: false},
-	{pattern: "6/lxc/42/kvm/0", valid: true},
+	{pattern: "6/lxc/42/kvm/0", valid: true, container: true},
 	{pattern: "06/lxc/42/kvm/0", valid: false},
 	{pattern: "6/lxc/042/kvm/0", valid: false},
 	{pattern: "6/lxc/42/kvm/00", valid: false},
@@ -56,5 +57,6 @@
 	for i, test := range machineIdTests {
 		c.Logf("test %d: %q", i, test.pattern)
 		c.Assert(names.IsMachine(test.pattern), gc.Equals, test.valid)
+		c.Assert(names.IsContainerMachine(test.pattern), gc.Equals, test.container)
 	}
 }

=== modified file 'provider/azure/environ.go'
--- provider/azure/environ.go	2014-04-21 23:50:20 +0000
+++ provider/azure/environ.go	2014-04-23 06:25:18 +0000
@@ -434,7 +434,10 @@
 }
 
 // PrecheckInstance is defined on the state.Prechecker interface.
-func (env *azureEnviron) PrecheckInstance(series string, cons constraints.Value) error {
+func (env *azureEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	if placement != "" {
+		return fmt.Errorf("unknown placement directive: %s", placement)
+	}
 	if !cons.HasInstanceType() {
 		return nil
 	}

=== modified file 'provider/azure/instancetype_test.go'
--- provider/azure/instancetype_test.go	2014-04-17 06:44:49 +0000
+++ provider/azure/instancetype_test.go	2014-04-23 06:25:18 +0000
@@ -449,13 +449,15 @@
 func (s *instanceTypeSuite) TestPrecheckInstanceValidInstanceType(c *gc.C) {
 	env := s.setupEnvWithDummyMetadata(c)
 	cons := constraints.MustParse("instance-type=Large")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.IsNil)
 }
 
 func (s *instanceTypeSuite) TestPrecheckInstanceInvalidInstanceType(c *gc.C) {
 	env := s.setupEnvWithDummyMetadata(c)
 	cons := constraints.MustParse("instance-type=Super")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.ErrorMatches, `invalid Azure instance "Super" specified`)
 }

=== modified file 'provider/common/policies.go'
--- provider/common/policies.go	2014-04-17 03:10:18 +0000
+++ provider/common/policies.go	2014-04-23 06:25:18 +0000
@@ -3,11 +3,6 @@
 
 package common
 
-import (
-	"launchpad.net/juju-core/constraints"
-	"launchpad.net/juju-core/state"
-)
-
 // SupportsUnitPlacementPolicy provides an
 // implementation of SupportsUnitPlacement
 // that never returns an error, and is
@@ -18,13 +13,3 @@
 func (*SupportsUnitPlacementPolicy) SupportsUnitPlacement() error {
 	return nil
 }
-
-// NopPrecheckerPolicy provides an implementation of the
-// state.Prechecker interface that passes all checks.
-type NopPrecheckerPolicy struct{}
-
-var _ state.Prechecker = (*NopPrecheckerPolicy)(nil)
-
-func (*NopPrecheckerPolicy) PrecheckInstance(series string, cons constraints.Value) error {
-	return nil
-}

=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go	2014-04-21 23:10:05 +0000
+++ provider/dummy/environs.go	2014-04-23 06:25:18 +0000
@@ -184,7 +184,6 @@
 // environ represents a client's connection to a given environment's
 // state.
 type environ struct {
-	common.NopPrecheckerPolicy
 	common.SupportsUnitPlacementPolicy
 
 	name         string
@@ -543,6 +542,14 @@
 	return true
 }
 
+// PrecheckInstance is specified in the state.Prechecker interface.
+func (*environ) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	if placement != "" && placement != "valid" {
+		return fmt.Errorf("%s placement is invalid", placement)
+	}
+	return nil
+}
+
 // GetImageSources returns a list of sources which are used to search for simplestreams image metadata.
 func (e *environ) GetImageSources() ([]simplestreams.DataSource, error) {
 	return []simplestreams.DataSource{

=== modified file 'provider/ec2/ec2.go'
--- provider/ec2/ec2.go	2014-04-21 23:50:20 +0000
+++ provider/ec2/ec2.go	2014-04-23 06:25:18 +0000
@@ -385,7 +385,10 @@
 }
 
 // PrecheckInstance is defined on the state.Prechecker interface.
-func (e *environ) PrecheckInstance(series string, cons constraints.Value) error {
+func (e *environ) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	if placement != "" {
+		return fmt.Errorf("unknown placement directive: %s", placement)
+	}
 	if !cons.HasInstanceType() {
 		return nil
 	}

=== modified file 'provider/ec2/local_test.go'
--- provider/ec2/local_test.go	2014-04-18 13:51:46 +0000
+++ provider/ec2/local_test.go	2014-04-23 06:25:18 +0000
@@ -381,21 +381,24 @@
 func (t *localServerSuite) TestPrecheckInstanceValidInstanceType(c *gc.C) {
 	env := t.Prepare(c)
 	cons := constraints.MustParse("instance-type=m1.small root-disk=1G")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.IsNil)
 }
 
 func (t *localServerSuite) TestPrecheckInstanceInvalidInstanceType(c *gc.C) {
 	env := t.Prepare(c)
 	cons := constraints.MustParse("instance-type=m1.invalid")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.ErrorMatches, `invalid AWS instance type "m1.invalid" specified`)
 }
 
 func (t *localServerSuite) TestPrecheckInstanceUnsupportedArch(c *gc.C) {
 	env := t.Prepare(c)
 	cons := constraints.MustParse("instance-type=cc1.4xlarge arch=i386")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.ErrorMatches, `invalid AWS instance type "cc1.4xlarge" and arch "i386" specified`)
 }
 

=== modified file 'provider/joyent/environ.go'
--- provider/joyent/environ.go	2014-04-17 06:53:42 +0000
+++ provider/joyent/environ.go	2014-04-23 06:25:18 +0000
@@ -76,7 +76,10 @@
 }
 
 // PrecheckInstance is defined on the state.Prechecker interface.
-func (env *joyentEnviron) PrecheckInstance(series string, cons constraints.Value) error {
+func (env *joyentEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	if placement != "" {
+		return fmt.Errorf("unknown placement directive: %s", placement)
+	}
 	if !cons.HasInstanceType() {
 		return nil
 	}

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2014-04-21 23:10:05 +0000
+++ provider/local/environ.go	2014-04-23 06:25:18 +0000
@@ -58,7 +58,6 @@
 var _ envtools.SupportsCustomSources = (*localEnviron)(nil)
 
 type localEnviron struct {
-	common.NopPrecheckerPolicy
 	common.SupportsUnitPlacementPolicy
 
 	localMutex       sync.Mutex
@@ -88,6 +87,13 @@
 	return false
 }
 
+func (*localEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	if placement != "" {
+		return fmt.Errorf("unknown placement directive: %s", placement)
+	}
+	return nil
+}
+
 // Name is specified in the Environ interface.
 func (env *localEnviron) Name() string {
 	return env.name

=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go	2014-04-21 23:10:05 +0000
+++ provider/maas/environ.go	2014-04-23 06:25:18 +0000
@@ -51,7 +51,6 @@
 }
 
 type maasEnviron struct {
-	common.NopPrecheckerPolicy
 	common.SupportsUnitPlacementPolicy
 
 	name string
@@ -171,6 +170,15 @@
 	return caps.Contains(capNetworksManagement)
 }
 
+func (env *maasEnviron) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	// TODO(axw) 2014-04-22 #1237709
+	// Handle maas-name placement directive.
+	if placement != "" {
+		return fmt.Errorf("unknown placement directive: %s", placement)
+	}
+	return nil
+}
+
 const capNetworksManagement = "networks-management"
 
 // getCapabilities asks the MAAS server for its capabilities, if

=== modified file 'provider/manual/environ.go'
--- provider/manual/environ.go	2014-04-21 23:10:05 +0000
+++ provider/manual/environ.go	2014-04-23 06:25:18 +0000
@@ -61,7 +61,6 @@
 }
 
 var _ envtools.SupportsCustomSources = (*manualEnviron)(nil)
-var _ state.Prechecker = (*manualEnviron)(nil)
 
 var errNoStartInstance = errors.New("manual provider cannot start instances")
 var errNoStopInstance = errors.New("manual provider cannot stop instances")
@@ -262,7 +261,7 @@
 	return err
 }
 
-func (*manualEnviron) PrecheckInstance(series string, cons constraints.Value) error {
+func (*manualEnviron) PrecheckInstance(series string, _ constraints.Value, placement string) error {
 	return errors.New(`use "juju add-machine ssh:[user@]<host>" to provision machines`)
 }
 

=== modified file 'provider/openstack/local_test.go'
--- provider/openstack/local_test.go	2014-04-21 23:50:20 +0000
+++ provider/openstack/local_test.go	2014-04-23 06:25:18 +0000
@@ -716,14 +716,16 @@
 func (s *localServerSuite) TestPrecheckInstanceValidInstanceType(c *gc.C) {
 	env := s.Open(c)
 	cons := constraints.MustParse("instance-type=m1.small")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.IsNil)
 }
 
 func (s *localServerSuite) TestPrecheckInstanceInvalidInstanceType(c *gc.C) {
 	env := s.Open(c)
 	cons := constraints.MustParse("instance-type=m1.large")
-	err := env.PrecheckInstance("precise", cons)
+	placement := ""
+	err := env.PrecheckInstance("precise", cons, placement)
 	c.Assert(err, gc.ErrorMatches, `invalid Openstack flavour "m1.large" specified`)
 }
 

=== modified file 'provider/openstack/provider.go'
--- provider/openstack/provider.go	2014-04-21 23:50:20 +0000
+++ provider/openstack/provider.go	2014-04-23 06:25:18 +0000
@@ -538,7 +538,10 @@
 }
 
 // PrecheckInstance is defined on the state.Prechecker interface.
-func (e *environ) PrecheckInstance(series string, cons constraints.Value) error {
+func (e *environ) PrecheckInstance(series string, cons constraints.Value, placement string) error {
+	if placement != "" {
+		return fmt.Errorf("unknown placement directive: %s", placement)
+	}
 	if !cons.HasInstanceType() {
 		return nil
 	}

=== modified file 'state/addmachine.go'
--- state/addmachine.go	2014-04-21 23:10:05 +0000
+++ state/addmachine.go	2014-04-23 06:25:18 +0000
@@ -69,6 +69,10 @@
 	// as unclean for unit-assignment purposes.
 	Dirty bool
 
+	// Placement holds the placement directive that will be associated
+	// with the machine.
+	Placement string
+
 	// principals holds the principal units that will
 	// associated with the machine.
 	principals []string
@@ -234,7 +238,7 @@
 		return nil, nil, err
 	}
 	if template.InstanceId == "" {
-		if err := st.precheckInstance(template.Series, template.Constraints); err != nil {
+		if err := st.precheckInstance(template.Series, template.Constraints, template.Placement); err != nil {
 			return nil, nil, err
 		}
 	}
@@ -359,13 +363,13 @@
 		return nil, nil, fmt.Errorf("no container type specified")
 	}
 	if parentTemplate.InstanceId == "" {
-		if err := st.precheckInstance(parentTemplate.Series, parentTemplate.Constraints); err != nil {
-			return nil, nil, err
-		}
 		// Adding a machine within a machine implies add-machine or placement.
 		if err := st.supportsUnitPlacement(); err != nil {
 			return nil, nil, err
 		}
+		if err := st.precheckInstance(parentTemplate.Series, parentTemplate.Constraints, parentTemplate.Placement); err != nil {
+			return nil, nil, err
+		}
 	}
 
 	parentDoc := machineDocForTemplate(parentTemplate, strconv.Itoa(seq))
@@ -403,6 +407,7 @@
 		Nonce:      template.Nonce,
 		Addresses:  instanceAddressesToAddresses(template.Addresses),
 		NoVote:     template.NoVote,
+		Placement:  template.Placement,
 	}
 }
 

=== modified file 'state/api/client.go'
--- state/api/client.go	2014-04-17 19:01:44 +0000
+++ state/api/client.go	2014-04-23 06:25:18 +0000
@@ -227,13 +227,26 @@
 	return results.CharmRelations, err
 }
 
+// AddMachines1dot18 adds new machines with the supplied parameters.
+//
+// TODO(axw) 2014-04-11 #XXX
+// This exists for backwards compatibility; remove in 1.21 (client only).
+func (c *Client) AddMachines1dot18(machineParams []params.AddMachineParams) ([]params.AddMachinesResult, error) {
+	args := params.AddMachines{
+		MachineParams: machineParams,
+	}
+	results := new(params.AddMachinesResults)
+	err := c.call("AddMachines", args, results)
+	return results.Machines, err
+}
+
 // AddMachines adds new machines with the supplied parameters.
 func (c *Client) AddMachines(machineParams []params.AddMachineParams) ([]params.AddMachinesResult, error) {
 	args := params.AddMachines{
 		MachineParams: machineParams,
 	}
 	results := new(params.AddMachinesResults)
-	err := c.call("AddMachines", args, results)
+	err := c.call("AddMachinesV2", args, results)
 	return results.Machines, err
 }
 

=== modified file 'state/api/params/internal.go'
--- state/api/params/internal.go	2014-04-18 16:37:28 +0000
+++ state/api/params/internal.go	2014-04-23 06:25:18 +0000
@@ -599,3 +599,23 @@
 type AgentVersionResult struct {
 	Version version.Number
 }
+
+// ProvisioningInfo holds machine provisioning info.
+type ProvisioningInfo struct {
+	Constraints     constraints.Value
+	Series          string
+	Placement       string
+	IncludeNetworks []string
+	ExcludeNetworks []string
+}
+
+// ProvisioningInfoResult holds machine provisioning info or an error.
+type ProvisioningInfoResult struct {
+	Error  *Error
+	Result *ProvisioningInfo
+}
+
+// ProvisioningInfoResults holds multiple machine provisioning info results.
+type ProvisioningInfoResults struct {
+	Results []ProvisioningInfoResult
+}

=== modified file 'state/api/params/params.go'
--- state/api/params/params.go	2014-04-12 05:53:58 +0000
+++ state/api/params/params.go	2014-04-23 06:25:18 +0000
@@ -92,6 +92,10 @@
 	Constraints constraints.Value
 	Jobs        []MachineJob
 
+	// If Placement is non-nil, it contains a placement directive
+	// that will be used to decide how to instantiate the machine.
+	Placement *instance.Placement
+
 	// If ParentId is non-empty, it specifies the id of the
 	// parent machine within which the new machine will
 	// be created. In that case, ContainerType must also be
@@ -117,7 +121,8 @@
 	Addrs                   []instance.Address
 }
 
-// AddMachines holds the parameters for making the AddMachines call.
+// AddMachines holds the parameters for making the
+// AddMachinesWithPlacement call.
 type AddMachines struct {
 	MachineParams []AddMachineParams
 }

=== modified file 'state/api/provisioner/machine.go'
--- state/api/provisioner/machine.go	2014-04-13 11:06:42 +0000
+++ state/api/provisioner/machine.go	2014-04-23 06:25:18 +0000
@@ -7,7 +7,6 @@
 	"errors"
 	"fmt"
 
-	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/names"
 	"launchpad.net/juju-core/state/api/params"
@@ -55,23 +54,22 @@
 	return nil
 }
 
-// RequestedNetworks returns a pair of lists of networks to
-// enable/disable on the machine.
-func (m *Machine) RequestedNetworks() (includeNetworks, excludeNetworks []string, err error) {
-	var results params.RequestedNetworksResults
+// ProvisioningInfo returns the information required to provisiong a machine.
+func (m *Machine) ProvisioningInfo() (*params.ProvisioningInfo, error) {
+	var results params.ProvisioningInfoResults
 	args := params.Entities{Entities: []params.Entity{{m.tag}}}
-	err = m.st.call("RequestedNetworks", args, &results)
+	err := m.st.call("ProvisioningInfo", args, &results)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	if len(results.Results) != 1 {
-		return nil, nil, fmt.Errorf("expected 1 result, got %d", len(results.Results))
+		return nil, fmt.Errorf("expected 1 result, got %d", len(results.Results))
 	}
 	result := results.Results[0]
 	if result.Error != nil {
-		return nil, nil, result.Error
+		return nil, result.Error
 	}
-	return result.IncludeNetworks, result.ExcludeNetworks, nil
+	return result.Result, nil
 }
 
 // SetStatus sets the status of the machine.
@@ -109,28 +107,6 @@
 	return result.Status, result.Info, nil
 }
 
-// Constraints returns the exact constraints that should apply when provisioning
-// an instance for the machine.
-func (m *Machine) Constraints() (constraints.Value, error) {
-	nothing := constraints.Value{}
-	var results params.ConstraintsResults
-	args := params.Entities{
-		Entities: []params.Entity{{Tag: m.tag}},
-	}
-	err := m.st.call("Constraints", args, &results)
-	if err != nil {
-		return nothing, err
-	}
-	if len(results.Results) != 1 {
-		return nothing, fmt.Errorf("expected 1 result, got %d", len(results.Results))
-	}
-	result := results.Results[0]
-	if result.Error != nil {
-		return nothing, result.Error
-	}
-	return result.Constraints, nil
-}
-
 // EnsureDead sets the machine lifecycle to Dead if it is Alive or
 // Dying. It does nothing otherwise.
 func (m *Machine) EnsureDead() error {

=== modified file 'state/api/provisioner/provisioner_test.go'
--- state/api/provisioner/provisioner_test.go	2014-04-18 15:31:56 +0000
+++ state/api/provisioner/provisioner_test.go	2014-04-23 06:25:18 +0000
@@ -388,55 +388,41 @@
 	c.Assert(err, jc.Satisfies, params.IsCodeNotFound)
 }
 
-func (s *provisionerSuite) TestConstraints(c *gc.C) {
-	// Create a fresh machine with some constraints.
-	template := state.MachineTemplate{
-		Series:      "quantal",
-		Jobs:        []state.MachineJob{state.JobHostUnits},
-		Constraints: constraints.MustParse("cpu-cores=12", "mem=8G"),
-	}
-	consMachine, err := s.State.AddOneMachine(template)
-	c.Assert(err, gc.IsNil)
-
-	apiMachine, err := s.provisioner.Machine(consMachine.Tag())
-	c.Assert(err, gc.IsNil)
-	cons, err := apiMachine.Constraints()
-	c.Assert(err, gc.IsNil)
-	c.Assert(cons, gc.DeepEquals, template.Constraints)
-
-	// Now try machine 0.
-	apiMachine, err = s.provisioner.Machine(s.machine.Tag())
-	c.Assert(err, gc.IsNil)
-	cons, err = apiMachine.Constraints()
-	c.Assert(err, gc.IsNil)
-	c.Assert(cons, gc.DeepEquals, constraints.Value{})
-}
-
-func (s *provisionerSuite) TestRequestedNetworks(c *gc.C) {
-	// Create a fresh machine with some requested networks.
+func (s *provisionerSuite) TestProvisioningInfo(c *gc.C) {
 	template := state.MachineTemplate{
 		Series:          "quantal",
 		Jobs:            []state.MachineJob{state.JobHostUnits},
+		Placement:       "valid",
+		Constraints:     constraints.MustParse("cpu-cores=12", "mem=8G"),
 		IncludeNetworks: []string{"net1", "net2"},
 		ExcludeNetworks: []string{"net3", "net4"},
 	}
-	netsMachine, err := s.State.AddOneMachine(template)
-	c.Assert(err, gc.IsNil)
-
-	apiMachine, err := s.provisioner.Machine(netsMachine.Tag())
-	c.Assert(err, gc.IsNil)
-	includeNetworks, excludeNetworks, err := apiMachine.RequestedNetworks()
-	c.Assert(err, gc.IsNil)
-	c.Assert(includeNetworks, gc.DeepEquals, template.IncludeNetworks)
-	c.Assert(excludeNetworks, gc.DeepEquals, template.ExcludeNetworks)
-
-	// Now try machine 0.
-	apiMachine, err = s.provisioner.Machine(s.machine.Tag())
-	c.Assert(err, gc.IsNil)
-	includeNetworks, excludeNetworks, err = apiMachine.RequestedNetworks()
-	c.Assert(err, gc.IsNil)
-	c.Assert(includeNetworks, gc.HasLen, 0)
-	c.Assert(excludeNetworks, gc.HasLen, 0)
+	machine, err := s.State.AddOneMachine(template)
+	c.Assert(err, gc.IsNil)
+	apiMachine, err := s.provisioner.Machine(machine.Tag())
+	c.Assert(err, gc.IsNil)
+	provisioningInfo, err := apiMachine.ProvisioningInfo()
+	c.Assert(err, gc.IsNil)
+	c.Assert(provisioningInfo.Series, gc.Equals, template.Series)
+	c.Assert(provisioningInfo.Placement, gc.Equals, template.Placement)
+	c.Assert(provisioningInfo.Constraints, gc.DeepEquals, template.Constraints)
+	c.Assert(provisioningInfo.IncludeNetworks, gc.DeepEquals, template.IncludeNetworks)
+	c.Assert(provisioningInfo.ExcludeNetworks, gc.DeepEquals, template.ExcludeNetworks)
+}
+
+func (s *provisionerSuite) TestProvisioningInfoMachineNotFound(c *gc.C) {
+	stateMachine, err := s.State.AddMachine("quantal", state.JobHostUnits)
+	c.Assert(err, gc.IsNil)
+	apiMachine, err := s.provisioner.Machine(stateMachine.Tag())
+	c.Assert(err, gc.IsNil)
+	err = apiMachine.EnsureDead()
+	c.Assert(err, gc.IsNil)
+	err = apiMachine.Remove()
+	c.Assert(err, gc.IsNil)
+	_, err = apiMachine.ProvisioningInfo()
+	c.Assert(err, gc.ErrorMatches, "machine 1 not found")
+	c.Assert(err, jc.Satisfies, params.IsCodeNotFound)
+	// auth tests in apiserver
 }
 
 func (s *provisionerSuite) TestWatchContainers(c *gc.C) {

=== modified file 'state/apiserver/client/client.go'
--- state/apiserver/client/client.go	2014-04-17 12:53:23 +0000
+++ state/apiserver/client/client.go	2014-04-23 06:25:18 +0000
@@ -587,6 +587,11 @@
 
 // AddMachines adds new machines with the supplied parameters.
 func (c *Client) AddMachines(args params.AddMachines) (params.AddMachinesResults, error) {
+	return c.AddMachinesV2(args)
+}
+
+// AddMachinesV2 adds new machines with the supplied parameters.
+func (c *Client) AddMachinesV2(args params.AddMachines) (params.AddMachinesResults, error) {
 	results := params.AddMachinesResults{
 		Machines: make([]params.AddMachinesResult, len(args.MachineParams)),
 	}
@@ -606,23 +611,50 @@
 }
 
 func (c *Client) addOneMachine(p params.AddMachineParams) (*state.Machine, error) {
-	if p.Series == "" {
-		conf, err := c.api.state.EnvironConfig()
-		if err != nil {
-			return nil, err
+	if p.ParentId != "" && p.ContainerType == "" {
+		return nil, fmt.Errorf("parent machine specified without container type")
+	}
+	if p.ContainerType != "" && p.Placement != nil {
+		return nil, fmt.Errorf("container type and placement are mutually exclusive")
+	}
+	if p.Placement != nil {
+		// Extract container type and parent from container placement directives.
+		containerType, err := instance.ParseContainerType(p.Placement.Scope)
+		if err == nil {
+			p.ContainerType = containerType
+			p.ParentId = p.Placement.Directive
+			p.Placement = nil
 		}
-		p.Series = config.PreferredSeries(conf)
 	}
-	if p.ContainerType != "" {
+
+	if p.ContainerType != "" || p.Placement != nil {
 		// Guard against dubious client by making sure that
 		// the following attributes can only be set when we're
-		// not making a new container.
+		// not using placement.
 		p.InstanceId = ""
 		p.Nonce = ""
 		p.HardwareCharacteristics = instance.HardwareCharacteristics{}
 		p.Addrs = nil
-	} else if p.ParentId != "" {
-		return nil, fmt.Errorf("parent machine specified without container type")
+	}
+
+	if p.Series == "" {
+		conf, err := c.api.state.EnvironConfig()
+		if err != nil {
+			return nil, err
+		}
+		p.Series = config.PreferredSeries(conf)
+	}
+
+	var placementDirective string
+	if p.Placement != nil {
+		env, err := c.api.state.Environment()
+		if err != nil {
+			return nil, err
+		}
+		if p.Placement.Scope != env.Name() {
+			return nil, fmt.Errorf("invalid environment name %q", p.Placement.Scope)
+		}
+		placementDirective = p.Placement.Directive
 	}
 
 	jobs, err := stateJobs(p.Jobs)
@@ -637,6 +669,7 @@
 		Nonce:       p.Nonce,
 		HardwareCharacteristics: p.HardwareCharacteristics,
 		Addresses:               p.Addrs,
+		Placement:               placementDirective,
 	}
 	if p.ContainerType == "" {
 		return c.api.state.AddOneMachine(template)

=== modified file 'state/apiserver/client/client_test.go'
--- state/apiserver/client/client_test.go	2014-04-17 12:53:23 +0000
+++ state/apiserver/client/client_test.go	2014-04-23 06:25:18 +0000
@@ -1665,8 +1665,8 @@
 
 	machines, err := s.APIState.Client().AddMachines([]params.AddMachineParams{{
 		Jobs:          []params.MachineJob{params.JobHostUnits},
+		ContainerType: instance.LXC,
 		ParentId:      "0",
-		ContainerType: instance.LXC,
 		Series:        "quantal",
 	}})
 	c.Assert(err, gc.IsNil)
@@ -1692,13 +1692,65 @@
 	}
 }
 
+func (s *clientSuite) TestClientAddMachinesWithPlacement(c *gc.C) {
+	apiParams := make([]params.AddMachineParams, 4)
+	for i := range apiParams {
+		apiParams[i] = params.AddMachineParams{
+			Jobs: []params.MachineJob{params.JobHostUnits},
+		}
+	}
+	apiParams[0].Placement = instance.MustParsePlacement("lxc")
+	apiParams[1].Placement = instance.MustParsePlacement("lxc:0")
+	apiParams[1].ContainerType = instance.LXC
+	apiParams[2].Placement = instance.MustParsePlacement("dummyenv:invalid")
+	apiParams[3].Placement = instance.MustParsePlacement("dummyenv:valid")
+	machines, err := s.APIState.Client().AddMachines(apiParams)
+	c.Assert(err, gc.IsNil)
+	c.Assert(len(machines), gc.Equals, 4)
+	c.Assert(machines[0].Machine, gc.Equals, "0/lxc/0")
+	c.Assert(machines[1].Error, gc.ErrorMatches, "container type and placement are mutually exclusive")
+	c.Assert(machines[2].Error, gc.ErrorMatches, "cannot add a new machine: invalid placement is invalid")
+	c.Assert(machines[3].Machine, gc.Equals, "1")
+
+	m, err := s.BackingState.Machine(machines[3].Machine)
+	c.Assert(err, gc.IsNil)
+	c.Assert(m.Placement(), gc.DeepEquals, apiParams[3].Placement.Directive)
+}
+
+func (s *clientSuite) TestClientAddMachines1dot18(c *gc.C) {
+	apiParams := make([]params.AddMachineParams, 2)
+	for i := range apiParams {
+		apiParams[i] = params.AddMachineParams{
+			Jobs: []params.MachineJob{params.JobHostUnits},
+		}
+	}
+	apiParams[1].ContainerType = instance.LXC
+	apiParams[1].ParentId = "0"
+	machines, err := s.APIState.Client().AddMachines1dot18(apiParams)
+	c.Assert(err, gc.IsNil)
+	c.Assert(len(machines), gc.Equals, 2)
+	c.Assert(machines[0].Machine, gc.Equals, "0")
+	c.Assert(machines[1].Machine, gc.Equals, "0/lxc/0")
+}
+
+func (s *clientSuite) TestClientAddMachines1dot18SomeErrors(c *gc.C) {
+	apiParams := []params.AddMachineParams{{
+		Jobs:     []params.MachineJob{params.JobHostUnits},
+		ParentId: "123",
+	}}
+	machines, err := s.APIState.Client().AddMachines1dot18(apiParams)
+	c.Assert(err, gc.IsNil)
+	c.Assert(len(machines), gc.Equals, 1)
+	c.Check(machines[0].Error, gc.ErrorMatches, "parent machine specified without container type")
+}
+
 func (s *clientSuite) TestClientAddMachinesSomeErrors(c *gc.C) {
 	// Here we check that adding a number of containers correctly handles the
 	// case that some adds succeed and others fail and report the errors
 	// accordingly.
-	// We will set up params to the AddMachines API to attempt to create 4 machines.
+	// We will set up params to the AddMachines API to attempt to create 3 machines.
 	// Machines 0 and 1 will be added successfully.
-	// Mchines 2 and 3 will fail due to different reasons.
+	// Remaining machines will fail due to different reasons.
 
 	// Create a machine to host the requested containers.
 	host, err := s.State.AddMachine("quantal", state.JobHostUnits)
@@ -1707,32 +1759,26 @@
 	err = host.SetSupportedContainers([]instance.ContainerType{instance.LXC})
 	c.Assert(err, gc.IsNil)
 
-	// Set up params for adding 4 containers.
-	apiParams := make([]params.AddMachineParams, 4)
-	for i := 0; i < 4; i++ {
+	// Set up params for adding 3 containers.
+	apiParams := make([]params.AddMachineParams, 3)
+	for i := range apiParams {
 		apiParams[i] = params.AddMachineParams{
 			Jobs: []params.MachineJob{params.JobHostUnits},
 		}
 	}
-	// Make it so that machines 2 and 3 will fail to be added.
-	// This will cause a machine add to fail because of an invalid parent.
-	apiParams[2].ParentId = "123"
 	// This will cause a machine add to fail due to an unsupported container.
-	apiParams[3].ParentId = host.Id()
-	apiParams[3].ContainerType = instance.KVM
+	apiParams[2].ContainerType = instance.KVM
+	apiParams[2].ParentId = host.Id()
 	machines, err := s.APIState.Client().AddMachines(apiParams)
 	c.Assert(err, gc.IsNil)
-	c.Assert(len(machines), gc.Equals, 4)
+	c.Assert(len(machines), gc.Equals, 3)
 
 	// Check the results - machines 2 and 3 will have errors.
 	c.Check(machines[0].Machine, gc.Equals, "1")
 	c.Check(machines[0].Error, gc.IsNil)
 	c.Check(machines[1].Machine, gc.Equals, "2")
 	c.Check(machines[1].Error, gc.IsNil)
-	c.Assert(machines[2].Error, gc.NotNil)
-	c.Check(machines[2].Error, gc.ErrorMatches, "parent machine specified without container type")
-	c.Assert(machines[2].Error, gc.NotNil)
-	c.Check(machines[3].Error, gc.ErrorMatches, "cannot add a new machine: machine 0 cannot host kvm containers")
+	c.Check(machines[2].Error, gc.ErrorMatches, "cannot add a new machine: machine 0 cannot host kvm containers")
 }
 
 func (s *clientSuite) TestClientAddMachinesWithInstanceIdSomeErrors(c *gc.C) {

=== modified file 'state/apiserver/provisioner/provisioner.go'
--- state/apiserver/provisioner/provisioner.go	2014-04-18 15:31:56 +0000
+++ state/apiserver/provisioner/provisioner.go	2014-04-23 06:25:18 +0000
@@ -285,6 +285,49 @@
 	return result, nil
 }
 
+// ProvisioningInfo returns the provisioning information for each given machine entity.
+func (p *ProvisionerAPI) ProvisioningInfo(args params.Entities) (params.ProvisioningInfoResults, error) {
+	result := params.ProvisioningInfoResults{
+		Results: make([]params.ProvisioningInfoResult, len(args.Entities)),
+	}
+	canAccess, err := p.getAuthFunc()
+	if err != nil {
+		return result, err
+	}
+	for i, entity := range args.Entities {
+		machine, err := p.getMachine(canAccess, entity.Tag)
+		if err == nil {
+			result.Results[i].Result, err = getProvisioningInfo(machine)
+		}
+		result.Results[i].Error = common.ServerError(err)
+	}
+	return result, nil
+}
+
+func getProvisioningInfo(m *state.Machine) (*params.ProvisioningInfo, error) {
+	cons, err := m.Constraints()
+	if err != nil {
+		return nil, err
+	}
+	// TODO(dimitern) For now, since network names and
+	// provider ids are the same, we return what we got
+	// from state. In the future, when networks can be
+	// added before provisioning, we should convert both
+	// slices from juju network names to provider-specific
+	// ids before returning them.
+	includeNetworks, excludeNetworks, err := m.RequestedNetworks()
+	if err != nil {
+		return nil, err
+	}
+	return &params.ProvisioningInfo{
+		Constraints:     cons,
+		Series:          m.Series(),
+		Placement:       m.Placement(),
+		IncludeNetworks: includeNetworks,
+		ExcludeNetworks: excludeNetworks,
+	}, nil
+}
+
 // DistributionGroup returns, for each given machine entity,
 // a slice of instance.Ids that belong to the same distribution
 // group as that machine. This information may be used to

=== modified file 'state/apiserver/provisioner/provisioner_test.go'
--- state/apiserver/provisioner/provisioner_test.go	2014-04-18 15:31:56 +0000
+++ state/apiserver/provisioner/provisioner_test.go	2014-04-23 06:25:18 +0000
@@ -726,6 +726,87 @@
 	})
 }
 
+func (s *withoutStateServerSuite) TestProvisioningInfo(c *gc.C) {
+	template := state.MachineTemplate{
+		Series:          "quantal",
+		Jobs:            []state.MachineJob{state.JobHostUnits},
+		Constraints:     constraints.MustParse("cpu-cores=123", "mem=8G"),
+		Placement:       "valid",
+		IncludeNetworks: []string{"net1", "net2"},
+		ExcludeNetworks: []string{"net3", "net4"},
+	}
+	placementMachine, err := s.State.AddOneMachine(template)
+	c.Assert(err, gc.IsNil)
+
+	args := params.Entities{Entities: []params.Entity{
+		{Tag: s.machines[0].Tag()},
+		{Tag: placementMachine.Tag()},
+		{Tag: "machine-42"},
+		{Tag: "unit-foo-0"},
+		{Tag: "service-bar"},
+	}}
+	result, err := s.provisioner.ProvisioningInfo(args)
+	c.Assert(err, gc.IsNil)
+	c.Assert(result, gc.DeepEquals, params.ProvisioningInfoResults{
+		Results: []params.ProvisioningInfoResult{
+			{
+				Result: &params.ProvisioningInfo{
+					Series:          "quantal",
+					IncludeNetworks: []string{},
+					ExcludeNetworks: []string{},
+				},
+			},
+			{
+				Result: &params.ProvisioningInfo{
+					Series:          "quantal",
+					Constraints:     template.Constraints,
+					Placement:       template.Placement,
+					IncludeNetworks: template.IncludeNetworks,
+					ExcludeNetworks: template.ExcludeNetworks,
+				},
+			},
+			{Error: apiservertesting.NotFoundError("machine 42")},
+			{Error: apiservertesting.ErrUnauthorized},
+			{Error: apiservertesting.ErrUnauthorized},
+		},
+	})
+}
+
+func (s *withoutStateServerSuite) TestProvisioningInfoPermissions(c *gc.C) {
+	// Login as a machine agent for machine 0.
+	anAuthorizer := s.authorizer
+	anAuthorizer.MachineAgent = true
+	anAuthorizer.EnvironManager = false
+	anAuthorizer.Tag = s.machines[0].Tag()
+	aProvisioner, err := provisioner.NewProvisionerAPI(s.State, s.resources, anAuthorizer)
+	c.Assert(err, gc.IsNil)
+	c.Assert(aProvisioner, gc.NotNil)
+
+	args := params.Entities{Entities: []params.Entity{
+		{Tag: s.machines[0].Tag()},
+		{Tag: s.machines[0].Tag() + "-lxc-0"},
+		{Tag: "machine-42"},
+		{Tag: s.machines[1].Tag()},
+		{Tag: "service-bar"},
+	}}
+
+	// Only machine 0 and containers therein can be accessed.
+	results, err := aProvisioner.ProvisioningInfo(args)
+	c.Assert(results, gc.DeepEquals, params.ProvisioningInfoResults{
+		Results: []params.ProvisioningInfoResult{
+			{Result: &params.ProvisioningInfo{
+				Series:          "quantal",
+				IncludeNetworks: []string{},
+				ExcludeNetworks: []string{},
+			}},
+			{Error: apiservertesting.NotFoundError("machine 0/lxc/0")},
+			{Error: apiservertesting.ErrUnauthorized},
+			{Error: apiservertesting.ErrUnauthorized},
+			{Error: apiservertesting.ErrUnauthorized},
+		},
+	})
+}
+
 func (s *withoutStateServerSuite) TestConstraints(c *gc.C) {
 	// Add a machine with some constraints.
 	template := state.MachineTemplate{

=== modified file 'state/machine.go'
--- state/machine.go	2014-04-21 23:10:05 +0000
+++ state/machine.go	2014-04-23 06:25:18 +0000
@@ -114,6 +114,9 @@
 	// machine is capable of hosting.
 	SupportedContainersKnown bool
 	SupportedContainers      []instance.ContainerType `bson:",omitempty"`
+	// Placement is the placement directive that should be used when provisioning
+	// an instance for the machine.
+	Placement string `bson:",omitempty"`
 	// Deprecated. InstanceId, now lives on instanceData.
 	// This attribute is retained so that data from existing machines can be read.
 	// SCHEMACHANGE
@@ -1090,6 +1093,12 @@
 	return m.doc.Id
 }
 
+// Placement returns the machine's Placement structure that should be used when
+// provisioning an instance for the machine.
+func (m *Machine) Placement() string {
+	return m.doc.Placement
+}
+
 // Constraints returns the exact constraints that should apply when provisioning
 // an instance for the machine.
 func (m *Machine) Constraints() (constraints.Value, error) {

=== modified file 'state/policy.go'
--- state/policy.go	2014-04-21 23:24:57 +0000
+++ state/policy.go	2014-04-23 06:25:18 +0000
@@ -49,7 +49,7 @@
 	// all invalid parameters. If PrecheckInstance returns nil, it is not
 	// guaranteed that the constraints are valid; if a non-nil error is
 	// returned, then the constraints are definitely invalid.
-	PrecheckInstance(series string, cons constraints.Value) error
+	PrecheckInstance(series string, cons constraints.Value, placement string) error
 }
 
 // ConfigValidator is a policy interface that is provided to State
@@ -78,7 +78,7 @@
 
 // precheckInstance calls the state's assigned policy, if non-nil, to obtain
 // a Prechecker, and calls PrecheckInstance if a non-nil Prechecker is returned.
-func (st *State) precheckInstance(series string, cons constraints.Value) error {
+func (st *State) precheckInstance(series string, cons constraints.Value, placement string) error {
 	if st.policy == nil {
 		return nil
 	}
@@ -95,7 +95,7 @@
 	if prechecker == nil {
 		return fmt.Errorf("policy returned nil prechecker without an error")
 	}
-	return prechecker.PrecheckInstance(series, cons)
+	return prechecker.PrecheckInstance(series, cons, placement)
 }
 
 func (st *State) constraintsValidator() (constraints.Validator, error) {

=== modified file 'state/prechecker_test.go'
--- state/prechecker_test.go	2014-04-21 23:10:05 +0000
+++ state/prechecker_test.go	2014-04-23 06:25:18 +0000
@@ -26,11 +26,13 @@
 	precheckInstanceError       error
 	precheckInstanceSeries      string
 	precheckInstanceConstraints constraints.Value
+	precheckInstancePlacement   string
 }
 
-func (p *mockPrechecker) PrecheckInstance(series string, cons constraints.Value) error {
+func (p *mockPrechecker) PrecheckInstance(series string, cons constraints.Value, placement string) error {
 	p.precheckInstanceSeries = series
 	p.precheckInstanceConstraints = cons
+	p.precheckInstancePlacement = placement
 	return p.precheckInstanceError
 }
 
@@ -44,13 +46,15 @@
 
 func (s *PrecheckerSuite) TestPrecheckInstance(c *gc.C) {
 	// PrecheckInstance should be called with the specified
-	// series, and the specified constraints merged with the
-	// environment constraints, when attempting to create an
-	// instance.
+	// series and placement, and the specified constraints
+	// merged with the environment constraints, when attempting
+	// to create an instance.
 	envCons := constraints.MustParse("mem=4G")
-	template, err := s.addOneMachine(c, envCons)
+	placement := "abc123"
+	template, err := s.addOneMachine(c, envCons, placement)
 	c.Assert(err, gc.IsNil)
 	c.Assert(s.prechecker.precheckInstanceSeries, gc.Equals, template.Series)
+	c.Assert(s.prechecker.precheckInstancePlacement, gc.Equals, placement)
 	validator := constraints.NewValidator()
 	cons, err := validator.Merge(envCons, template.Constraints)
 	c.Assert(err, gc.IsNil)
@@ -60,14 +64,14 @@
 func (s *PrecheckerSuite) TestPrecheckErrors(c *gc.C) {
 	// Ensure that AddOneMachine fails when PrecheckInstance returns an error.
 	s.prechecker.precheckInstanceError = fmt.Errorf("no instance for you")
-	_, err := s.addOneMachine(c, constraints.Value{})
+	_, err := s.addOneMachine(c, constraints.Value{}, "placement")
 	c.Assert(err, gc.ErrorMatches, ".*no instance for you")
 
 	// If the policy's Prechecker method fails, that will be returned first.
 	s.policy.getPrechecker = func(*config.Config) (state.Prechecker, error) {
 		return nil, fmt.Errorf("no prechecker for you")
 	}
-	_, err = s.addOneMachine(c, constraints.Value{})
+	_, err = s.addOneMachine(c, constraints.Value{}, "placement")
 	c.Assert(err, gc.ErrorMatches, ".*no prechecker for you")
 }
 
@@ -76,10 +80,10 @@
 	s.policy.getPrechecker = func(*config.Config) (state.Prechecker, error) {
 		return nil, precheckerErr
 	}
-	_, err := s.addOneMachine(c, constraints.Value{})
+	_, err := s.addOneMachine(c, constraints.Value{}, "placement")
 	c.Assert(err, gc.ErrorMatches, "cannot add a new machine: policy returned nil prechecker without an error")
 	precheckerErr = errors.NotImplementedf("Prechecker")
-	_, err = s.addOneMachine(c, constraints.Value{})
+	_, err = s.addOneMachine(c, constraints.Value{}, "placement")
 	c.Assert(err, gc.IsNil)
 }
 
@@ -89,11 +93,11 @@
 		return nil, nil
 	}
 	state.SetPolicy(s.State, nil)
-	_, err := s.addOneMachine(c, constraints.Value{})
+	_, err := s.addOneMachine(c, constraints.Value{}, "placement")
 	c.Assert(err, gc.IsNil)
 }
 
-func (s *PrecheckerSuite) addOneMachine(c *gc.C, envCons constraints.Value) (state.MachineTemplate, error) {
+func (s *PrecheckerSuite) addOneMachine(c *gc.C, envCons constraints.Value, placement string) (state.MachineTemplate, error) {
 	err := s.State.SetEnvironConstraints(envCons)
 	c.Assert(err, gc.IsNil)
 	oneJob := []state.MachineJob{state.JobHostUnits}
@@ -102,6 +106,7 @@
 		Series:      "precise",
 		Constraints: extraCons,
 		Jobs:        oneJob,
+		Placement:   placement,
 	}
 	_, err = s.State.AddOneMachine(template)
 	return template, err
@@ -113,22 +118,26 @@
 		Series:     "precise",
 		Nonce:      state.BootstrapNonce,
 		Jobs:       []state.MachineJob{state.JobManageEnviron},
+		Placement:  "anyoldthing",
 	}
 	_, err := s.State.AddOneMachine(template)
 	c.Assert(err, gc.IsNil)
 	// PrecheckInstance should not have been called, as we've
 	// injected a machine with an existing instance.
 	c.Assert(s.prechecker.precheckInstanceSeries, gc.Equals, "")
+	c.Assert(s.prechecker.precheckInstancePlacement, gc.Equals, "")
 }
 
 func (s *PrecheckerSuite) TestPrecheckContainerNewMachine(c *gc.C) {
 	// Attempting to add a container to a new machine should cause
 	// PrecheckInstance to be called.
 	template := state.MachineTemplate{
-		Series: "precise",
-		Jobs:   []state.MachineJob{state.JobHostUnits},
+		Series:    "precise",
+		Jobs:      []state.MachineJob{state.JobHostUnits},
+		Placement: "intertubes",
 	}
 	_, err := s.State.AddMachineInsideNewMachine(template, template, instance.LXC)
 	c.Assert(err, gc.IsNil)
 	c.Assert(s.prechecker.precheckInstanceSeries, gc.Equals, template.Series)
+	c.Assert(s.prechecker.precheckInstancePlacement, gc.Equals, template.Placement)
 }

=== modified file 'worker/provisioner/provisioner_task.go'
--- worker/provisioner/provisioner_task.go	2014-04-18 16:37:28 +0000
+++ worker/provisioner/provisioner_task.go	2014-04-23 06:25:18 +0000
@@ -5,6 +5,7 @@
 
 import (
 	"fmt"
+	"time"
 
 	"launchpad.net/tomb"
 
@@ -450,26 +451,19 @@
 }
 
 func (task *provisionerTask) startMachine(machine *apiprovisioner.Machine) error {
-	cons, err := machine.Constraints()
-	if err != nil {
-		return err
-	}
-	series, err := machine.Series()
-	if err != nil {
-		return err
-	}
-	possibleTools, err := task.possibleTools(series, cons)
-	if err != nil {
-		return err
-	}
-	machineConfig, err := task.machineConfig(machine)
+	provisioningInfo, err := task.provisioningInfo(machine)
+	if err != nil {
+		return err
+	}
+	possibleTools, err := task.possibleTools(provisioningInfo.Series, provisioningInfo.Constraints)
 	if err != nil {
 		return err
 	}
 	inst, metadata, networkInfo, err := task.broker.StartInstance(environs.StartInstanceParams{
-		Constraints:       cons,
+		Constraints:       provisioningInfo.Constraints,
 		Tools:             possibleTools,
-		MachineConfig:     machineConfig,
+		MachineConfig:     provisioningInfo.MachineConfig,
+		Placement:         provisioningInfo.Placement,
 		DistributionGroup: machine.DistributionGroup,
 	})
 	if err != nil {
@@ -478,7 +472,7 @@
 		// error; just keep going with the other machines.
 		return task.setErrorStatus("cannot start instance for machine %q: %v", machine, err)
 	}
-	nonce := machineConfig.MachineNonce
+	nonce := provisioningInfo.MachineConfig.MachineNonce
 	networks, ifaces := task.prepareNetworkAndInterfaces(networkInfo)
 
 	err = machine.SetInstanceInfo(inst.Id(), nonce, metadata, networks, ifaces)
@@ -512,7 +506,14 @@
 	panic(fmt.Errorf("broker of type %T does not provide any tools", task.broker))
 }
 
-func (task *provisionerTask) machineConfig(machine *apiprovisioner.Machine) (*cloudinit.MachineConfig, error) {
+type provisioningInfo struct {
+	Constraints   constraints.Value
+	Series        string
+	Placement     string
+	MachineConfig *cloudinit.MachineConfig
+}
+
+func (task *provisionerTask) provisioningInfo(machine *apiprovisioner.Machine) (*provisioningInfo, error) {
 	stateInfo, apiInfo, err := task.auth.SetupAuthentication(machine)
 	if err != nil {
 		logger.Errorf("failed to setup authentication: %v", err)
@@ -525,11 +526,32 @@
 	if err != nil {
 		return nil, err
 	}
-	includeNetworks, excludeNetworks, err := machine.RequestedNetworks()
-	if err != nil {
+	// ProvisioningInfo is new in 1.20; wait for the API server to be upgraded
+	// so we don't spew errors on upgrade.
+	var pInfo *params.ProvisioningInfo
+	for {
+		if pInfo, err = machine.ProvisioningInfo(); err == nil {
+			break
+		}
+		if params.IsCodeNotImplemented(err) {
+			logger.Infof("waiting for state server to be upgraded")
+			select {
+			case <-task.tomb.Dying():
+				return nil, tomb.ErrDying
+			case <-time.After(15 * time.Second):
+				continue
+			}
+		}
 		return nil, err
 	}
+	includeNetworks := pInfo.IncludeNetworks
+	excludeNetworks := pInfo.ExcludeNetworks
 	nonce := fmt.Sprintf("%s:%s", task.machineTag, uuid.String())
 	machineConfig := environs.NewMachineConfig(machine.Id(), nonce, includeNetworks, excludeNetworks, stateInfo, apiInfo)
-	return machineConfig, nil
+	return &provisioningInfo{
+		Constraints:   pInfo.Constraints,
+		Series:        pInfo.Series,
+		Placement:     pInfo.Placement,
+		MachineConfig: machineConfig,
+	}, nil
 }