[389-devel] Please Review: Fix parsing of start-slapd scripts

[Nathan Kinder]

On 03/31/2010 09:52 PM, Endi Sukma Dewata wrote:
> ----- "Nathan Kinder"<nkinder at redhat.com>  wrote:
>
>    
>>>> The admin server CGIs parse the start-slapd scripts to determine the
>>>> DS instance names.  A recent format change to start-slapd caused this
>>>> parsing to break.  These patches make the instance name easier to
>>>> parse from the script.  One patch is for DS itself and one is for the
>>>> Admin Server.
>>>>
>>>>          
>>> ack - much better
>>>
>>>        
>> Thanks, but I need to nak my own patch since it's imcomplete.
>>
>> This isn't going to work well when upgrading an instance.  We don't
>> regenerate the start-slapd script when running 'setup-ds.pl -u'.  This
>> means that an upgraded instance will not work properly with any of the
>> admin server CGIs that need to parse the instance name from
>> start-slapd.  This issue is already a problem not related to this patch,
>> but it seems we should fix it along with this issue.
>>
>> I suppose the right thing to do is to make 'setup-ds.pl -u' generate a
>> new start-slapd script for the existing instances as well as a new
>> instance specific initconfig script if one doesn't exist.  I think we
>> need to avoid wiping out an existing instance specific sysconfig script
>> since it may have been modified by an admin to add other stuff to it
>> (like KRB5_KTNAME for Kerberos).  Do you see any problems with this
>> approach?
>>      
> Sorry that my changes caused this problem. Peace... :)
>    
Actually, a previous change of mine actually broke the upgrade case.  
You just broke the admin server parsing code. :)
> Would it be better to have a pure configuration file (not script) in the
> instance directory that contains things like instance name, etc.? The
> start-dirsrv is a script, and parsing a script without a proper parser is
> risky. Same thing with the sysconfig scripts. Even a slight change in that
> file could break some regular expressions in the Perl modules. What do you
> think?
>    
My previous change that breaks the upgrade case is trying to accomplish 
this.  We previously had all sorts of paths in the start-slapd scripts 
which were parsed in various places.  I created the instance specific 
config files in /etc/sysconfig that now contain the paths and other 
things needed to start an instance in an easily parsed format.  The one 
wrinkle is that we need to know the instance name to know which 
sysconfig file to read since the file name is "dirsrv-<instance>".  The 
admin server tries to get the instance name from the start script to 
find the sysconfig file so we can determine other things, such as the 
pid file location.  The only other way I can think of to get the 
instance name would be to parse it out of the instance path that we use 
to locate the start-slapd script.  This is usually something like 
"/usr/lib/dirsrv/slapd-<instance>".  I think that my proposed change to 
the start-slapd format is less error prone since there is no complex 
parsing needed.

-NGK
> Thanks.
>
> --
> Endi S. Dewata
> --
> 389-devel mailing list
> 389-devel at lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>