Updates Podman Instructions, Docker-Compose, & Spark Script #1553
Conversation
docs/contributing/podman.md - Podman instructions have been updated for clarity and to emphasis its usage in a Linux environment. crates/integration_tests/testdata/docker-compose.yaml - Updated to remove deprecated Link feature, which breaks when used with Podman. https://stackoverflow.com/questions/35832095/difference-between-links-and-depends-on-in-docker-compose-yml crates/integration_tests/testdata/spark/provision.py - Added Create Namespace `default` to fix breaking tests.
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
LGTM! Thanks for adding this to the docs!
| .getOrCreate() | ||
| ) | ||
|
|
||
| spark.sql(f"""CREATE NAMESPACE IF NOT EXISTS rest.default""") |
There was a problem hiding this comment.
👍 this is due to the new default behavior change for iceberg-rest-fixture
apache/iceberg#13599
There was a problem hiding this comment.
Thank you for the added context!
| links: | ||
| - rest:rest | ||
| - minio:minio |
There was a problem hiding this comment.
looks like links and depends_on are doing the same thing, so we can remove this
| 2. Create a docker wrapper script: | ||
|
|
||
| * Create a fresh `/usr/bin/docker` file and add the below contents: | ||
| ```bash | ||
| #!/bin/sh | ||
| [ -e /etc/containers/nodocker ] || \ | ||
| echo "Emulate Docker CLI using podman. Create /etc/containers/nodocker to quiet msg." >&2 | ||
| exec sudo /usr/bin/podman "$@" | ||
| ``` | ||
|
|
||
| * Set new `/usr/bin/docker` file to executable. | ||
| ```shell | ||
| sudo chmod +x /usr/bin/docker | ||
| ``` |
There was a problem hiding this comment.
nit: is the goal for the docker wrapper script to alias podman as docker?
can we add an alias instead? https://docs.podman.io/en/latest/#:~:text=Most%20users%20can,the%20running%20containers.
There was a problem hiding this comment.
Adding an alias was a step I forgot to document, but it wouldn't be a replacement for this script. Without the above script I was seeing issues where podman wasn't looking in the correct folder for the docker-compose.yaml file to pass to docker-compose plugin.
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @Kurtiscwright for this pr. Generally I'm fine with the code changes, but I have concerns to claim that podman is perferred on linux, since currently most of our cis are running using docker.
CONTRIBUTING.md
Outdated
| For MacOS users, you can install [OrbStack as a docker alternative](docs/contributing/orbstack.md). | ||
|
|
||
| For podman, refer to [Using Podman instead of Docker](docs/contributing/podman.md) | ||
| For Linux users Podman is recommended, refer to [Using Podman instead of Docker](docs/contributing/podman.md) |
There was a problem hiding this comment.
I'm hesitating to add this change. I think on linux both podman and docker are equal important, and in fact our ci is using docker on linux.
There was a problem hiding this comment.
I can remove this line and the same below and will try to work making docker work on my machine then write up steps for that use case as well.
Out of curiosity (can also move this discussion to async slack), how does the CI work for Iceberg spceifically Iceberg-Rust? I noticed while making these changes that I had to blow away cached files a number of times and I worry that CI might be using cached values in a few places.
There was a problem hiding this comment.
I can remove this line and the same below and will try to work making docker work on my machine then write up steps for that use case as well.
Thanks.
Out of curiosity (can also move this discussion to async slack), how does the CI work for Iceberg spceifically Iceberg-Rust? I noticed while making these changes that I had to blow away cached files a number of times and I worry that CI might be using cached values in a few places.
Sorry, I don't get your point. You are supposed to be able to run make test to run all tests locally, as long as you have docker and rust installed.
There was a problem hiding this comment.
Latest commit has removed the recommendation wording.
docs/contributing/podman.md
Outdated
| # Using Podman instead of Docker | ||
|
|
||
| Iceberg-rust does not require containerization, except for integration tests, where "docker" and "docker-compose" are used to start containers for minio and various catalogs. Below instructions setup "rootful podman" and docker's official docker-compose plugin to run integration tests as an alternative to docker or Orbstack. | ||
| Iceberg-rust does not require containerization, except for integration tests, where "docker" and "docker-compose" are used to start containers for minio and various catalogs. Below instructions setup "rootful podman" and docker's official docker-compose plugin to run integration tests as an alternative to docker or Orbstack. It is recommended to setup Podman if you are developing in a Linux or WSL environment. |
There was a problem hiding this comment.
I don't think we should add this statement.
There was a problem hiding this comment.
Same comment as above, I can send out an update to this PR removing this language.
docs/contributing/podman.md - Podman instructions have been updated for clarity and to emphasis its usage in a Linux environment. crates/integration_tests/testdata/docker-compose.yaml - Updated to remove deprecated Link feature, which breaks when used with Podman. https://stackoverflow.com/questions/35832095/difference-between-links-and-depends-on-in-docker-compose-yml crates/integration_tests/testdata/spark/provision.py - Added Create Namespace `default` to fix breaking tests.
liurenjie1024
left a comment
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @Kurtiscwright for this pr!
) ## Which issue does this PR close? - No current PR, was found while testing 0.6.0 RC ## What changes are included in this PR? `CONTRIBUTING.md` `docs/contributing/podman.md` - Podman instructions have been updated for clarity and to emphasis its usage in a Linux environment. `crates/integration_tests/testdata/docker-compose.yaml` - Updated to remove deprecated Link feature, which breaks when used with Podman. https://stackoverflow.com/questions/35832095/difference-between-links-and-depends-on-in-docker-compose-yml `crates/integration_tests/testdata/spark/provision.py` - Added Create Namespace `default` to fix breaking tests. ## Are these changes tested? - Tested by running `make build && make test` against the following platforms: * Arch Linux x86_64 with Podman && Docker-Compose * MacOS M1 arm64 with Orbstack
4 participants
Which issue does this PR close?
What changes are included in this PR?
CONTRIBUTING.mddocs/contributing/podman.mdcrates/integration_tests/testdata/docker-compose.yamlcrates/integration_tests/testdata/spark/provision.pydefaultto fix breaking tests.Are these changes tested?
make build && make testagainst the following platforms: