From fb78d4a71a23f8a1a547356e1ab4bf9214a34c35 Mon Sep 17 00:00:00 2001 From: William Brown Date: Mar 24 2017 07:09:37 +0000 Subject: Ticket 10 - Improve command line tool arguments Bug Description: Previously our tools were a poc, and didn't have very good layout of commands. Fix Description: Improve these, to be of the form: [] For example: usage: dsadm [-h] [-v] instance {restart,start,stop,status,db2index} ... positional arguments: instance The name of the instance to act upon {restart,start,stop,status,db2index} action restart Restart an instance of Directory Server, if it is running: else start it. start Start an instance of Directory Server, if it is not currently running stop Stop an instance of Directory Server, if it is currently running status Check running status of an instance of Directory Server db2index Initialise a reindex of the server database. The server must be stopped for this to proceed. usage: dsidm instance user [-h] {list,get,get_dn,create,delete} ... positional arguments: {list,get,get_dn,create,delete} action list list get get get_dn get_dn create create delete deletes the object usage: dsidm instance user create [-h] [--uid [UID]] [--cn [CN]] [--sn [SN]] [--uidNumber [UIDNUMBER]] [--gidNumber [GIDNUMBER]] [--homeDirectory [HOMEDIRECTORY]] optional arguments: -h, --help show this help message and exit --uid [UID] Value of uid --cn [CN] Value of cn --sn [SN] Value of sn --uidNumber [UIDNUMBER] Value of uidNumber --gidNumber [GIDNUMBER] Value of gidNumber --homeDirectory [HOMEDIRECTORY] Value of homeDirectory https://pagure.io/lib389/issue/10 Author: wibrown Review by: spichugi (Thanks!) --- diff --git a/cli/dsadm b/cli/dsadm index 0ee0970..4779b25 100755 --- a/cli/dsadm +++ b/cli/dsadm @@ -12,12 +12,15 @@ import argparse import logging import sys +# This has to happen before we import DirSrv else it tramples our config ... :( +logging.basicConfig(format='%(message)s') + from lib389.cli_base import _get_arg from lib389 import DirSrv from lib389.cli_adm import instance as cli_instance +from lib389.cli_adm import dbtasks as cli_dbtasks from lib389.cli_base import disconnect_instance -logging.basicConfig() log = logging.getLogger("dsadm") if __name__ == '__main__': @@ -28,17 +31,16 @@ if __name__ == '__main__': help="Display verbose operation tracing during command execution", action='store_true', default=False ) - # Should this actually be in the sub modules? That way they can set the requirements. - parser.add_argument('-Z', '--instance', + parser.add_argument('instance', help="The name of the instance to act upon", - default=None, ) - subparsers = parser.add_subparsers(help="resources to act upon") + subparsers = parser.add_subparsers(help="action") # We stack our needed options in via submodules. cli_instance.create_parser(subparsers) + cli_dbtasks.create_parser(subparsers) # Then we tell it to execute. @@ -58,7 +60,7 @@ if __name__ == '__main__': # Assert we have a resources to work on. if not hasattr(args, 'func'): - log.error("No resource provided to act upon") + log.error("No action provided") log.error("USAGE: dsadm [options] [action options]") sys.exit(1) @@ -68,18 +70,13 @@ if __name__ == '__main__': result = True - # we allocate an instance in all cases unless we are CREATING the server. - if not (hasattr(args, 'noinst') and args.noinst is True): - # Get the instance name if needed. - inst_id = _get_arg( args.instance, msg="Directory Server instance name") - - # Allocate the instance based on name - insts = inst.list(serverid=inst_id) - if len(insts) != 1: - raise ValueError("No such instance %s" % inst_id) + # Allocate the instance based on name + insts = inst.list(serverid=args.instance) + if len(insts) != 1: + raise ValueError("No such instance %s" % args.instance) - inst.allocate(insts[0]) - log.debug('Instance allocated') + inst.allocate(insts[0]) + log.debug('Instance allocated') if args.verbose: result = args.func(inst, log, args) diff --git a/cli/dsconf b/cli/dsconf index 72ec909..36834fa 100755 --- a/cli/dsconf +++ b/cli/dsconf @@ -17,6 +17,7 @@ import sys logging.basicConfig(format='%(message)s') from lib389 import DirSrv +from lib389._constants import DN_CONFIG from lib389.cli_conf import backend as cli_backend from lib389.cli_conf import plugin as cli_plugin from lib389.cli_conf import schema as cli_schema @@ -34,26 +35,21 @@ if __name__ == '__main__': # Can we get default options for these from .rc file? + parser.add_argument('instance', + help="The instance name OR the LDAP url to connect to, IE localhost, ldap://mai.example.com:389", + ) + parser.add_argument('-v', '--verbose', + help="Display verbose operation tracing during command execution", + action='store_true', default=False + ) parser.add_argument('-D', '--binddn', help="The account to bind as for executing operations", default='cn=Directory Manager', ) - parser.add_argument('-H', '--ldapurl', - help="The LDAP url to connect to, IE ldap://mai.example.com:389", - default='ldap://localhost', - ) - parser.add_argument('-b', '--basedn', - help="The basedn for this operation.", - default=None, - ) parser.add_argument('-Z', '--starttls', help="Connect with StartTLS", default=False, action='store_true' ) - parser.add_argument('-v', '--verbose', - help="Display verbose operation tracing during command execution", - action='store_true', default=False - ) subparsers = parser.add_subparsers(help="resources to act upon") @@ -83,15 +79,19 @@ if __name__ == '__main__': log.error("USAGE: dsadm [options] [action options]") sys.exit(1) + # TODO: Parse instance to a url in some cases. + ldapurl = args.instance + # Connect + # We don't need a basedn, because the config objects derive it properly inst = None if args.verbose: - inst = connect_instance(args.ldapurl, args.binddn, args.verbose, args.starttls) - args.func(inst, args.basedn, log, args) + inst = connect_instance(ldapurl=ldapurl, binddn=args.binddn, verbose=args.verbose, starttls=args.starttls) + args.func(inst, None, log, args) else: try: - inst = connect_instance(args.ldapurl, args.binddn, args.verbose, args.starttls) - args.func(inst, args.basedn, log, args) + inst = connect_instance(ldapurl=ldapurl, binddn=args.binddn, verbose=args.verbose, starttls=args.starttls) + args.func(inst, None, log, args) except Exception as e: log.debug(e, exc_info=True) log.error("Error: %s" % e) diff --git a/cli/dscreate b/cli/dscreate new file mode 100755 index 0000000..cec121f --- /dev/null +++ b/cli/dscreate @@ -0,0 +1,84 @@ +#!/usr/bin/python + +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2016 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- + +import argparse +import logging +import sys + +# This has to happen before we import DirSrv else it tramples our config ... :( +logging.basicConfig(format='%(message)s') + +from lib389 import DirSrv +from lib389.cli_adm import instance as cli_instance + +log = logging.getLogger("dscreate") + +if __name__ == '__main__': + + parser = argparse.ArgumentParser() + + parser.add_argument('-v', '--verbose', + help="Display verbose operation tracing during command execution", + action='store_true', default=False + ) + + subparsers = parser.add_subparsers(help="action") + + fromfile_parser = subparsers.add_parser('fromfile', help="Create an instance of Directory Server from an inf answer file") + fromfile_parser.add_argument('file', help="Inf file to use with prepared answers. You can generate an example of this with 'dscreate example'") + fromfile_parser.add_argument('-n', '--dryrun', help="Validate system and configurations only. Do not alter the system.", action='store_true', default=False) + fromfile_parser.add_argument('--IsolemnlyswearthatIamuptonogood', dest="ack", + help="""You are here likely here by mistake! You want setup-ds.pl! +By setting this value you acknowledge and take responsibility for the fact this command is UNTESTED and NOT READY. You are ON YOUR OWN! +""", + action='store_true', default=False) + fromfile_parser.add_argument('-c', '--containerised', help="Indicate to the installer that this is running in a container. Used to disable systemd native components, even if they are installed.", action='store_true', default=False) + fromfile_parser.set_defaults(func=cli_instance.instance_create) + + example_parser = subparsers.add_parser('example', help="Display an example ini answer file, with comments") + example_parser.set_defaults(func=cli_instance.instance_example) + + args = parser.parse_args() + + if args.verbose: + log.setLevel(logging.DEBUG) + else: + log.setLevel(logging.INFO) + + log.debug("The 389 Directory Server Creation Tool") + # Leave this comment here: UofA let me take this code with me provided + # I gave attribution. -- wibrown + log.debug("Inspired by works of: ITS, The University of Adelaide") + + log.debug("Called with: %s", args) + + inst = DirSrv(verbose=args.verbose) + + result = False + + if args.verbose: + result = args.func(inst, log, args) + else: + try: + result = args.func(inst, log, args) + except Exception as e: + log.debug(e, exc_info=True) + log.error("Error: %s" % e.message) + + if result is True: + log.info('FINISH: Command succeeded') + elif result is False: + log.info('FAIL: Command failed. See output for details.') + + # Done! + log.debug("dscreate is brought to you by the letter S and the number 22.") + + if result is False: + sys.exit(1) diff --git a/cli/dsidm b/cli/dsidm index fd697ba..789c7c1 100755 --- a/cli/dsidm +++ b/cli/dsidm @@ -13,6 +13,9 @@ import argparse # import argcomplete import logging +# This has to happen before we import DirSrv else it tramples our config ... :( +logging.basicConfig(format='%(message)s') + from lib389.cli_idm import organisationalunit as cli_ou from lib389.cli_idm import group as cli_group from lib389.cli_idm import posixgroup as cli_posixgroup @@ -20,7 +23,6 @@ from lib389.cli_idm import user as cli_user from lib389.cli_base import connect_instance, disconnect_instance -logging.basicConfig() log = logging.getLogger("dsidm") if __name__ == '__main__': @@ -29,17 +31,9 @@ if __name__ == '__main__': parser = argparse.ArgumentParser() # First, add the LDAP options - parser.add_argument('-D', '--binddn', - help="The account to bind as for executing operations", - default=None, - ) - parser.add_argument('-H', '--ldapurl', - help="The LDAP url to connect to, IE ldap://mai.example.com:389", - default=None, - ) - parser.add_argument('-Z', '--starttls', - help="Connect with StartTLS", - default=False, action='store_true' + + parser.add_argument('instance', + help="The instance name OR the LDAP url to connect to, IE localhost, ldap://mai.example.com:389", ) parser.add_argument('-b', '--basedn', help="Basedn (root naming context) of the instance to manage", @@ -49,6 +43,14 @@ if __name__ == '__main__': help="Display verbose operation tracing during command execution", action='store_true', default=False ) + parser.add_argument('-D', '--binddn', + help="The account to bind as for executing operations", + default='cn=Directory Manager', + ) + parser.add_argument('-Z', '--starttls', + help="Connect with StartTLS", + default=False, action='store_true' + ) subparsers = parser.add_subparsers(help="resources to act upon") @@ -74,14 +76,16 @@ if __name__ == '__main__': log.debug("Called with: %s", args) + ldapurl = args.instance + # Connect inst = None if args.verbose: - inst = connect_instance(args.ldapurl, args.binddn, args.verbose, args.starttls) + inst = connect_instance(ldapurl=ldapurl, binddn=args.binddn, verbose=args.verbose, starttls=args.starttls) args.func(inst, args.basedn, log, args) else: try: - inst = connect_instance(args.ldapurl, args.binddn, args.verbose, args.starttls) + inst = connect_instance(ldapurl=ldapurl, binddn=args.binddn, verbose=args.verbose, starttls=args.starttls) args.func(inst, args.basedn, log, args) except Exception as e: log.debug(e, exc_info=True) diff --git a/lib389/__init__.py b/lib389/__init__.py index ecd9936..df9f55e 100644 --- a/lib389/__init__.py +++ b/lib389/__init__.py @@ -433,7 +433,7 @@ class DirSrv(SimpleLDAPObject, object): self.state) if SER_SERVERID_PROP not in args: - self.log.debug('SER_SERVERID_PROP not provided') + self.log.debug('SER_SERVERID_PROP not provided, assuming non-local instance') # The lack of this value basically rules it out in most cases self.isLocal = False self.ds_paths = Paths(instance=self) @@ -1118,7 +1118,7 @@ class DirSrv(SimpleLDAPObject, object): self.state = DIRSRV_STATE_OFFLINE - def start(self, timeout=120): + def start(self, timeout=120, post_open=True): ''' It starts an instance and rebind it. Its final state after rebind (open) is DIRSRV_STATE_ONLINE @@ -1174,7 +1174,8 @@ class DirSrv(SimpleLDAPObject, object): count -= 1 if not pid_exists(pid): raise Exception("Failed to start DS") - self.open() + if post_open: + self.open() def stop(self, timeout=120): ''' @@ -1241,7 +1242,7 @@ class DirSrv(SimpleLDAPObject, object): # Wait return pid_exists(pid) - def restart(self, timeout=120): + def restart(self, timeout=120, post_open=True): ''' It restarts an instance and rebind it. Its final state after rebind (open) is DIRSRV_STATE_ONLINE. @@ -1255,7 +1256,7 @@ class DirSrv(SimpleLDAPObject, object): ''' self.stop(timeout) time.sleep(1) - self.start(timeout) + self.start(timeout, post_open) def _infoBackupFS(self): """ diff --git a/lib389/cli_adm/dbtasks.py b/lib389/cli_adm/dbtasks.py new file mode 100644 index 0000000..276f478 --- /dev/null +++ b/lib389/cli_adm/dbtasks.py @@ -0,0 +1,18 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2016 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- + +def dbtasks_db2index(inst, log, args): + # inst.db2index(suffixes=[args.suffix,]) + inst.db2index(bename=args.backend) + +def create_parser(subcommands): + db2index_parser = subcommands.add_parser('db2index', help="Initialise a reindex of the server database. The server must be stopped for this to proceed.") + # db2index_parser.add_argument('suffix', help="The suffix to reindex. IE dc=example,dc=com.") + db2index_parser.add_argument('backend', help="The backend to reindex. IE userRoot") + db2index_parser.set_defaults(func=dbtasks_db2index) + diff --git a/lib389/cli_adm/instance.py b/lib389/cli_adm/instance.py index e14fa79..f59fb2b 100644 --- a/lib389/cli_adm/instance.py +++ b/lib389/cli_adm/instance.py @@ -30,13 +30,14 @@ def instance_list(inst, log, args): log.info(e) log.info("Perhaps you need to be a different user?") +def instance_restart(inst, log, args): + inst.restart(post_open=False) + def instance_start(inst, log, args): - if inst.status() is False: - inst.start() + inst.start(post_open=False) def instance_stop(inst, log, args): - if inst.status() is True: - inst.stop() + inst.stop() def instance_status(inst, log, args): if inst.status() is True: @@ -113,40 +114,20 @@ def instance_example(inst, log, args): print(g2b.collect_help()) print(s2b.collect_help()) -def create_parser(subparsers): - instance_parser = subparsers.add_parser('instance', help="Manager instances of Directory Server") - - subcommands = instance_parser.add_subparsers(help="action") - - list_parser = subcommands.add_parser('list', help="List installed instances of Directory Server") - list_parser.set_defaults(func=instance_list) - list_parser.set_defaults(noinst=True) +def create_parser(subcommands): + # list_parser = subcommands.add_parser('list', help="List installed instances of Directory Server") + # list_parser.set_defaults(func=instance_list) + # list_parser.set_defaults(noinst=True) + restart_parser = subcommands.add_parser('restart', help="Restart an instance of Directory Server, if it is running: else start it.") + restart_parser.set_defaults(func=instance_restart) start_parser = subcommands.add_parser('start', help="Start an instance of Directory Server, if it is not currently running") - # start_parser.add_argument('instance', nargs=1, help="The name of the instance to start.") start_parser.set_defaults(func=instance_start) stop_parser = subcommands.add_parser('stop', help="Stop an instance of Directory Server, if it is currently running") - # stop_parser.add_argument('instance', nargs=1, help="The name of the instance to stop.") stop_parser.set_defaults(func=instance_stop) status_parser = subcommands.add_parser('status', help="Check running status of an instance of Directory Server") - # status_parser.add_argument('instance', nargs=1, help="The name of the instance to check.") status_parser.set_defaults(func=instance_status) - create_parser = subcommands.add_parser('create', help="Create an instance of Directory Server. Can be interactive or silent with an inf answer file") - create_parser.add_argument('-n', '--dryrun', help="Validate system and configurations only. Do not alter the system.", action='store_true', default=False) - create_parser.add_argument('-f', '--file', help="Inf file to use with prepared answers") - create_parser.add_argument('--IsolemnlyswearthatIamuptonogood', dest="ack", - help="""You are here likely here by mistake! You want setup-ds.pl! -By setting this value you acknowledge and take responsibility for the fact this command is UNTESTED and NOT READY. You are ON YOUR OWN! -""", - action='store_true', default=False) - create_parser.add_argument('-c', '--containerised', help="Indicate to the installer that this is running in a container. Used to disable systemd native components, even if they are installed.", action='store_true', default=False) - create_parser.set_defaults(func=instance_create) - create_parser.set_defaults(noinst=True) - - example_parser = subcommands.add_parser('example', help="Display an example ini answer file, with comments") - example_parser.set_defaults(func=instance_example) - example_parser.set_defaults(noinst=True) diff --git a/lib389/cli_base/__init__.py b/lib389/cli_base/__init__.py index a3e59ac..9dca625 100644 --- a/lib389/cli_base/__init__.py +++ b/lib389/cli_base/__init__.py @@ -50,8 +50,8 @@ def _get_args(args, kws): def _get_attributes(args, attrs): kwargs = {} for attr in attrs: - if args is not None and len(args) > 0: - kwargs[attr] = args.pop(0) + if args is not None and hasattr(args, attr) and getattr(args, attr) is not None: + kwargs[attr] = getattr(args, attr) else: if attr.lower() == 'userpassword': kwargs[attr] = getpass("Enter value for %s : " % attr) @@ -88,6 +88,10 @@ def disconnect_instance(inst): if inst is not None: inst.close() +def populate_attr_arguments(parser, attributes): + for attr in attributes: + parser.add_argument('--%s' % attr, nargs='?', help="Value of %s" % attr) + def _generic_list(inst, basedn, log, manager_class, **kwargs): mc = manager_class(inst, basedn) ol = mc.list() diff --git a/lib389/cli_idm/group.py b/lib389/cli_idm/group.py index 373d1de..a64eb86 100644 --- a/lib389/cli_idm/group.py +++ b/lib389/cli_idm/group.py @@ -7,9 +7,10 @@ # --- END COPYRIGHT BLOCK --- import argparse -from lib389.idm.group import Group, Groups +from lib389.idm.group import Group, Groups, MUST_ATTRIBUTES from lib389.cli_base import ( + populate_attr_arguments, _generic_list, _generic_get, _generic_get_dn, @@ -65,9 +66,7 @@ def create_parser(subparsers): create_parser = subcommands.add_parser('create', help='create') create_parser.set_defaults(func=create) - create_parser.add_argument('extra', nargs=argparse.REMAINDER, - help='A create may take one or more extra arguments. This parameter provides them' - ) + populate_attr_arguments(create_parser, MUST_ATTRIBUTES) delete_parser = subcommands.add_parser('delete', help='deletes the object') delete_parser.set_defaults(func=delete) diff --git a/lib389/cli_idm/organisationalunit.py b/lib389/cli_idm/organisationalunit.py index 46ec267..a9a9a08 100644 --- a/lib389/cli_idm/organisationalunit.py +++ b/lib389/cli_idm/organisationalunit.py @@ -7,9 +7,10 @@ # --- END COPYRIGHT BLOCK --- import argparse -from lib389.idm.organisationalunit import OrganisationalUnit, OrganisationalUnits +from lib389.idm.organisationalunit import OrganisationalUnit, OrganisationalUnits, MUST_ATTRIBUTES from lib389.cli_base import ( + populate_attr_arguments, _generic_list, _generic_get, _generic_get_dn, @@ -65,9 +66,7 @@ def create_parser(subparsers): create_parser = subcommands.add_parser('create', help='create') create_parser.set_defaults(func=create) - create_parser.add_argument('extra', nargs=argparse.REMAINDER, - help='A create may take one or more extra arguments. This parameter provides them' - ) + populate_attr_arguments(create_parser, MUST_ATTRIBUTES) delete_parser = subcommands.add_parser('delete', help='deletes the object') delete_parser.set_defaults(func=delete) diff --git a/lib389/cli_idm/posixgroup.py b/lib389/cli_idm/posixgroup.py index 2f5cbda..abf6d53 100644 --- a/lib389/cli_idm/posixgroup.py +++ b/lib389/cli_idm/posixgroup.py @@ -10,6 +10,7 @@ import argparse from lib389.idm.posixgroup import PosixGroup, PosixGroups, MUST_ATTRIBUTES from lib389.cli_base import ( + populate_attr_arguments, _generic_list, _generic_get, _generic_get_dn, @@ -66,9 +67,7 @@ def create_parser(subparsers): create_parser = subcommands.add_parser('create', help='create') create_parser.set_defaults(func=create) - create_parser.add_argument('extra', nargs=argparse.REMAINDER, - help='A create may take one or more extra arguments. This parameter provides them' - ) + populate_attr_arguments(create_parser, MUST_ATTRIBUTES) delete_parser = subcommands.add_parser('delete', help='deletes the object') delete_parser.set_defaults(func=delete) diff --git a/lib389/cli_idm/user.py b/lib389/cli_idm/user.py index 197a035..5b57862 100644 --- a/lib389/cli_idm/user.py +++ b/lib389/cli_idm/user.py @@ -7,9 +7,10 @@ # --- END COPYRIGHT BLOCK --- import argparse -from lib389.idm.user import UserAccount, UserAccounts +from lib389.idm.user import UserAccount, UserAccounts, MUST_ATTRIBUTES from lib389.cli_base import ( + populate_attr_arguments, _generic_list, _generic_get, _generic_get_dn, @@ -65,9 +66,7 @@ def create_parser(subparsers): create_parser = subcommands.add_parser('create', help='create') create_parser.set_defaults(func=create) - create_parser.add_argument('extra', nargs=argparse.REMAINDER, - help='A create may take one or more extra arguments. This parameter provides them' - ) + populate_attr_arguments(create_parser, MUST_ATTRIBUTES) delete_parser = subcommands.add_parser('delete', help='deletes the object') delete_parser.set_defaults(func=delete) diff --git a/setup.py b/setup.py index c10347c..52e96cf 100644 --- a/setup.py +++ b/setup.py @@ -21,7 +21,7 @@ here = path.abspath(path.dirname(__file__)) with open(path.join(here, 'VERSION'), 'r') as version_file: version = version_file.read().strip() -with open(path.join(here, 'README'), 'r') as f: +with open(path.join(here, 'README.md'), 'r') as f: long_description = f.read() setup( @@ -55,6 +55,7 @@ setup( # 'lib389/clitools/ds_setup', 'cli/dsadm', 'cli/dsconf', + 'cli/dscreate', 'cli/dsidm', ]), ],