Swarm mode modifications #73
Conversation
| @@ -51,6 +51,27 @@ services: | |||
| - INCLUDE_LOGS=true | |||
| ``` | |||
|
|
|||
| #### Running in Docker Swarm | |||
| To run the backup service in Docker Swarm, you should configure it as a global service. This ensures that the backup process runs on each node in the Swarm and backs up all matching containers on the local node. | |||
| ```yml | |||
There was a problem hiding this comment.
Nit: A lot of this YAML is duplicated from the above example. Perhaps it would be better to link out to the Docker documentation for making a service "global"?
|
|
||
| duration = (datetime.now() - now).total_seconds() | ||
| print(f"Backup of {len(backed_up_services)} services complete in {duration:.2f} seconds.") | ||
| else: |
There was a problem hiding this comment.
Question: What's the reason for the below duplication? Could the extra logic be handled in the existing single loop?
| """ | ||
| Create a backup file name with a timestamp prefix and the container name. | ||
| """ | ||
| timestamp = datetime.now().strftime('%Y%m%d_%H%M%S') |
There was a problem hiding this comment.
Issue: This is a very big change. Backups are intentionally not timestamped. Please remove this.
|
|
||
| print(f"Found {len(containers)} containers.") | ||
| def get_local_node_tasks() -> list: | ||
| docker_client = docker.from_env() |
There was a problem hiding this comment.
Issue: This code creates a lot of extra connections to the docker API - could a client be passed around instead?
|
|
||
| def get_local_node_id() -> str: |
There was a problem hiding this comment.
Question: Does the new logic need to happen only in swarm mode? Could checking for the container config image name be done all the time instead? It feels like it's a purely additive change
| image_name = tag.split(":")[0].split("@")[0] | ||
| image_name = image_name.split("/")[-1] |
There was a problem hiding this comment.
Question: Why are you removing resolve_repository_name here? Was it not working correctly?
#72