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

Official GLPI Release Image #67

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link

This is still a work in progress.

Example .env:

COMPOSE_PROFILES=full
DB_ROOT_PASSWORD=glpi
DB_USER=glpi
DB_PASSWORD=glpi
WEB_PORT=8067
DB_PORT=9906
GLPI_RELEASE=https://github.com/glpi-project/glpi/releases/download/10.0.0/glpi-10.0.0.tgz
IMAGE_TAG=10.0.0
GLPI_LANG=en_US

When building an image, it will automatically download the archive specified by GLPI_RELEASE and tag the image based on IMAGE_TAG. When starting the containers, it checks if GLPI is already installed and if it isn't, it will automatically start the install so the user lands on the login page and not the install.

Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

  1. We already provide nightly build images, and, IMHO, we should rely on existing logic. The only difference is that released versions of GLPI are already built, so we can directly use the build archive.
    See Nightly docker image of GLPI #24.

  2. Providing a docker-compose.yml file will add complexity, and may require more work for us. Thus, people may want to use a docker image on an existing environment which already contains database services and defined networks.
    I would prefer to add some documentation in which we provide an example of docker-compose.yml file.

  3. Definition of user/group ID does not seems usefull to me. Indeed, in a production environment, files should not be used in both container and host contexts, so there should not have any files ownership issues.

@cconard96
Copy link
Author

The goal, as far as I read it, was to provide a production-ready GLPI environment with performance and security in mind. To me that means at least an optional database, redis server, etc.

The user/group ID are left over from the reference docker files. I believe the other docker config used mounted volumes instead of named ones.

Comment on lines +111 to +114
# Download GLPI Files
ARG GLPI_RELEASE
RUN chmod u+x /opt/download_glpi.sh
RUN /opt/download_glpi.sh ${GLPI_RELEASE}
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, the GLPI files should be directly included in images. It would permit to tag images according to contained GLPI version.
For instance, we could use following images names/tags :

  • GLPI 10.0.0 -> glpi:10.0.0, glpi:10.0 (latest tag of 10.0 intermediate release), glpi:10 (latest tag of 10 major release), glpi:latest
  • GLPI 9.5.7 -> glpi:9.5.7, glpi:9.5 (latest tag of 9.5 major release) glpi:9 (cannot consider that 9.1 and 9.5 are intermediate versions of a same major release)
  • GLPI 9.5.6 -> glpi:9.5.6

So, people using glpi or glpi:latest will always get the latest stable release when using a pull command, but people using glpi:10.0 would ensure to only get bugfixes versions of GLPI 10.0.

Also, if we let people choose the release to use in this image, we will have to maintain images that are compatible with multiple GLPI versions, which can be a pain.

Comment on lines +7 to +15
apache2-foreground

if [[ -f "/var/www/glpi/config/config_db.php"]]; then
echo "GLPI installed already"
else
echo "GLPI config missing. Installing GLPI..."
./opt/install_glpi.sh
echo "Installation finished"
fi
Copy link
Member

Choose a reason for hiding this comment

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

Unless I am wrong, this will not work.

Indeed, a container stops as long as the startup script is exiting. This is why we use apache2-foreground here, as it is a foreground process that is not exiting (unless on failure). So to be sure that this if/else block is executed, you have to move it prior to apache2-foreground execution.

@@ -0,0 +1,11 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

These scripts should exit as long as an error occurred.

Suggested change
#!/bin/bash
#!/bin/bash -e

@cconard96
Copy link
Author

I think this will need split up since it has mixed concerns.
The first part is the tooling needed to build an official GLPI image while the second is providing a sample docker compose file for building an environment with the official image.

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.

None yet

2 participants