Code review comment for lp:~axwalk/juju-core/addmachine-placement

Revision history for this message
Andrew Wilkins (axwalk) wrote :

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

« Back to merge proposal