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

Use bookworm/debian 12, python 3.12, update python deps, fix buildx warnings #162

Merged
merged 8 commits into from
Feb 4, 2025

Conversation

fredvd
Copy link
Member

@fredvd fredvd commented Jan 31, 2025

Fix buildx warning for ARG and ENV usage.
update relstorage, python-ldap, add sasl2 for ldap installation.
Fix libtiff5 to 6
Update and pin pip and wheel
Remove importlibmetadata env var for older pip issue.
Fix FROM/as casing warning from buildx.

Fix buildx warning for ARG and ENV usage.
update relstorage, python-ldap, add sasl2 for ldap installation.
Fix libtiff5 to 6
Update and pin pip and wheel
Remove importlibmetadata env var for older pip issue.
Fix FROM/as casing warning from buildx.
@fredvd fredvd marked this pull request as draft January 31, 2025 19:50
@fredvd
Copy link
Member Author

fredvd commented Jan 31, 2025

@mauritsvanrees @davisagli The images build for me now locally (arm64), without warnings. But when I try to start plone-backend or plone-classicui, the image halts because python cannot import Zope2.

Comparing against local builds for 6.0.14, I notice that plone-backend is missing the path to site-packages, which explains why Zope2 cannot be found.

But also my plone-backend:6.1.0rc1 local image still has Python 3.11, not 3.12 :-O

6.0.14 plone-backend

root@e8eadcbb1a86:/app# python3
Python 3.11.11 (main, Dec  4 2024, 23:37:51) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path
['', '/usr/local/lib/python311.zip', '/usr/local/lib/python3.11', '/usr/local/lib/python3.11/lib-dynload', '/app/lib/python3.11/site-packages']
>>>
(app) root@e8eadcbb1a86:/app# which python
/app/bin/python
(app) root@e8eadcbb1a86:/app# grep Products.CMFPlone constraints.txt
Products.CMFPlone==6.0.14

6.1.0rc1 plone-backend

root@ffa942febe9c:/app# python
Python 3.11.11 (main, Jan 14 2025, 10:54:20) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path
['', '/usr/local/lib/python311.zip', '/usr/local/lib/python3.11', '/usr/local/lib/python3.11/lib-dynload']
>>> exit
Use exit() or Ctrl-D (i.e. EOF) to exit
>>>
(app) root@ffa942febe9c:/app# python3
Python 3.11.11 (main, Jan 14 2025, 10:54:20) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>>
(app) root@ffa942febe9c:/app# grep Products.CMFPlone constraints.txt
Products.CMFPlone==6.1.0rc1

…traints.txt from rc1, add no-cache. Now build-images work locally.
@fredvd
Copy link
Member Author

fredvd commented Jan 31, 2025

@mauritsvanrees @davisagli @ericof Found the issue, I think. my local build was still fetching/preferring already built images from hub.docker.com.

I've set the PLONE_VERSION to 6.1.0rc2 here, but have hardcoded 6.1.0rc2 in Dockerfile.builder where it still fetches the 6.1.0rc1/constraints.txt from dist.plone.org.

Should/can we remove the 6.1.0rc1 images from hub.docker.com? Then we can still release under rc1 later.

@fredvd
Copy link
Member Author

fredvd commented Jan 31, 2025

note: needs some feedback, fixes and cleanup before it can be merged

@fredvd
Copy link
Member Author

fredvd commented Jan 31, 2025

Oh, one more request from me, but can be dealt with in another issue/PR. But now that I might have your attention here ;-) :

can we move/rename the /app folder in the backend images to /backend ?

this would make it much easier to build a single plone-demo image with both of them in. Dockerfile later

@mauritsvanrees
Copy link
Member

I don't completely follow what is happening here, I am not the greatest Docker guru. But I see a lot of good things here. +1 for moving to Python 3.12 and latest RelStorage.

@fredvd
Copy link
Member Author

fredvd commented Jan 31, 2025

And: we need to check and maybe mention an issue when migrating from Relstorage 3.5 to 4.0 or 4.x and ZODB issues . Maurits found the report back when I mentioned it to him this afternoon:

zodb/relstorage#505

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile.builder Outdated Show resolved Hide resolved
Dockerfile.classicui Outdated Show resolved Hide resolved
version.txt Outdated Show resolved Hide resolved
@davisagli
Copy link
Member

@fredvd

Should/can we remove the 6.1.0rc1 images from hub.docker.com? Then we can still release under rc1 later.

I don't see 6.1.0rc1 images there, so I guess you already did that? Yes, I'd say it's important to release under rc1 to match what is on dist.plone.org.

can we move/rename the /app folder in the backend images to /backend ? this would make it much easier to build a single plone-demo image with both of them in. Dockerfile later

That sounds like a breaking change that would affect most downstream images that build on top of this, so I'm pretty strongly -1 on that. But maybe it could be an ARG? I would not do it pre-emptively until you actually try to create a unified image and see whether it makes sense to reuse these Dockerfiles or not.

Co-authored-by: David Glick <[email protected]>
Co-authored-by: David Glick <[email protected]>
…s constraints.

Remove version nmber from PLONE_VERSION ARG in Docker files so that the build will fall. This will re-introduce output warnings from docker buildx that possible uninitialised variables are used.
README.md Outdated Show resolved Hide resolved
@fredvd
Copy link
Member Author

fredvd commented Feb 3, 2025

@fredvd

Should/can we remove the 6.1.0rc1 images from hub.docker.com? Then we can still release under rc1 later.

I don't see 6.1.0rc1 images there, so I guess you already did that? Yes, I'd say it's important to release under rc1 to match what is on dist.plone.org.

can we move/rename the /app folder in the backend images to /backend ? this would make it much easier to build a single plone-demo image with both of them in. Dockerfile later

That sounds like a breaking change that would affect most downstream images that build on top of this, so I'm pretty strongly -1 on that. But maybe it could be an ARG? I would not do it pre-emptively until you actually try to create a unified image and see whether it makes sense to reuse these Dockerfiles or not.

Ah, a variable to parameterise the docker-entrypoint.sh is a good suggestion, I can do so something with that, thanks!

@fredvd
Copy link
Member Author

fredvd commented Feb 3, 2025

I have removed the rc2 testing setup from the branch and removed the default ARG variables. I have also removed the plone/server-prod-config 6.1.0rc1 tag from hub.docker.com as that was likely the cause why local test builds of the rc1 images had a mixed python3.11/3.12 environment.

If this PR is merged, rc1 should build and be deployed to hub.docker.com

@fredvd fredvd marked this pull request as ready for review February 3, 2025 19:55
@fredvd fredvd merged commit 41edebe into 6.1.x Feb 4, 2025
3 checks passed
@fredvd fredvd deleted the updates-bookworm-python312 branch February 4, 2025 09:15
@mauritsvanrees
Copy link
Member

I think the old v6.1.0rc1 tag in this repo can be removed, and a new tag made, right?

@fredvd
Copy link
Member Author

fredvd commented Feb 4, 2025

I think the old v6.1.0rc1 tag in this repo can be removed, and a new tag made, right?

Yes, that archive/release you can download is a bit superfluous, because what counts are the released images. But the contents of the tar.gz source files should match the generated container image tags, which should match the Plone (backend) release.

I'll remove the tag and will try to build/release the 6.1.0rc1 images again.

@mauritsvanrees
Copy link
Member

I don't really care about the archive/release on GitHub. I only meant that a new tag was needed, because that is the only way I know that an image will be generated.
But I see docker images can be made by manually triggering the workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants