juju-core

Merge lp:~axwalk/juju-core/lp1266748-state-environment-isalive-1.16 into lp:~go-bot/juju-core/trunk

  1. lp1266748-state-environment-isalive-1.16
  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: 2191
Proposed branch: lp:~axwalk/juju-core/lp1266748-state-environment-isalive-1.16
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 102 lines (+79/-2)
2 files modified
state/compat_test.go (+68/-0)
state/environ.go (+11/-2)
To merge this branch: bzr merge lp:~axwalk/juju-core/lp1266748-state-environment-isalive-1.16
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+200759@code.launchpad.net

Commit message

state: fix Environ life assertion to handle null

If the life field is missing, it should be
considered Alive.

Fixes #1266748

https://codereview.appspot.com/48670045/

Description of the change

state: fix Environ life assertion to handle null

If the life field is missing, it should be
considered Alive.

Fixes #1266748

https://codereview.appspot.com/48670045/

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

Reviewers: mp+200759_code.launchpad.net,

Message:
Please take a look.

Description:
state: fix Environ life assertion to handle null

If the life field is missing, it should be
considered Alive.

Fixes #1266748

https://code.launchpad.net/~axwalk/juju-core/lp1266748-state-environment-isalive-1.16/+merge/200759

(do not edit description out of merge proposal)

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

Affected files (+81, -2 lines):
   A [revision details]
   A state/compat_test.go
   M state/environ.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-20140106181553-6uzfea71izl68d4l
+New revision: <email address hidden>

Index: state/compat_test.go
=== added file 'state/compat_test.go'
--- state/compat_test.go 1970-01-01 00:00:00 +0000
+++ state/compat_test.go 2014-01-08 02:46:32 +0000
@@ -0,0 +1,68 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package state
+
+import (
+ "labix.org/v2/mgo/txn"
+ gc "launchpad.net/gocheck"
+
+ "launchpad.net/juju-core/testing"
+ "launchpad.net/juju-core/testing/testbase"
+)
+
+// compatSuite contains backwards compatibility tests,
+// for ensuring state operations behave correctly across
+// schema changes.
+type compatSuite struct {
+ testbase.LoggingSuite
+ testing.MgoSuite
+ state *State
+ env *Environment
+}
+
+var _ = gc.Suite(&compatSuite{})
+
+func (s *compatSuite) SetUpSuite(c *gc.C) {
+ s.LoggingSuite.SetUpSuite(c)
+ s.MgoSuite.SetUpSuite(c)
+}
+
+func (s *compatSuite) TearDownSuite(c *gc.C) {
+ s.MgoSuite.TearDownSuite(c)
+ s.LoggingSuite.TearDownSuite(c)
+}
+
+func (s *compatSuite) SetUpTest(c *gc.C) {
+ s.LoggingSuite.SetUpTest(c)
+ s.MgoSuite.SetUpTest(c)
+ s.state = TestingInitialize(c, nil)
+ env, err := s.state.Environment()
+ c.Assert(err, gc.IsNil)
+ s.env = env
+}
+
+func (s *compatSuite) TearDownTest(c *gc.C) {
+ s.state.Close()
+ s.MgoSuite.TearDownTest(c)
+ s.LoggingSuite.TearDownTest(c)
+}
+
+func (s *compatSuite) TestEnvironAssertAlive(c *gc.C) {
+ // 1.17+ has a "Life" field in environment documents.
+ // We remove it here, to test 1.16 compatibility.
+ ops := []txn.Op{{
+ C: s.state.environments.Name,
+ Id: s.env.doc.UUID,
+ Update: D{{"$unset", D{{"life", Dying}}}},
+ }}
+ err := s.state.runTransaction(ops)
+ c.Assert(err, gc.IsNil)
+
+ // Now check the assertAliveOp and Destroy work as if
+ // the environment is Alive.
+ err = s.state.runTransaction([]txn.Op{s.env.assertAliveOp()})
+ c.Assert(err, gc.IsNil)
+ err = s.env.Destroy()
+ c.Assert(err, gc.IsNil)
+}

Index: state/environ.go
=== modified file 'state/environ.go'
--- state/environ.go 2013-12-11 08:50:47 +0000
+++ state/environ.go 2014-01-08 02:46:32 +0000
@@ -98,7 +98,7 @@
    C: e.st.environments.Name,
    Id: e.doc.UUID,
    Update: D{{"$set", D{{"life", Dying}}}},
- Assert: isAliveDoc,
+ Assert: isEnvAliveDoc,
   }, e.st.newCleanupOp("services", "")}
   err := e.st.runTransaction(ops)
   switch err {
@@ -130,6 +130,15 @@
   return txn...

Read more...

Revision history for this message
John A Meinel (jameinel) wrote :

LGTM, with a small question. (most of the unset examples I've seen do
pass something, but it is "")

https://codereview.appspot.com/48670045/diff/1/state/compat_test.go
File state/compat_test.go (right):

https://codereview.appspot.com/48670045/diff/1/state/compat_test.go#newcode57
state/compat_test.go:57: Update: D{{"$unset", D{{"life", Dying}}}},
It seems odd to pass a value here for unset.

https://codereview.appspot.com/48670045/

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

Please take a look.

https://codereview.appspot.com/48670045/diff/1/state/compat_test.go
File state/compat_test.go (right):

https://codereview.appspot.com/48670045/diff/1/state/compat_test.go#newcode57
state/compat_test.go:57: Update: D{{"$unset", D{{"life", Dying}}}},
On 2014/01/09 10:24:21, jameinel wrote:
> It seems odd to pass a value here for unset.

Changed to nil.

https://codereview.appspot.com/48670045/

Revision history for this message
Go Bot (go-bot) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

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

The attempt to merge lp:~axwalk/juju-core/lp1266748-state-environment-isalive-1.16 into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.764s
ok launchpad.net/juju-core/agent/tools 0.221s
ok launchpad.net/juju-core/bzr 7.104s
ok launchpad.net/juju-core/cert 3.493s
ok launchpad.net/juju-core/charm 0.575s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.071s
ok launchpad.net/juju-core/cloudinit/sshinit 1.142s
ok launchpad.net/juju-core/cmd 0.260s
ok launchpad.net/juju-core/cmd/charm-admin 0.825s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 190.521s
ok launchpad.net/juju-core/cmd/jujud 60.514s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 3.341s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/constraints 0.027s
ok launchpad.net/juju-core/container 0.050s
ok launchpad.net/juju-core/container/factory 0.071s
ok launchpad.net/juju-core/container/kvm 0.294s
ok launchpad.net/juju-core/container/kvm/mock 0.053s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 0.355s
? 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.332s
ok launchpad.net/juju-core/environs 3.249s
ok launchpad.net/juju-core/environs/bootstrap 4.598s
ok launchpad.net/juju-core/environs/cloudinit 0.899s
ok launchpad.net/juju-core/environs/config 1.116s
ok launchpad.net/juju-core/environs/configstore 0.106s
ok launchpad.net/juju-core/environs/filestorage 0.032s
ok launchpad.net/juju-core/environs/httpstorage 0.953s
ok launchpad.net/juju-core/environs/imagemetadata 0.570s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.087s
ok launchpad.net/juju-core/environs/jujutest 0.226s
ok launchpad.net/juju-core/environs/manual 10.313s
ok launchpad.net/juju-core/environs/simplestreams 0.407s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 1.265s
ok launchpad.net/juju-core/environs/storage 1.215s
ok launchpad.net/juju-core/environs/sync 31.628s
ok launchpad.net/juju-core/environs/testing 0.207s
ok launchpad.net/juju-core/environs/tools 6.927s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.016s
ok launchpad.net/juju-core/instance 0.024s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 16.139s
ok launchpad.net/juju-core/juju/osenv 0.019s
? launchpad.net/juju-core/juju/testing [no test files]
ok launchpad.net/juju-core/log 0.016s
ok launchpad.net/juju-core/log/syslog 0.025s
ok launchpad.net/juju-core/names 0.025s
? launchpad.net/juju-core/provider [no test files]
? launch...

Read more...

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

The attempt to merge lp:~axwalk/juju-core/lp1266748-state-environment-isalive-1.16 into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 0.658s
ok launchpad.net/juju-core/agent/tools 0.257s
ok launchpad.net/juju-core/bzr 7.130s
ok launchpad.net/juju-core/cert 3.362s
ok launchpad.net/juju-core/charm 0.613s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.068s
ok launchpad.net/juju-core/cloudinit/sshinit 1.132s
ok launchpad.net/juju-core/cmd 0.273s
ok launchpad.net/juju-core/cmd/charm-admin 0.809s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 181.489s

----------------------------------------------------------------------
FAIL: machine_test.go:322: MachineSuite.TestManageEnvironRunsAddressUpdater

[LOG] 88.99443 DEBUG juju.environs.configstore Making /tmp/gocheck-1905388747193831650/31/home/ubuntu/.juju/environments
[LOG] 89.09040 DEBUG juju.environs.tools reading v1.* tools
[LOG] 89.09045 INFO juju environs/testing: uploading FAKE tools 1.17.1-precise-amd64
[LOG] 89.09141 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 89.09143 DEBUG juju.environs.tools no series specified when finding tools, looking for any
[LOG] 89.09150 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found not found
[LOG] 89.09152 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.sjson": invalid URL "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.sjson" not found
[LOG] 89.09156 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.json": file "tools/streams/v1/index.json" not found not found
[LOG] 89.09158 DEBUG juju.environs.simplestreams cannot load index "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.json": invalid URL "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.json" not found
[LOG] 89.09187 INFO juju.environs.tools Writing tools/streams/v1/index.json
[LOG] 89.09193 INFO juju.environs.tools Writing tools/streams/v1/com.ubuntu.juju:released:tools.json
[LOG] 89.09206 INFO juju.environs.bootstrap bootstrapping environment "dummyenv"
[LOG] 89.09209 DEBUG juju.environs.bootstrap looking for bootstrap tools: series="precise", arch=<nil>, version=1.17.1
[LOG] 89.09211 INFO juju.environs.tools reading tools with major.minor version 1.17
[LOG] 89.09212 INFO juju.environs.tools filtering tools by version: 1.17.1
[LOG] 89.09212 INFO juju.environs.tools filtering tools by series: precise
[LOG] 89.09214 DEBUG juju.environs.tools no architecture specified when finding tools, looking for any
[LOG] 89.09218 DEBUG juju.environs.simplestreams fetchData failed for "http://127.0.0.1:54146/dummyenv/private/tools/streams/v1/index.sjson": file "tools/streams/v1/index.sjson" not found not found
[LOG] 89.09220 DEBUG juju.environs.simpl...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'state/compat_test.go'
2--- state/compat_test.go 1970-01-01 00:00:00 +0000
3+++ state/compat_test.go 2014-01-09 10:38:29 +0000
4@@ -0,0 +1,68 @@
5+// Copyright 2014 Canonical Ltd.
6+// Licensed under the AGPLv3, see LICENCE file for details.
7+
8+package state
9+
10+import (
11+ "labix.org/v2/mgo/txn"
12+ gc "launchpad.net/gocheck"
13+
14+ "launchpad.net/juju-core/testing"
15+ "launchpad.net/juju-core/testing/testbase"
16+)
17+
18+// compatSuite contains backwards compatibility tests,
19+// for ensuring state operations behave correctly across
20+// schema changes.
21+type compatSuite struct {
22+ testbase.LoggingSuite
23+ testing.MgoSuite
24+ state *State
25+ env *Environment
26+}
27+
28+var _ = gc.Suite(&compatSuite{})
29+
30+func (s *compatSuite) SetUpSuite(c *gc.C) {
31+ s.LoggingSuite.SetUpSuite(c)
32+ s.MgoSuite.SetUpSuite(c)
33+}
34+
35+func (s *compatSuite) TearDownSuite(c *gc.C) {
36+ s.MgoSuite.TearDownSuite(c)
37+ s.LoggingSuite.TearDownSuite(c)
38+}
39+
40+func (s *compatSuite) SetUpTest(c *gc.C) {
41+ s.LoggingSuite.SetUpTest(c)
42+ s.MgoSuite.SetUpTest(c)
43+ s.state = TestingInitialize(c, nil)
44+ env, err := s.state.Environment()
45+ c.Assert(err, gc.IsNil)
46+ s.env = env
47+}
48+
49+func (s *compatSuite) TearDownTest(c *gc.C) {
50+ s.state.Close()
51+ s.MgoSuite.TearDownTest(c)
52+ s.LoggingSuite.TearDownTest(c)
53+}
54+
55+func (s *compatSuite) TestEnvironAssertAlive(c *gc.C) {
56+ // 1.17+ has a "Life" field in environment documents.
57+ // We remove it here, to test 1.16 compatibility.
58+ ops := []txn.Op{{
59+ C: s.state.environments.Name,
60+ Id: s.env.doc.UUID,
61+ Update: D{{"$unset", D{{"life", nil}}}},
62+ }}
63+ err := s.state.runTransaction(ops)
64+ c.Assert(err, gc.IsNil)
65+
66+ // Now check the assertAliveOp and Destroy work as if
67+ // the environment is Alive.
68+ err = s.state.runTransaction([]txn.Op{s.env.assertAliveOp()})
69+ c.Assert(err, gc.IsNil)
70+ err = s.env.Destroy()
71+ c.Assert(err, gc.IsNil)
72+}
73
74=== modified file 'state/environ.go'
75--- state/environ.go 2013-12-11 08:50:47 +0000
76+++ state/environ.go 2014-01-09 10:38:29 +0000
77@@ -98,7 +98,7 @@
78 C: e.st.environments.Name,
79 Id: e.doc.UUID,
80 Update: D{{"$set", D{{"life", Dying}}}},
81- Assert: isAliveDoc,
82+ Assert: isEnvAliveDoc,
83 }, e.st.newCleanupOp("services", "")}
84 err := e.st.runTransaction(ops)
85 switch err {
86@@ -130,6 +130,15 @@
87 return txn.Op{
88 C: e.st.environments.Name,
89 Id: e.UUID(),
90- Assert: isAliveDoc,
91+ Assert: isEnvAliveDoc,
92 }
93 }
94+
95+// isEnvAlive is an Environment-specific versio nof isAliveDoc.
96+//
97+// Environment documents from versions of Juju prior to 1.17
98+// do not have the life field; if it does not exist, it should
99+// be considered to have the value Alive.
100+var isEnvAliveDoc = D{
101+ {"life", D{{"$in", []interface{}{Alive, nil}}}},
102+}

Subscribers

People subscribed via source and target branches

to status/vote changes: