Re: New DS CLI design

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

 



On Fri, 2018-05-25 at 13:04 +0200, Viktor Ashirov wrote:
> On Fri, May 25 2018 at 10:19:26 +1000, William Brown <william@blackha
> ts.net.au> wrote:
> > > 
> > > 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.
> 
> ds* instance -i <instance> thing
> > 
> > > 
> > > 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.
> 
> As I mentioned before, we have too many tools already, which sit in
> ds*
> namespace. It's hard to tell by name which one is doing what. Why
> there
> is dscreate, but no dsremove?

why do we have so many perl tools that also are like this?

>  Why dsidm is so opinionated? 

So is every tool we have. Everything that exists is "opinionated". We
have 500kloc, and we expose a fraction of that in the cli. Most
projects do. So really, everything is opinionated, it's that today you
are seeing that opinion because you have the domain specific knowledge.

> Each
> installation has its own requirements to OCs, has custom schema, etc.
> This tool doesn't fit everyone's needs. I can't use it on my custom
> DIT
> that is tailored for my organization needs, so why ship this tool at
> all? If I want IdM, I'd use IPA.

Great! Then use your own tools. 

But for some IPA is too heavy (or fragile ...). For some people, they
just want to get users and groups work easily. For some people they'll
extend and develop their own extensions. There are many use cases that
exist here.   These tools will be a massive help to deployments, and
people are still free to choose to use something else if they want.

> 
> My point is that we shouldn't have so many tools, but rather one,
> that
> can be used for remote and local instances. Then there is no need to
> jump around different command line syntaxes since there will be just
> one.

No. Because this breaks the python logic and seperation. Do I need root
access on the local machine when I create a group now? Do I need a
directory manager account to restart my dirsrv? 

This seperation exists for very clear logical reasons that are baked
into the python code.

> > 
> > 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. :)  
> 
> Well, here's my first experience and I find it confusing. Does that
> count? And if this tool has been around since 2016, why I still can't
> find man pages for it?

Because the tool changed so much in those years, trying to write man
pages was not worth my time to rewrite them over and over. Because it
was "in design" for so long. Because when I was working on it *no one
else* was helping, and I had so much to do that I could not find time.
And you know the other reasons why I could not work on this.

I wanted to write man pages, but was unable to do so. 

> > 
> > > 
> > > 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.
> 
> We use multiple instances in QA and development. That 5% would
> benefit
> to all, since the tool won't be so stiff and can be used for both
> scenarios.

for i in "instances ..."; do dsctl $i action; done

You also probably would have access to for loops inside of python
during pytest also: testing by hand sucks. :) 

Do you really expect a complete rewrite of a tool, all it's test,
completely changing it's argument processing, breaking syntax with the
other suite of tools, all because you want "multiple instance", that
only applies to a tiny fraction of the possible commands - and doesn't
benefit most of our users?

There are other ways to achieve what you want, that don't require the
rewrite of this processing - not to mention the rewrite would be a
horrible layering violation and additional complexity into the python
code that I would not accept.  

> > 
> > For example: do I want to perform "user create" on 13 instances at
> > once? See how suddenly the command logic breaks down suddently?
> 
> Some actions can require multiple instances as an argument. 'status'
> is
> one of them. 'user create' is not. Other commands that might require
> that - stop/start replication agreements, add indices, reindex db,
> etc. 

And then replication requires customised parameters to each instance.
reindexing shouldn't be performed like that. About the only suggestion
here is adding indexes, and even that should be more carefully applied
than just "blanket changes". Sysadmins are VERY hesistant to send
commands at a production ldap cluster. Just because we do it with
disregard, does not mean everyone else does.

I remember that when I wanted to add indexes in production it was a two
week planning process, documentation, a submission for change, I had to
have a backup/backout plan. It wasn't just "run this command in a batch
against 4 servers".

> 
> > 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.
> 
> See my argument above, that we should not have so many commands.

They must remain seperate for good design reasons like seperation of
privelege (each tool requires less priv as you step down), clear
identification of access vecton (local, local/remote, remote), and
removing ambiguity to the user who is running the tool (do I need root
to change group details?).

I wanted a single unified tool. I spent hours on this topic. It's not
viable. It's too confusing. The python becomes a complete tangled mess.

> > 
> > 
> > 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.
> 
> Since we're stuck with it, why not embrace it and make it work
> better?

Since we may not always be stuck with it, since there are other ways
(for loops, systemctl), and since we all have better things to do with
our time. 

> > 
> > > 
> > > 
> > > > 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_qu
> > > ery
> > > 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.
> 
> I think that REPL is a very powerful way to provide context help,
> increase discoverability. See for example python vs bpython REPL.

It's fine from your normal shell to tab complete, but it's a huge
amount of extra work in a program to maintain and test it. I'm not
willing to invest my time into this, and I will advise that you don't
either because it's really really rare to again, shoot off a stack of
commands to production. People plan them, they'll copy paste, they'll
prepare their ansible playbooks, or scripts. REPL may be "neat" for
some quick admin jobs, but really, having a proper cli is going to see
99% of the work through it.

The current cli behaviour is fine, it's just missing tab complete. That
would be a far better investment and one that I always wanted to see
happen.

> > 
> > > 
> > > 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/build
> > > ing_
> > > 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.
> 
> Bug report was filed some time ago: https://pagure.io/389-ds-base/iss
> ue/49705
> > 
> > > 
> > > > 
> > > > 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.fedorap
> > > > > roje
> > > > > 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/QFJVIX5CTRPBA2VPZIEXUDCZG7L
> > > > > 2TS2
> > > > > O/
> > > > 
> > > > _______________________________________________
> > > > 389-devel mailing list -- 389-devel@xxxxxxxxxxxxxxxxxxxxxxx
> > > > To unsubscribe send an email to 389-devel-leave@lists.fedorapro
> > > > ject
> > > > .org
> > > > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.h
> > > > tml
> > > > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_gu
> > > > idel
> > > > ines
> > > > List Archives: https://lists.fedoraproject.org/archives/list/38
> > > > 9-de
> > > > vel@xxxxxxxxxxxxxxxxxxxxxxx/message/GABWBNQO55TW2OE2PZCT5PI3BJZ
> > > > KXC4
> > > > 2/
> > 
> > -- 
> > --
> > Sincerely,
> > 
> > William
-- 
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/QB4THSVVFTGIX5QSRNLMQMTOVKE3TMDX/




[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