-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Package 'cryptography' is missing from non-amd64 images in ansible-operator images > v1.26.0 #6302
Comments
I wrote a little test script to check the contents of ansible-operator images: #!/usr/bin/env bash
version=${1:-v1.27.0}
outdir=${2:-site-packages}-${version}
image_name=quay.io/operator-framework/ansible-operator
function check() {
local arch=$1
docker pull --quiet --platform ${arch} ${image_name}:${version}
docker tag ${image_name}:${version} ${image_name}:${version}-${arch}
docker rmi ${image_name}:${version}
mkdir -p ${outdir}/${arch}
local container=${image_name##*/}-${arch}
docker create --name ${container} ${image_name}:${version}-${arch}
docker cp --quiet ${container}:/usr/local/lib64/python3.8/site-packages/. ${outdir}/${arch}
docker rm ${container}
}
for arch in amd64 ppc64le s390x; do
check ${arch}
done
ls -ld ${outdir}/*/crypto* With v1.26.0 I get:
but with v1.26.1 and v1.27.0 I only get this:
|
@efussi Thanks for raising this issue! We updated our cryptography dependency in #6269 I would have assumed our build process would have properly built and fetched the dependencies correctly but seems this wasn't the case... I will do some digging on this to try and figure out what went wrong /assign |
Interestingly enough there seems to have been an error that occurred when building the images for ppc64le (here) and s390x (here), although they say immediately after "Will try again!" and appears to succeed... It looks like it was successful for arm64 (here). After doing some digging, it looks like ppc64le and s390x prebuilt wheels don't exist as the maintainers of the project state they can't test these architectures (although it looks like ppc64le may be coming later this year?) :
I think we have a few options here:
I will make sure to raise this issue for discussion in the next community issue triage meeting on Monday (02/13/2023) and gather some thoughts on what we should do. |
Thanks @everettraven for your quick assessment! Taking a step back, I am wondering why the missing cryptography package on ppc64le and s390x isn't causing unit test or runtime issues -- isn't cryptography a dependency that is needed at ansible-operator runtime? If yes, then I think (2) isn't a viable option. If not, may I ask why cryptography was included in the first place? If it was a courtesy to consumers, then imho option (2) is a breaking change and might require a major version bump according to semver (one could argue about this, if this courtesy wasn't officially documented :-) ). Is it possible to do a variant of option (1), reverting just on those platforms where no prebuilt wheels are available? To throw in one last data point: Anaconda provides cryptography for a variety of platforms, see https://anaconda.org/main/cryptography. But this is not readily installable in a plain pip environment (if at all). There might also be license considerations. |
To be honest, I'm not sure. I'm not very familiar with all the python side of things for the ansible operator but am slowly learning a bit more at a time to try and help cover a bit of a knowledge gap we have in the currently active maintainers (all of our maintainers that were SMEs on the ansible side of things have since moved on to other things). My assumption is that if the unit tests aren't failing it isn't required but more so a courtesy as you mentioned.
I 100% agree here and option 2 is my least favorite option but figured I would include it since it is one path that could be taken.
IIUC how we currently build the images it uses the same Dockerfile and uses
That's good to know, thanks for sharing! |
I ran another quick test using this small Dockerfile based on the ones in https://github.com/operator-framework/operator-sdk/blob/master/images/ansible-operator:
This installs plain
We see that this installs
We get this output:
The relevant line is
So |
I think the |
Going to bring this up at the ansible community call next week, will report back. |
We discussed this in the ansible community meeting and it was recommended that we first try updating our pip version that we install when building the images from |
so that we can ensure all packages are always included in the images we deliver by installing/building all packages in a build stage and copying them over to the final build stage. fixes operator-framework#6302 Signed-off-by: Bryce Palmer <[email protected]>
* update ansible base images to multi-stage builds so that we can ensure all packages are always included in the images we deliver by installing/building all packages in a build stage and copying them over to the final build stage. fixes #6302 Signed-off-by: Bryce Palmer <[email protected]> * update 2.11 preview image changes Signed-off-by: Bryce Palmer <[email protected]> --------- Signed-off-by: Bryce Palmer <[email protected]>
@everettraven Do we have any updates on this ? |
fwiw, #6333 was reverted through #6348, see also #6342 (comment). |
Bug Report
What did you do?
Use
cryptography
in ppc64le and s290x images based on ansible-operator. Minimal example:What did you expect to see?
Package
cryptography
can be used (python-cryptography-fernet-wrapper
installs successfully like it did in <= 1.26.0)What did you see instead? Under which circumstances?
Package 'cryptography' is missing in ppc64le and s390x images (didn't check arm64), causing an installation failure because
cryptography
is no longer preinstalled and the rust toolchain required to buildcryptography
from source is missing:Environment
Operator type:
/language ansible
Kubernetes cluster type:
OpenShift
$ operator-sdk version
$ go version
(if language is Go)$ kubectl version
Possible Solution
Additional context
Seemingly a regression caused by #6269
The text was updated successfully, but these errors were encountered: