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

[Rich Megginson]

Nathan Kinder wrote:
> On 03/31/2010 10:04 PM, Nathan Kinder wrote:
>> 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?
>>>>        
> I've attached a new set of patches that implements the solution 
> outlined above.
>
> -NGK
>>>>
>>>>        
>>> 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
ack
>>   
>>> Thanks.
>>>
>>> -- 
>>> Endi S. Dewata
>>> -- 
>>> 389-devel mailing list
>>> 389-devel at lists.fedoraproject.org
>>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>>
>>>      
>> -- 
>> 389-devel mailing list
>> 389-devel at lists.fedoraproject.org
>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>    
>
> ------------------------------------------------------------------------
>
> --
> 389-devel mailing list
> 389-devel at lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/389-devel