Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrade: Run configuration upgrade under empty ccache collection #1725

Closed
wants to merge 1 commit into from

Conversation

abbra
Copy link
Contributor

@abbra abbra commented Mar 22, 2018

Use temporary empty DIR-based ccache collection to prevent upgrade
failures in case KCM: or KEYRING: ccache type is used by default in
krb5.conf and is not available. We don't need any user credentials
during upgrade procedure but kadmin.local would attempt to resolve
default ccache and if that's not available, kadmin.local will fail.

This approach was successfully tested with OpenQA tests that upgrade from F27 to F28.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1558818

os.environ['KRB5CCNAME'] = old_path
else:
del os.environ['KRB5CCNAME']
for f in os.listdir(kpath_dir):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use shutil.rmtree(), it removes the directory and all its children.

# Bug https://bugzilla.redhat.com/show_bug.cgi?id=1558818
kpath_dir = tempfile.mkdtemp(prefix="upgrade_ccaches",
dir=paths.IPA_CCACHES)
kpath = "DIR:{dir}s".format(dir=kpath_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra s after the directory name looks like a mistake.

kpath = "DIR:{}".format(kpath) is shorter and easier to read.

Use temporary empty DIR-based ccache collection to prevent upgrade
failures in case KCM: or KEYRING: ccache type is used by default in
krb5.conf and is not available. We don't need any user credentials
during upgrade procedure but kadmin.local would attempt to resolve
default ccache and if that's not available, kadmin.local will fail.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1558818
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
@abbra
Copy link
Contributor Author

abbra commented Mar 22, 2018

Updated to use shutil.rmtree() and simplify the formatting.

@tiran
Copy link
Member

tiran commented Mar 22, 2018

Thanks, LGTM to me!

@felipevolpone felipevolpone added the re-run Trigger a new run of PR-CI label Mar 22, 2018
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Mar 22, 2018
@flo-renaud
Copy link
Contributor

Hi @abbra
thank you for the patch, works for me. I tested by doing the following:

  • install on fedora 27
  • systemctl stop sssd-kcm.socket; systemctl disable sssd-kcm.socket
  • ipa-server-upgrade => success (without the patch, this would be failing)

@flo-renaud flo-renaud added the ack Pull Request approved, can be merged label Mar 23, 2018
@tiran tiran added the pushed Pull Request has already been pushed label Mar 23, 2018
@tiran
Copy link
Member

tiran commented Mar 23, 2018

master:

  • a678336 upgrade: Run configuration upgrade under empty ccache collection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
5 participants