Merge authors: James Hunt (jamesodhunt) Related merge proposals: https://code.launchpad.net/~jamesodhunt/upstart/fix-system-reexec-test/+merge/185238 proposed by: James Hunt (jamesodhunt) review: Approve - Dmitrijs Ledkovs (xnox) ------------------------------------------------------------ revno: 1550 [merge] committer: Dmitrijs Ledkovs <[email protected]> branch nick: upstart timestamp: Sun 2013-11-03 00:28:07 +0000 message: merge lp:~jamesodhunt/upstart/fix-system-reexec-test modified: ChangeLog scripts/pyupstart.py scripts/tests/test_pyupstart_session_init.py scripts/tests/test_pyupstart_system_init.py
-- lp:upstart https://code.launchpad.net/~upstart-devel/upstart/trunk Your team Upstart Reviewers is subscribed to branch lp:upstart. To unsubscribe from this branch go to https://code.launchpad.net/~upstart-devel/upstart/trunk/+edit-subscription
=== modified file 'ChangeLog' --- ChangeLog 2013-11-03 00:19:02 +0000 +++ ChangeLog 2013-11-03 00:28:07 +0000 @@ -1,4 +1,25 @@ -2013-11-01 James Hunt <[email protected]> +2013-11-03 James Hunt <[email protected]> + + * 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(). + +2013-11-03 James Hunt <[email protected]> * extra/upstart-file-bridge.c: - create_handler(): Use original_file() for directories === modified file 'scripts/pyupstart.py' --- scripts/pyupstart.py 2013-07-30 08:25:59 +0000 +++ scripts/pyupstart.py 2013-09-12 10:56:22 +0000 @@ -91,8 +91,13 @@ Note that in the special case of the specified string being None or the nul string, it is encoded as '_'. - Example: 'hello-world' would be encoded as 'hello_2dworld' since - '-' is 2d in hex. + Examples: + + 'hello-world' would be encoded as 'hello_2dworld' since + '-' is 2d in hex (resulting in '_2d'). + + Similarly, '_2f' would be the encoding for '/' since that character + has hex value 2f. """ if not str: @@ -244,7 +249,8 @@ the main loop starts. """ - self.new_job = Job(self, self.test_dir, self.test_dir_name, name, body=body) + self.new_job = Job(self, self.test_dir, self.test_dir_name, + name, body=body, retain=self.retain) # deregister return False @@ -377,13 +383,15 @@ """ raise NotImplementedError('method must be implemented by subclass') - def job_create(self, name, body): + def job_create(self, name, body, retain=False): """ Create a Job Configuration File. @name: Name to give the job. @body: String representation of configuration file, or list of strings. + @retain: if True, don't remove the Job Configuration File when + object is cleaned up. Strategy: @@ -421,6 +429,8 @@ self.job_seen = False self.new_job = None + self.retain = retain + # construct the D-Bus path for the new job job_path = '{}/{}'.format(self.test_dir_name, name) @@ -452,6 +462,27 @@ return self.new_job + def job_recreate(self, name, conf_path): + """ + Create a job object from an existing Job Configuration File. + + @name: Name prefix of existing job configuration file. + @conf_path: Full path to *existing* Job Configuration File. + """ + + assert (name) + assert (conf_path) + + job_path = '{}/{}'.format(self.test_dir_name, name) + self.job_object_path = '{}/{}/{}'.format( + OBJECT_PATH, 'jobs', dbus_encode(job_path) + ) + + self.new_job = Job(self, self.test_dir, self.test_dir_name, + name, body=None, reuse_path=conf_path) + self.jobs.append(self.new_job) + return self.new_job + class Job: """ @@ -470,7 +501,8 @@ """ - def __init__(self, upstart, dir_name, subdir_name, job_name, body=None): + def __init__(self, upstart, dir_name, subdir_name, job_name, + body=None, reuse_path=None, retain=False): """ @upstart: Upstart() parent object. @dir_name: Full path to job configuration files directory. @@ -479,6 +511,10 @@ @job_name: Name of job. @body: Contents of job configuration file (either a string, or a list of strings). + @reuse_path: If set and @body is None, (re)create the job object + using the existing specified job configuration file path. + @retain: If True, don't delete the Job Configuration File on + object destruction. """ self.logger = logging.getLogger(self.__class__.__name__) @@ -491,6 +527,8 @@ self.name = job_name self.job_dir = dir_name self.body = body + self.reuse_path = reuse_path + self.retain = retain self.instance_name = None @@ -499,19 +537,30 @@ self.properties = None - if not self.body: - raise UpstartException('No body specified') + # need some way to create the job + if not self.body and not self.reuse_path: + raise UpstartException('No body or reusable path specified') - self.conffile = os.path.join(self.job_dir, self.name + '.conf') + if self.reuse_path: + self.conffile = self.reuse_path + else: + self.conffile = os.path.join(self.job_dir, self.name + '.conf') if self.body and isinstance(self.body, str): # Assume body cannot be a bytes object. body = body.splitlines() - with open(self.conffile, 'w', encoding='utf-8') as fh: - for line in body: - print(line.strip(), file=fh) - print(file=fh) + if not self.body and self.reuse_path: + # Just check conf file exists + if not os.path.exists(self.conffile): + raise UpstartException( + 'File {} does not exist for reuse'.format(self.conffile)) + else: + # Create conf file + with open(self.conffile, 'w', encoding='utf-8') as fh: + for line in body: + print(line.strip(), file=fh) + print(file=fh) self.valid = True @@ -533,7 +582,8 @@ for instance in self.instances: instance.destroy() - os.remove(self.conffile) + if not self.retain: + os.remove(self.conffile) except FileNotFoundError: pass @@ -551,19 +601,44 @@ if env is None: env = [] instance_path = self.interface.Start(dbus.Array(env, 's'), wait) - instance_name = instance_path.replace("%s/" % self.object_path, '') - - # store the D-Bus encoded instance name ('_' for single-instance jobs) - if instance_name not in self.instance_names: - self.instance_names.append(instance_name) - - instance = JobInstance(self, instance_name, instance_path) - self.instances.append(instance) - return instance - - def _get_instance(self, name): - """ - Retrieve job instance and its properties. + + instance_name = instance_path.replace("%s/" % self.object_path, '') + + # store the D-Bus encoded instance name ('_' for single-instance jobs) + if instance_name not in self.instance_names: + self.instance_names.append(instance_name) + + instance = JobInstance(self, instance_name, instance_path) + self.instances.append(instance) + return instance + + def get_instance(self): + """ + Returns: JobInstance of calling Job. + """ + + # construct the D-Bus path for the new job + job_path = '{}/{}'.format(self.upstart.test_dir_name, self.name) + + instance_path = '{}/{}/{}/{}'.format( + OBJECT_PATH, 'jobs', dbus_encode(job_path), + + # XXX: LIMITATION - only support default instance. + '_' + ) + instance_name = instance_path.replace("%s/" % self.object_path, '') + + # store the D-Bus encoded instance name ('_' for single-instance jobs) + if instance_name not in self.instance_names: + self.instance_names.append(instance_name) + + instance = JobInstance(self, instance_name, instance_path) + self.instances.append(instance) + return instance + + def _get_dbus_instance(self, name): + """ + Retrieve D-Bus job instance and its properties. @name: D-Bus encoded instance name. @@ -586,7 +661,7 @@ """ for name in self.instance_names: - instance = self._get_instance(name) + instance = self._get_dbus_instance(name) try: instance.Stop(wait) except dbus.exceptions.DBusException: @@ -600,7 +675,7 @@ @wait: if False, stop job instances asynchronously. """ for name in self.instance_names: - instance = self._get_instance(name) + instance = self._get_dbus_instance(name) instance.Restart(wait) def instance_object_paths(self): @@ -629,9 +704,13 @@ assert(name in self.instance_names) - instance = self._get_instance(name) + instance = self._get_dbus_instance(name) + assert (instance) properties = dbus.Interface(instance, FREEDESKTOP_PROPERTIES) + assert (properties) + + # don't assert as there may not be any processes procs = properties.Get(INSTANCE_INTERFACE_NAME, 'processes') pid_map = {} @@ -845,10 +924,13 @@ def destroy(self): """ Stop the instance and cleanup. + + Note: If the instance specified retain when created, this will + be a NOP. """ - self.stop() - self.logfile.destroy() - + if not self.job.retain: + self.stop() + self.logfile.destroy() class SystemInit(Upstart): === modified file 'scripts/tests/test_pyupstart_session_init.py' --- scripts/tests/test_pyupstart_session_init.py 2013-09-10 16:57:59 +0000 +++ scripts/tests/test_pyupstart_session_init.py 2013-11-03 00:28:07 +0000 @@ -340,6 +340,23 @@ # Ensure no stateful-reexec already performed. self.assertFalse(re.search('state-fd', output)) + version = self.upstart.version() + self.assertTrue(version) + + # create a job and start it, marking it such that the .conf file + # will be retained when object becomes unusable (after re-exec). + job = self.upstart.job_create('sleeper', 'exec sleep 123', retain=True) + self.assertTrue(job) + + # Used when recreating the job + conf_path = job.conffile + + inst = job.start() + self.assertTrue(inst) + pids = job.pids() + self.assertEqual(len(pids), 1) + pid = pids['main'] + # Trigger re-exec and catch the D-Bus exception resulting # from disconnection from Session Init when it severs client # connections. @@ -359,8 +376,40 @@ # instantaneous, try a few times. self.upstart.polling_connect(force=True) + # Since the parent job was created with 'retain', this is actually + # a NOP but is added to denote that the old instance is dead. + inst.destroy() + # check that we can still operate on the re-exec'd Upstart - self.assertTrue(self.upstart.version()) + version_postexec = self.upstart.version() + self.assertTrue(version_postexec) + self.assertEqual(version, version_postexec) + + # Ensure the job is still running with the same PID + os.kill(pid, 0) + + # XXX: The re-exec will have severed the D-Bus connection to + # Upstart. Hence, revivify the job with some magic. + job = self.upstart.job_recreate('sleeper', conf_path) + self.assertTrue(job) + + # Recreate the instance + inst = job.get_instance() + self.assertTrue(inst) + + self.assertTrue(job.running('_')) + pids = job.pids() + self.assertEqual(len(pids), 1) + self.assertTrue(pids['main']) + + # The pid should not have changed after a restart + self.assertEqual(pid, pids['main']) + + job.stop() + + # Ensure the pid has gone + with self.assertRaises(ProcessLookupError): + os.kill(pid, 0) self.stop_session_init() === 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:56:22 +0000 @@ -63,9 +63,12 @@ # create a job and start it, marking it such that the .conf file # will be retained when object becomes unusable (after re-exec). - job = self.upstart.job_create('sleeper', 'exec sleep 123') + job = self.upstart.job_create('sleeper', 'exec sleep 123', retain=True) self.assertTrue(job) + # Used when recreating the job + conf_path = job.conffile + inst = job.start() self.assertTrue(inst) pids = job.pids() @@ -80,6 +83,10 @@ # try a few times. self.upstart.polling_connect(force=True) + # Since the parent job was created with 'retain', this is actually + # a NOP but is added to denote that the old instance is dead. + inst.destroy() + # check that we can still operate on the re-exec'd Upstart version_postexec = self.upstart.version() self.assertTrue(version_postexec) @@ -88,32 +95,29 @@ # Ensure the job is still running with the same PID os.kill(pid, 0) + # XXX: The re-exec will have severed the D-Bus connection to + # Upstart. Hence, revivify the job with some magic. + job = self.upstart.job_recreate('sleeper', conf_path) + self.assertTrue(job) + + # Recreate the instance + inst = job.get_instance() + self.assertTrue(inst) + self.assertTrue(job.running('_')) - pids = job.pids() self.assertEqual(len(pids), 1) self.assertTrue(pids['main']) - # Ensure pid remains the same + # The pid should not have changed after a restart self.assertEqual(pid, pids['main']) - # Exceptions will be caught by the unittest framework - inst.stop() - - job.start() - - pids = job.pids() - self.assertEqual(len(pids), 1) - self.assertTrue(pids['main']) - - os.kill(pids['main'], 0) - self.assertTrue(job.running('_')) - - # The pid should have changed after a restart - self.assertNotEqual(pid, pids['main']) - job.stop() + # Ensure the pid has gone + with self.assertRaises(ProcessLookupError): + os.kill(pid, 0) + # Clean up self.upstart.destroy()
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
