juju-core

Merge lp:~axwalk/juju-core/fix-supportsunitplacement-injectmachine into lp:~go-bot/juju-core/trunk

  1. fix-supportsunitplacement-injectmachine
  2. Merge into trunk
Proposed by Andrew Wilkins
Status: Merged
Approved by: Andrew Wilkins
Approved revision: no longer in the source branch.
Merged at revision: 2557
Proposed branch: lp:~axwalk/juju-core/fix-supportsunitplacement-injectmachine
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 57 lines (+19/-3)
2 files modified
state/addmachine.go (+2/-2)
state/environcapability_test.go (+17/-1)
To merge this branch: bzr merge lp:~axwalk/juju-core/fix-supportsunitplacement-injectmachine
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+213989@code.launchpad.net

Commit message

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://codereview.appspot.com/83940043/

Description of the change

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://codereview.appspot.com/83940043/

To post a comment you must log in.
Revision history for this message
Andrew Wilkins (axwalk) wrote :

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)

Revision history for this message
William Reade (fwereade) wrote :

LGTM

https://codereview.appspot.com/83940043/diff/1/state/addmachine.go
File state/addmachine.go (left):

https://codereview.appspot.com/83940043/diff/1/state/addmachine.go#oldcode137
state/addmachine.go:137: // only permitted unit placement is supported.
s/permitted/permitted if/

https://codereview.appspot.com/83940043/

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

Please take a look.

https://codereview.appspot.com/83940043/diff/1/state/addmachine.go
File state/addmachine.go (left):

https://codereview.appspot.com/83940043/diff/1/state/addmachine.go#oldcode137
state/addmachine.go:137: // only permitted unit placement is supported.
On 2014/04/03 10:06:57, fwereade wrote:
> s/permitted/permitted if/

Done.

https://codereview.appspot.com/83940043/

Revision history for this message
Go Bot (go-bot) wrote :
Download full text (56.9 KiB)

The attempt to merge lp:~axwalk/juju-core/fix-supportsunitplacement-injectmachine into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.154s
ok launchpad.net/juju-core/agent/mongo 0.546s
ok launchpad.net/juju-core/agent/tools 0.200s
ok launchpad.net/juju-core/bzr 5.335s
ok launchpad.net/juju-core/cert 2.499s
ok launchpad.net/juju-core/charm 0.393s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.029s
ok launchpad.net/juju-core/cloudinit/sshinit 0.781s
ok launchpad.net/juju-core/cmd 0.143s
ok launchpad.net/juju-core/cmd/charm-admin 0.723s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.188s
ok launchpad.net/juju-core/cmd/juju 213.975s
ok launchpad.net/juju-core/cmd/jujud 66.483s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 10.112s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.151s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.021s
ok launchpad.net/juju-core/container 0.029s
ok launchpad.net/juju-core/container/factory 0.030s
ok launchpad.net/juju-core/container/kvm 0.193s
ok launchpad.net/juju-core/container/kvm/mock 0.033s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.395s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.220s
ok launchpad.net/juju-core/environs 2.340s
ok launchpad.net/juju-core/environs/bootstrap 12.295s
ok launchpad.net/juju-core/environs/cloudinit 0.529s
ok launchpad.net/juju-core/environs/config 1.963s
ok launchpad.net/juju-core/environs/configstore 0.036s
ok launchpad.net/juju-core/environs/filestorage 0.028s
ok launchpad.net/juju-core/environs/httpstorage 0.643s
ok launchpad.net/juju-core/environs/imagemetadata 0.431s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.036s
ok launchpad.net/juju-core/environs/jujutest 0.149s
ok launchpad.net/juju-core/environs/manual 16.745s
ok launchpad.net/juju-core/environs/simplestreams 0.260s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.898s
ok launchpad.net/juju-core/environs/storage 0.908s
ok launchpad.net/juju-core/environs/sync 48.600s
ok launchpad.net/juju-core/environs/testing 0.168s
ok launchpad.net/juju-core/environs/tools 4.809s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.013s
ok launchpad.net/juju-core/instance 0.021s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juj...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'state/addmachine.go'
2--- state/addmachine.go 2014-04-03 04:03:00 +0000
3+++ state/addmachine.go 2014-04-03 10:10:30 +0000
4@@ -134,8 +134,8 @@
5 var mdocs []*machineDoc
6 for _, template := range templates {
7 // Adding a machine without any principals is
8- // only permitted unit placement is supported.
9- if len(template.principals) == 0 {
10+ // only permitted if unit placement is supported.
11+ if len(template.principals) == 0 && template.InstanceId == "" {
12 if err := st.supportsUnitPlacement(); err != nil {
13 return nil, err
14 }
15
16=== modified file 'state/environcapability_test.go'
17--- state/environcapability_test.go 2014-04-03 03:37:45 +0000
18+++ state/environcapability_test.go 2014-04-03 10:10:30 +0000
19@@ -52,6 +52,15 @@
20 })
21 }
22
23+func (s *EnvironCapabilitySuite) addOneMachineWithInstanceId(c *gc.C) (*state.Machine, error) {
24+ return s.State.AddOneMachine(state.MachineTemplate{
25+ Series: "quantal",
26+ Jobs: []state.MachineJob{state.JobHostUnits},
27+ InstanceId: "i-rate",
28+ Nonce: "ya",
29+ })
30+}
31+
32 func (s *EnvironCapabilitySuite) addMachineInsideNewMachine(c *gc.C) error {
33 template := state.MachineTemplate{
34 Series: "quantal",
35@@ -62,7 +71,7 @@
36 }
37
38 func (s *EnvironCapabilitySuite) TestSupportsUnitPlacementAddMachine(c *gc.C) {
39- // Ensure that AddOneMachine fails when PrecheckInstance returns an error.
40+ // Ensure that AddOneMachine fails when SupportsUnitPlacement returns an error.
41 s.capability.supportsUnitPlacementError = fmt.Errorf("no add-machine for you")
42 _, err := s.addOneMachine(c)
43 c.Assert(err, gc.ErrorMatches, ".*no add-machine for you")
44@@ -76,6 +85,13 @@
45 c.Assert(err, gc.ErrorMatches, ".*incapable of EnvironCapability")
46 }
47
48+func (s *EnvironCapabilitySuite) TestSupportsUnitPlacementAddMachineInstanceId(c *gc.C) {
49+ // Ensure that AddOneMachine with a non-empty InstanceId does not fail.
50+ s.capability.supportsUnitPlacementError = fmt.Errorf("no add-machine for you")
51+ _, err := s.addOneMachineWithInstanceId(c)
52+ c.Assert(err, gc.IsNil)
53+}
54+
55 func (s *EnvironCapabilitySuite) TestSupportsUnitPlacementUnitAssignment(c *gc.C) {
56 m, err := s.addOneMachine(c)
57 c.Assert(err, gc.IsNil)

Subscribers

People subscribed via source and target branches

to status/vote changes: