On 02/22/2016 07:15 PM, Martin Basti wrote: > > > On 22.02.2016 17:05, Martin Basti wrote: >> >> >> On 19.02.2016 15:02, Alexander Bokovoy wrote: >>> On Fri, 19 Feb 2016, Petr Vobornik wrote: >>>> On 02/19/2016 11:12 AM, Alexander Bokovoy wrote: >>>>> On Fri, 19 Feb 2016, Martin Basti wrote: >>>>>> WIP patch attached >>>>>> >>>>>> https://fedorahosted.org/freeipa/ticket/5665 >>>>>> >>>>> Comments inline. >>>>> >>>>>> + # we need to run sidgen task >>>>>> + sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks," >>>>>> + "cn=config") >>>>>> + sidgen_tasks_attr = { >>>>>> + "objectclass": ["top", "extensibleObject"], >>>>>> + "cn": ["sidgen"], >>>>>> + "delay": [0], >>>>>> + "nsslapd-basedn": [self.api.env.basedn], >>>>>> + } >>>>> May be you are better to name this task more uniquely? >>>>> Something like 'cn=generate domain sid,cn=...'? >>>>> >>>>>> + >>>>>> + task_entry = ldap.make_entry(sidgen_task_dn, >>>>>> + **sidgen_tasks_attr) >>>>>> + try: >>>>>> + ldap.add_entry(task_entry) >>>>>> + except errors.DuplicateEntry: >>>>>> + self.log.debug("sidgen task already created") >>>>>> + else: >>>>>> + self.log.debug("sidgen task has been created") >>>>> There could be multiple tasks running in parallel, that's why it could >>>>> be good to use a different and unique name. >>>>> >>>>>> + # we have to check all trusts domains which have been added >>>>>> after the >>>>>> + # upgrade that caused bug was done. >>>>>> + >>>>>> + base_dn = DN(self.api.env.container_adtrusts, >>>>>> self.api.env.basedn) >>>>>> + trust_domain_entries, truncated = ldap.find_entries( >>>>>> + base_dn=base_dn, >>>>>> + scope=ldap.SCOPE_ONELEVEL, >>>>>> + attrs_list=[attr_name, "cn"], >>>>>> + ) >>>>>> + >>>>>> + if truncated: >>>>>> + self.log.warning("update_sids: Search results were >>>>>> truncated") >>>>>> + >>>>>> + for entry in trust_domain_entries: >>>>>> + if entry.single_value[attr_name] is None: >>>>>> + domain = entry.single_value["cn"] >>>>>> + self.log.error( >>>>>> + "Your trust to %s is broken. Please re-create it >>>>>> by " >>>>>> + "running 'ipa trust-add' again", domain) >>>>>> + >>>>>> + sysupgrade.set_upgrade_state('sidgen', 'update_sids', False) >>>>>> + return False, () >>>>> This part looks fine. Basically, a similar check needs to be added to >>>>> trust_find, trust_show, and may be trust_add. >>>>> >>>> >>>> Why trust-add? >>>> >>>> I'm not a big fan of cluttering existing commands(find, show, mod) >>>> with logic to fix one upgrade bug. But I understand a need to >>>> communicate it somehow. >>>> >>>> Would it make sense to move such logic to a separate command, e.g. >>>> trust-check/trust-verify? The command can do additional check in a >>>> future. >>> No. What is the value of trust-add if it says you 'Trust established and >>> verified' while in reality it didn't? This specific issue is very easy >>> to catch. >>> >> Patch attached, patch with warning will land soon. >> >> > Updated patches attached
During the RPM upgrade, the ipa-server-upgrade times out and leaves directory server stopped. Issues seem to be fixed after manual DS start&upgrade, but we should get the upgrade during RPM cleanup sorted out to have a seamless experience. Tomas -- 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
