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

ANW-2218 support docker on production #3420

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

Conversation

thimios
Copy link
Collaborator

@thimios thimios commented Dec 4, 2024

Description

See also: archivesspace/tech-docs#205
And: archivesspace/tech-docs#210

Here is an example zip file that contains the docker "configuration package" as created with:

./build/run build-docker-conf-package -Dversion="v4.0.0-RC1" -Ddocker-tag="4.0.0-RC1"          

archivesspace-docker-v4.0.0-RC1.zip

Related JIRA Ticket or GitHub Issue

ANW-2218

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have read the CONTRIBUTING document.
  • I have authority to submit this code.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@thimios thimios force-pushed the ANW-2218_docker_support_for_prod branch from b5846ad to 6a9b39d Compare December 4, 2024 17:30
@thimios thimios force-pushed the ANW-2218_docker_support_for_prod branch from 6a9b39d to 4516082 Compare December 5, 2024 13:21
@@ -0,0 +1,38 @@
services:
app:
image: archivesspace/archivesspace:@TAG@
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mark-cooper The @tag@ token is set by ant: https://github.com/archivesspace/archivesspace/pull/3420/files#diff-bffc59192cdccaf509753556461ff111ed11b8611616b7ebf7bbfd0700c3a711R318

to the git tag as passed to it from the github workflow: https://github.com/archivesspace/archivesspace/pull/3420/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R47

Which leaves this strange @tag@ token in the compose file but serves our release workflow... I do not have a better idea for this...

Copy link
Member

Choose a reason for hiding this comment

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

I think it looks fine. A possible alternative might be to use latest as the default in place of @TAG@ so it is runnable locally if you want to try it out easily (i.e. with docker compose -f docker-compose.prod.yml up -d for testing). But that exposes a potential issue in the case that "latest" isn't properly replaced during the build workflow (although then there could be checks for that etc.).

@thimios thimios requested a review from mark-cooper December 5, 2024 14:34
@thimios thimios marked this pull request as ready for review December 14, 2024 13:42
@thimios thimios requested a review from brianzelip December 14, 2024 13:53
@thimios thimios requested a review from Blake- December 14, 2024 13:55
Blake-
Blake- previously approved these changes Dec 18, 2024
brianzelip
brianzelip previously approved these changes Dec 19, 2024
@mark-cooper
Copy link
Member

I haven't read up on everything so sorry if parts of the feedback are irrelevant (don't have time to deep dive r/n):

I think it broadly looks good. But there are a few things that are still needed. The major thing is data persistence. At least:

Additional host volume mounts for:

  • the archivesspace data directory (something like: ./data:/archivesspace/data)
  • the mysql database (something like: ./database:/var/lib/mysql)
  • the solr data (something like: ./solr:/var/solr/data)

I'm going off memory so paths may be somewhat off. I recommend host volume mounts for this use case because it's a bit more straightforwardly "visible" to anyone less familiar with Docker (vs. Docker managed volumes).

It may or may not be desirable to directly expose ports for the db and solr. A "safer" default would be to not expose the ports and just let ASpace connect to them.

IIRC the use case is simplify deployment as much as possible and in that case I think I'd recommend bundling an nginx proxy to listen on port 4000 (but configurable) and then also restrict the ASpace ports. The only exposed port then would be 4000 for whatever other proxy / load balancer is going to connect to ASpace, and it just needs to proxy pass / forward to 4000 as the paths would be predefined for the individual apps like: https://github.com/archivesspace/archivesspace/blob/master/proxy/default.conf.release

There should be instructions about backing stuff up that may need to be tailored to how the docker bundle is working. For example if the db ports are not exposed then ASpace can be used to run a SQL dump (assuming that script still works!). Restoring would be about destroying the container, deleting the ./database files and putting the dump file in ./sql.

Use latest images by default in docker-compose
@mark-cooper
Copy link
Member

@thimios on second thoughts I think host volume mounts are probably not a good idea for data. At the very least there were issues on Windows using them. Using Docker volumes should always work equivalently across operating systems. That would look something like:

services:
  app:
    # ...
    volumes:
      - appdata:/archivesspace/data
  db:
    # ...
    volumes:
      - dbdata:/var/lib/mysql
  solr:
    # ...
    volumes:
      - solrdata:/var/solr/data

volumes:
  appdata:
  dbdata:
  solrdata:

There would just need to be docs about how data will be lost if the volumes are removed and about the importance of backups (but that's as always).

If it helps you out I can make a PR to this branch with the update after testing across Windows, Mac and Ubuntu (I got them all). I can also add the proxy if you're interested in seeing that.

@thimios
Copy link
Collaborator Author

thimios commented Jan 4, 2025

@mark-cooper I actually started working on setting up external mounts but did not know about the windows issue... Sure it would be great if you would create a PR to this branch with the proposed changes and I can take care of the documentation

@thimios
Copy link
Collaborator Author

thimios commented Jan 5, 2025

@mark-cooper I added one more commit with the WIP changes I started doing after your feedback. I opted for a single .env file because I feel it is easier to manage and because I could not find a way to pass an environment variable to be used within the docker-compose file for the proxy port configuration without making the standard command docker compose up more complex...

@mark-cooper
Copy link
Member

@thimios I pushed one commit directly to use the required mount path when using docker volumes. I've tested Ubuntu and Mac. I just need to test Windows tomorrow. I also wanted to look into an integrated backup container (if it seems doable quickly).

@thimios
Copy link
Collaborator Author

thimios commented Jan 10, 2025

@mark-cooper sounds great!

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.

4 participants