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

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



Discussed on #irc.  Changes made as below.  ACKed by Endi and pushed to
master.

Ade
On Fri, 2015-05-08 at 11:24 -0400, Ade Lee wrote:
> 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.
> 
> 
> _______________________________________________
> Pki-devel mailing list
> Pki-devel redhat com
> https://www.redhat.com/mailman/listinfo/pki-devel



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