[Freeipa-devel] [PATCH]Add -f option to ipactl

[Martin Kosek]

On 02/19/2014 10:58 PM, Dmitri Pal wrote:
> On 02/19/2014 03:13 PM, Petr Spacek wrote:
>> On 19.2.2014 21:10, Dmitri Pal wrote:
>>> On 02/19/2014 11:58 AM, Adam Misnyovszki wrote:
>>>> Hi,
>>>> I reviewed this old patch:
>>>>
>>>> If an error occurs in the start up sequence in ipactl start/restart,
>>>> all the services are stopped. Using the --force/-f option prevents
>>>> stopping of services that have successfully started.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/3509
>>>
>>>
>>> I have not read the code but looked at the patch and bug.
>>> I do not understand the -force option especially with help string being:
>>> "If ipactl action fails, do not stop the services that are already running"
>>> force usually means the reverse: if something did not happen force it to
>>> happen.
>>> I am not sure that --force option is the right name for the option with this
>>> help.
>>
>> I have proposed --no-rollback but it didn't fit to habits in IPA:
>> https://fedorahosted.org/freeipa/ticket/3509#comment:2
>>
> then it should be -fs/--force-start
> 
> or something of this kind.
> 

IMO --force is not that bad, it forces start procedure to finish regardless of
the result (if some service started or not). --force-start may be redundant:

# ipactl start --force-start
# ipactl restart --force-start

But I am not insisting on --force, if there is better option I am open to it.

Few comments for the patch itself:

1) Please update it so that it does not use this construct:

+        try:
+            svc_off.stop(capture_output=False)
+        except:
+            pass

Bare except clauses also catch for example KeyboardInterrupt. Then if the stop
command is stuck, I would not be able to CTRL+C it. "except Exception:" is better.

2) The --force does not work as I would wish. When --force option is used, I
would like ipactl to try to start all services it can, regardless to if they
failed or not.

Now it just does not rollback, but it still does not start all services it can:

# ipactl start --force
Existing service file detected!
Assuming stale, cleaning and proceeding
Starting Directory Service
Starting krb5kdc Service
Starting kadmin Service
Starting named Service
Starting ipa_memcached Service
Starting httpd Service
Job for httpd.service failed. See 'systemctl status httpd.service' and
'journalctl -xn' for details.
Failed to start httpd Service
Shutting down <==
Aborting ipactl

See? HTTPD did not start, ipactl stopped and it did not start pki-tomcatd or
ipa-otpd even though they do not use HTTPD when starting.

3) I see we still write "Shutting down" even though we use --force. I would
rather print "Shutting down suppressed" or "Forced start, no service rollback".

Martin