fix(podman): Ensure compatibility with Podman - #1061
Open
theCalcaholic wants to merge 2 commits into
Open
Conversation
- lib/docker.ts: Fix volume syntax for createContainer - cypress.config.ts, docker.spec.ts, runExec.spec.ts, start-nextcloud-server.mjs: Cleanup apps volume after tests Signed-off-by: Tobias Knöppler <6317548+theCalcaholic@users.noreply.github.com>
- lib/docker.ts: Read docker socket path from env - cypress.config.ts, docker.spec.ts, runExec.spec.ts: Expose port explicitly (because podman doesn't do that by default) - cypress.config.ts: Expose runExec command from lib/docker.ts as cypress task Signed-off-by: Tobias Knöppler <6317548+theCalcaholic@users.noreply.github.com>
theCalcaholic
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a few issues preventing full compatibility with podman (as well as a few general issues I hit, see below)
Fixes #927
Fixes for podman compatibility
Socket selection
The socket is now read from the environment variable
DOCKER_SOCKET(if set). This allows to configure the tool to use/run/podman/podman.sockor/run/user/$(id -u)/podman/podman.sock(for rootfull/rootless podman).No
docker execInstead of
cy.exec('docker exec ...'), cypress uses now the same dockerode based implementation of runExec as playwright.Explicit port mappings
Podman doesn't expose ports by default, breaking the expectation that Nextcloud will be available at http://:80 if the
exposePortconfiguration option is not provided. This has been fixed for the tests, but still needs to be set by any projects that use the framework. One could be tempted to set a default of{exposePort: 80}, however I do not recommend this, because rootless Podman cannot expose that port by default (because only root can control ports < 1024, which are privileged ports on Linux) and it would cause confusing errors.General fixes
Invalid usage of the docker API
For some reason, the
createContainercall was ill configured - it was using theVolumeskey to set the volume, but this is wrong according to the Docker API Spec which expects volumes to be configured viaHostConfig.Mounts(using type "volume").Missing await on container teardown
When stopping Nextcloud, the async function was not awaited properly.
Volume cleanup
The automated tests never cleaned up the apps_writable volume. I added this, however, it might make sense to provide a way to override (or generate) the volume name to not accidentally remove a volume that is used by anything else.
One option would be to prefix it with the current directory (similar to the container name), however, that might be a breaking change for some obscure scenarios (where users manually manipulate the volume/container between runs).
Testing
I mainly tested the included test (both
npm run node:testandnpm run playwright) each with podman rootless, podman rootfull and docker.🤖 AI (if applicable)