# HG changeset patch # User Alain Leufroy # Date 1615059541 -3600 # Sat Mar 06 20:39:01 2021 +0100 # Node ID a323fcaec0f5b487d10c38a63fdf344fa30a10db # Parent 1c7539e31415e64bbb6690b672aa05bcd118e5cf interrup/preserve: refactorize and rename options diff --git a/overlayctl b/overlayctl --- a/overlayctl +++ b/overlayctl @@ -54,6 +54,10 @@ """Raised on exception.""" +class ActiveBranchError(SystemExit): + """Raised when a layer branch has unexpected active descendants.""" + + class NoResult(OverlayctlError): """Raised when no result where found.""" @@ -560,6 +564,36 @@ return result[layer][1:] +@contextmanager +def _stop_branch(current, interrupt=False, preserve=False, restart=False, allbranch=False): + """Ensure layer and its descendant are stopped inside this context. + If current or any of its descendan is active, raise ActiveBranchError error. + + :stop_others: automatically stop activated descendants. Then restart them at exit. + + :kepp_others: make this a null context if True + """ + if preserve: + yield + else: + branch = chain(_iter_descendants(current), [current]) + if allbranch: + branch = chain(branch, (x for x in _iter_ascendants(current) if x.is_managed())) + activated = [x for x in branch if x.automountunit.is_active()] + if activated: + if interrupt: + for layer in activated: + layer.stop() + else: + raise ActiveBranchError( + 'The following units should be stopped: %s' + % ', '.join(x.name for x in activated)) + yield + if interrupt and restart: + for layer in activated: + layer.start() + + def creater(name, lowers, start=None, permanent=None): """create an overlay at `name` that inherites from `lowerdir`. @@ -605,89 +639,54 @@ def editer( name, appended=None, prepended=None, removed=None, - stop_others=False, keep_others=False): + interrupt=False, preserve=False): """Ask user to edit lowers of the given overlay name, rewrite the metadata and unit files accordingly, finaly retart systemd units of the edited overlay. """ layer = build_layer(name) - to_stop = [] - if not keep_others: - to_stop[:] = [layer for layer in _iter_descendants(layer) - if layer.automountunit.is_active()] - if to_stop: - if stop_others: - for descendant in to_stop: - descendant.stop() - else: - logger.error( - 'You should prefere to stop the following overlays: \n\n%s\n', - ', '.join(x.name for x in to_stop)) + with _stop_branch(layer, interrupt=interrupt, preserve=preserve, restart=interrupt): + if not appended and not prepended and not removed: + with NamedTemporaryFile() as fobj: + fobj.write(b'# -*- encoding: utf-8 -*-\n') + fobj.write(b'# Note: Leave unchanged or fully empty to ignore changes\n') + fobj.write(b'\n'.join(lower.name.encode('utf-8') for lower in layer.lowers)) + fobj.flush() + _get_editor_on_file(fobj.name) + fobj.seek(0) + lowers = [ + build_layer(line.strip()) + for line in fobj.read().decode('utf-8').splitlines() + if not line.strip().startswith('#') and line.strip() + ] + fobj.seek(0) + if not fobj.read(1): + return # Ignore empty file for convenience + else: + prepended = map(build_layer, prepended or ()) + appended = map(build_layer, appended or ()) + removed = list(map(build_layer, removed or ())) + lowers = chain( + prepended, + (lower for lower in layer.lowers if lower not in removed), + appended) + if lowers == layer.lowers: return - if not appended and not prepended and not removed: - with NamedTemporaryFile() as fobj: - fobj.write(b'# -*- encoding: utf-8 -*-\n') - fobj.write(b'# Note: Leave unchanged or fully empty to ignore changes\n') - fobj.write(b'\n'.join(lower.name.encode('utf-8') for lower in layer.lowers)) - fobj.flush() - _get_editor_on_file(fobj.name) - fobj.seek(0) - lowers = [ - build_layer(line.strip()) - for line in fobj.read().decode('utf-8').splitlines() - if not line.strip().startswith('#') and line.strip() - ] - fobj.seek(0) - if not fobj.read(1): - return # Ignore empty file for convenience - else: - prepended = map(build_layer, prepended or ()) - appended = map(build_layer, appended or ()) - removed = list(map(build_layer, removed or ())) - lowers = chain( - prepended, - (lower for lower in layer.lowers if lower not in removed), - appended) - if lowers == layer.lowers: - return - with layer.temporary_stop(): layer.lowers[:] = lowers layer.dump() systemctl.daemon_reload() - for descendant in to_stop: - descendant.start() # XXX propagate to descendants logger.debug("Systemd \"%s\" restarted", layer.unitname) logger.info("\"%s\" updated", layer.name) -def starter(name, preserve_others=False, stop_others=False, permanent=False): +def starter(name, interrupt=False, preserve=False, permanent=False): layer = build_layer(name) if not layer.is_managed(): logger.info("\"%s\" not managed. Nothing to do.", layer.name) return - if not preserve_others: - to_stop = [] - for _layer in chain(_iter_descendants(layer), _iter_ascendants(layer)): - if not _layer.is_managed(): - continue - if _layer.automountunit.is_active(): - to_stop.append(_layer) - if to_stop: - if not stop_others: - logger.error( - 'You should prefere to stop the following overlays: \n\n%s\n', - ', '.join(x.name for x in to_stop)) - logger.info( - 'You may want to use `--stop-others`. ' - 'Or see --help for more options.') - return - for _layer in to_stop: - _layer.disable() if permanent else _layer.stop() - logger.info( - 'The following overlays were stopped: \n\n' - + ', '.join(x.name for x in to_stop)) - layer.start(permanent) + with _stop_branch(layer, interrupt, preserve, restart=False, allbranch=True): + layer.start(permanent) def stoper(name, permanent=False): @@ -754,7 +753,7 @@ raise NoResult("No overlay matches.") -def deplacer(oldname, newname): +def deplacer(oldname, newname, interrupt=False, preserve=False): # Checks _new = Layer(newname) @@ -768,38 +767,22 @@ logger.info('Not a new unit name, nothing to do.') return - descendants = list(_iter_descendants(layer)) - logger.debug( - '%s overlays affected: %s', len(descendants), - ', '.join(x.name for x in descendants) - ) - activated = [x.name for x in descendants if x.automountunit.is_active()] - if activated: - # XXX add --stop - logger.error( - 'The following units must be stopped: %s', ', '.join(activated)) + with _stop_branch(layer, interrupt=interrupt, preserve=preserve, restart=interrupt): - is_automonted = layer.automountunit.is_active() - if is_automonted: - layer.stop() - - # Move overlay dirs. - layer.upperdir.rename(_new.upperdir) - layer.workdir.rename(_new.workdir) + # Move overlay dirs. + layer.upperdir.rename(_new.upperdir) + layer.workdir.rename(_new.workdir) - layer.delete() + layer.delete() - layer.name = newname - layer.dump() - - # Update existing layers that depends on the moved one. - for layer in descendants: + layer.name = newname layer.dump() - systemctl.daemon_reload() + # Update existing layers that depends on the moved one. + for layer in _iter_descendants(layer): + layer.dump() - if is_automonted: - layer.start() + systemctl.daemon_reload() def _setup_logger(level): @@ -854,6 +837,23 @@ help="available commands', description='Valid commands to manipulate overlays.", ) + def add_interrupt_preserve_arguments(parser): + parser.add_argument( + '--interrupt', '-i', + action='store_true', + default=False, + help=('automatically interrupt descendants to prevent broken mount point. ' + 'Restart them afterward.') + ) + parser.add_argument( + '--preserve', '-K', + action='store_true', + default=False, + help=('do not check if descendant overlays are started ' + '(may produce inconsistant behaviours)') + ) + + # create create = subparser.add_parser( 'create', @@ -1024,20 +1024,7 @@ edit.add_argument( '-d', '--delete', action='append', help="append the following names", ) - edit.add_argument( - '--stop-others', '-s', - action='store_true', - default=False, - help=('automatically stop descendants to prevent broken mount point. ' - 'Restart them afterward.') - ) - edit.add_argument( - '--keep-others', '-K', - action='store_true', - default=False, - help=('do not check if descendant overlays are started ' - '(may produce inconsistant behaviours)') - ) + add_interrupt_preserve_arguments(edit) edit.add_argument( 'mountdir', metavar='MOUNTDIR', @@ -1048,7 +1035,8 @@ ) def _editer(args): - editer(args.mountdir, args.append, args.prepend, args.delete, args.stop_others, args.keep_others) + editer(args.mountdir, args.append, args.prepend, args.delete, + interrupt=args.interrupt, preserve=args.preserve) edit.set_defaults(func=_editer) @@ -1060,11 +1048,12 @@ "move an overlay created by this tool. " "Other overlays that depends on it are updated"), ) + add_interrupt_preserve_arguments(move) move.add_argument('old', metavar='OLD', help="existing overlay name.") move.add_argument('new', metavar='NEW', help="New overlay name.") def _deplacer(args): - deplacer(args.old, args.new) + deplacer(args.old, args.new, interrupt=args.interrupt, preserve=args.preserve) move.set_defaults(func=_deplacer) @@ -1086,19 +1075,7 @@ "it is assumed to be %s/{MOUNTDIR}" % (os.path.sep, MOUNTDIR) ), ) - start.add_argument( - '--stop-others', '-s', - action='store_true', - default=False, - help='stop ascendants and descendants if needed' - ) - start.add_argument( - '--keep-others', '-K', - action='store_true', - default=False, - help=('do not check if descendant or ascendants overlays are started ' - '(may produce inconsistant behaviours)') - ) + add_interrupt_preserve_arguments(start) start.add_argument( '--permanent', '-p', action='store_true', default=False, @@ -1106,7 +1083,8 @@ ) def _start(args): - starter(args.mountdir, args.keep_others, args.stop_others, args.permanent) + starter(args.mountdir, interrupt=args.interrupt, + preserve=args.preserve, permanent=args.permanent) start.set_defaults(func=_start) diff --git a/test_overlayctl.py b/test_overlayctl.py --- a/test_overlayctl.py +++ b/test_overlayctl.py @@ -298,6 +298,7 @@ self.get_mount_dir('archbase').mkdir() overlayctl.creater('layer1', ['archbase']) self.systemctl.reset_mock() + self.systemctl.status.return_value = {'Active': 'inactive'} overlayctl.starter('layer1') self.assert_layer_started('layer1') self.systemctl.enable.assert_not_called() @@ -309,6 +310,7 @@ self.get_mount_dir('archbase').mkdir() overlayctl.creater('layer1', ['archbase']) self.systemctl.reset_mock() + self.systemctl.status.return_value = {'Active': 'inactive'} overlayctl.starter('layer1', permanent=True) self.assert_layer_started('layer1') self.assert_layer_enabled('layer1') @@ -328,24 +330,25 @@ overlayctl.creater('layer1', ['archbase']) overlayctl.creater('layer2', ['layer1']) self.systemctl.reset_mock() - overlayctl.starter('layer1') + with self.assertRaises(SystemExit): + overlayctl.starter('layer1') self.systemctl.start.assert_not_called() def test_user_can_force_to_start_a_lower_with_active_descendants(self): # A layer with active_descendants can be started with the - # --preserve_others option. This may be dangereous, but the + # --preserve option. This may be dangereous, but the # user is the master. self.systemctl.status.side_effect = [{'Active': 'inactive'}] self.get_mount_dir('archbase').mkdir() overlayctl.creater('layer1', ['archbase']) overlayctl.creater('layer2', ['layer1']) self.systemctl.reset_mock() - overlayctl.starter('layer1', preserve_others=True) + overlayctl.starter('layer1', preserve=True) self.assert_layer_started('layer1') def test_starting_stops_descendants_if_needed(self): # Active descendant layers can be stopped automatically before - # starting the layer with the --stop-others options. + # starting the layer with the --interrupt options. self.systemctl.status.side_effect = [ {'Active': 'active'}, {'Active': 'active'}, {'Active': 'inactive'}] @@ -354,7 +357,7 @@ overlayctl.creater('layer2', ['layer1']) overlayctl.creater('layer3', ['layer2']) self.systemctl.reset_mock() - overlayctl.starter('layer1', stop_others=True) + overlayctl.starter('layer1', interrupt=True) self.assert_layer_stopped('layer2') self.assert_layer_stopped('layer3') self.assert_layer_started('layer1') @@ -479,7 +482,8 @@ overlayctl.creater('layer', ['lower']) self.systemctl.reset_mock() self.systemctl.status.return_value = {'Active': 'active'} - overlayctl.editer('lower', removed=['base']) + with self.assertRaises(SystemExit): + overlayctl.editer('lower', removed=['base']) self.check_mount_unit('lower', [self.get_mount_dir('base')]) def test_editing_layer_with_active_descendant(self): @@ -490,7 +494,7 @@ overlayctl.creater('layer', ['lower']) self.systemctl.reset_mock() self.systemctl.status.return_value = {'Active': 'active'} - overlayctl.editer('lower', removed=['base'], keep_others=True) + overlayctl.editer('lower', removed=['base'], preserve=True) self.check_mount_unit('lower', []) @@ -536,13 +540,13 @@ self.assert_daemon_reload_called() def test_moved_layer_is_restarted(self): - # Moving an activated terminal layer is ok, It's still + # Moving an activated terminal layer is ok with --interrupt, It's still # activated at the end. self.get_mount_dir('base').mkdir() overlayctl.creater('layer1', ['base']) self.systemctl.reset_mock() self.systemctl.status.return_value = {'Active': 'active'} - overlayctl.deplacer('layer1', 'renamed') + overlayctl.deplacer('layer1', 'renamed', interrupt=True) self.systemctl.stop.assert_any_call(self.get_unit_name('layer1') + '.mount') self.systemctl.stop.assert_any_call(self.get_unit_name('layer1') + '.automount') self.systemctl.start.assert_any_call(self.get_unit_name('renamed') + '.automount') @@ -565,7 +569,8 @@ overlayctl.creater('layer', ['lower']) self.systemctl.reset_mock() self.systemctl.status.return_value = {'Active': 'active'} - overlayctl.deplacer('lower', 'newlower') + with self.assertRaises(SystemExit): + overlayctl.deplacer('lower', 'newlower') def assert_layer_not_exist(self, name): self.assertFalse(self.get_unit_path(name, 'mount').exists())