Code review comment for lp://qastaging/~frankban/juju-quickstart/azure-bootstrap-node

Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+225618_code.launchpad.net,

Message:
Please take a look.

Description:
Do not colocate GUI on machine 0 for Azure.

https://code.launchpad.net/~frankban/juju-quickstart/azure-bootstrap-node/+merge/225618

(do not edit description out of merge proposal)

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

Affected files (+25, -3 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/tests/test_app.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: quickstart/app.py
=== modified file 'quickstart/app.py'
--- quickstart/app.py 2014-06-13 13:45:06 +0000
+++ quickstart/app.py 2014-07-04 07:49:03 +0000
@@ -388,9 +388,15 @@
          # provided charm URL and just use the already deployed charm.
          charm_url = service_data['CharmURL']
      charm = utils.parse_gui_charm_url(charm_url)
- # Deploy on the bootstrap node if we are not using the local provider,
and
- # if the requested charm and the bootstrap node have the same series.
- if (env_type != 'local') and (charm.series == bootstrap_node_series):
+ # Deploy on the bootstrap node if the following conditions are
satisfied:
+ # - we are not using the local provider (which uses localhost);
+ # - we are not using the azure provider (in which availability sets
prevent
+ # us from relying on the bootstrap node);
+ # - the requested charm and the bootstrap node have the same series.
+ if (
+ (env_type not in ('local', 'azure')) and
+ (charm.series == bootstrap_node_series)
+ ):
          machine = '0'
      return charm_url, machine, service_data, unit_data

Index: quickstart/tests/test_app.py
=== modified file 'quickstart/tests/test_app.py'
--- quickstart/tests/test_app.py 2014-06-13 13:45:06 +0000
+++ quickstart/tests/test_app.py 2014-07-04 07:49:03 +0000
@@ -947,6 +947,20 @@
          # The Juju GUI unit cannot be deployed to localhost.
          self.assertIsNone(machine)

+ def test_azure_provider(self, mock_print):
+ # When using the azure provider, availability sets are enabled and
+ # this prevents us from co-locating the Juju GUI on machine 0.
+ env = self.make_env()
+ charm_url = None
+ env_type = 'azure'
+ bootstrap_node_series = 'trusty'
+ check_preexisting = False
+ with
self.patch_get_charm_url(return_value='cs:trusty/juju-gui-42'):
+ url, machine, service_data, unit_data = app.check_environment(
+ env, 'my-gui', charm_url, env_type, bootstrap_node_series,
+ check_preexisting)
+ self.assertIsNone(machine)
+
      def test_default_charm_url(self, mock_print):
          # A default charm URL suitable to be deployed in the bootstrap
node is
          # returned if the charmworld API is not reachable.

« Back to merge proposal