Merge lp:~gz/pyjuju/os_remove_placement_config_really into lp:pyjuju

[Martin Packman]

Really remove placement from openstack environment config

Missed the most important line in the previous branch killing
'placement' as a config key in the openstack provider.

As penance have added the testing that should have existed previously.
Note that default-series and default-instance-type are still required
in config, but could perhaps be optional now.

https://codereview.appspot.com/6493069/

[Martin Packman (gz)]

Reviewers: mp+122301_code.launchpad.net,

Message:
Please take a look.

Description:
Really remove placement from openstack environment config

Missed the most important line in the previous branch killing
'placement' as a config key in the openstack provider.

As penance have added the testing that should have existed previously.
Note that default-series and default-instance-type are still required
in config, but could perhaps be optional now.

https://code.launchpad.net/~gz/juju/os_remove_placement_config_really/+merge/122301

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/environment/config.py
   M juju/environment/tests/test_config.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/environment/config.py
=== modified file 'juju/environment/config.py'
--- juju/environment/config.py 2012-08-29 16:41:48 +0000
+++ juju/environment/config.py 2012-08-31 15:03:31 +0000
@@ -67,7 +67,6 @@
              "auth-url": String(),
              "project-name": String(),
              "use-floating-ip": Bool(),
- "placement": _EITHER_PLACEMENT,
              "auth-mode": _OPENSTACK_AUTH_MODE,
              "region": String(),
              "default-series": String(),

Index: juju/environment/tests/test_config.py
=== modified file 'juju/environment/tests/test_config.py'
--- juju/environment/tests/test_config.py 2012-04-13 18:01:18 +0000
+++ juju/environment/tests/test_config.py 2012-08-31 15:05:28 +0000
@@ -59,6 +59,17 @@
       default-series: oneiric
  """

+SAMPLE_OPENSTACK = """
+environments:
+ sample:
+ type: openstack
+ admin-secret: sekret
+ control-bucket: container
+ default-image-id: 42
+ default-instance-type: m1-sample
+ default-series: precise
+"""
+

  class EnvironmentsConfigTestBase(TestCase):

@@ -750,3 +761,37 @@
          error = self.assertRaises(
              EnvironmentsConfigError, self.config.load, self.other_path)
          self.assertIn("expected 'local', got 'unassigned'", str(error))
+
+ def test_openstack_requires_default_image_id(self):
+ """A VM image must be supplied for openstack provider."""
+ config = yaml.load(SAMPLE_OPENSTACK)
+ del config["environments"]["sample"]["default-image-id"]
+ self.write_config(yaml.dump(config), other_path=True)
+ error = self.assertRaises(
+ EnvironmentsConfigError, self.config.load, self.other_path)
+ self.assertIn("default-image-id: required value not found",
str(error))
+
+ def test_openstack_ignores_placement(self):
+ """The placement config is not verified for openstack provider."""
+ config = yaml.load(SAMPLE_OPENSTACK)
+ config["environments"]["sample"]["type"] = "openstack_s3"
+ config["environments"]["sample"]["placement"] = "whatever"
+ self.writ...

[lp:~gz/pyjuju/os_remove_placement_config_really]

564. By Martin Packman on 2012-08-31

Rearrange added tests so names match up with reality

=== modified file 'juju/environment/config.py'
--- juju/environment/config.py	2012-08-29 16:41:48 +0000
+++ juju/environment/config.py	2012-08-31 15:19:18 +0000
@@ -67,7 +67,6 @@
             "auth-url": String(),
             "project-name": String(),
             "use-floating-ip": Bool(),
-            "placement": _EITHER_PLACEMENT,
             "auth-mode": _OPENSTACK_AUTH_MODE,
             "region": String(),
             "default-series": String(),

=== modified file 'juju/environment/tests/test_config.py'
--- juju/environment/tests/test_config.py	2012-04-13 18:01:18 +0000
+++ juju/environment/tests/test_config.py	2012-08-31 15:19:18 +0000
@@ -59,6 +59,17 @@
      default-series: oneiric
 """
 
+SAMPLE_OPENSTACK = """
+environments:
+   sample:
+     type: openstack
+     admin-secret: sekret
+     control-bucket: container
+     default-image-id: 42
+     default-instance-type: m1-sample
+     default-series: precise
+"""
+
 
 class EnvironmentsConfigTestBase(TestCase):
 
@@ -750,3 +761,37 @@
         error = self.assertRaises(
             EnvironmentsConfigError, self.config.load, self.other_path)
         self.assertIn("expected 'local', got 'unassigned'", str(error))
+
+    def test_openstack_requires_default_image_id(self):
+        """A VM image must be supplied for openstack provider."""
+        config = yaml.load(SAMPLE_OPENSTACK)
+        del config["environments"]["sample"]["default-image-id"]
+        self.write_config(yaml.dump(config), other_path=True)
+        error = self.assertRaises(
+            EnvironmentsConfigError, self.config.load, self.other_path)
+        self.assertIn("default-image-id: required value not found", str(error))
+
+    def test_openstack_ignores_placement(self):
+        """The placement config is not verified for openstack provider."""
+        config = yaml.load(SAMPLE_OPENSTACK)
+        config["environments"]["sample"]["placement"] = "whatever"
+        self.write_config(yaml.dump(config), other_path=True)
+        self.config.load(self.other_path)
+
+    def test_openstack_s3_requires_default_image_id(self):
+        """A VM image must be supplied for openstack_s3 provider."""
+        config = yaml.load(SAMPLE_OPENSTACK)
+        config["environments"]["sample"]["type"] = "openstack_s3"
+        del config["environments"]["sample"]["default-image-id"]
+        self.write_config(yaml.dump(config), other_path=True)
+        error = self.assertRaises(
+            EnvironmentsConfigError, self.config.load, self.other_path)
+        self.assertIn("default-image-id: required value not found", str(error))
+
+    def test_openstack_s3_ignores_placement(self):
+        """The placement config is not verified for openstack_s3 provider."""
+        config = yaml.load(SAMPLE_OPENSTACK)
+        config["environments"]["sample"]["type"] = "openstack_s3"
+        config["environments"]["sample"]["placement"] = "whatever"
+        self.write_config(yaml.dump(config), other_path=True)
+        self.config.load(self.other_path)