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 _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
