-
Notifications
You must be signed in to change notification settings - Fork 1
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
ceph-mgr/dashboard: python-cryptography PyO3 modules may only be initialized once per interpreter process #20
Comments
@bazaah, it's about your basic fact 8) I am seeing 2 of my 3-node cluster where dashboard is loaded before restful, so dashboard is loaded and works. |
I've personally never seen that (restful always loads first) but I'm unsure if this ordering is enforced in the code somewhere. Its very possible that for whatever reason, your dashboard loads first, and the restful module aborts |
Seems the upstream has become aware of the issue:
However, it appears that this may still not be enough because of bcrypt usages, and moreover, we all still basically need PyO3 to provide a path forward for subinterpreters. That said, I'll probably experiment a bit with building a py-crypto-less dashboard. |
Although imperfect, I am now applying follow patches for making Ceph MGR working happily with bcrypt:
Also see: |
Good to know, though I'm somewhat limited until the python-bcrypt package updates to the (yet to merged) PR above |
Please don't expect bcrypt will accept that PR as pyca/bcrypt#714 (comment) mentioned. I am creating that PR for upstream bcrypt just because it works with my own OBS packaging pipeline, and I like to share the possible workaround with other else downstream distro packagers. |
Yeah, this is now completely broken on arch latest. During my testing for the 18.2.2-1 release:
I'm going to have to fork either the modules (bcrypt, cryptography) or pyo3 and remove the safety check, as this issue is now intractable |
So, with a two line change to pyo3, removing the import error, and the equivalent of rebuilding diff --git a/src/_bcrypt/Cargo.toml b/src/_bcrypt/Cargo.toml
index a9c7f7c..02317c8 100644
--- a/src/_bcrypt/Cargo.toml
+++ b/src/_bcrypt/Cargo.toml
@@ -6,7 +6,7 @@ edition = "2018"
publish = false
[dependencies]
-pyo3 = { version = "0.20.0", features = ["abi3"] }
+pyo3 = { git = "https://git.st8l.com/luxolus/pyo3", tag = "v0.20.3-subint+1", features = ["abi3", "unsafe-allow-subinterpreters"] }
bcrypt = "0.15"
bcrypt-pbkdf = "0.10.0"
base64 = "0.21.5" The bcrypt issue affecting all mgr modules has receded. I realize now that ceph v18.2.2 doesn't include ceph/ceph#55689, so I'll need to backport that myself and rebuild. Unsure how I'm going to expose my python-bcrypt fork, yet. |
Removes the (limited) runtime usages of python-jwt, and therefore, python-cryptography from the mgr dashboard module. This should restore dashboard functionality, if used alongside a patched pyo3 for python-bcrypt. Upstream-Ref: ceph/ceph@33d8bef References: ceph/ceph#55689 References: https://tracker.ceph.com/issues/63529 References: #20 References: https://git.st8l.com/luxolus/pyo3/commit/44d6919168621b46e4e884130ca43338655b020c
Removes the (limited) runtime usages of python-jwt, and therefore, python-cryptography from the mgr dashboard module. This should restore dashboard functionality, if used alongside a patched pyo3 for python-bcrypt. Upstream-Ref: ceph/ceph@33d8bef References: ceph/ceph#55689 References: https://tracker.ceph.com/issues/63529 References: #20 References: https://git.st8l.com/luxolus/pyo3/commit/44d6919168621b46e4e884130ca43338655b020c
- python-bcrypt-allow-subinterpreters.patch - python-bcrypt-prefix-ceph.patch Together, these allow us to build a renamed python-bcrypt package, as ceph_bcrypt. This will allow us to bypass the thorny pyo3 issues we have with various mgr modules exploding at runtime. References: #20
Removes the (limited) runtime usages of python-jwt, and therefore, python-cryptography from the mgr dashboard module. This should restore dashboard functionality, if used alongside a patched pyo3 for python-bcrypt. Upstream-Ref: ceph/ceph@33d8bef References: ceph/ceph#55689 References: https://tracker.ceph.com/issues/63529 References: #20 References: https://git.st8l.com/luxolus/pyo3/commit/44d6919168621b46e4e884130ca43338655b020c
- python-bcrypt-allow-subinterpreters.patch - python-bcrypt-prefix-ceph.patch Together, these allow us to build a renamed python-bcrypt package, as ceph_bcrypt. This will allow us to bypass the thorny pyo3 issues we have with various mgr modules exploding at runtime. References: #20
It appears this chicken has come home to roost now: https://tracker.ceph.com/issues/64213. I'm curious what the outcome of this will be. |
Allow the subinterpreter safeguards to be disabled, so that applications like Ceph's manager can continue to use pyo3 modules without soft crashing. Enabling this feature should be done with caution, as any storage of Py objects in rust statics can lead to undefined behavior. However, not all consumers of pyo3 use global state, and thus a subset of them (such as python-bcrypt) are safe to use in subinterpreter contexts. References: bazaah/aur-ceph#20 References: PyO3#2523 References: pyca/cryptography#9016 References: PyO3#2346 (comment) References: PyO3#2346 (comment) References: PyO3#3451 Signed-off-by: Paul Stemmet <[email protected]>
Opening this to track progress / news on the upstream project(s). See #16 for the history of this issue.
The basic facts are:
python-cryptography
modulev0.17.0
(or really: pymodule: only allow initializing once per process PyO3/pyo3#2523) the maintainers allowed one to initialize the same PyO3 backed module multiple times per process invocationpython-cryptography
upgraded to the problem version of PyO3 in v41, which is the current latest in Archlinux: https://archlinux.org/packages/extra/x86_64/python-cryptography/python-jwt
which initializespython-cryptography
firstAt the moment, I don't see a maintainable path forward without help from the upstream packages.
The text was updated successfully, but these errors were encountered: