Landscape Client

Merge lp:~fcorrea/landscape-client/wait-for-valid-session-id-on-resynchronize into lp:~landscape/landscape-client/trunk

  1. wait-for-valid-session-id-on-resynchronize
  2. Merge into trunk
Proposed by Fernando Correa Neto
Status: Merged
Approved by: Fernando Correa Neto
Approved revision: 829
Merged at revision: 826
Proposed branch: lp:~fcorrea/landscape-client/wait-for-valid-session-id-on-resynchronize
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 249 lines (+64/-25)
4 files modified
landscape/broker/client.py (+2/-2)
landscape/broker/tests/test_client.py (+11/-0)
landscape/monitor/tests/test_usermonitor.py (+44/-20)
landscape/monitor/usermonitor.py (+7/-3)
To merge this branch: bzr merge lp:~fcorrea/landscape-client/wait-for-valid-session-id-on-resynchronize
Reviewer Review Type Date Requested Status
Chad Smith Approve
Free Ekanayaka (community) Approve
Benji York (community) Approve
Review via email: mp+282483@code.launchpad.net

Commit message

Changes the user monitor plugin to wait on a valid session id.

Description of the change

With the scope resync work, the user monitor plugin stopped working.
Since now we rely on a valid session id in order to send a message to the server, the user monitor plugin must wait until it gets a valid session.
The fix was provided by Free and a test and some cleanups were added as well.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

Thanks for the branch, it looks good.

review: Approve
Revision history for this message
Free Ekanayaka (free.ekanayaka) :
review: Needs Fixing
lp:~fcorrea/landscape-client/wait-for-valid-session-id-on-resynchronize updated
828. By Fernando Correa Neto

- fix the unit test so it can actually fail if the fix is removed.
- improve test coveration for BrokerClientPlugin base class

Revision history for this message
Fernando Correa Neto (fcorrea) wrote :

Free

Thanks for reviewing it.
I've changed the test as we discussed to make sure it will fail when the fix is reverted. Please, take a look.

Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

+1 with a nit

review: Approve
lp:~fcorrea/landscape-client/wait-for-valid-session-id-on-resynchronize updated
829. By Fernando Correa Neto

- rename test and address review comment

Revision history for this message
Fernando Correa Neto (fcorrea) :
Revision history for this message
Chad Smith (chad.smith) wrote :

Good work here and additional unit tests. It took a while for me to go through upgrade path testing w/ broken released client TO patched trunk client.

Looks like a plain upgrade of a broken existing released client will still sit missing the initial delta of all system users. But, those broken clients will properly send any users or groups that were modified after the initial registration of that client.

To truly "fix" the broken clients, the customer would have to upgrade landscape-client and click the resynch button for that computer as a support person.

+1. This was tough.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'landscape/broker/client.py'
--- landscape/broker/client.py 2013-07-25 13:54:29 +0000
+++ landscape/broker/client.py 2016-01-15 16:29:39 +0000
@@ -1,6 +1,6 @@
1from logging import info, exception1from logging import info, exception
22
3from twisted.internet.defer import maybeDeferred3from twisted.internet.defer import maybeDeferred, succeed
44
5from landscape.log import format_object5from landscape.log import format_object
6from landscape.lib.twisted_util import gather_results6from landscape.lib.twisted_util import gather_results
@@ -67,7 +67,7 @@
67 """67 """
68 if not (scopes is None or self.scope in scopes):68 if not (scopes is None or self.scope in scopes):
69 # This resynchronize event is out of scope for us. Do nothing69 # This resynchronize event is out of scope for us. Do nothing
70 return70 return succeed(None)
7171
72 # Because the broker will drop session IDs already associated to scope72 # Because the broker will drop session IDs already associated to scope
73 # of the resynchronisation, it isn't safe to send messages until the73 # of the resynchronisation, it isn't safe to send messages until the
7474
=== modified file 'landscape/broker/tests/test_client.py'
--- landscape/broker/tests/test_client.py 2013-07-19 09:57:57 +0000
+++ landscape/broker/tests/test_client.py 2016-01-15 16:29:39 +0000
@@ -47,6 +47,17 @@
47 self.client.add(plugin)47 self.client.add(plugin)
48 self.assertEqual(test_session_id, plugin._session_id)48 self.assertEqual(test_session_id, plugin._session_id)
4949
50 def test_resynchronizing_out_of_scope(self):
51 """
52 When a 'reysnchronize' event happens and the plugin scope is not part
53 of the scopes that were passed, BrokerClientPlugin succeeds.
54 """
55 plugin = BrokerClientPlugin()
56 plugin.scope = "foo"
57 self.client.add(plugin)
58 deferred = self.client_reactor.fire("resynchronize", scopes=["bar"])[0]
59 self.assertIsNone(self.successResultOf(deferred))
60
50 def test_resynchronizing_refreshes_session_id(self):61 def test_resynchronizing_refreshes_session_id(self):
51 """62 """
52 When a 'reysnchronize' event fires a new session ID is acquired as the63 When a 'reysnchronize' event fires a new session ID is acquired as the
5364
=== modified file 'landscape/monitor/tests/test_usermonitor.py'
--- landscape/monitor/tests/test_usermonitor.py 2016-01-11 18:34:49 +0000
+++ landscape/monitor/tests/test_usermonitor.py 2016-01-15 16:29:39 +0000
@@ -23,13 +23,13 @@
23 def got_result(result):23 def got_result(result):
24 self.assertMessages(24 self.assertMessages(
25 self.broker_service.message_store.get_pending_messages(),25 self.broker_service.message_store.get_pending_messages(),
26 [{"create-group-members": {u"webdev":[u"jdoe"]},26 [{"create-group-members": {u"webdev": [u"jdoe"]},
27 "create-groups": [{"gid": 1000, "name": u"webdev"}],27 "create-groups": [{"gid": 1000, "name": u"webdev"}],
28 "create-users": [{"enabled": True, "home-phone": None,28 "create-users": [{"enabled": True, "home-phone": None,
29 "location": None, "name": u"JD",29 "location": None, "name": u"JD",
30 "primary-gid": 1000, "uid": 1000,30 "primary-gid": 1000, "uid": 1000,
31 "username": u"jdoe", "work-phone": None}],31 "username": u"jdoe", "work-phone": None}],
32 "type": "users"}])32 "type": "users"}])
33 plugin.stop()33 plugin.stop()
3434
35 users = [("jdoe", "x", 1000, 1000, "JD,,,,", "/home/jdoe", "/bin/sh")]35 users = [("jdoe", "x", 1000, 1000, "JD,,,,", "/home/jdoe", "/bin/sh")]
@@ -91,20 +91,20 @@
91 self.assertTrue(persist.get("groups"))91 self.assertTrue(persist.get("groups"))
92 self.assertMessages(92 self.assertMessages(
93 self.broker_service.message_store.get_pending_messages(),93 self.broker_service.message_store.get_pending_messages(),
94 [{"create-group-members": {u"webdev":[u"jdoe"]},94 [{"create-group-members": {u"webdev": [u"jdoe"]},
95 "create-groups": [{"gid": 1000, "name": u"webdev"}],95 "create-groups": [{"gid": 1000, "name": u"webdev"}],
96 "create-users": [{"enabled": True, "home-phone": None,96 "create-users": [{"enabled": True, "home-phone": None,
97 "location": None, "name": u"JD",97 "location": None, "name": u"JD",
98 "primary-gid": 1000, "uid": 1000,98 "primary-gid": 1000, "uid": 1000,
99 "username": u"jdoe", "work-phone": None}],99 "username": u"jdoe", "work-phone": None}],
100 "type": "users"}])100 "type": "users"}])
101 self.broker_service.message_store.delete_all_messages()101 self.broker_service.message_store.delete_all_messages()
102 deferred = self.monitor.reactor.fire(102 deferred = self.monitor.reactor.fire(
103 "resynchronize", scopes=["users"])[0]103 "resynchronize", scopes=["users"])[0]
104 self.successResultOf(deferred)104 self.successResultOf(deferred)
105 self.assertMessages(105 self.assertMessages(
106 self.broker_service.message_store.get_pending_messages(),106 self.broker_service.message_store.get_pending_messages(),
107 [{"create-group-members": {u"webdev":[u"jdoe"]},107 [{"create-group-members": {u"webdev": [u"jdoe"]},
108 "create-groups": [{"gid": 1000, "name": u"webdev"}],108 "create-groups": [{"gid": 1000, "name": u"webdev"}],
109 "create-users": [{"enabled": True, "home-phone": None,109 "create-users": [{"enabled": True, "home-phone": None,
110 "location": None, "name": u"JD",110 "location": None, "name": u"JD",
@@ -113,6 +113,29 @@
113 "work-phone": None}],113 "work-phone": None}],
114 "type": "users"}])114 "type": "users"}])
115115
116 def test_new_message_after_resynchronize_event(self):
117 """
118 When a 'resynchronize' reactor event is fired, a new session is
119 created and the UserMonitor creates a new message.
120 """
121 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",
122 "/home/jdoe", "/bin/sh")]
123 self.provider.groups = [("webdev", "x", 1000, ["jdoe"])]
124 self.broker_service.message_store.set_accepted_types(["users"])
125 self.monitor.add(self.plugin)
126 self.plugin.client.broker.message_store.drop_session_ids()
127 deferred = self.reactor.fire("resynchronize")[0]
128 self.successResultOf(deferred)
129 self.assertMessages(
130 self.broker_service.message_store.get_pending_messages(),
131 [{"create-group-members": {u"webdev": [u"jdoe"]},
132 "create-groups": [{"gid": 1000, "name": u"webdev"}],
133 "create-users": [{"enabled": True, "home-phone": None,
134 "location": None, "name": u"JD",
135 "primary-gid": 1000, "uid": 1000,
136 "username": u"jdoe", "work-phone": None}],
137 "type": "users"}])
138
116 def test_wb_resynchronize_event_with_global_scope(self):139 def test_wb_resynchronize_event_with_global_scope(self):
117 """140 """
118 When a C{resynchronize} event, with global scope, occurs we act exactly141 When a C{resynchronize} event, with global scope, occurs we act exactly
@@ -129,19 +152,19 @@
129 self.assertTrue(persist.get("groups"))152 self.assertTrue(persist.get("groups"))
130 self.assertMessages(153 self.assertMessages(
131 self.broker_service.message_store.get_pending_messages(),154 self.broker_service.message_store.get_pending_messages(),
132 [{"create-group-members": {u"webdev":[u"jdoe"]},155 [{"create-group-members": {u"webdev": [u"jdoe"]},
133 "create-groups": [{"gid": 1000, "name": u"webdev"}],156 "create-groups": [{"gid": 1000, "name": u"webdev"}],
134 "create-users": [{"enabled": True, "home-phone": None,157 "create-users": [{"enabled": True, "home-phone": None,
135 "location": None, "name": u"JD",158 "location": None, "name": u"JD",
136 "primary-gid": 1000, "uid": 1000,159 "primary-gid": 1000, "uid": 1000,
137 "username": u"jdoe", "work-phone": None}],160 "username": u"jdoe", "work-phone": None}],
138 "type": "users"}])161 "type": "users"}])
139 self.broker_service.message_store.delete_all_messages()162 self.broker_service.message_store.delete_all_messages()
140 deferred = self.monitor.reactor.fire("resynchronize")[0]163 deferred = self.monitor.reactor.fire("resynchronize")[0]
141 self.successResultOf(deferred)164 self.successResultOf(deferred)
142 self.assertMessages(165 self.assertMessages(
143 self.broker_service.message_store.get_pending_messages(),166 self.broker_service.message_store.get_pending_messages(),
144 [{"create-group-members": {u"webdev":[u"jdoe"]},167 [{"create-group-members": {u"webdev": [u"jdoe"]},
145 "create-groups": [{"gid": 1000, "name": u"webdev"}],168 "create-groups": [{"gid": 1000, "name": u"webdev"}],
146 "create-users": [{"enabled": True, "home-phone": None,169 "create-users": [{"enabled": True, "home-phone": None,
147 "location": None, "name": u"JD",170 "location": None, "name": u"JD",
@@ -166,7 +189,7 @@
166 self.assertTrue(persist.get("groups"))189 self.assertTrue(persist.get("groups"))
167 self.assertMessages(190 self.assertMessages(
168 self.broker_service.message_store.get_pending_messages(),191 self.broker_service.message_store.get_pending_messages(),
169 [{"create-group-members": {u"webdev":[u"jdoe"]},192 [{"create-group-members": {u"webdev": [u"jdoe"]},
170 "create-groups": [{"gid": 1000, "name": u"webdev"}],193 "create-groups": [{"gid": 1000, "name": u"webdev"}],
171 "create-users": [{"enabled": True, "home-phone": None,194 "create-users": [{"enabled": True, "home-phone": None,
172 "location": None, "name": u"JD",195 "location": None, "name": u"JD",
@@ -189,13 +212,13 @@
189 def got_result(result):212 def got_result(result):
190 self.assertMessages(213 self.assertMessages(
191 self.broker_service.message_store.get_pending_messages(),214 self.broker_service.message_store.get_pending_messages(),
192 [{"create-group-members": {u"webdev":[u"jdoe"]},215 [{"create-group-members": {u"webdev": [u"jdoe"]},
193 "create-groups": [{"gid": 1000, "name": u"webdev"}],216 "create-groups": [{"gid": 1000, "name": u"webdev"}],
194 "create-users": [{"enabled": True, "home-phone": None,217 "create-users": [{"enabled": True, "home-phone": None,
195 "location": None, "name": u"JD",218 "location": None, "name": u"JD",
196 "primary-gid": 1000, "uid": 1000,219 "primary-gid": 1000, "uid": 1000,
197 "username": u"jdoe", "work-phone": None}],220 "username": u"jdoe", "work-phone": None}],
198 "type": "users"}])221 "type": "users"}])
199222
200 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",223 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",
201 "/home/jdoe", "/bin/sh")]224 "/home/jdoe", "/bin/sh")]
@@ -230,14 +253,14 @@
230 def got_result(result):253 def got_result(result):
231 self.assertMessages(254 self.assertMessages(
232 self.broker_service.message_store.get_pending_messages(),255 self.broker_service.message_store.get_pending_messages(),
233 [{"create-group-members": {u"webdev":[u"jdoe"]},256 [{"create-group-members": {u"webdev": [u"jdoe"]},
234 "create-groups": [{"gid": 1000, "name": u"webdev"}],257 "create-groups": [{"gid": 1000, "name": u"webdev"}],
235 "create-users": [{"enabled": True, "home-phone": None,258 "create-users": [{"enabled": True, "home-phone": None,
236 "location": None, "name": u"JD",259 "location": None, "name": u"JD",
237 "primary-gid": 1000, "uid": 1000,260 "primary-gid": 1000, "uid": 1000,
238 "username": u"jdoe", "work-phone": None}],261 "username": u"jdoe", "work-phone": None}],
239 "operation-id": 1001,262 "operation-id": 1001,
240 "type": "users"}])263 "type": "users"}])
241264
242 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",265 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",
243 "/home/jdoe", "/bin/sh")]266 "/home/jdoe", "/bin/sh")]
@@ -253,12 +276,12 @@
253 def got_result(result):276 def got_result(result):
254 self.assertMessages(277 self.assertMessages(
255 self.broker_service.message_store.get_pending_messages(),278 self.broker_service.message_store.get_pending_messages(),
256 [{"create-group-members": {u"webdev":[u"jdoe"]},279 [{"create-group-members": {u"webdev": [u"jdoe"]},
257 "create-groups": [{"gid": 1000, "name": u"webdev"}],280 "create-groups": [{"gid": 1000, "name": u"webdev"}],
258 "create-users": [{"enabled": True, "home-phone": None,281 "create-users": [{"enabled": True, "home-phone": None,
259 "location": None, "name": u"JD",282 "location": None, "name": u"JD",
260 "primary-gid": 1000, "uid": 1000,283 "primary-gid": 1000, "uid": 1000,
261 "username": u"jdoe", "work-phone": None}],284 "username": u"jdoe", "work-phone": None}],
262 "type": "users"}])285 "type": "users"}])
263286
264 self.broker_service.message_store.set_accepted_types(["users"])287 self.broker_service.message_store.set_accepted_types(["users"])
@@ -283,7 +306,7 @@
283 def got_result(result):306 def got_result(result):
284 self.assertMessages(307 self.assertMessages(
285 self.broker_service.message_store.get_pending_messages(),308 self.broker_service.message_store.get_pending_messages(),
286 [{"create-group-members": {u"webdev":[u"jdoe"]},309 [{"create-group-members": {u"webdev": [u"jdoe"]},
287 "create-groups": [{"gid": 1000, "name": u"webdev"}],310 "create-groups": [{"gid": 1000, "name": u"webdev"}],
288 "create-users": [{"enabled": True, "home-phone": None,311 "create-users": [{"enabled": True, "home-phone": None,
289 "location": None, "name": u"JD",312 "location": None, "name": u"JD",
@@ -332,8 +355,9 @@
332355
333 def got_result(result):356 def got_result(result):
334 mstore = self.broker_service.message_store357 mstore = self.broker_service.message_store
335 self.assertMessages(mstore.get_pending_messages(),358 self.assertMessages(
336 [{"create-group-members": {u"webdev":[u"jdoe"]},359 mstore.get_pending_messages(),
360 [{"create-group-members": {u"webdev": [u"jdoe"]},
337 "create-groups": [{"gid": 1000, "name": u"webdev"}],361 "create-groups": [{"gid": 1000, "name": u"webdev"}],
338 "create-users": [{"enabled": True, "home-phone": None,362 "create-users": [{"enabled": True, "home-phone": None,
339 "location": None, "name": u"JD",363 "location": None, "name": u"JD",
@@ -376,7 +400,7 @@
376 self.mocker.replay()400 self.mocker.replay()
377401
378 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",402 self.provider.users = [("jdoe", "x", 1000, 1000, "JD,,,,",
379 "/home/jdoe", "/bin/sh")]403 "/home/jdoe", "/bin/sh")]
380 self.provider.groups = [("webdev", "x", 1000, ["jdoe"])]404 self.provider.groups = [("webdev", "x", 1000, ["jdoe"])]
381 self.monitor.add(self.plugin)405 self.monitor.add(self.plugin)
382 connector = RemoteUserMonitorConnector(self.reactor, self.config)406 connector = RemoteUserMonitorConnector(self.reactor, self.config)
383407
=== modified file 'landscape/monitor/usermonitor.py'
--- landscape/monitor/usermonitor.py 2016-01-11 18:34:49 +0000
+++ landscape/monitor/usermonitor.py 2016-01-15 16:29:39 +0000
@@ -39,10 +39,14 @@
39 self._publisher.stop()39 self._publisher.stop()
40 self._publisher = None40 self._publisher = None
4141
42 def _reset(self):42 def _resynchronize(self, scopes=None):
43 """Reset user and group data."""43 """Reset user and group data."""
44 super(UserMonitor, self)._reset()44 deferred = super(UserMonitor, self)._resynchronize(scopes=scopes)
45 return self._run_detect_changes()45 # Wait for the superclass' asynchronous _resynchronize method to
46 # complete, so we have a new session ID at hand and we can craft a
47 # valid message (l.broker.client.BrokerClientPlugin._resynchronize).
48 deferred.addCallback(lambda _: self._run_detect_changes())
49 return deferred
4650
47 @remote51 @remote
48 def detect_changes(self, operation_id=None):52 def detect_changes(self, operation_id=None):

Subscribers

People subscribed via source and target branches

to all changes: