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

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(a)&lt;instance_name&gt;.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(a)&lt;instance_name&gt;.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(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel