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

Added dockerfiles to create container for moodle. #130

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

Conversation

mohammedzee1000
Copy link
Contributor

No description provided.

Copy link

@scara scara left a comment

Choose a reason for hiding this comment

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

Hi @mohammedzee1000,
great that you're working w/ introducing Moodle here, openshift-wise: is there any particular target e.g. for personal use?

Some refs among the ones you probably got in touch with:

HTH,
Matteo

HTTPD_WELCOME="/etc/httpd/conf.d/welcome.conf"
INSTALL_PKGS2="httpd nss_wrapper gettext";
PHP_REQUIRED="php php-mysql php-pgsql php-xml php-xmlrpc php-gd"
PHP_GOOD_TO_HAVE="php-xcache php-intl php-soap php-xmlrpc php-mbstring"
Copy link

Choose a reason for hiding this comment

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

No need for xcache: we should play with opcache only since Moodle supports it e.g. in terms of some invalidations during updates (not available for other opcode cachers).

MOODLE_HOST="`hostname -i`";
fi

if [ $1 == "moodle" ]; then
Copy link

Choose a reason for hiding this comment

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

Moodle could play w/ other services too, starting from an "external" DB then with an unoconv listener and could benefit from memcached, redis and other services including mail services.
What should be the goal of this docker file? Something to be used within a compose?

Copy link
Contributor Author

@mohammedzee1000 mohammedzee1000 Apr 5, 2017

Choose a reason for hiding this comment

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

I would say get it functional with openshift as well as compose and i have succeeded in this to a good extent.
I used following template for this

apiVersion: v1
items:
- apiVersion: v1
  kind: Service
  metadata:
    creationTimestamp: null
    labels:
      service: moodle
    name: moodle
  spec:
    ports:
    - name: "8080"
      port: 8080
      targetPort: 8080
    - name: "8443"
      port: 8443
      targetPort: 8443
    selector:
      service: moodle
  status:
    loadBalancer: {}
- apiVersion: v1
  kind: Service
  metadata:
    creationTimestamp: null
    labels:
      service: postgres
    name: postgres
  spec:
    clusterIP: None
    ports:
    - name: headless
      port: 55555
      targetPort: 0
    selector:
      service: postgres
  status:
    loadBalancer: {}
- apiVersion: v1
  kind: DeploymentConfig
  metadata:
    creationTimestamp: null
    labels:
      service: moodle
    name: moodle
  spec:
    replicas: 1
    selector:
      service: moodle
    strategy:
      resources: {}
    template:
      metadata:
        creationTimestamp: null
        labels:
          service: moodle
      spec:
        containers:
        - env:
          - name: DB_HOST
            value: postgres
          - name: DB_TYPE
            value: pgsql
          - MOODLE_HOST
            value: "URL_FROM_ROUTE"
          - MOODLE_HOST_NOPORT
            value: "True"
          image: ' '
          name: moodle
          ports:
          - containerPort: 8080
          - containerPort: 8443
          resources: {}
        restartPolicy: Always
    test: false
    triggers:
    - type: ConfigChange
    - imageChangeParams:
        automatic: true
        containerNames:
        - moodle
        from:
          kind: ImageStreamTag
          name: moodle:latest
      type: ImageChange
  status: {}
- apiVersion: v1
  kind: ImageStream
  metadata:
    creationTimestamp: null
    name: moodle
  spec:
    tags:
    - annotations: null
      from:
        kind: DockerImage
        name: mohammedzee1000/testc
      generation: null
      importPolicy: {}
      name: latest
  status:
    dockerImageRepository: ""
- apiVersion: v1
  kind: DeploymentConfig
  metadata:
    creationTimestamp: null
    labels:
      service: postgres
    name: postgres
  spec:
    replicas: 1
    selector:
      service: postgres
    strategy:
      resources: {}
    template:
      metadata:
        creationTimestamp: null
        labels:
          service: postgres
      spec:
        containers:
        - env:
          - name: POSTGRESQL_DATABASE
            value: moodle
          - name: POSTGRESQL_PASSWORD
            value: moodle
          - name: POSTGRESQL_USER
            value: moodle
          image: ' '
          name: postgres
          resources: {}
        restartPolicy: Always
    test: false
    triggers:
    - type: ConfigChange
    - imageChangeParams:
        automatic: true
        containerNames:
        - postgres
        from:
          kind: ImageStreamTag
          name: postgres:9.6
      type: ImageChange
  status: {}
- apiVersion: v1
  kind: ImageStream
  metadata:
    creationTimestamp: null
    name: postgres
  spec:
    tags:
    - annotations: null
      from:
        kind: DockerImage
        name: registry.centos.org/postgresql/postgresql:9.6
      generation: null
      importPolicy: {}
      name: "9.6"
  status:
    dockerImageRepository: ""
kind: List
metadata: {}

Copy link

Choose a reason for hiding this comment

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

TNX for your comments and your example, appreciated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: mohammedzee1000/testc is a namespace where i house any container i am testing. Currently thats moodle. Once i am done here, and this container is added. The container will get proper name which will be published on https://wiki.centos.org/ContainerPipeline

# Install moodle
pushd /var/www;
wget ${MOODLE_DOWNLOAD_URL} && tar zxvf ${MOODLE_TAR} && mv /var/www/${MOODLE}/* /var/www/html;
mkdir -p /var/moodledata
Copy link

Choose a reason for hiding this comment

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

moodledata must be persistent (VOLUME): it contains caching files and the deduplicated (at file level) storage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks i will see if i can make it a VOLUME but generally i have noticed this plays havoc with permission fixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok i tried making it a volume as in VOLUME /var/moodledata in dockerfile i got
"Fatal error: $CFG->dataroot is not writable, admin has to fix directory permissions! Exiting."

Copy link

Choose a reason for hiding this comment

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

Hi @mohammedzee1000,
the user, under which the Moodle instance runs on, should have rwx rights there since it must write on it folders and files during the installation and the life cycle of the instance.
Could we run kind of fix-permissions.sh (e.g. chown -R on run.sh) or should we add a requirement when using an "external" volume to fix the permission (not sure how to automate it in case of a dynamic uid as in openshift)?

Copy link
Contributor Author

@mohammedzee1000 mohammedzee1000 Apr 6, 2017

Choose a reason for hiding this comment

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

Yea that is always a problem. I had this issue with other containers as well. As soon as i added VOLUME /somepath, anywhere in dockerfile. Then any fix-permissions that i ran on /somepath broke on openshift. But as soon as i removed that the fix-permissions took affect properly and it worked fine on openshift.

I will attempt what you said with run.sh however, lets see if it works.

@mohammedzee1000
Copy link
Contributor Author

As far as why moodle, its one of things we plan to bring onto Container Pipeline Service (https://wiki.centos.org/ContainerPipeline)

@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 5, 2017

I am having issues with oci (oracle) and mssql there are no rpms available and atleast for mssql pecl install needs php7.

I am attempting with oci now. Hopefully i can get that to work.
@scara @kbsingh any ideas

@scara
Copy link

scara commented Apr 5, 2017

Hi @mohammedzee1000,
I do not use/play w/ mssql but I'm used to run Moodle on PHP 7.0.x via @remicollet's repo (php70 for 3.1).

Please note that for security reasons you should download the latest weekly when packaging the image and not just the initial release: this is due to the way Moodle is used to release upgrades (and security updates!) within a supported release (like 3.1, which is LTS: https://docs.moodle.org/dev/Releases#Version_support).
You're fine here since you're downloading from https://download.moodle.org/download.php/stable31/moodle-latest-31.tgz which is the latest weekly for 3.1: maybe somewhere you should add a comment about this strategy and, maybe, the build date to let people deduce what 3.1 weekly their container is based on (the exact version is also available logging in as Administrator in the Notification page but... of course you're running the image to access to the Moodle instance).

HTH,
Matteo

@@ -8,22 +8,28 @@ HTTPD_WELCOME="/etc/httpd/conf.d/welcome.conf"

INSTALL_PKGS1="wget";
INSTALL_PKGS2="httpd nss_wrapper gettext";
REMI_REPO="http://rpms.famillecollet.com/enterprise/remi-release-7.rpm"

Choose a reason for hiding this comment

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

Please use proper URL, which is https://rpms.remirepo.net/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok thanks

PHP4="${PHP_REMI_VERSION}-php-pecl-apcu-bc ${PHP_REMI_VERSION}-php-pecl-redis ${PHP_REMI_VERSION}-php-phpiredis"
PHP5="${PHP_REMI_VERSION}-php-opcache ${PHP_REMI_VERSION}-php-pecl-memcache ${PHP_REMI_VERSION}-php-pecl-memcached"
PHP6="${PHP_REMI_VERSION}-php-intl ${PHP_REMI_VERSION}-php-mbstring ${PHP_REMI_VERSION}-php-pecl-solr2"
PHP7="${PHP_REMI_VERSION}-php-pecl-zip ${PHP_REMI_VERSION}-php-soap"

Choose a reason for hiding this comment

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

Notice than using php71-php from "remi-safe" repository (SCL package) is very different from using php from "remi-php71" repository (base package).

BTW, if SCL packages are installed in a different path (/opt) they are smaller, so perhaps more suitable for Docker image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, could not catch that. Am i doing something wrong?

Copy link

@remicollet remicollet Apr 6, 2017

Choose a reason for hiding this comment

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

Nothing wrong, just a different set of packages. Both work, only files locations differ.

Copy link

Choose a reason for hiding this comment

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

HI @mohammedzee1000,
that's probably means: PHP as module via SCL requires an adjustment in the Apache conf files.

HTH,
Matteo

Copy link
Contributor Author

@mohammedzee1000 mohammedzee1000 Apr 6, 2017

Choose a reason for hiding this comment

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

@scara cool. I was able to get the container running based on current changes in source through docker compose
@remicollet thanks

@mohammedzee1000
Copy link
Contributor Author

@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 6, 2017

@remicollet thanks well then i can avoid most of those steps then, most.
I will need to still populate freetds file dynamically

@remicollet
Copy link

I will need to still populate freetds file dynamically

Using php-sqlsrv, you don't need freetds ;)

@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 6, 2017

@remicollet ok so one more package and mssql is solved does the same apply to oci just install php-oci8 and im good from that side?

@remicollet
Copy link

remicollet commented Apr 6, 2017

just install php-oci8 and im good from that side?

For this one, you also need the Oracle client library (which can't be in my repository because of License issues, not even redistributable), and some tuning to ensure the lib are found in the library path (and because their RPM sucks), such as

echo "/usr/lib/oracle/12.1/client64/lib" >/etc/ld.so.conf.d/oracle-instantclient-x86_64.conf

@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 6, 2017

So i guess il skip oracle for now and add a warning for the same. Is that ok with you @scara?
Or do you want me to pull form rajeshtaneja since he apparently can keep that thing in his repo.

@mohammedzee1000 mohammedzee1000 force-pushed the 2017-04-03_16-56-49-moodle_container branch from 35ce799 to 1356b17 Compare April 6, 2017 13:32
@scara
Copy link

scara commented Apr 6, 2017

Hi @mohammedzee1000,
up to you: I'm fine with your message and your nice README. You could add Oracle support one day when it will be requested, maybe via a PR.
Thanks for your hard working! 👍

Thanks to @remicollet for stepping into too, appreciated 😉.

@mohammedzee1000 mohammedzee1000 changed the title [WIP] Added dockerfiles to create container for moodle. Added dockerfiles to create container for moodle. Apr 7, 2017
@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 7, 2017

@scara @remicollet Thanks for all the help. I have pushed in couple of last fixed here for MOODLE_URL as its passed to sed, so users must be made aware of using http:\/\/ instead of http:// for example which i have added to the README as well. Otherwise there is no other change.

@kbsingh @jimperrin i think we should be good here.

@mohammedzee1000
Copy link
Contributor Author

@kbsingh i have updated the information for tracking updates.

@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 10, 2017

@kbsingh @jperrin any more feedback or can we get this in.

@scara
Copy link

scara commented Apr 11, 2017

Hi @mohammedzee1000,
did you already read about two small typos in https://github.com/CentOS/CentOS-Dockerfiles/pull/130/files#diff-2f7a31c9e648aff543a099083dff59e4R32 (moodle/centos7/README.md) at #130 (comment)?

TIA,
Matteo

@mohammedzee1000
Copy link
Contributor Author

@scara no, i cant seem to be able to find the coment either. Do you remember what they were? Or if you could point it out, i will fix it.

@scara
Copy link

scara commented Apr 12, 2017

A couple of typos around https://github.com/CentOS/CentOS-Dockerfiles/pull/130/files#diff-2f7a31c9e648aff543a099083dff59e4R32:

  • doubled "the" in "... include the the protocol ..."
  • "Its" should be "It's"

HTH,
Matteo

@mohammedzee1000
Copy link
Contributor Author

@scara thanks, il fix them right away :)

@mohammedzee1000
Copy link
Contributor Author

Updated

@mohammedzee1000
Copy link
Contributor Author

CentOS/container-index#146
PR raised to include this container on Container Pipeline Service. This container will be published as
registry.centos.org/centos/moodle:3.1

@mohammedzee1000
Copy link
Contributor Author

Container now available on registry.centos.org as registry.centos.org/centos/moodle:3.1
@scara ^

@mohammedzee1000
Copy link
Contributor Author

mohammedzee1000 commented Apr 21, 2017

@scara thanks i will take a look at this possibility

EDIT:
Yes, it works. Now to get it into the install.sh :)

@mohammedzee1000
Copy link
Contributor Author

@scara updated

@scara
Copy link

scara commented Apr 21, 2017

OK
The rationale behind my proposal is to avoid writing in the layer (CoW) something read "outside": the "drawback" is that logs are now collected by the container engine and no more present in the container itself.

HTH,
Matteo

@mohammedzee1000
Copy link
Contributor Author

Which should be fine. A lot people already are using some sort of log aggregation with containers.

Infact it's not recommended to have file based logs in containers.

@scara
Copy link

scara commented Jun 13, 2017

Hi @mohammedzee1000,
not sure about progresses here, but found some more comments to be shared w/ you (maybe useful or other of your branches too):

  1. in https://github.com/mohammedzee1000/CentOS-Dockerfiles/blob/183c3ca568e53e859b059f48871323486f61ed41/moodle/centos7/install.sh#L56 we should only change 80 into 8080 since no mod_ssl has been installed and configured. For the same reason in https://github.com/mohammedzee1000/CentOS-Dockerfiles/blob/183c3ca568e53e859b059f48871323486f61ed41/moodle/centos7/Dockerfile#L17 we should expose just 8080
  2. why https://github.com/mohammedzee1000/CentOS-Dockerfiles/blob/183c3ca568e53e859b059f48871323486f61ed41/moodle/centos7/Dockerfile#L5 is commented out? Is it a policy to stick with the base image maybe since the update should happen there?
  3. ACCEPT_EULA is declared but not used (https://github.com/mohammedzee1000/CentOS-Dockerfiles/blob/183c3ca568e53e859b059f48871323486f61ed41/moodle/centos7/install.sh#L36). Should it be required for MSSQL client, here missing?
  4. Typo, this this directory, in https://github.com/mohammedzee1000/CentOS-Dockerfiles/blob/183c3ca568e53e859b059f48871323486f61ed41/moodle/centos7/README.md

HTH,
Matteo

@mohammedzee1000
Copy link
Contributor Author

@scara Thanks i will look into it. I am planning to try to get sclo rpms in there. But the image is already building and available on r.c.o.

Will continue to add as i keep figuring things out.
Need to figure out how to factor this in.
This is due to licensing issues with oracle client.
The MOODLE_URL is passed to a sed in run and hence a user must be
aware that he must escape appropriate characters such as http://
which should actually be http:\/\/.
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