Merge into shadow-trunk-1204 : sync-env-when-required : Code : pyjuju

Status:	Merged
Approved by:	William Reade on 2012-03-30
Approved revision:	521
Merged at revision:	515
Proposed branch:	lp:~fwereade/pyjuju/sync-env-when-required
Merge into:	lp:~fwereade/pyjuju/shadow-trunk-1204
Prerequisite:	lp:~fwereade/pyjuju/warn-ignored-constraints
Diff against target:	0 lines
To merge this branch:	bzr merge lp:~fwereade/pyjuju/sync-env-when-required
Related bugs:	Link a bug report

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

Revision history for this message

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

#

Reviewers: mp+99941_code.launchpad.net,

Message:
Please take a look.

Description:
All actions which require remote environment state now sync it

add-unit had lost it somewhere in the pipeline due to expectation of
syncing
being retired; terminate-machine never had it; deploy and
[gs]et-constraints' tests slightly improved.

https://code.launchpad.net/~fwereade/juju/sync-env-when-required/+merge/99941

Requires:
https://code.launchpad.net/~fwereade/juju/warn-ignored-constraints/+merge/99861

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/control/add_unit.py
   M juju/control/constraints_get.py
   M juju/control/constraints_set.py
   M juju/control/deploy.py
   M juju/control/terminate_machine.py
   M juju/control/tests/test_add_unit.py
   M juju/control/tests/test_constraints_get.py
   M juju/control/tests/test_constraints_set.py
   M juju/control/tests/test_terminate_machine.py
   M juju/control/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/add_unit.py
=== modified file 'juju/control/add_unit.py'
--- juju/control/add_unit.py 2012-03-26 13:26:58 +0000
+++ juju/control/add_unit.py 2012-03-29 13:40:45 +0000
@@ -3,7 +3,7 @@
from twisted.internet.defer import inlineCallbacks

  from juju.control import legacy
-from juju.control.utils import get_environment
+from juju.control.utils import get_environment, sync_environment_state
  from juju.state.placement import place_unit
  from juju.state.service import ServiceStateManager

@@ -47,8 +47,8 @@
      try:
          yield legacy.check_environment(
              client, provider.get_legacy_config_keys())
+ yield sync_environment_state(client, config, environment.name)

- # TODO: handle legacy environment syncing?
          service_manager = ServiceStateManager(client)
          service_state = yield
service_manager.get_service_state(service_name)
          for i in range(num_units):

Index: juju/control/constraints_get.py
=== modified file 'juju/control/constraints_get.py'
--- juju/control/constraints_get.py 2012-03-29 02:00:04 +0000
+++ juju/control/constraints_get.py 2012-03-29 13:40:45 +0000
@@ -4,7 +4,7 @@

from twisted.internet.defer import inlineCallbacks

-from juju.control.utils import get_environment
+from juju.control.utils import get_environment, sync_environment_state
  from juju.state.environment import EnvironmentStateManager
  from juju.state.machine import MachineStateManager
  from juju.state.service import ServiceStateManager
@@ -47,8 +47,7 @@
      client = yield provider.connect()
      result = {}
      try:
- esm = EnvironmentStateManager(client)
- yield esm.set_config_state(env_config, environment.name)
+ yield sync_environment_state(client, env_config, environment.name)
          if entity_names:
              msm = MachineStateMan...

Reviewers: mp+99941_code.launchpad.net,

Message:
Please take a look.

Description:
All actions which require remote environment state now sync it

add-unit had lost it somewhere in the pipeline due to expectation of
syncing
being retired; terminate-machine never had it; deploy and
[gs]et-constraints' tests slightly improved.

https://code.launchpad.net/~fwereade/juju/sync-env-when-required/+merge/99941

Requires:
https://code.launchpad.net/~fwereade/juju/warn-ignored-constraints/+merge/99861

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/control/add_unit.py
   M juju/control/constraints_get.py
   M juju/control/constraints_set.py
   M juju/control/deploy.py
   M juju/control/terminate_machine.py
   M juju/control/tests/test_add_unit.py
   M juju/control/tests/test_constraints_get.py
   M juju/control/tests/test_constraints_set.py
   M juju/control/tests/test_terminate_machine.py
   M juju/control/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-20120329102730-9vtlwg17rh9jzmdp
+New revision: fwereade@gmail.com-20120329134045-zgrxzo49m522wgo6

Index: juju/control/add_unit.py
=== modified file 'juju/control/add_unit.py'
--- juju/control/add_unit.py	2012-03-26 13:26:58 +0000
+++ juju/control/add_unit.py	2012-03-29 13:40:45 +0000
@@ -3,7 +3,7 @@
  from twisted.internet.defer import inlineCallbacks

from juju.control import legacy
-from juju.control.utils import get_environment
+from juju.control.utils import get_environment, sync_environment_state
  from juju.state.placement import place_unit
  from juju.state.service import ServiceStateManager

@@ -47,8 +47,8 @@
      try:
          yield legacy.check_environment(
              client, provider.get_legacy_config_keys())
+        yield sync_environment_state(client, config, environment.name)

-        # TODO: handle legacy environment syncing?
          service_manager = ServiceStateManager(client)
          service_state = yield  
service_manager.get_service_state(service_name)
          for i in range(num_units):

Index: juju/control/constraints_get.py
=== modified file 'juju/control/constraints_get.py'
--- juju/control/constraints_get.py	2012-03-29 02:00:04 +0000
+++ juju/control/constraints_get.py	2012-03-29 13:40:45 +0000
@@ -4,7 +4,7 @@

from twisted.internet.defer import inlineCallbacks

-from juju.control.utils import get_environment
+from juju.control.utils import get_environment, sync_environment_state
  from juju.state.environment import EnvironmentStateManager
  from juju.state.machine import MachineStateManager
  from juju.state.service import ServiceStateManager
@@ -47,8 +47,7 @@
      client = yield provider.connect()
      result = {}
      try:
-        esm = EnvironmentStateManager(client)
-        yield esm.set_config_state(env_config, environment.name)
+        yield sync_environment_state(client, env_config, environment.name)
          if entity_names:
              msm = MachineStateManager(client)
              ssm = ServiceStateManager(client)
@@ -62,6 +61,7 @@
                  constraints = yield entity.get_constraints()
                  result[name] = dict(constraints)
          else:
+            esm = EnvironmentStateManager(client)
              constraints = yield esm.get_constraints()
              result = dict(constraints)
          yaml.safe_dump(result, sys.stdout)

Index: juju/control/constraints_set.py
=== modified file 'juju/control/constraints_set.py'
--- juju/control/constraints_set.py	2012-03-29 10:16:41 +0000
+++ juju/control/constraints_set.py	2012-03-29 13:40:45 +0000
@@ -3,7 +3,7 @@
  from twisted.internet.defer import inlineCallbacks

from juju.control import legacy
-from juju.control.utils import get_environment
+from juju.control.utils import get_environment, sync_environment_state
  from juju.state.environment import EnvironmentStateManager
  from juju.state.service import ServiceStateManager

@@ -89,9 +89,9 @@
      client = yield provider.connect()
      try:
          yield legacy.check_constraints(client, constraint_strs)
-        esm = EnvironmentStateManager(client)
-        yield esm.set_config_state(env_config, environment.name)
+        yield sync_environment_state(client, env_config, environment.name)
          if service_name is None:
+            esm = EnvironmentStateManager(client)
              yield esm.set_constraints(constraints)
          else:
              ssm = ServiceStateManager(client)

Index: juju/control/deploy.py
=== modified file 'juju/control/deploy.py'
--- juju/control/deploy.py	2012-03-29 03:20:51 +0000
+++ juju/control/deploy.py	2012-03-29 13:40:45 +0000
@@ -5,14 +5,14 @@
  from twisted.internet.defer import inlineCallbacks

from juju.control import legacy
-from juju.control.utils import expand_constraints, expand_path,  
get_environment
+from juju.control.utils import (
+    expand_constraints, expand_path, get_environment,  
sync_environment_state)

from juju.charm.errors import ServiceConfigValueError
  from juju.charm.publisher import CharmPublisher
  from juju.charm.repository import resolve
  from juju.errors import CharmError
  from juju.state.endpoint import RelationEndpoint
-from juju.state.environment import EnvironmentStateManager
  from juju.state.placement import place_unit
  from juju.state.relation import RelationStateManager
  from juju.state.service import ServiceStateManager
@@ -141,12 +141,7 @@
          yield legacy.check_constraints(client, constraint_strs)
          yield legacy.check_environment(
              client, provider.get_legacy_config_keys())
-
-        # TODO: handle legacy environment syncing
-        service_manager = ServiceStateManager(client)
-        environment_state_manager = EnvironmentStateManager(client)
-        yield environment_state_manager.set_config_state(
-            env_config, environment.name)
+        yield sync_environment_state(client, env_config, environment.name)

# Publish the charm to juju
          storage = yield provider.get_file_storage()
@@ -160,6 +155,7 @@
          charm_state = result[0]

# Create the service state
+        service_manager = ServiceStateManager(client)
          service_state = yield service_manager.add_service_state(
              service_name, charm_state, constraints)

Index: juju/control/terminate_machine.py
=== modified file 'juju/control/terminate_machine.py'
--- juju/control/terminate_machine.py	2011-09-15 18:50:23 +0000
+++ juju/control/terminate_machine.py	2012-03-29 13:40:45 +0000
@@ -2,6 +2,7 @@

from twisted.internet.defer import inlineCallbacks

+from juju.control.utils import sync_environment_state
  from juju.errors import CannotTerminateMachine
  from juju.environment.errors import EnvironmentsConfigError
  from juju.state.errors import MachineStateNotFound
@@ -57,6 +58,7 @@
      client = yield provider.connect()
      terminated_machine_ids = []
      try:
+        yield sync_environment_state(client, config, environment.name)
          machine_state_manager = MachineStateManager(client)
          for machine_id in machine_ids:
              if machine_id == 0:

Index: juju/control/utils.py
=== modified file 'juju/control/utils.py'
--- juju/control/utils.py	2012-03-29 01:37:57 +0000
+++ juju/control/utils.py	2012-03-29 13:40:45 +0000
@@ -5,6 +5,7 @@

from juju.environment.errors import EnvironmentsConfigError
  from juju.state.errors import ServiceUnitStateMachineNotAssigned
+from juju.state.environment import EnvironmentStateManager
  from juju.state.machine import MachineStateManager
  from juju.state.service import ServiceStateManager

@@ -20,6 +21,11 @@
      return environment

+def sync_environment_state(client, config, name):
+    esm = EnvironmentStateManager(client)
+    return esm.set_config_state(config, name)
+
+
  @inlineCallbacks
  def get_ip_address_for_machine(client, provider, machine_id):
      """Returns public DNS name and machine state for the machine id.

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-29 01:37:57 +0000
+++ juju/control/tests/test_add_unit.py	2012-03-29 13:40:45 +0000
@@ -3,6 +3,7 @@
  from twisted.internet.defer import inlineCallbacks

from juju.control import main
+from juju.state.environment import EnvironmentStateManager

from .common import MachineControlToolTest

@@ -32,9 +33,15 @@
          finished = self.setup_cli_reactor()
          self.setup_exit(0)
          self.mocker.replay()
+        # trash environment to check syncing
+        yield self.client.delete("/environment")
          main(["add-unit", "mysql"])
          yield finished

+        # verify the env state was synced
+        esm = EnvironmentStateManager(self.client)
+        yield esm.get_config()
+
          # verify the unit and its machine assignment.
          unit_names = yield self.service_state1.get_unit_names()
          self.assertEqual(len(unit_names), 2)

Index: juju/control/tests/test_constraints_get.py
=== modified file 'juju/control/tests/test_constraints_get.py'
--- juju/control/tests/test_constraints_get.py	2012-03-29 02:00:04 +0000
+++ juju/control/tests/test_constraints_get.py	2012-03-29 13:40:45 +0000
@@ -95,3 +95,5 @@
          yield self.finished
          result = yaml.load(self.stdout.getvalue())
          self.assertEquals(result, {"mysql/0": self.expect_unit})
+        esm = EnvironmentStateManager(self.client)
+        yield esm.get_config()

Index: juju/control/tests/test_constraints_set.py
=== modified file 'juju/control/tests/test_constraints_set.py'
--- juju/control/tests/test_constraints_set.py	2012-03-29 03:20:51 +0000
+++ juju/control/tests/test_constraints_set.py	2012-03-29 13:40:45 +0000
@@ -55,6 +55,7 @@
          yield finished

esm = EnvironmentStateManager(self.client)
+        yield esm.get_config()
          constraints = yield esm.get_constraints()
          self.assertEquals(constraints, {
              "ubuntu-series": None,

Index: juju/control/tests/test_terminate_machine.py
=== modified file 'juju/control/tests/test_terminate_machine.py'
--- juju/control/tests/test_terminate_machine.py	2012-03-27 23:56:09 +0000
+++ juju/control/tests/test_terminate_machine.py	2012-03-29 13:40:45 +0000
@@ -6,6 +6,7 @@
  from juju.control.tests.common import MachineControlToolTest
  from juju.errors import CannotTerminateMachine
  from juju.state.errors import MachineStateInUse, MachineStateNotFound
+from juju.state.environment import EnvironmentStateManager

class ControlTerminateMachineTest(MachineControlToolTest):
@@ -106,8 +107,16 @@
          yield wordpress_unit_state.unassign_from_machine()
          yield mysql_unit_state.unassign_from_machine()
          yield self.assert_machine_states([0, 1, 2, 3], [])
+
+        # trash environment to check syncing
+        yield self.client.delete("/environment")
          main(["terminate-machine", "1", "3"])
          yield wait_on_reactor_stopped
+
+        # check environment synced
+        esm = EnvironmentStateManager(self.client)
+        yield esm.get_config()
+
          self.assertIn(
              "Machines terminated: 1, 3", self.output.getvalue())
          yield self.assert_machine_states([0, 2], [1, 3])

Revision history for this message

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

#

Please take a look.

https://codereview.appspot.com/5957044/

lp:~fwereade/pyjuju/sync-env-when-required updated on 2012-03-29

520. By William Reade on 2012-03-29: added explanatory docstring to sync_environment_state (rather than adding one at evry call site)
521. By William Reade on 2012-03-29: merge parent

Revision history for this message

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

#

lgtm, centralizes all the env syncing across control.

https://codereview.appspot.com/5957044/diff/1012/juju/control/tests/test_constraints_get.py
File juju/control/tests/test_constraints_get.py (right):

https://codereview.appspot.com/5957044/diff/1012/juju/control/tests/test_constraints_get.py#newcode99
juju/control/tests/test_constraints_get.py:99: yield esm.get_config()
what is this bit verifying?

https://codereview.appspot.com/5957044/diff/1012/juju/control/utils.py
File juju/control/utils.py (right):

https://codereview.appspot.com/5957044/diff/1012/juju/control/utils.py#newcode38
juju/control/utils.py:38: that state code can use an
EnvironmentStateManager to get a provider.
thank you.. this is just what i was looking for in an earlier branch.

https://codereview.appspot.com/5957044/

Revision history for this message

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

#

https://codereview.appspot.com/5957044/diff/1012/juju/control/tests/test_constraints_get.py
File juju/control/tests/test_constraints_get.py (right):

https://codereview.appspot.com/5957044/diff/1012/juju/control/tests/test_constraints_get.py#newcode99
juju/control/tests/test_constraints_get.py:99: yield esm.get_config()
On 2012/03/29 21:04:03, hazmat wrote:
> what is this bit verifying?

It's actually redundant; I thought it would have a clarifying effect,
but clearly not. Commenting instead.

https://codereview.appspot.com/5957044/

lp:~fwereade/pyjuju/sync-env-when-required updated on 2012-03-30

522. By William Reade on 2012-03-30: merge parent
523. By William Reade on 2012-03-30: merge parent

Revision history for this message

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

#

*** Submitted:

All actions which require remote environment state now sync it

add-unit had lost it somewhere in the pipeline due to expectation of
syncing
being retired; terminate-machine never had it; deploy and
[gs]et-constraints' tests slightly improved.

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

https://codereview.appspot.com/5957044/

pyjuju

Merge lp:~fwereade/pyjuju/sync-env-when-required into lp:~fwereade/pyjuju/shadow-trunk-1204

Commit message

Description of the change

Preview Diff

Subscribers