Merge into shadow-trunk-1204 : private-cloud-fixes : Code : pyjuju

Status:	Merged
Approved by:	William Reade on 2012-03-28
Approved revision:	511
Merged at revision:	511
Proposed branch:	lp:~fwereade/pyjuju/private-cloud-fixes
Merge into:	lp:~fwereade/pyjuju/shadow-trunk-1204
Diff against target:	202 lines (+37/-43) 6 files modified juju/control/legacy.py (+10/-25) juju/control/tests/test_add_unit.py (+3/-6) juju/control/tests/test_deploy.py (+2/-6) juju/providers/ec2/__init__.py (+9/-4) juju/providers/ec2/tests/test_provider.py (+12/-1) juju/providers/ec2/utils.py (+1/-1)
To merge this branch:	bzr merge lp:~fwereade/pyjuju/private-cloud-fixes
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
William Reade			Pending
Review via email: mp+99810@code.launchpad.net

Description of the change

Small fixes:

* Private EC2 clouds no longer expose constraints
* deploy/add-unit no longer pointlessly warn about legacy keys which remain
necessary in legacy environments

https://codereview.appspot.com/5933058/

Revision history for this message

William Reade (fwereade) wrote on 2012-03-28:

#

Download full text (8.3 KiB)

Reviewers: mp+99810_code.launchpad.net,

Message:
Please take a look.

Description:
Small fixes:

* Private EC2 clouds no longer expose constraints
* deploy/add-unit no longer pointlessly warn about legacy keys which
remain
necessary in legacy environments

https://code.launchpad.net/~fwereade/juju/private-cloud-fixes/+merge/99810

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/control/legacy.py
   M juju/control/tests/test_add_unit.py
   M juju/control/tests/test_deploy.py
   M juju/providers/ec2/__init__.py
   M juju/providers/ec2/tests/test_provider.py
   M juju/providers/ec2/utils.py

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: <email address hidden>
+New revision: <email address hidden>

Index: juju/control/legacy.py
=== modified file 'juju/control/legacy.py'
--- juju/control/legacy.py 2012-03-26 13:26:58 +0000
+++ juju/control/legacy.py 2012-03-28 19:48:24 +0000
@@ -1,41 +1,26 @@
-import sys
-
from twisted.internet.defer import inlineCallbacks

from juju.errors import JujuError
from juju.state.environment import EnvironmentStateManager

-_WARNING = """
-WARNING: Your environments.yaml contains deprecated keys; changes to these
keys
-will no longer be propagated. The affected keys are:
-"""
-
  _ERROR = """
  Your environments.yaml contains deprecated keys; they must not be used
other
  than in legacy deployments. The affected keys are:
-"""
-
-_INFO = """
-%s
-This %s can be resolved according to the instructions available at:
+
+ %s
+
+This error can be resolved according to the instructions available at:

https://juju.ubuntu.com/DeprecatedEnvironmentSettings
"""

-def _format_keys(keys):
- return " %s\n" % "\n ".join(sorted(keys))
-
-def warn(keys):
- print >>sys.stderr, _WARNING + _INFO % (_format_keys(keys), "warning")
-
def error(keys):
- raise JujuError(_ERROR + _INFO % (_format_keys(keys), "error"))
+ raise JujuError(_ERROR % "\n ".join(sorted(keys)))

@inlineCallbacks
def check_environment(client, keys):
- if keys:
- esm = EnvironmentStateManager(client)
- if (yield esm.get_in_legacy_environment()):
- warn(keys)
- else:
- error(keys)
+ if not keys:
+ return
+ esm = EnvironmentStateManager(client)
+ if not (yield esm.get_in_legacy_environment()):
+ error(keys)

Index: juju/control/tests/test_add_unit.py
=== modified file 'juju/control/tests/test_add_unit.py'
--- juju/control/tests/test_add_unit.py 2012-03-27 23:56:09 +0000
+++ juju/control/tests/test_add_unit.py 2012-03-28 19:48:24 +0000
@@ -145,27 +145,24 @@
yield self.assert_machine_assignments("mysql", [0, 0])

      @inlineCallbacks
- def test_legacy_option_warning(self):
+ def test_legacy_option_in_legacy_env(self):
          config = {
              "environments": {"firstenv": {
                      "some-legacy-key": "blah...

Reviewers: mp+99810_code.launchpad.net,

Message:
Please take a look.

Description:
Small fixes:

* Private EC2 clouds no longer expose constraints
* deploy/add-unit no longer pointlessly warn about legacy keys which
remain
   necessary in legacy environments

https://code.launchpad.net/~fwereade/juju/private-cloud-fixes/+merge/99810

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/control/legacy.py
   M juju/control/tests/test_add_unit.py
   M juju/control/tests/test_deploy.py
   M juju/providers/ec2/__init__.py
   M juju/providers/ec2/tests/test_provider.py
   M juju/providers/ec2/utils.py

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: fwereade@gmail.com-20120328190611-wk36rese28j3oomi
+New revision: fwereade@gmail.com-20120328194824-y3y27cl0gphyqkj0

Index: juju/control/legacy.py
=== modified file 'juju/control/legacy.py'
--- juju/control/legacy.py	2012-03-26 13:26:58 +0000
+++ juju/control/legacy.py	2012-03-28 19:48:24 +0000
@@ -1,41 +1,26 @@
-import sys
-
  from twisted.internet.defer import inlineCallbacks

from juju.errors import JujuError
  from juju.state.environment import EnvironmentStateManager

-_WARNING = """
-WARNING: Your environments.yaml contains deprecated keys; changes to these  
keys
-will no longer be propagated. The affected keys are:
-"""
-
  _ERROR = """
  Your environments.yaml contains deprecated keys; they must not be used  
other
  than in legacy deployments. The affected keys are:
-"""
-
-_INFO = """
-%s
-This %s can be resolved according to the instructions available at:
+
+    %s
+
+This error can be resolved according to the instructions available at:

https://juju.ubuntu.com/DeprecatedEnvironmentSettings
  """

-def _format_keys(keys):
-    return "    %s\n" % "\n    ".join(sorted(keys))
-
-def warn(keys):
-    print >>sys.stderr, _WARNING + _INFO % (_format_keys(keys), "warning")
-
  def error(keys):
-    raise JujuError(_ERROR + _INFO % (_format_keys(keys), "error"))
+    raise JujuError(_ERROR % "\n    ".join(sorted(keys)))

@inlineCallbacks
  def check_environment(client, keys):
-    if keys:
-        esm = EnvironmentStateManager(client)
-        if (yield esm.get_in_legacy_environment()):
-            warn(keys)
-        else:
-            error(keys)
+    if not keys:
+        return
+    esm = EnvironmentStateManager(client)
+    if not (yield esm.get_in_legacy_environment()):
+        error(keys)

Index: juju/control/tests/test_add_unit.py
=== modified file 'juju/control/tests/test_add_unit.py'
--- juju/control/tests/test_add_unit.py	2012-03-27 23:56:09 +0000
+++ juju/control/tests/test_add_unit.py	2012-03-28 19:48:24 +0000
@@ -145,27 +145,24 @@
          yield self.assert_machine_assignments("mysql", [0, 0])

@inlineCallbacks
-    def test_legacy_option_warning(self):
+    def test_legacy_option_in_legacy_env(self):
          config = {
              "environments": {"firstenv": {
                      "some-legacy-key": "blah",
                      "type": "dummy"}}}
          yield self.push_config("firstenv", config)
+
          finished = self.setup_cli_reactor()
          self.setup_exit(0)
          self.mocker.replay()
-        stderr = self.capture_stream("stderr")
          main(["add-unit", "mysql"])
          yield finished

-        self.assertIn(
-            "Your environments.yaml contains deprecated keys",
-            stderr.getvalue())
          unit_names = yield self.service_state1.get_unit_names()
          self.assertEqual(len(unit_names), 2)

@inlineCallbacks
-    def test_legacy_option_error(self):
+    def test_legacy_option_in_fresh_env(self):
          local_config = {
              "environments": {"firstenv": {
                      "some-legacy-key": "blah",

Index: juju/control/tests/test_deploy.py
=== modified file 'juju/control/tests/test_deploy.py'
--- juju/control/tests/test_deploy.py	2012-03-27 23:22:28 +0000
+++ juju/control/tests/test_deploy.py	2012-03-28 19:48:24 +0000
@@ -506,7 +506,7 @@
          self.assertEqual(machine_id, 0)

@inlineCallbacks
-    def test_deploy_legacy_warning(self):
+    def test_deploy_legacy_keys_in_legacy_env(self):
          config = {
              "environments": {"firstenv": {
                      "type": "dummy",
@@ -516,20 +516,16 @@
          finished = self.setup_cli_reactor()
          self.setup_exit(0)
          self.mocker.replay()
-        stderr = self.capture_stream("stderr")

main(["deploy", "--repository", self.unbundled_repo_path,
                "local:sample", "beekeeper"])
          yield finished

-        self.assertIn(
-            "Your environments.yaml contains deprecated keys",
-            stderr.getvalue())
          service_manager = ServiceStateManager(self.client)
          yield service_manager.get_service_state("beekeeper")

@inlineCallbacks
-    def test_deploy_legacy_error(self):
+    def test_deploy_legacy_keys_in_fresh_env(self):
          yield self.push_default_config()
          local_config = {
              "environments": {"firstenv": {

Index: juju/providers/ec2/__init__.py
=== modified file 'juju/providers/ec2/__init__.py'
--- juju/providers/ec2/__init__.py	2012-03-28 02:00:16 +0000
+++ juju/providers/ec2/__init__.py	2012-03-28 19:48:24 +0000
@@ -42,19 +42,24 @@
      def provider_type(self):
          return "ec2"

+    @property
+    def using_amazon(self):
+        return "ec2-uri" not in self.config
+
      @inlineCallbacks
      def get_constraint_set(self):
          """Return the set of constraints that are valid for this  
provider."""
          cs = yield super(MachineProvider, self).get_constraint_set()
-        cs.register_generics(INSTANCE_TYPES.keys())
-        cs.register("ec2-zone", converter=convert_zone)
+        if self.using_amazon:
+            # Expose EC2 instance types/zones on AWS itelf, not private  
clouds.
+            cs.register_generics(INSTANCE_TYPES.keys())
+            cs.register("ec2-zone", converter=convert_zone)
          returnValue(cs)

def get_legacy_config_keys(self):
          """Return any deprecated config keys that are set"""
          legacy = super(MachineProvider, self).get_legacy_config_keys()
-        using_amazon = "ec2-uri" not in self.config
-        if using_amazon:
+        if self.using_amazon:
              # In the absence of a generic instance-type/image-id mechanism,
              # these keys remain valid on private clouds.
              amazon_legacy =  
set(("default-image-id", "default-instance-type"))

Index: juju/providers/ec2/utils.py
=== modified file 'juju/providers/ec2/utils.py'
--- juju/providers/ec2/utils.py	2012-03-28 17:57:56 +0000
+++ juju/providers/ec2/utils.py	2012-03-28 19:48:24 +0000
@@ -58,7 +58,7 @@
      if len(s) == 1:
          if s in _PLAUSIBLE_ZONES:
              return s
-    raise ValueError("expected single ascii char")
+    raise ValueError("expected single ascii letter")

def get_region_uri(region):

Index: juju/providers/ec2/tests/test_provider.py
=== modified file 'juju/providers/ec2/tests/test_provider.py'
--- juju/providers/ec2/tests/test_provider.py	2012-03-28 17:57:56 +0000
+++ juju/providers/ec2/tests/test_provider.py	2012-03-28 19:48:24 +0000
@@ -172,7 +172,10 @@
              "mem": None})

yield self.assert_invalid(
-            "Bad 'ec2-zone' constraint 'blob': expected single ascii char",
+            "Bad 'ec2-zone' constraint '7': expected single ascii letter",
+            "ec2-zone=7")
+        yield self.assert_invalid(
+            "Bad 'ec2-zone' constraint 'blob': expected single ascii  
letter",
              "ec2-zone=blob")
          yield self.assert_invalid(
              "Bad 'instance-type' constraint 'qq1.moar': unknown instance  
type",
@@ -194,6 +197,14 @@
          self.assertFalse(a.can_satisfy(b))
          self.assertFalse(b.can_satisfy(a))

+    @inlineCallbacks
+    def test_non_amazon_constraints(self):
+        provider = MachineProvider("some-non-ec2-env", {"ec2-uri": "blah"})
+        cs = yield provider.get_constraint_set()
+        self.assertEquals(cs.parse([]), {
+            "provider-type": "ec2",
+            "ubuntu-series": None})
+

class FailCreateTest(TestCase):

Revision history for this message

Kapil Thangavelu (hazmat) wrote on 2012-03-28:

#

lgtm

https://codereview.appspot.com/5933058/

Revision history for this message

William Reade (fwereade) wrote on 2012-03-28:

#

*** Submitted:

Small fixes:

* Private EC2 clouds no longer expose constraints
* deploy/add-unit no longer pointlessly warn about legacy keys which
remain
necessary in legacy environments

R=hazmat
CC=
https://codereview.appspot.com/5933058

https://codereview.appspot.com/5933058/

pyjuju

Merge lp:~fwereade/pyjuju/private-cloud-fixes into lp:~fwereade/pyjuju/shadow-trunk-1204

Commit message

Description of the change

Preview Diff

Subscribers

1	=== modified file 'juju/control/legacy.py'
2	--- juju/control/legacy.py 2012-03-26 13:26:58 +0000
3	+++ juju/control/legacy.py 2012-03-28 19:54:20 +0000
4	@@ -1,41 +1,26 @@
5	-import sys
6	-
7	from twisted.internet.defer import inlineCallbacks
8
9	from juju.errors import JujuError
10	from juju.state.environment import EnvironmentStateManager
11
12	-_WARNING = """
13	-WARNING: Your environments.yaml contains deprecated keys; changes to these keys
14	-will no longer be propagated. The affected keys are:
15	-"""
16	-
17	_ERROR = """
18	Your environments.yaml contains deprecated keys; they must not be used other
19	than in legacy deployments. The affected keys are:
20	-"""
21	-
22	-_INFO = """
23	-%s
24	-This %s can be resolved according to the instructions available at:
25	+
26	+ %s
27	+
28	+This error can be resolved according to the instructions available at:
29
30	https://juju.ubuntu.com/DeprecatedEnvironmentSettings
31	"""
32
33	-def _format_keys(keys):
34	- return " %s\n" % "\n ".join(sorted(keys))
35	-
36	-def warn(keys):
37	- print >>sys.stderr, _WARNING + _INFO % (_format_keys(keys), "warning")
38	-
39	def error(keys):
40	- raise JujuError(_ERROR + _INFO % (_format_keys(keys), "error"))
41	+ raise JujuError(_ERROR % "\n ".join(sorted(keys)))
42
43	@inlineCallbacks
44	def check_environment(client, keys):
45	- if keys:
46	- esm = EnvironmentStateManager(client)
47	- if (yield esm.get_in_legacy_environment()):
48	- warn(keys)
49	- else:
50	- error(keys)
51	+ if not keys:
52	+ return
53	+ esm = EnvironmentStateManager(client)
54	+ if not (yield esm.get_in_legacy_environment()):
55	+ error(keys)
56
57	=== modified file 'juju/control/tests/test_add_unit.py'
58	--- juju/control/tests/test_add_unit.py 2012-03-27 23:56:09 +0000
59	+++ juju/control/tests/test_add_unit.py 2012-03-28 19:54:20 +0000
60	@@ -145,27 +145,24 @@
61	yield self.assert_machine_assignments("mysql", [0, 0])
62
63	@inlineCallbacks
64	- def test_legacy_option_warning(self):
65	+ def test_legacy_option_in_legacy_env(self):
66	config = {
67	"environments": {"firstenv": {
68	"some-legacy-key": "blah",
69	"type": "dummy"}}}
70	yield self.push_config("firstenv", config)
71	+
72	finished = self.setup_cli_reactor()
73	self.setup_exit(0)
74	self.mocker.replay()
75	- stderr = self.capture_stream("stderr")
76	main(["add-unit", "mysql"])
77	yield finished
78
79	- self.assertIn(
80	- "Your environments.yaml contains deprecated keys",
81	- stderr.getvalue())
82	unit_names = yield self.service_state1.get_unit_names()
83	self.assertEqual(len(unit_names), 2)
84
85	@inlineCallbacks
86	- def test_legacy_option_error(self):
87	+ def test_legacy_option_in_fresh_env(self):
88	local_config = {
89	"environments": {"firstenv": {
90	"some-legacy-key": "blah",
91
92	=== modified file 'juju/control/tests/test_deploy.py'
93	--- juju/control/tests/test_deploy.py 2012-03-27 23:22:28 +0000
94	+++ juju/control/tests/test_deploy.py 2012-03-28 19:54:20 +0000
95	@@ -506,7 +506,7 @@
96	self.assertEqual(machine_id, 0)
97
98	@inlineCallbacks
99	- def test_deploy_legacy_warning(self):
100	+ def test_deploy_legacy_keys_in_legacy_env(self):
101	config = {
102	"environments": {"firstenv": {
103	"type": "dummy",
104	@@ -516,20 +516,16 @@
105	finished = self.setup_cli_reactor()
106	self.setup_exit(0)
107	self.mocker.replay()
108	- stderr = self.capture_stream("stderr")
109
110	main(["deploy", "--repository", self.unbundled_repo_path,
111	"local:sample", "beekeeper"])
112	yield finished
113
114	- self.assertIn(
115	- "Your environments.yaml contains deprecated keys",
116	- stderr.getvalue())
117	service_manager = ServiceStateManager(self.client)
118	yield service_manager.get_service_state("beekeeper")
119
120	@inlineCallbacks
121	- def test_deploy_legacy_error(self):
122	+ def test_deploy_legacy_keys_in_fresh_env(self):
123	yield self.push_default_config()
124	local_config = {
125	"environments": {"firstenv": {
126
127	=== modified file 'juju/providers/ec2/__init__.py'
128	--- juju/providers/ec2/__init__.py 2012-03-28 02:00:16 +0000
129	+++ juju/providers/ec2/__init__.py 2012-03-28 19:54:20 +0000
130	@@ -42,19 +42,24 @@
131	def provider_type(self):
132	return "ec2"
133
134	+ @property
135	+ def using_amazon(self):
136	+ return "ec2-uri" not in self.config
137	+
138	@inlineCallbacks
139	def get_constraint_set(self):
140	"""Return the set of constraints that are valid for this provider."""
141	cs = yield super(MachineProvider, self).get_constraint_set()
142	- cs.register_generics(INSTANCE_TYPES.keys())
143	- cs.register("ec2-zone", converter=convert_zone)
144	+ if self.using_amazon:
145	+ # Expose EC2 instance types/zones on AWS itelf, not private clouds.
146	+ cs.register_generics(INSTANCE_TYPES.keys())
147	+ cs.register("ec2-zone", converter=convert_zone)
148	returnValue(cs)
149
150	def get_legacy_config_keys(self):
151	"""Return any deprecated config keys that are set"""
152	legacy = super(MachineProvider, self).get_legacy_config_keys()
153	- using_amazon = "ec2-uri" not in self.config
154	- if using_amazon:
155	+ if self.using_amazon:
156	# In the absence of a generic instance-type/image-id mechanism,
157	# these keys remain valid on private clouds.
158	amazon_legacy = set(("default-image-id", "default-instance-type"))
159
160	=== modified file 'juju/providers/ec2/tests/test_provider.py'
161	--- juju/providers/ec2/tests/test_provider.py 2012-03-28 17:57:56 +0000
162	+++ juju/providers/ec2/tests/test_provider.py 2012-03-28 19:54:20 +0000
163	@@ -172,7 +172,10 @@
164	"mem": None})
165
166	yield self.assert_invalid(
167	- "Bad 'ec2-zone' constraint 'blob': expected single ascii char",
168	+ "Bad 'ec2-zone' constraint '7': expected single ascii letter",
169	+ "ec2-zone=7")
170	+ yield self.assert_invalid(
171	+ "Bad 'ec2-zone' constraint 'blob': expected single ascii letter",
172	"ec2-zone=blob")
173	yield self.assert_invalid(
174	"Bad 'instance-type' constraint 'qq1.moar': unknown instance type",
175	@@ -194,6 +197,14 @@
176	self.assertFalse(a.can_satisfy(b))
177	self.assertFalse(b.can_satisfy(a))
178
179	+ @inlineCallbacks
180	+ def test_non_amazon_constraints(self):
181	+ provider = MachineProvider("some-non-ec2-env", {"ec2-uri": "blah"})
182	+ cs = yield provider.get_constraint_set()
183	+ self.assertEquals(cs.parse([]), {
184	+ "provider-type": "ec2",
185	+ "ubuntu-series": None})
186	+
187
188	class FailCreateTest(TestCase):
189
190
191	=== modified file 'juju/providers/ec2/utils.py'
192	--- juju/providers/ec2/utils.py 2012-03-28 17:57:56 +0000
193	+++ juju/providers/ec2/utils.py 2012-03-28 19:54:20 +0000
194	@@ -58,7 +58,7 @@
195	if len(s) == 1:
196	if s in _PLAUSIBLE_ZONES:
197	return s
198	- raise ValueError("expected single ascii char")
199	+ raise ValueError("expected single ascii letter")
200
201
202	def get_region_uri(region):