Re: [PATCH] Upgrade process for obsolete options

On 24/09/10 03:57, Alex Rousskov wrote:
> On 09/23/2010 08:30 AM, Amos Jeffries wrote:
>> One problem we currently have with upgrades is leaving the parser able
>> to avoid its bungled/unknown option message for directives which have
>> been fully removed or massively syntax altered.
>>
>> We are able to handle this for flags and option syntax easily but the
>> parser has been particularly dense and strict on the directives (first
>> word of each line).
>>
>> This patch updates the cf_* and cfgman code to allow a special directive
>> type "obsolete" which pumps old config lines off to a parse_obsolete()
>> function for handling without causing the directives to remain in the
>> publicly visible config documentation.
>>
>> cf.data.pre has entries added for all the squid-2.6+ directives I could
>> find that were removed up to 3.HEAD
>>
>> parse_obsolete(char*) is added with tests to catch and report the above
>> entries as obsolete while ignoring. Several of the directives have
>> additional explanation text matching the release notes to aid manual
>> upgrades.
>
> There is a clash of approaches here. I think it is best for Squid not to
> start if the wrong option may significantly affect its functionality.

That is a matter for the parse_obsolete handling of it. This is intended
for obsolete / useless / ineffective directives.
The current set are fairly trivial, however its possible to write the
explanation do something complicated and self_destruct from
parse_obsolete() as easily as elsewhere in the parser.

> This is especially true during upgrades, when no sane admin would try to
> upgrade without a backup readily available (even if they did not offline
> checking). However, I know that many developers prefer when Squid
> bypasses configuration errors and starts if at all possible instead.

I think I understand what you mean. I've been careful to leave squid
barfing if actual typos or unknown directives are entered into squid.conf.

This only reduces useless obsolete options to warnings for later
cleaning up.

>
> I like the absolete feature as such. I would recommend allowing
> cf.data.pre COMMENTs for obsolete options and recording upgrade
> instructions there instead of hard-coding them in parse_obsolete(). A
> generated obsolete option parser would then display those comments. That
> would be a much cleaner solution, IMO.

Interesting. I'll think about how to do that without clashing with the
exports. COMMENTS and DOC_ are a nasty case.

>
> It is fine to not even try upgrading automatically, IMO. If we want to
> upgrade automatically, we should do it via a dedicated hand-written
> upgrade_* function, referenced from the cf.data.pre blob of an obsolete
> feature.

Have not gone there yet. thought about it, decided its still to hard for
now. It's vaguely possible after this if someone wants to. For now I'm
concentrating on keeping working installs going after automatic upgrades.

>
> I cannot object to the proposed changes even though I think there is a
> better way to support the same good idea.

Which better way would that be?

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.8
   Beta testers wanted for 3.2.0.2