Skip to content

Commit

Permalink
Changes for running containers
Browse files Browse the repository at this point in the history
  • Loading branch information
@markusweigelt
markusweigelt committed Feb 24, 2023
1 parent 4f5b3c2 commit 02162ad
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 5 deletions.
4 changes: 1 addition & 3 deletions .env
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,12 @@ MANAGER_DATA=${PWD}/kitodo/data/metadata # persistent data volume to mount
MONITOR_IMAGE=ghcr.io/slub/ocrd_monitor:latest # name and tag of image
MONITOR_HOST=ocrd-monitor # name/address of server
MONITOR_PORT_WEB=5000 # host-side port to exposed Web server
MONITOR_PORT_GTK=8500 # host-side port to exposed Broadwayd (Gtk Web server)
MONITOR_PORT_LOG=8088 # host-side port to exposed Dozzle (Docker log viewer)
MONITOR_DATA=${PWD}/kitodo/data/metadata # persistent data volume to mount

# Controller SSH server (with-ocrd-controller)
CONTROLLER_IMAGE=ghcr.io/slub/ocrd_controller:latest # name and tag of image
CONTROLLER_HOST=ocrd-controller # name/address of server (for Manager/Monitor)
CONTROLLER_PORT_SSH=22 # SSH port (for Manager/Monitor)
CONTROLLER_PORT_SSH=8022 # SSH port (for Manager/Monitor)

This comment has been minimized.

Sign in to view

Copy link
@bertsky

bertsky Feb 27, 2023

Member

Why the non-default port here? (If Controller runs as a service under Compose, it should have a dedicated virtual host, so no conflicts for native SSH are to be expected. Otherwise this setting will have to be adapted to the actual value of the external server anyway.)

This comment has been minimized.

Sign in to view

Copy link
@markusweigelt

markusweigelt Feb 27, 2023

Author Collaborator

We should not use here a standard port cause on my local system the port is already used for root or so far I could not find out through which application. Maybe this is the port that makes it possible for IT to get to my system in case of emergency or is part of an Ansible profile etc.

I think that can happen in other institutions as well.

In addition, there is a problem:

As soon as this port is occupied and we change the number we get a problem with using the local OCR-D Controller. As default we use the bridge network and thus all ports of services are only accessible on the exposed port and not the host port. For OCR-D Controller that mean 22 no matter which port was configured for CONTROLLER_PORT_SSH. Currently we also use this as environment variable for the OCR-D Manager to access the OCR-D Controller. So if this CONTROLLER_PORT_SSH is != 22 it does not work with enabled controller profile anymore.

As a workaround I set defaults for the port in docker-compose files and commented out the variable CONTROLLER_PORT_SSH. However, I wanted to agree beforehand whether this is a good solution. Otherwise we need two variables here or we have to change the network type. But I would be for the workaround or the variables.

This comment has been minimized.

Sign in to view

Copy link
@bertsky

bertsky Feb 27, 2023

Member

I understand now, but please be more precise in your language next time. (Differentiate between bridge default network and Compose-generated bridge-type network, between local-but-external and local-but-virtual / Compose-controlled...)

So perhaps we should not even expose 22 to CONTROLLER_PORT_SSH via ocrd_controller/docker-compose.yml ports in the first place?

There should be no need for such an exposition if we manage the Controller service via Compose. (It can be reached within the Compose network on the internal port 22.) In contrast, for an external Controller, we must have it. (But so far we rely on the Makefile for that. Our mkdocs currently explain how Compose can be used for standalone as well, though.)

Indeed, we might as well expose to some non-conflicting unpriviledged port like 8022 to cover both cases alike. That's your current solution.

When I think of it, I actually do like this best. This way, we can keep the documentation regarding standalone Compose usage. 👍

This comment has been minimized.

Sign in to view

Copy link
@markusweigelt

markusweigelt Feb 27, 2023 •
edited
Loading

Author Collaborator

That's your current solution.

Atm my local solution is following:

BaseRepo .env #CONTROLLER_PORT_SSH=8022 # SSH port (for Manager/Monitor)

adjusted docker-compose of Module OCR-D Controller - ${CONTROLLER_PORT_SSH:-8022}:22

adjusted docker-compose of Module OCR-D Monitor and OCR-D Manager - CONTROLLER: "${CONTROLLER_HOST}:${CONTROLLER_PORT_SSH:-22}"

Problem is the same when 8022 is occupied. So to cover all eventualities I am for the opinion to assign two environment variables one for the host CONTROLLER_PORT_SSH_HOST=8022 and one on which the manager and monitor can reach the controller CONTROLLER_PORT_SSH=22.

Our mkdocs currently explain how Compose can be used for standalone as well, though.)

Yes but that is not a problem to change documentation for that. :)

This comment has been minimized.

Sign in to view

Copy link
@bertsky

bertsky Feb 27, 2023

Member

Problem is the same when 8022 is occupied.

Sure, but you have that with MANAGER_PORT_SSH, too. And with APP_PORT etc. And MONITOR_PORT_WEB. We cannot "cover" the eventuality that the user does not know what ports he wants/allows to bind.

I say keep it simple. CONTROLLER_PORT_SSH is not strictly needed (under with-ocrd-controller) like the other ports are. But it does not hurt to expose it for other purposes, either.

This comment has been minimized.

Sign in to view

Copy link
@markusweigelt

markusweigelt Feb 27, 2023 •
edited
Loading

Author Collaborator

Problem is the same when 8022 is occupied.

Sure, but you have that with MANAGER_PORT_SSH, too. And with APP_PORT etc. And MONITOR_PORT_WEB. We cannot "cover" the eventuality that the user does not know what ports he wants/allows to bind.

No you do not have the same problem. For these service a change of ports is possible when using with-ocrd-controller profile. When we change 8022 to 10022 the CONTROLLER env of manager and monitor has following value ocrd-controller:10022 and thus cannot be accessed, since we can only access 22.

This comment has been minimized.

Sign in to view

Copy link
@bertsky

bertsky Feb 27, 2023

Member

Oh, right! Thanks for being thorough.

But keeping two separate variables is ugly IMO.

Let's go with unexposing 22 to CONTROLLER_PORT_SSH in ocrd_controller/docker-compose.yml then – the only place where we need that is in the standalone case when run via Compose. For that specific case, we can add a ocrd_controller/docker-compose.override.yml – it will not be used by Compose from ocrd_kitodo (because it's in a subdirectory) but only from ocrd_controller standalone (because it's the default override).

So we don't even have to change our documentation, just move the port mapping from docker-compose.yml to docker-compose.override.yml

This comment has been minimized.

Sign in to view

Copy link
@markusweigelt

markusweigelt Mar 1, 2023

Author Collaborator

All right, that's what we'll do. PR for that is set. slub/ocrd_controller#23
CONTROLLER_ENV_UID=1001 # user id of SSH user (`id -u` when using `make`)
CONTROLLER_ENV_GID=1001 # group id of SSH user (`id -g` when using `make`)
CONTROLLER_ENV_UMASK=0002 # SSH user specific permission mask
Expand Down
4 changes: 2 additions & 2 deletions docker-compose.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ services:

ocrd-monitor:
extends:
file: _modules/ocrd_manager/ocrd_monitor/docker-compose.yml
file: _modules/ocrd_monitor/docker-compose.yml
service: ocrd-monitor

ocrd-logview:
extends:
file: _modules/ocrd_manager/ocrd_monitor/docker-compose.yml
file: _modules/ocrd_monitor/docker-compose.yml
service: ocrd-logview

# OCR-D Controller
Expand Down

0 comments on commit 02162ad

Please sign in to comment.