Re: New DS CLI design

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2018-05-24 at 09:57 +0200, Viktor Ashirov wrote:
> Hi William,
> On Thu, May 24 2018 at 09:06:14 +1000, William Brown <william@blackha
> ts.net.au> wrote:
> > On Wed, 2018-05-23 at 11:34 +0200, Viktor Ashirov wrote:
> > > Hi,
> > > 
> > > I'd like to continue our discussion about feature parity with
> > > perl
> > > tools
> > > and overall user experience of the new CLI tools.
> > > 
> > > Here are some projects where we can take inspiration for our own
> > > design:
> > > 
> > > * bpython interpreter
> > >   https://bpython-interpreter.org/screenshots.html
> > > 
> > > * fish shell
> > >   http://fishshell.com/docs/current/design.html
> > > 
> > > * Python Prompt Toolkit
> > >   https://github.com/jonathanslenders/python-prompt-toolkit
> > >   (it's used to build tools like pgcli and mycli: https://github.
> > > com/
> > > dbcli)
> > > 
> > > * OpenStack CLI:
> > >   https://docs.openstack.org/python-openstackclient/pike/cli/comm
> > > ands
> > > .html
> > >   https://docs.openstack.org/python-openstackclient/pike/cli/inte
> > > ract
> > > ive.html
> > > 
> > > Common theme is that all of these CLIs are primarily used in REPL
> > > mode,
> > > but they can be used in script mode as well.
> > > 
> > > Here's the quote from fish shell's design page about the law of
> > > discoverability:
> > > 
> > > * Everything should be tab-completable, and every tab completion
> > > should
> > >   have a description.
> > > 
> > > * Every syntax error and error in a built-in command should
> > > contain
> > > an
> > >   error message describing what went wrong and a relevant help
> > > page.
> > >   Whenever possible, errors should be flagged red by the syntax
> > >   highlighter.
> > > 
> > > * The help manual should be easy to read, easily available from
> > > the
> > >   shell, complete and contain many examples
> > > 
> > > * The language should be uniform, so that once the user
> > > understands
> > > the
> > >   command/argument syntax, they will know the whole language, and
> > > be
> > >   able to use tab-completion to discover new features.
> > > 
> > 
> > Hey there,
> > 
> > When building these tools I took a lot of inspriation from the
> > openshift tools actually, but there is a logic to the design.
> 
> I like the design of openshift tools, but I think we have a flaw in
> our
> implementation of this logic.
> 
> > Everything goes from "least specific" to "most specific"
> > 
> > So for example, 
> > 
> > dsctl start <instance>
> > 
> > This doesn't make sense because you have a "specific" action,
> > before
> > the "broad concept" of an instance.

> > 
> > So everything is in the pattern of:
> > 
> > dsctl <instance> start/thing ....
> > dsidm <instance> user create ....
> > 
> > Everything goes from least specific to most. 
> > 
> 
> You treat <instance> as a broad concept, when in fact it is a
> parameter
> that is a subject to change. We should use keyword, not a parameter.


Instance IS a broad concept. I don't administer 2 or more instances
generally on a server. There is a 1:1 ratio of DS to servers in the
wild generally. And even if I have multiple servers, I am generally
focused on one.

So I want to do:

ds* instance thing ....

and I'm going to change "...." between commands.

> 
> Here are few problems with the current design:
> * dsctl has "remove" subcommand, but "create" is separate in dscreate
>   command. 
> 
> It can be solved with the same logic that you mentioned above:
> dsctl instance create ...
> dsctl instance remove <instance>

There is excellent logic and research behind this design. It comes down
to instance discovery and the allocation of the DirSrv object.

Earlier versions of this tool had:

dsctl create
dsctl stop instance
dsctl remove instance

What resulted was every Subcommand group had to parse and setup
instance itself. It was a huge spew of boilerplate, and it also made
the command NOT match the dsconf and dsidm commands that took:

dsidm instance user <actions>

So in order to keep a consistent design across all the tools, I decided
that instance should be second - I didn't just make the decision, I
actually contacted a group of LDAP admins and asked them what they
preferred. Instance is the broad topic I am working under, and I tend
to change parameters at the tail of the command, like user create, then
user edit, or user list. 

Now the second part. If you have:

dsctl instance create

What's the issue? Well in the design of dsctl, this means we have to
delay instance allocation to very late in the command's processing, and
means that errors are not reported correctly, it makes the design
really complex, and horrendous, and it just made for bad code logic.

So in the end I made a choice to split dscreate out. 

dscreate works on the absence of an instance. 
dsctl works on a local instance you have root access too. 
dsconf works on a remote instance via cn=config. 
dsidm manages a backend of an instance. 

Huge amounts of work were put in into this design, it's not just magic
from no where. The first versions of these tools were created in 2016
at pycon AU, and have been evolving (and rewritten) since based on
usage and trials. :)  

> 
> We can prepend all instance specific tasks with "instance" word, we
> can
> even short it to "inst" or "i", similar to GDB commands for those,
> who
> doesn't like type. So we have a broad concept of "instance", and
> specific actions are followed by parameters like instance name.
> 
> * "dsctl <instance> status" shows only one instance status, and there
> is
>   no easy way to get status of all instances on the server, besides
>   falling back to systemctl.
> 
> Consider this:
> dsctl status -- shows status of all instances
> dsctl status <instance1> <instance2> .. <instanceN> -- shows status
> of
>   specified instances

I'm sorry, but I do not agree. I do not work across 16 instances at
once. I focus on one and conduct a task. Most installs only have one.

We should not focus on the idea that we have the capability for
multiple instances. I would guarantee almost 95% or more deployments
have a single instance per server, and would never "gain" from the
excess work done here to support many instances, and the "divergent"
command line formats would cause confusion. 

For example: do I want to perform "user create" on 13 instances at
once? See how suddenly the command logic breaks down suddently? the
instance IS the largest unit of work, and must be early in the command.
 We must consider that it's not just dsctl, but dsconf and dsidm's
command line syntax that must also remain consistent. If we make it
inconsistent, we ruin the user experience rapidly. 


If you want to see the status of an instance (or all instances), use
systemctl. We should never have allowed stop/start/status into the
dsctl command. It's not our job to manage services, that's the role of
systemd/whatever else.
It only exists because of containers *without* systemd support, and
because the container ecosystem is a mess, we can not programmatically
detect if we are in one or not. Therfore we are now stuck with a set of
distracting commands that really shouldn't be our job. 

> 
> 
> > I actually did put a lot of work into these based on design
> > principles
> > already. About all we are missing is tab complete, and IIRC there
> > is a
> > python argparse module for that which should work given our
> > design. 
> 
> I'm not sure how argparse can help here, because in that case we're
> relying on shell's support for autocompletion. It might not work, it
> might be not enabled. Moreover, "ds" namespace is overloaded. Here's
> the
> output on my F28 box:

Argparse has a module that Mark once found allowing autocompletion
generation. I wanted to add support for it, but never found time
because I was always doing too much ;) 

> 
> # ds <TAB>
> ds-cockpit-
> setup             dsidm                        ds_selinux_port_query
> dsconf                       ds-
> logpipe.py                ds_systemd_ask_password_acl
> dscreate                     ds-replcheck                 
> dsctl                        ds_selinux_enabled           
> 
> Some of these commands are moved to a proper place in master, but
> that
> still leaves us with at least:
> dsconf
> dscreate
> dsctl
> dsidm
> 
> Without looking at man pages (which do not exist, btw), it's hard to
> tell what each of these commands does. What's the difference between
> dsctl and dsconf? I need to run all of these with --help to get more
> information.
> 
> In REPL mode I can press <TAB> and get context-aware help and
> autocompletion.
> 
> > 
> > I don't REPL mode is super important, I don't think I've EVER used
> > a
> > REPL in all my years as a sysadmin because I want to be able to
> > copy
> > paste out whole command sequences.
> 
> Your shell is REPL! Did you use shell? :)

I meant a REPL inside of the command line too IE nsupdate. I don't want
to support that. 

> 
> Look at openstack cii. You can use it as REPL mode with
> tab-completion:
> $ openstack
> (openstack) server <TAB> 
> add fixed ip           image create           resume
> add floating ip        list                   set
> add port               lock                   shelve
> add security group     migrate                show
> add volume             pause                  ssh
> backup create          reboot                 start
> create                 rebuild                stop
> delete                 remove fixed ip        suspend
> dump create            remove floating ip     unlock
> event list             remove port            unpause
> event show             remove security group  unrescue
> group create           remove volume          unset
> group delete           rescue                 unshelve
> group list             resize                 
> group show             restore                
> (openstack) server
> 
> or just from the command line:
> $ openstack server list
> ...
> 
> If you want to copy-paste commands from the docs - no problem.
> If you want to save history, prompt_toolkit provides very nice
> interface
> for that:
> http://python-prompt-toolkit.readthedocs.io/en/master/pages/building_
> prompts.html
> Paged output, editing current command in $EDITOR (similar to C-x C-e
> in
> bash), etc.
> 
> I also would really love to get back interactive installer.
> Installation
> from the template is nice, but it has its own issues.
> For example, dscreate provides only 2 actions: show an example INI
> file
> and load INF file. Are these two files different? Anyway, let's try
> to
> run
> $ dscreate example
> ...
> Here's the wall of text
> ...
> 

I have commented on this in the response to Marc. 

> This should be more helpful and at least provide a hint, that this
> output should be redirected to a file, since later it will be loaded
> by
> 'fromfile' subcommand.
> 
> Anyway, let's look at the file. It's very nice documented! Look at
> all
> the options! Some of them are uncommented, that must be required
> parameters. But ports are commented, are they not required? Maybe
> something else I should change? And the DM's password is in a plain
> text. Can I pass hashed value here? Can I be asked for the password
> so
> that it won't be sitting in the template on a filesystem for ages
> until
> someone discovers this template?
> 
> Let's change port at least to something else and run dscreate.
> # dscreate fromfile localhost.ini
> READY: Preparing installation for localhost
> READY: Beginning installation for localhost
> Job for dirsrv@localhost.service failed because the control process
> exited with error code.
> See "systemctl status dirsrv@localhost.service" and "journalctl -xe"
> for details.
> Error: Command '['/usr/bin/systemctl', 'start', 'dirsrv@localhost']'
> returned non-zero exit status 1.
> FAIL: Command failed. See output for details.
> 
> # ausearch -m AVC
> ----
> time->Thu May 24 02:20:39 2018
> type=AVC msg=audit(1527142839.292:366): avc:  denied  { name_bind }
> for  pid=3598 comm="ns-slapd" src=1389
> scontext=system_u:system_r:dirsrv_t:s0
> tcontext=system_u:object_r:unreserved_port_t:s0 tclass=tcp_socket
> permissive=0
> 
> 
> Alright, now I need to make SELinux happy, label ports manually with
> ldap_port_t...
> 
> Now, I think it would be nice to have something like this in addition
> to
> the unattended install, instead of having unattended mode as the only
> way to create an instance.
> 
> DS> instance create --name=master1 --fqdn=server.example.com --
> port=1389 --secure-port=1636 --suffix=dc=example,dc=com --password=
> or 
> # ds instance create --name=master1 --fqdn=server.example.com --
> port=1389 --secure-port=1636 --suffix=dc=example,dc=com
> Enter Directory Manager's password:
> ...

Again, I've asked for a bug report to me made here is this is a simple
fix. 

> 
> > 
> > Hope that helps! 
> > 
> > 
> > > Please let me know what do you think.
> > > 
> > > Thanks!
> > > 
> > > --
> > > Viktor
> > > _______________________________________________
> > > 389-devel mailing list -- 389-devel@xxxxxxxxxxxxxxxxxxxxxxx
> > > To unsubscribe send an email to 389-devel-leave@lists.fedoraproje
> > > ct.o
> > > rg
> > > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.htm
> > > l
> > > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guid
> > > elin
> > > es
> > > List Archives: https://lists.fedoraproject.org/archives/list/389-
> > > deve
> > > l@xxxxxxxxxxxxxxxxxxxxxxx/message/QFJVIX5CTRPBA2VPZIEXUDCZG7L2TS2
> > > O/
> > 
> > _______________________________________________
> > 389-devel mailing list -- 389-devel@xxxxxxxxxxxxxxxxxxxxxxx
> > To unsubscribe send an email to 389-devel-leave@lists.fedoraproject
> > .org
> > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidel
> > ines
> > List Archives: https://lists.fedoraproject.org/archives/list/389-de
> > vel@xxxxxxxxxxxxxxxxxxxxxxx/message/GABWBNQO55TW2OE2PZCT5PI3BJZKXC4
> > 2/
-- 
--
Sincerely,

William
_______________________________________________
389-devel mailing list -- 389-devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to 389-devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/389-devel@xxxxxxxxxxxxxxxxxxxxxxx/message/TVHNF72TFCHVUEYHZFR42WVNMWXYBEX5/




[Index of Archives]     [Fedora Directory Announce]     [Fedora Users]     [Older Fedora Users Mail]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Review]     [Fedora Art]     [Fedora Music]     [Fedora Packaging]     [CentOS]     [Fedora SELinux]     [Big List of Linux Books]     [KDE Users]     [Fedora Art]     [Fedora Docs]

  Powered by Linux