[Pki-devel] [PATCH] patch to pki-core for nuxwdog systemd support

[Ade Lee]

See comments below:

On Thu, 2015-05-07 at 13:44 -0500, Endi Sukma Dewata wrote:
> On 5/6/2015 3:25 PM, Ade Lee wrote:
> >     Patches to get nuxwdog working with systemd
> >
> >      This patch adds some new unit files and targets for starting instances
> >      with nuxwdog, as well as logic within the pki-server nuxwdog module to
> >      switch to/from the old and new systemd unit files.
> >
> >      It also corrects some issues found in additional testing of the nuxwdog
> >      change scripts.
> >
> >      To use nuxwdog to start the instance, a user needs to do the following:
> >
> >      1. Create an instance normally.
> >      2. Run: pki-server instance-nuxwdog-enable <instance_name>
> >      3. Start the instance using:
> >         systemctl start pki-tomcatd-nuxwdog@<instance_name>.service
> >
> >      To revert the instance, simply do the following:
> >
> >      1. Run: pki-server instance-nuxwdog-disable <instance_name>
> >      2. Start the instance using:
> >         systemctl start pki-tomcatd@<instance_name>.service
> >
> >      To do all this, you need the latest nuxwdog (with the patches I just posted).
> >
> >
> > Whats missing:
> >
> > 1. documentation.  That will come next.
> > 2. right now -- under nuxwdog, java runs as root.  We will need to change this.
> > 3. Not integrated with pkispawn.  Basically, if you want to add a new subsystem to an nuxwdog-ed instance,
> >      you will need to revert to a non-nuxwdog instance first.
> >
> > Ade
> 
> Some comments:
> 
> 1. The patch currently obtains both uid and gid using the same 
> TOMCAT_USER variable. In the instance registry file there are actually 
> separate PKI_USER and PKI_GROUP variables. In the latest code the 
> PKIInstance reads these variables and makes them available as 
> instance.uid and instance.gid. The patch should use them instead.
> 
yup - we talked about adding this code.  Glad it merged - will change
accordingly.

> 2. The capitalization and punctuation in the output messages are 
> inconsistent:
> 
> $ pki-server nuxwdog-enable
> ---------------------------
> Nuxwdog enabled for system.
> ---------------------------
> 
> $ pki-server instance-nuxwdog-enable pki-tomcat
> ---------------------------------------
> nuxwdog enabled for instance pki-tomcat
> ---------------------------------------
> 
will fix.

> 3. Existing issue. In base/server/tomcatN/conf/server.xml the nuxwdog 
> Listener is already added by default, so it's not necessary to 
> add/remove the Listener while enabling/disabling nuxwdog. Alternatively 
> the Listener shouldn't be added by default, and only added if nuxwdog is 
> enabled.
> 
> If we are keeping the Listener added by default, we probably should 
> rename it into something more generic such as PKIListener (we can use it 
> later for other things, not just nuxwdog). Having a nuxwdog Listener 
> might confuse users. Also it should be moved above GlobalNamingResources 
> for consistency.
> 

Ok - we still need to add it in case someone has an older instance that
does not have the listener.  We'll just remove the removal code.

> 4. It would be nice to show the nuxwdog enablement status in the 
> pki-server instance-show output.

Perhaps we can do that as a separate patch.

> 
> 5. To simplify server startup (and avoid mistakes) the pki-server 
> instance-start/stop command can call the appropriate service name based 
> on the nuxwdog status. 

Agreed.  Let me add that as a follow-on patch.

> Alternatively, the service name probably can stay 
> the same regardless of nuxwdog status since only one of them will work 
> at a time (the link will point to the appropriate systemd service file).
> 
That won't work.  You can only link to one template file for a specific
template.  When we restart systems , or possibly when you do a systemd
daemon-reload, systemd automatically resets links.

The unit files are quite different, so we need two different ones.

> 6. When the pkispawn integration is added, the link to nuxwdog.jar can 
> be added at install time since there's already an RPM dependency on 
> nuxwdog. We'll also need to add an upgrade script to update existing 
> instances.
> 
yes - when pkispawn integration is added. Separate patch ..

> 7. This check is not necessary because it's never null:
> 
>      if not instance.subsystems:
>          print "Error: Instance has no subsystems."
>          sys.exit(1)
> 
> And if it's empty (which never happens), the subsequent loop will simply 
> exit.
> 
OK - will remove check.

> 8. Existing issue. The password prompts should display the instance name 
> (in case there are multiple instances, for example:
> 
>    [pki-tomcat] Password for internal: ************
>    [pki-tomcat] Password for internaldb: *********
>    [pki-tomcat] Password for replicationdb: **********
> 
Good idea - will add.