[Freeipa-devel] [PATCHES 0227-0229] Server upgrade: introduce ipa-server-upgrade command

[Martin Basti]

On 23/04/15 12:55, Martin Basti wrote:
> On 21/04/15 10:31, Martin Basti wrote:
>> On 21/04/15 08:12, Jan Cholasta wrote:
>>> Hi,
>>>
>>> Dne 15.4.2015 v 16:26 Martin Basti napsal(a):
>>>> https://fedorahosted.org/freeipa/ticket/4904
>>>>
>>>> Patches attached.
>>>>
>>>> Also ipa-upgradeconfig part is called as a subprocess. This will be
>>>> removed after installer modifications.
>>>>
>>>> This patch may cause temporal upgrade issues (corner cases), until
>>>> installer part will be finished.
>>>>
>>>> If somebody will be hit by them, please use --skip-version-check for
>>>> ipactl and ipa-server-upgrade.
>>>
>>> Regarding that option vs. --force: I think the common assumption is 
>>> that --force ignores *all* non-fatal errors, but you break that 
>>> assumption in ipactl. IMO --force should both ignore errors in 
>>> service startup *and* skip version check, and a new option should be 
>>> added to just ignore errors in service startup (e.g. 
>>> --ignore-service-failures).
>> Originally I used --force option to skip detection, but there was 
>> objections against it on list.
>>
>> However, to have option --force, which set true for both 
>> --ignore-service-failures and --skip-version-check options, might be 
>> better.
>>
>>>
>>> ipa-server-upgrade should probably also have --force, even if it 
>>> does the same thing as --skip-version-check, again because --force 
>>> is common.
>>>
>>>
>>> This is a weird API:
>>>
>>> +        if data_upgrade.badsyntax:
>>> +            raise admintool.ScriptError(
>>> +                'Bad syntax detected in upgrade file(s).', 1)
>>> +        elif data_upgrade.upgradefailed:
>>> +            raise admintool.ScriptError('IPA upgrade failed.', 1)
>>> +        elif data_upgrade.modified:
>>> +            self.log.info('Data update complete')
>>> +        else:
>>> +            self.log.info('Data update complete, no data were 
>>> modified')
>>>
>>> Why does not IPAUpgrade raise errors instead?
>>>
>> For historical reasons, I can investigate what would break this 
>> change, I will send it in separate patch.
>>>
>>> +class IPAVersionError(Exception):
>>> +    pass
>>> +
>>> +class PlatformMismatchError(IPAVersionError):
>>> +    pass
>>> +
>>> +class DataUpgradeRequiredError(IPAVersionError):
>>> +    pass
>>> +
>>> +class DataInNewerVersionError(IPAVersionError):
>>> +    pass
>>>
>>> I don't like the "IPA" in "IPAVersionError", it does not tell you 
>>> much about what kind of version is that. Also data version errors 
>>> should only tell you what is wrong, not how you fix it. IMO better 
>>> names for these would be e.g. "UpgradeVersionError", 
>>> "UpgradePlatformError", "UpgradeDataOlderVersionError", 
>>> "UpgradeDataNewerVersionError". Similar for store_ipa_version and 
>>> check_ipa_version.
>>>
>> Ok.
>>>
>>> Why is it not an error if there is no version in check_ipa_version? 
>>> IMO it should, even if you then ignore the exception most of the time.
>> I can raise error in that case and ignore the exception.
>>>
>>>
>>> Honza
>>>
>> Martin^2
>>
> Updated patches attached.
>
>
>
Updated patches attached

-- 
Martin Basti

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150427/e83f4751/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0227.4-Server-Upgrade-ipa-server-upgrade-command.patch
Type: text/x-patch
Size: 6829 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150427/e83f4751/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0228.4-Server-Upgrade-Verify-version-and-platform.patch
Type: text/x-patch
Size: 17421 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150427/e83f4751/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0229.4-Server-Upgrade-use-ipa-server-upgrade-in-RPM-upgrade.patch
Type: text/x-patch
Size: 996 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150427/e83f4751/attachment-0002.bin>