fix(podman): Ensure compatibility with Podman - #1061
Merged
Conversation
theCalcaholic
force-pushed
the
main
branch
2 times, most recently
from
July 25, 2026 00:48
bd55092 to
ee8f3a5
Compare
theCalcaholic
changed the title
susnux
reviewed
Jul 29, 2026
Contributor
Author
|
@susnux I applied the requested change |
- 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>
Contributor
|
@theCalcaholic one remaining CI issue with linter. |
theCalcaholic
force-pushed
the
main
branch
2 times, most recently
from
July 30, 2026 09:32
ccf9a4b to
c6ca89f
Compare
Contributor
Author
|
@susnux Should be fixed now (apart from one remaining linting issue which is unrelated to my changes). |
susnux
reviewed
Jul 30, 2026
susnux
reviewed
Jul 30, 2026
- 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>
susnux
approved these changes
Jul 30, 2026
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
2 participants
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)