URL: https://github.com/freeipa/freeipa/pull/5312 Author: rcritten Title: #5312: Move where the restore state is marked during IPA server upgrade Action: opened
PR body: """ There is still some exposure to killing in a bad place. This was reproduced by killing the process in the parser.parse() call within __restore_config (line 230) so the values were restored from the backup but the new dse.ldif never written or copied. But the values had already been restored from the state file. I'm not sure this can ever be 100% bullet-proof since it can be externally killed but if rather than calling restore_state() on the values in __restore_config we use get_state() which will peek at the values in the state file without removing them. Then the last step is to pop upgrade-in-progress and then the rest. If the values have been restored and the new ldif written and copied then it's only upgrade-in-progress that really matters. The rest will be overwritten. https://pagure.io/freeipa/issue/7534 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/5312/head:pr5312 git checkout pr5312
From fc2c0772210b1a4b8664b9ec8b592e08da890d8f Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 1 Dec 2020 16:35:26 -0500 Subject: [PATCH] Move where the restore state is marked during IPA server upgrade There is still some exposure to killing in a bad place. This was reproduced by killing the process in the parser.parse() call within __restore_config (line 230) so the values were restored from the backup but the new dse.ldif never written or copied. But the values had already been restored from the state file. I'm not sure this can ever be 100% bullet-proof since it can be externally killed but if rather than calling restore_state() on the values in __restore_config we use get_state() which will peek at the values in the state file without removing them. Then the last step is to pop upgrade-in-progress and then the rest. If the values have been restored and the new ldif written and copied then it's only upgrade-in-progress that really matters. The rest will be overwritten. https://pagure.io/freeipa/issue/7534 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> --- ipaserver/install/upgradeinstance.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py index 855d7f236c5..a239dd035a8 100644 --- a/ipaserver/install/upgradeinstance.py +++ b/ipaserver/install/upgradeinstance.py @@ -199,11 +199,11 @@ def __enable_ds_global_write_lock(self): shutil.copy2(ldif_outfile, self.filename) def __restore_config(self): - port = self.restore_state('nsslapd-port') - security = self.restore_state('nsslapd-security') - global_lock = self.restore_state('nsslapd-global-backend-lock') - schema_compat_enabled = self.restore_state('schema_compat_enabled') - self.restore_state('upgrade-in-progress') + # peek the values during the restoration + port = self.get_state('nsslapd-port') + security = self.get_state('nsslapd-security') + global_lock = self.get_state('nsslapd-global-backend-lock') + schema_compat_enabled = self.get_state('schema_compat_enabled') ldif_outfile = "%s.modified.out" % self.filename with open(ldif_outfile, "w") as out_file: @@ -231,6 +231,15 @@ def __restore_config(self): shutil.copy2(ldif_outfile, self.filename) + # Now the restore is really done, remove upgrade-in-progress + self.restore_state('upgrade-in-progress') + + # the values are restored, remove from the state file + self.restore_state('nsslapd-port') + self.restore_state('nsslapd-security') + self.restore_state('nsslapd-global-backend-lock') + self.restore_state('schema_compat_enabled') + def __disable_listeners(self): ldif_outfile = "%s.modified.out" % self.filename with open(ldif_outfile, "w") as out_file:
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
