Skip to content

declare volumes #36

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

declare volumes #36

wants to merge 10 commits into from

Conversation

nelu
Copy link

@nelu nelu commented Feb 2, 2025

  • create the persistent data from the image when volume is empty
  • multiple architecture build arg
  • remove imported data from sql file - breaks postfixadmin install ; create the db structure from postfixadmin install
  • make postfix package version optional
  • github docker build and image publish action

nelu and others added 10 commits February 2, 2025 10:16
to create the persistent data from the image when volume is empty
multiple architecture build arg
used linked directories for persistent data
copy postfix config on data volume
CHARACTER SET latin1 fix for postfixadmin migrations setup
… since upstream updates the package regularly

the version number should be a build-time env variable. or use an official postfix image versioned accordingly and use it with FROM
@technicalguru
Copy link
Owner

Hello @nelu - Thanks for your contribution. Some comments:

  • create the persistent data from the image when volume is empty - accepted
  • multiple architecture build arg - I am not able to test this feature, hence I cannot ensure that any image built will actually run
  • remove imported data from sql file - breaks postfixadmin install ; create the db structure from postfixadmin install - strongly disagree here. Postfix will not work without it. PFA is an optional component that users might install or not. However, some recent upgrade bug was discovered and fixed to setup the tables in a better way.
  • make postfix package version optional - Disagree. The version must be in there as scripts and configuration depend on the correct version of Postfix. Experiences show that a Dockerfile without it is prone to different image results every time it is built.
  • github docker build and image publish action - Again - I will not be able to test this and not sure it shall be built using GitHub at all. However, I can leave it in as informational YAML file

Sorry for the very late response to your request.

@nelu
Copy link
Author

nelu commented May 21, 2025

PFA is an optional component that users might install or not. -- its present in the build imagine its more like enabling it or not based on user preference, you can run a cli php script to populate the db from pfa

--

make postfix package version optional - Disagree. The version must be in there as scripts and configuration depend on the correct version of Postfix. Experiences show that a Dockerfile without it is prone to different image results every time it is built. -- it breaks the docker build since those versions used at build time get removed compared to the distro ones which come with the same default config on newer version and its the right step towards multiarch build -- using a postfix version that was tested on multiple hardware/distros/users

--

regarding multiarch builds, should work without any trouble, since most of the configs are rewriten. debian arm64 is an almost replica of amd64.

its nice to run it on new high end routers or rasbperry pi which can run docker. i tested it running openwrt on https://www.gl-inet.com/products/gl-mt6000/

@nelu nelu closed this May 21, 2025
@technicalguru technicalguru reopened this May 21, 2025
@technicalguru
Copy link
Owner

Reopened to follow up on your contribution.

Hi all, thanks for your comments. I will add the SQL setup then as optional - configurable via environment, defaulting to yes.

The Dockerfile changes: I need to test with my own build and publish process. In case of issues, we can add an additional Dockerfile for other architectures.

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.

2 participants