Code review comment for lp:~axwalk/juju-core/fix-supportsunitplacement-injectmachine

Reviewers: mp+213989_code.launchpad.net,

Message:
Please take a look.

Description:
state: no pre-checks for injected machines

We should not be second guessing the bootstrap agent or
manual provisioning. If the instance-id is non-empty when
adding a machine to state, do not check SupportsUnitPlacement.

https://code.launchpad.net/~axwalk/juju-core/fix-supportsunitplacement-injectmachine/+merge/213989

(do not edit description out of merge proposal)

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

Affected files (+20, -2 lines):
   A [revision details]
   M state/addmachine.go
   M state/environcapability_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140403093134-znbl1cwzwu3cb3es
+New revision: <email address hidden>

Index: state/addmachine.go
=== modified file 'state/addmachine.go'
--- state/addmachine.go 2014-04-03 04:03:00 +0000
+++ state/addmachine.go 2014-04-03 09:52:10 +0000
@@ -135,7 +135,7 @@
   for _, template := range templates {
    // Adding a machine without any principals is
    // only permitted unit placement is supported.
- if len(template.principals) == 0 {
+ if len(template.principals) == 0 && template.InstanceId == "" {
     if err := st.supportsUnitPlacement(); err != nil {
      return nil, err
     }

Index: state/environcapability_test.go
=== modified file 'state/environcapability_test.go'
--- state/environcapability_test.go 2014-04-03 03:37:45 +0000
+++ state/environcapability_test.go 2014-04-03 09:52:10 +0000
@@ -52,6 +52,15 @@
   })
  }

+func (s *EnvironCapabilitySuite) addOneMachineWithInstanceId(c *gc.C)
(*state.Machine, error) {
+ return s.State.AddOneMachine(state.MachineTemplate{
+ Series: "quantal",
+ Jobs: []state.MachineJob{state.JobHostUnits},
+ InstanceId: "i-rate",
+ Nonce: "ya",
+ })
+}
+
  func (s *EnvironCapabilitySuite) addMachineInsideNewMachine(c *gc.C) error
{
   template := state.MachineTemplate{
    Series: "quantal",
@@ -62,7 +71,7 @@
  }

  func (s *EnvironCapabilitySuite) TestSupportsUnitPlacementAddMachine(c
*gc.C) {
- // Ensure that AddOneMachine fails when PrecheckInstance returns an error.
+ // Ensure that AddOneMachine fails when SupportsUnitPlacement returns an
error.
   s.capability.supportsUnitPlacementError = fmt.Errorf("no add-machine for
you")
   _, err := s.addOneMachine(c)
   c.Assert(err, gc.ErrorMatches, ".*no add-machine for you")
@@ -76,6 +85,13 @@
   c.Assert(err, gc.ErrorMatches, ".*incapable of EnvironCapability")
  }

+func (s *EnvironCapabilitySuite)
TestSupportsUnitPlacementAddMachineInstanceId(c *gc.C) {
+ // Ensure that AddOneMachine with a non-empty InstanceId does not fail.
+ s.capability.supportsUnitPlacementError = fmt.Errorf("no add-machine for
you")
+ _, err := s.addOneMachineWithInstanceId(c)
+ c.Assert(err, gc.IsNil)
+}
+
  func (s *EnvironCapabilitySuite) TestSupportsUnitPlacementUnitAssignment(c
*gc.C) {
   m, err := s.addOneMachine(c)
   c.Assert(err, gc.IsNil)