Merge lp:~jamesodhunt/upstart/fix-system-reexec-test into lp:upstart

Proposed by James Hunt on 2013-09-12
Status: Merged
Merged at revision: 1550
Proposed branch: lp:~jamesodhunt/upstart/fix-system-reexec-test
Merge into: lp:upstart
Diff against target: 443 lines (+207/-51)
4 files modified
ChangeLog (+21/-0)
scripts/pyupstart.py (+114/-32)
scripts/tests/test_pyupstart_session_init.py (+50/-1)
scripts/tests/test_pyupstart_system_init.py (+22/-18)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/fix-system-reexec-test
Reviewer Review Type Date Requested Status
Dimitri John Ledkov 2013-09-12 Approve on 2013-11-03
Review via email: mp+185238@code.launchpad.net

Description of the change

Various fixes and improvements to the system and session tests:

* scripts/pyupstart.py:
  - dbus_encode(): Comments.
  - Upstart::_idle_create_job_cb(): Pass retain option.
  - Upstart::job_create(): Allow specification of retain option to keep
    job configuration file on object destruction.
  - Upstart::job_recreate(): New method to vivify a Job object using an
    existing job configuration file.
  - Job::__init__(): New 'retain' and 'reuse_path' options.
  - Job::get_instance(): New method to obtain a Jobs JobInstance.
  - Job::get_dbus_instance(): Renamed from get_instance().
  - JobInstance::destroy(): NOP for 'retain'ed jobs.
* scripts/tests/test_pyupstart_system_init.py:
  - test_pid1_reexec():
    - Retain the job configuration to allow the object to be recreated
      after re-exec.
* scripts/tests/test_pyupstart_session_init.py:
  - test_session_init_reexec: Add job creation for parity with
    test_pid1_reexec().

To post a comment you must log in.
Dimitri John Ledkov (xnox) wrote :

What "::" is in C++ is "." in python =)

pyflakes 3 .
pep8 .

give plenty of warnings, but that's a pre-existing (health) condition.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ChangeLog'
--- ChangeLog 2013-09-05 16:19:06 +0000
+++ ChangeLog 2013-09-12 10:58:02 +0000
@@ -1,3 +1,24 @@
12013-09-12 James Hunt <james.hunt@ubuntu.com>
2
3 * scripts/pyupstart.py:
4 - dbus_encode(): Comments.
5 - Upstart::_idle_create_job_cb(): Pass retain option.
6 - Upstart::job_create(): Allow specification of retain option to keep
7 job configuration file on object destruction.
8 - Upstart::job_recreate(): New method to vivify a Job object using an
9 existing job configuration file.
10 - Job::__init__(): New 'retain' and 'reuse_path' options.
11 - Job::get_instance(): New method to obtain a Jobs JobInstance.
12 - Job::get_dbus_instance(): Renamed from get_instance().
13 - JobInstance::destroy(): NOP for 'retain'ed jobs.
14 * scripts/tests/test_pyupstart_system_init.py:
15 - test_pid1_reexec():
16 - Retain the job configuration to allow the object to be recreated
17 after re-exec.
18 * scripts/tests/test_pyupstart_session_init.py:
19 - test_session_init_reexec: Add job creation for parity with
20 test_pid1_reexec().
21
12013-09-05 James Hunt <james.hunt@ubuntu.com>222013-09-05 James Hunt <james.hunt@ubuntu.com>
223
3 * util/tests/test_initctl.c: test_quiesce():24 * util/tests/test_initctl.c: test_quiesce():
425
=== modified file 'scripts/pyupstart.py'
--- scripts/pyupstart.py 2013-07-30 08:25:59 +0000
+++ scripts/pyupstart.py 2013-09-12 10:58:02 +0000
@@ -91,8 +91,13 @@
91 Note that in the special case of the specified string being None91 Note that in the special case of the specified string being None
92 or the nul string, it is encoded as '_'.92 or the nul string, it is encoded as '_'.
9393
94 Example: 'hello-world' would be encoded as 'hello_2dworld' since94 Examples:
95 '-' is 2d in hex.95
96 'hello-world' would be encoded as 'hello_2dworld' since
97 '-' is 2d in hex (resulting in '_2d').
98
99 Similarly, '_2f' would be the encoding for '/' since that character
100 has hex value 2f.
96101
97 """102 """
98 if not str:103 if not str:
@@ -244,7 +249,8 @@
244 the main loop starts.249 the main loop starts.
245250
246 """251 """
247 self.new_job = Job(self, self.test_dir, self.test_dir_name, name, body=body)252 self.new_job = Job(self, self.test_dir, self.test_dir_name,
253 name, body=body, retain=self.retain)
248254
249 # deregister255 # deregister
250 return False256 return False
@@ -377,13 +383,15 @@
377 """383 """
378 raise NotImplementedError('method must be implemented by subclass')384 raise NotImplementedError('method must be implemented by subclass')
379385
380 def job_create(self, name, body):386 def job_create(self, name, body, retain=False):
381 """387 """
382 Create a Job Configuration File.388 Create a Job Configuration File.
383389
384 @name: Name to give the job.390 @name: Name to give the job.
385 @body: String representation of configuration file, or list of391 @body: String representation of configuration file, or list of
386 strings.392 strings.
393 @retain: if True, don't remove the Job Configuration File when
394 object is cleaned up.
387395
388 Strategy:396 Strategy:
389397
@@ -421,6 +429,8 @@
421 self.job_seen = False429 self.job_seen = False
422 self.new_job = None430 self.new_job = None
423431
432 self.retain = retain
433
424 # construct the D-Bus path for the new job434 # construct the D-Bus path for the new job
425 job_path = '{}/{}'.format(self.test_dir_name, name)435 job_path = '{}/{}'.format(self.test_dir_name, name)
426436
@@ -452,6 +462,27 @@
452462
453 return self.new_job463 return self.new_job
454464
465 def job_recreate(self, name, conf_path):
466 """
467 Create a job object from an existing Job Configuration File.
468
469 @name: Name prefix of existing job configuration file.
470 @conf_path: Full path to *existing* Job Configuration File.
471 """
472
473 assert (name)
474 assert (conf_path)
475
476 job_path = '{}/{}'.format(self.test_dir_name, name)
477 self.job_object_path = '{}/{}/{}'.format(
478 OBJECT_PATH, 'jobs', dbus_encode(job_path)
479 )
480
481 self.new_job = Job(self, self.test_dir, self.test_dir_name,
482 name, body=None, reuse_path=conf_path)
483 self.jobs.append(self.new_job)
484 return self.new_job
485
455486
456class Job:487class Job:
457 """488 """
@@ -470,7 +501,8 @@
470501
471 """502 """
472503
473 def __init__(self, upstart, dir_name, subdir_name, job_name, body=None):504 def __init__(self, upstart, dir_name, subdir_name, job_name,
505 body=None, reuse_path=None, retain=False):
474 """506 """
475 @upstart: Upstart() parent object.507 @upstart: Upstart() parent object.
476 @dir_name: Full path to job configuration files directory.508 @dir_name: Full path to job configuration files directory.
@@ -479,6 +511,10 @@
479 @job_name: Name of job.511 @job_name: Name of job.
480 @body: Contents of job configuration file (either a string, or a512 @body: Contents of job configuration file (either a string, or a
481 list of strings).513 list of strings).
514 @reuse_path: If set and @body is None, (re)create the job object
515 using the existing specified job configuration file path.
516 @retain: If True, don't delete the Job Configuration File on
517 object destruction.
482 """518 """
483519
484 self.logger = logging.getLogger(self.__class__.__name__)520 self.logger = logging.getLogger(self.__class__.__name__)
@@ -491,6 +527,8 @@
491 self.name = job_name527 self.name = job_name
492 self.job_dir = dir_name528 self.job_dir = dir_name
493 self.body = body529 self.body = body
530 self.reuse_path = reuse_path
531 self.retain = retain
494532
495 self.instance_name = None533 self.instance_name = None
496534
@@ -499,19 +537,30 @@
499537
500 self.properties = None538 self.properties = None
501539
502 if not self.body:540 # need some way to create the job
503 raise UpstartException('No body specified')541 if not self.body and not self.reuse_path:
542 raise UpstartException('No body or reusable path specified')
504543
505 self.conffile = os.path.join(self.job_dir, self.name + '.conf')544 if self.reuse_path:
545 self.conffile = self.reuse_path
546 else:
547 self.conffile = os.path.join(self.job_dir, self.name + '.conf')
506548
507 if self.body and isinstance(self.body, str):549 if self.body and isinstance(self.body, str):
508 # Assume body cannot be a bytes object.550 # Assume body cannot be a bytes object.
509 body = body.splitlines()551 body = body.splitlines()
510552
511 with open(self.conffile, 'w', encoding='utf-8') as fh:553 if not self.body and self.reuse_path:
512 for line in body:554 # Just check conf file exists
513 print(line.strip(), file=fh)555 if not os.path.exists(self.conffile):
514 print(file=fh)556 raise UpstartException(
557 'File {} does not exist for reuse'.format(self.conffile))
558 else:
559 # Create conf file
560 with open(self.conffile, 'w', encoding='utf-8') as fh:
561 for line in body:
562 print(line.strip(), file=fh)
563 print(file=fh)
515564
516 self.valid = True565 self.valid = True
517566
@@ -533,7 +582,8 @@
533 for instance in self.instances:582 for instance in self.instances:
534 instance.destroy()583 instance.destroy()
535584
536 os.remove(self.conffile)585 if not self.retain:
586 os.remove(self.conffile)
537 except FileNotFoundError:587 except FileNotFoundError:
538 pass588 pass
539589
@@ -551,19 +601,44 @@
551 if env is None:601 if env is None:
552 env = []602 env = []
553 instance_path = self.interface.Start(dbus.Array(env, 's'), wait)603 instance_path = self.interface.Start(dbus.Array(env, 's'), wait)
554 instance_name = instance_path.replace("%s/" % self.object_path, '')604
555605 instance_name = instance_path.replace("%s/" % self.object_path, '')
556 # store the D-Bus encoded instance name ('_' for single-instance jobs)606
557 if instance_name not in self.instance_names:607 # store the D-Bus encoded instance name ('_' for single-instance jobs)
558 self.instance_names.append(instance_name)608 if instance_name not in self.instance_names:
559609 self.instance_names.append(instance_name)
560 instance = JobInstance(self, instance_name, instance_path)610
561 self.instances.append(instance)611 instance = JobInstance(self, instance_name, instance_path)
562 return instance612 self.instances.append(instance)
563613 return instance
564 def _get_instance(self, name):614
565 """615 def get_instance(self):
566 Retrieve job instance and its properties.616 """
617 Returns: JobInstance of calling Job.
618 """
619
620 # construct the D-Bus path for the new job
621 job_path = '{}/{}'.format(self.upstart.test_dir_name, self.name)
622
623 instance_path = '{}/{}/{}/{}'.format(
624 OBJECT_PATH, 'jobs', dbus_encode(job_path),
625
626 # XXX: LIMITATION - only support default instance.
627 '_'
628 )
629 instance_name = instance_path.replace("%s/" % self.object_path, '')
630
631 # store the D-Bus encoded instance name ('_' for single-instance jobs)
632 if instance_name not in self.instance_names:
633 self.instance_names.append(instance_name)
634
635 instance = JobInstance(self, instance_name, instance_path)
636 self.instances.append(instance)
637 return instance
638
639 def _get_dbus_instance(self, name):
640 """
641 Retrieve D-Bus job instance and its properties.
567642
568 @name: D-Bus encoded instance name.643 @name: D-Bus encoded instance name.
569644
@@ -586,7 +661,7 @@
586 """661 """
587662
588 for name in self.instance_names:663 for name in self.instance_names:
589 instance = self._get_instance(name)664 instance = self._get_dbus_instance(name)
590 try:665 try:
591 instance.Stop(wait)666 instance.Stop(wait)
592 except dbus.exceptions.DBusException:667 except dbus.exceptions.DBusException:
@@ -600,7 +675,7 @@
600 @wait: if False, stop job instances asynchronously.675 @wait: if False, stop job instances asynchronously.
601 """676 """
602 for name in self.instance_names:677 for name in self.instance_names:
603 instance = self._get_instance(name)678 instance = self._get_dbus_instance(name)
604 instance.Restart(wait)679 instance.Restart(wait)
605680
606 def instance_object_paths(self):681 def instance_object_paths(self):
@@ -629,9 +704,13 @@
629704
630 assert(name in self.instance_names)705 assert(name in self.instance_names)
631706
632 instance = self._get_instance(name)707 instance = self._get_dbus_instance(name)
708 assert (instance)
633709
634 properties = dbus.Interface(instance, FREEDESKTOP_PROPERTIES)710 properties = dbus.Interface(instance, FREEDESKTOP_PROPERTIES)
711 assert (properties)
712
713 # don't assert as there may not be any processes
635 procs = properties.Get(INSTANCE_INTERFACE_NAME, 'processes')714 procs = properties.Get(INSTANCE_INTERFACE_NAME, 'processes')
636715
637 pid_map = {}716 pid_map = {}
@@ -845,10 +924,13 @@
845 def destroy(self):924 def destroy(self):
846 """925 """
847 Stop the instance and cleanup.926 Stop the instance and cleanup.
927
928 Note: If the instance specified retain when created, this will
929 be a NOP.
848 """930 """
849 self.stop()931 if not self.job.retain:
850 self.logfile.destroy()932 self.stop()
851933 self.logfile.destroy()
852934
853class SystemInit(Upstart):935class SystemInit(Upstart):
854936
855937
=== modified file 'scripts/tests/test_pyupstart_session_init.py'
--- scripts/tests/test_pyupstart_session_init.py 2013-08-06 16:54:28 +0000
+++ scripts/tests/test_pyupstart_session_init.py 2013-09-12 10:58:02 +0000
@@ -229,6 +229,23 @@
229 # Ensure no stateful-reexec already performed.229 # Ensure no stateful-reexec already performed.
230 self.assertFalse(re.search('state-fd', output))230 self.assertFalse(re.search('state-fd', output))
231231
232 version = self.upstart.version()
233 self.assertTrue(version)
234
235 # create a job and start it, marking it such that the .conf file
236 # will be retained when object becomes unusable (after re-exec).
237 job = self.upstart.job_create('sleeper', 'exec sleep 123', retain=True)
238 self.assertTrue(job)
239
240 # Used when recreating the job
241 conf_path = job.conffile
242
243 inst = job.start()
244 self.assertTrue(inst)
245 pids = job.pids()
246 self.assertEqual(len(pids), 1)
247 pid = pids['main']
248
232 # Trigger re-exec and catch the D-Bus exception resulting249 # Trigger re-exec and catch the D-Bus exception resulting
233 # from disconnection from Session Init when it severs client250 # from disconnection from Session Init when it severs client
234 # connections.251 # connections.
@@ -248,8 +265,40 @@
248 # instantaneous, try a few times.265 # instantaneous, try a few times.
249 self.upstart.polling_connect(force=True)266 self.upstart.polling_connect(force=True)
250267
268 # Since the parent job was created with 'retain', this is actually
269 # a NOP but is added to denote that the old instance is dead.
270 inst.destroy()
271
251 # check that we can still operate on the re-exec'd Upstart272 # check that we can still operate on the re-exec'd Upstart
252 self.assertTrue(self.upstart.version())273 version_postexec = self.upstart.version()
274 self.assertTrue(version_postexec)
275 self.assertEqual(version, version_postexec)
276
277 # Ensure the job is still running with the same PID
278 os.kill(pid, 0)
279
280 # XXX: The re-exec will have severed the D-Bus connection to
281 # Upstart. Hence, revivify the job with some magic.
282 job = self.upstart.job_recreate('sleeper', conf_path)
283 self.assertTrue(job)
284
285 # Recreate the instance
286 inst = job.get_instance()
287 self.assertTrue(inst)
288
289 self.assertTrue(job.running('_'))
290 pids = job.pids()
291 self.assertEqual(len(pids), 1)
292 self.assertTrue(pids['main'])
293
294 # The pid should not have changed after a restart
295 self.assertEqual(pid, pids['main'])
296
297 job.stop()
298
299 # Ensure the pid has gone
300 with self.assertRaises(ProcessLookupError):
301 os.kill(pid, 0)
253302
254 self.stop_session_init()303 self.stop_session_init()
255304
256305
=== modified file 'scripts/tests/test_pyupstart_system_init.py'
--- scripts/tests/test_pyupstart_system_init.py 2013-08-06 16:54:28 +0000
+++ scripts/tests/test_pyupstart_system_init.py 2013-09-12 10:58:02 +0000
@@ -63,9 +63,12 @@
6363
64 # create a job and start it, marking it such that the .conf file64 # create a job and start it, marking it such that the .conf file
65 # will be retained when object becomes unusable (after re-exec).65 # will be retained when object becomes unusable (after re-exec).
66 job = self.upstart.job_create('sleeper', 'exec sleep 123')66 job = self.upstart.job_create('sleeper', 'exec sleep 123', retain=True)
67 self.assertTrue(job)67 self.assertTrue(job)
6868
69 # Used when recreating the job
70 conf_path = job.conffile
71
69 inst = job.start()72 inst = job.start()
70 self.assertTrue(inst)73 self.assertTrue(inst)
71 pids = job.pids()74 pids = job.pids()
@@ -80,6 +83,10 @@
80 # try a few times.83 # try a few times.
81 self.upstart.polling_connect(force=True)84 self.upstart.polling_connect(force=True)
8285
86 # Since the parent job was created with 'retain', this is actually
87 # a NOP but is added to denote that the old instance is dead.
88 inst.destroy()
89
83 # check that we can still operate on the re-exec'd Upstart90 # check that we can still operate on the re-exec'd Upstart
84 version_postexec = self.upstart.version()91 version_postexec = self.upstart.version()
85 self.assertTrue(version_postexec)92 self.assertTrue(version_postexec)
@@ -88,32 +95,29 @@
88 # Ensure the job is still running with the same PID95 # Ensure the job is still running with the same PID
89 os.kill(pid, 0)96 os.kill(pid, 0)
9097
98 # XXX: The re-exec will have severed the D-Bus connection to
99 # Upstart. Hence, revivify the job with some magic.
100 job = self.upstart.job_recreate('sleeper', conf_path)
101 self.assertTrue(job)
102
103 # Recreate the instance
104 inst = job.get_instance()
105 self.assertTrue(inst)
106
91 self.assertTrue(job.running('_'))107 self.assertTrue(job.running('_'))
92
93 pids = job.pids()108 pids = job.pids()
94 self.assertEqual(len(pids), 1)109 self.assertEqual(len(pids), 1)
95 self.assertTrue(pids['main'])110 self.assertTrue(pids['main'])
96111
97 # Ensure pid remains the same112 # The pid should not have changed after a restart
98 self.assertEqual(pid, pids['main'])113 self.assertEqual(pid, pids['main'])
99114
100 # Exceptions will be caught by the unittest framework
101 inst.stop()
102
103 job.start()
104
105 pids = job.pids()
106 self.assertEqual(len(pids), 1)
107 self.assertTrue(pids['main'])
108
109 os.kill(pids['main'], 0)
110 self.assertTrue(job.running('_'))
111
112 # The pid should have changed after a restart
113 self.assertNotEqual(pid, pids['main'])
114
115 job.stop()115 job.stop()
116116
117 # Ensure the pid has gone
118 with self.assertRaises(ProcessLookupError):
119 os.kill(pid, 0)
120
117 # Clean up121 # Clean up
118 self.upstart.destroy()122 self.upstart.destroy()
119123

Subscribers

People subscribed via source and target branches