liurenjie1024 commented on code in PR #489: URL: https://github.com/apache/iceberg-rust/pull/489#discussion_r1695336217
########## website/src/reference/podman.md: ########## @@ -0,0 +1,87 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# Using Podman instead of Docker + +The majority of iceberg-rust is agnostic as to which containerization provider is used. However, +for integration tests, "docker" and "docker-compose" are used to spinup containers for minio and various catalogs. +It is possible with the below instructions to run integrations tests with no source-code changes via "podman" and docker's official docker-compose plugin. + +1. Have podman v4 or newer. + ```console + $ podman --version + podman version 4.9.4-rhel + ``` + +2. Open file `/usr/bin/docker` 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 "$@" + ``` + +3. Install the [docker compose plugin](https://docs.docker.com/compose/install/linux). Check for successful installation. + ```console + $ docker compose version + Docker Compose version v2.28.1 + ``` + +4. Append the below to `~/.bashrc` or equivalent shell config: + ```bash + export DOCKER_HOST=unix:///run/podman/podman.sock + ``` + +5. Start the "rootful" podman socket. + ```shell + sudo systemctl start podman.socket + sudo systemctl status podman.socket + ``` + +6. Check that the following symlink exists. + ```console + $ ls -al /var/run/docker.sock + lrwxrwxrwx 1 root root 27 Jul 24 12:18 /var/run/docker.sock -> /var/run/podman/podman.sock + ``` + If the symlink does not exist, create it. + ```shell + sudo ln -s /var/run/podman/podman.sock /var/run/docker.sock + ``` + +7. Check that the docker socket is working. + ```shell + sudo curl -H "Content-Type: application/json" --unix-socket /var/run/docker.sock http://localhost/_ping + ``` + +8. Try some integration tests! + ```shell + cargo test -p iceberg --test file_io_s3_test + ``` + +### References Review Comment: Shoule this be `#`? ########## crates/test_utils/src/cmd.rs: ########## @@ -28,6 +28,17 @@ pub fn run_command(mut cmd: Command, desc: impl ToString) { } } +pub fn get_cmd_output_dont_panic(mut cmd: Command, desc: impl ToString) -> Result<String, String> { + let desc = desc.to_string(); + log::info!("Starting to {}, command: {:?}", &desc, cmd); + let output = cmd.output().unwrap(); Review Comment: It still panics here. ########## crates/catalog/glue/tests/glue_catalog_test.rs: ########## @@ -83,15 +81,15 @@ async fn get_catalog() -> GlueCatalog { (AWS_REGION_NAME.to_string(), "us-east-1".to_string()), ( S3_ENDPOINT.to_string(), - format!("http://{}:{}";, minio_ip, MINIO_PORT), + format!("http://{}";, minio_socket_addr.to_string()), Review Comment: ```suggestion format!("http://{}";, minio_socket_addr), ``` ########## website/src/reference/podman.md: ########## @@ -0,0 +1,87 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# Using Podman instead of Docker + +The majority of iceberg-rust is agnostic as to which containerization provider is used. However, +for integration tests, "docker" and "docker-compose" are used to spinup containers for minio and various catalogs. +It is possible with the below instructions to run integrations tests with no source-code changes via "podman" and docker's official docker-compose plugin. + +1. Have podman v4 or newer. + ```console + $ podman --version + podman version 4.9.4-rhel + ``` + +2. Open file `/usr/bin/docker` 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 "$@" + ``` + +3. Install the [docker compose plugin](https://docs.docker.com/compose/install/linux). Check for successful installation. + ```console + $ docker compose version + Docker Compose version v2.28.1 + ``` + +4. Append the below to `~/.bashrc` or equivalent shell config: + ```bash + export DOCKER_HOST=unix:///run/podman/podman.sock + ``` + +5. Start the "rootful" podman socket. + ```shell + sudo systemctl start podman.socket + sudo systemctl status podman.socket + ``` + +6. Check that the following symlink exists. + ```console + $ ls -al /var/run/docker.sock + lrwxrwxrwx 1 root root 27 Jul 24 12:18 /var/run/docker.sock -> /var/run/podman/podman.sock + ``` + If the symlink does not exist, create it. + ```shell + sudo ln -s /var/run/podman/podman.sock /var/run/docker.sock + ``` + +7. Check that the docker socket is working. + ```shell + sudo curl -H "Content-Type: application/json" --unix-socket /var/run/docker.sock http://localhost/_ping + ``` + +8. Try some integration tests! + ```shell + cargo test -p iceberg --test file_io_s3_test + ``` + +### References + +* <https://docs.docker.com/compose/install/linux> +* <https://www.redhat.com/sysadmin/podman-docker-compose> + +### Note on rootless containers + +As of podman v4, ["To be succinct and simple, when running rootless containers, the container itself does not have an IP address"](https://www.redhat.com/sysadmin/container-ip-address-podman) This causes issues with iceberg-rust's integration tests, which rely upon ip-addressable containers via docker-compose. As a result, podman "rootful" containers are required throughout to ensure containers have IP addresses. Perhaps as a future work or with updates to default podman networking, the need for "rootful" podman containers can be eliminated. Review Comment: I'm not familiar with podman, but this statement shows that "rootful" podman is essential, so maybe we should put the "rootful" requirement in a better place? ########## crates/test_utils/src/docker.rs: ########## @@ -40,15 +41,28 @@ impl DockerCompose { self.project_name.as_str() } + // docker/podman do not consistently place OSArch in the same json path across OS and versions + // below function tries two common places then gives up fn get_os_arch() -> String { let mut cmd = Command::new("docker"); cmd.arg("info") .arg("--format") .arg("{{.OSType}}/{{.Architecture}}"); - get_cmd_output(cmd, "Get os arch".to_string()) - .trim() - .to_string() + let result = get_cmd_output_dont_panic(cmd, "Get os arch".to_string()); + match result { + Ok(value) => value.trim().to_string(), + Err(_err) => { + let mut alt_cmd = Command::new("docker"); Review Comment: Please add some comments here to explain what this fallback is for. ########## website/src/reference/podman.md: ########## @@ -0,0 +1,87 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# Using Podman instead of Docker + +The majority of iceberg-rust is agnostic as to which containerization provider is used. However, +for integration tests, "docker" and "docker-compose" are used to spinup containers for minio and various catalogs. Review Comment: Unnecessary newline? ########## crates/catalog/rest/testdata/rest_catalog/docker-compose.yaml: ########## @@ -15,6 +15,9 @@ # specific language governing permissions and limitations # under the License. +networks: Review Comment: Is there any reason we need to add this? We didn't add this before so that we can start multi docker compose cluster together. I still don't see the benefit of adding this. ########## crates/test_utils/src/cmd.rs: ########## @@ -28,6 +28,17 @@ pub fn run_command(mut cmd: Command, desc: impl ToString) { } } +pub fn get_cmd_output_dont_panic(mut cmd: Command, desc: impl ToString) -> Result<String, String> { Review Comment: ```suggestion pub fn get_cmd_output_result(mut cmd: Command, desc: impl ToString) -> Result<String, String> { ``` ########## crates/test_utils/src/cmd.rs: ########## @@ -28,6 +28,17 @@ pub fn run_command(mut cmd: Command, desc: impl ToString) { } } +pub fn get_cmd_output_dont_panic(mut cmd: Command, desc: impl ToString) -> Result<String, String> { + let desc = desc.to_string(); + log::info!("Starting to {}, command: {:?}", &desc, cmd); + let output = cmd.output().unwrap(); + if output.status.success() { + Ok(String::from_utf8(output.stdout).unwrap()) + } else { + Err(format!("{} failed: {:?}", desc, output.status)) + } +} + Review Comment: We should replace implementation of `get_cmd_output` with this method and just panic when error happens? ########## website/src/reference/podman.md: ########## @@ -0,0 +1,87 @@ +<!-- + ~ Licensed to the Apache Software Foundation (ASF) under one + ~ or more contributor license agreements. See the NOTICE file + ~ distributed with this work for additional information + ~ regarding copyright ownership. The ASF licenses this file + ~ to you under the Apache License, Version 2.0 (the + ~ "License"); you may not use this file except in compliance + ~ with the License. You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, + ~ software distributed under the License is distributed on an + ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + ~ KIND, either express or implied. See the License for the + ~ specific language governing permissions and limitations + ~ under the License. +--> + +# Using Podman instead of Docker + +The majority of iceberg-rust is agnostic as to which containerization provider is used. However, +for integration tests, "docker" and "docker-compose" are used to spinup containers for minio and various catalogs. +It is possible with the below instructions to run integrations tests with no source-code changes via "podman" and docker's official docker-compose plugin. Review Comment: Ditto. ########## crates/catalog/glue/tests/glue_catalog_test.rs: ########## @@ -83,15 +81,15 @@ async fn get_catalog() -> GlueCatalog { (AWS_REGION_NAME.to_string(), "us-east-1".to_string()), ( S3_ENDPOINT.to_string(), - format!("http://{}:{}";, minio_ip, MINIO_PORT), + format!("http://{}";, minio_socket_addr.to_string()), ), (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), (S3_REGION.to_string(), "us-east-1".to_string()), ]); let config = GlueCatalogConfig::builder() - .uri(format!("http://{}:{}";, glue_catalog_ip, GLUE_CATALOG_PORT)) + .uri(format!("http://{}";, glue_socket_addr.to_string())) Review Comment: ```suggestion .uri(format!("http://{}";, glue_socket_addr)) ``` ########## crates/catalog/hms/testdata/hms_catalog/Dockerfile: ########## @@ -15,8 +15,6 @@ FROM --platform=$BUILDPLATFORM openjdk:8-jre-slim AS build -ARG BUILDPLATFORM Review Comment: Please don't remove this, it's used for docker compose to select correct image for architecture. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
