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

tools: specify project name in docker-compose files #4396

Merged
merged 10 commits into from
Nov 3, 2023

Conversation

matzf
Copy link
Contributor

@matzf matzf commented Sep 22, 2023

Simplify the usage of the various docker-compose configurations by including the project name in the configuration file. This has been supported for a while now in docker-compose v2. This allows to drop the -p/--project-name from all docker-compose incantations.

Also, streamline the docker-compose files generated by the topogen scripts; remove the explicit container_name configurations and drop all the explicit scion_ prefixes -- managing these prefixes is docker-compose's job.
Shut up the warnings on "SCION_EXPERIMENTAL_... variable is not set. Defaulting to a blank string." by using the variable expansion syntax to explicitly default to a blank string.

Also, drop the docker compose --compatibility flag. The compatibility flag affects what word separator is used in the
container name and it has long been deprecated. We don't usually rely on specific container names, so this should be
fine. In some exception cases where expecting specific container names seems more practical (containers for bazel-remote-cache and go-module-proxy) due to special casing in the CI scripts, container_name is set explicitly.


This change is Reviewable

@matzf matzf force-pushed the topology-docker-project-name branch 5 times, most recently from 9f8c967 to 6fb92e3 Compare September 28, 2023 08:53
@matzf matzf marked this pull request as ready for review September 28, 2023 09:18
@matzf matzf force-pushed the topology-docker-project-name branch 3 times, most recently from e26deb3 to 84a1628 Compare October 30, 2023 21:06
@matzf matzf force-pushed the topology-docker-project-name branch from 84a1628 to d0faa46 Compare October 31, 2023 12:50
matzf added 2 commits October 31, 2023 15:12
The compatibility flag affects what word separator is used in the
container name.

> Compose generates container names based on the project name, service name, and scale/replica count.
> In Compose V1, an underscore (_) was used as the word separator. In Compose V2, a hyphen (-) is used as the word separator.

We don't usually rely on specific container names, so this should be
fine.
In some exception cases where this seems more practical (containers for
bazel-remote-cache and go-module-proxy) due to special casing in the CI
scripts, container_name is set explicitly.
Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 22 of 22 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @matzf)

@matzf matzf merged commit cab6ce3 into scionproto:master Nov 3, 2023
1 check passed
@matzf matzf deleted the topology-docker-project-name branch November 3, 2023 08:36
@matzf
Copy link
Contributor Author

matzf commented Nov 3, 2023

Just as a heads-up for developers using the docker compose based local topology setup: a docker compose local topology setup started before this change may not be compatible with the scripts or compose file created after this change. To avoid confusing docker compose make sure to stop your local topology before changing to this commit. Otherwise, you might have to manually remove dangling containers etc. (e.g. the nuclear option: stop all running containers, docker stop $(docker ps -a -q), then clean up everything docker system prune -- consult your friendly docker manuals for minimally invasive options).

juagargi pushed a commit to netsec-ethz/scion that referenced this pull request Mar 8, 2024
Simplify the usage of the various docker-compose configurations by including the project name in the configuration file. This has been supported for a while now in docker-compose v2. This allows to drop the `-p`/`--project-name` from all `docker-compose` incantations.

Also, streamline the docker-compose files generated by the topogen scripts; remove the explicit `container_name` configurations and drop all the explicit `scion_` prefixes -- managing these prefixes is docker-compose's job. 
Shut up the warnings on "SCION_EXPERIMENTAL_... variable is not set. Defaulting to a blank string." by using the variable expansion syntax to explicitly default to a blank string.

Also, drop the `docker compose` `--compatibility` flag. The compatibility flag affects what word separator is used in the
container name and it has long been deprecated. We don't usually rely on specific container names, so this should be
fine. In some exception cases where expecting specific container names seems more practical (containers for bazel-remote-cache and go-module-proxy) due to special casing in the CI scripts, container_name is set explicitly.
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.

2 participants