David Kupka wrote: > On 23/02/16 10:14, Martin Kosek wrote: >> On 02/23/2016 09:47 AM, David Kupka wrote: >>> On 22/02/16 16:15, Martin Kosek wrote: >>>> On 02/22/2016 04:04 PM, Jan Cholasta wrote: >>>>> On 22.2.2016 15:56, David Kupka wrote: >>>>>> On 22/02/16 07:28, Jan Cholasta wrote: >>>>>>> On 18.2.2016 10:10, David Kupka wrote: >>>>>>>> On 19/01/16 16:10, David Kupka wrote: >>>>>>>>> On 19/01/16 14:38, Jan Cholasta wrote: >>>>>>>>>> On 19.1.2016 14:26, Martin Kosek wrote: >>>>>>>>>>> On 01/19/2016 01:47 PM, David Kupka wrote: >>>>>>>>>>>> I've polished the patch attached to #5586 by Timo Aaltonen. >>>>>>>>>>>> >>>>>>>>>>>> Thanks for the patch. I've fixed the path in specfile and >>>>>>>>>>>> removed >>>>>>>>>>>> unused import >>>>>>>>>>>> but otherwise it works, ACK. >>>>>>>>>>>> >>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5586 >>>>>>>>>>> >>>>>>>>>>> Won't this break existing certmonger requests depending on >>>>>>>>>>> the old >>>>>>>>>>> path? >>>>>>>>>> >>>>>>>>>> It will, I don't see any upgrade code. >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> # getcert list | grep '/usr/lib64/ipa/certmonger' >>>>>>>>>>> pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert >>>>>>>>>>> "auditSigningCert >>>>>>>>>>> cert-pki-ca" >>>>>>>>>>> pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert >>>>>>>>>>> "ocspSigningCert >>>>>>>>>>> cert-pki-ca" >>>>>>>>>>> pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert >>>>>>>>>>> "subsystemCert >>>>>>>>>>> cert-pki-ca" >>>>>>>>>>> pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert >>>>>>>>>>> "caSigningCert >>>>>>>>>>> cert-pki-ca" >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/renew_ra_cert >>>>>>>>>>> pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert >>>>>>>>>>> "Server-Cert >>>>>>>>>>> cert-pki-ca" >>>>>>>>>>> post-save command: >>>>>>>>>>> /usr/lib64/ipa/certmonger/restart_dirsrv >>>>>>>>>>> RHEL72 >>>>>>>>>>> post-save command: /usr/lib64/ipa/certmonger/restart_httpd >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> You're right it will break the upgrade. I haven't noticed that >>>>>>>>> Server-Cert for DS and HTTPD are not handled by >>>>>>>>> certificate_renewal_update (ipaserver.install.server.upgrade) >>>>>>>>> where all >>>>>>>>> the other trackings are stopped and then configured again with the >>>>>>>>> paths.CERTMONGER_COMMAND_TEMPLATE already updated. >>>>>>>>> >>>>>>>>> Thanks for the catch. >>>>>>>>> >>>>>>>> >>>>>>>> I've updated Timo's patch little more and added >>>>>>>> start_tracking_certificates() for dsinstance and httpinstance. >>>>>>>> Now the >>>>>>>> upgrade works as expected. >>>>>>> >>>>>>> The way the patches are split is kind of weird and apparently >>>>>>> confusing >>>>>>> (see the other thread). IMO there should be 2 patches: the first >>>>>>> should >>>>>>> add the ability to change DS and HTTP certmonger config during >>>>>>> upgrade >>>>>>> (i.e. the start_tracking_certificates() methods and >>>>>>> certificate_renewal_update() changes), the second should move the >>>>>>> helpers (i.e. the actual move and certificate_renewal_update() >>>>>>> version >>>>>>> bump). >>>>>>> >>>>>> Honza, do I understand it correctly that the code is OK but I did not >>>>>> split it to the patches correctly? >>>>> >>>>> Yes. >>>> >>>> Before acking or pushing, can you please explain for me how the >>>> upgrade of >>>> certmonger tracking requests work? I want to make sure this is >>>> right, so please >>>> bear with me: >>>> >>>> 1) How does it edit existing tracking requests with the new helper >>>> paths? >>>> >>>> 2) Does it go and try to edit the requests on every upgrade? Or is >>>> there some >>>> check that requests were updated? >>>> >>>> Thanks, >>>> Martin >>>> >>> >>> Whole upgrade of renewal requests is done in >>> ipaserver/install/server/upgrade.py in certificate_renewal_upgrade(). >>> >>> First there is version of requests and if it's the same as in state file >>> upgrade is skipped. >>> Then every request is searched over certmonger's DBus interface and >>> if at least >>> one is not found it means that there was change in request >>> configuration. All >>> tracking requests are then stopped and started again with new >>> configuration. >>> >>> So to answer you questions: >>> 1) By stopping the old request with the old parameters (including >>> path) and >>> starting new with new parameters. >>> >>> 2) Only if version was bumped which happens only if some of the >>> requests changes. >> >> Ah, so IIUC, if you bump the version, requests should be properly >> updated. The >> change looks fine then. >> > > After discussion with Honza, we decided to drop the part comparing only > base names of pre- and post-save commands and use it as whole. I've also > split the patches so it's obvious what is going on. > > Patches should be applied in this order: > > freeipa-dkupka-0091.0
A cert could silently fail to be tracked in start_tracking_certificates() if no serverid can be found. > freeipa-dkupka-0087.1 > freeipa-dkupka-0088.1 > freeipa-tjaalton-0011.2 > freeipa-dkupka-0092.0 > > > -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code