chihsuan commented on code in PR #11829:
URL: https://github.com/apache/ozone/pull/11829#discussion_r4413117652
##########
hadoop-ozone/dist/src/main/compose/ozonesecure-ha/ranger-testlib.sh:
##########
@@ -41,32 +41,29 @@ setup_ranger_acceptance_env() {
fi
fi
- if [[ -z "${RANGER_VERSION:-}" ]]; then
- export RANGER_VERSION="${ranger.version}"
- fi
-
- : "${DOWNLOAD_DIR:=${TEMP_DIR:-/tmp}}"
+ # where tarballs will be extracted
+ : "${TEMP_DIR:=/tmp}"
+ # where tarballs will be downloaded
+ : "${DOWNLOAD_DIR:=${TEMP_DIR}}"
export COMPOSE_FILE=docker-compose.yaml:ranger.yaml:../common/ranger.yaml
export OM_SERVICE_ID="omservice"
export SCM=scm1.org
export SECURITY_ENABLED=true
- if [[ "${SKIP_APACHE_VERIFY_DOWNLOAD}" != "true" ]]; then
- curl -LO https://downloads.apache.org/ranger/KEYS
- gpg --import KEYS
+ if [[ "${SKIP_APACHE_VERIFY_DOWNLOAD:-}" != "true" ]]; then
+ import_keys https://downloads.apache.org/ranger/KEYS
fi
- download_and_verify_apache_release
"ranger/${RANGER_VERSION}/apache-ranger-${RANGER_VERSION}.tar.gz"
- tar -C "${DOWNLOAD_DIR}" -x -z -f
"${DOWNLOAD_DIR}/apache-ranger-${RANGER_VERSION}.tar.gz"
- export RANGER_SOURCE_DIR="${DOWNLOAD_DIR}/apache-ranger-${RANGER_VERSION}"
+ tar -C "${TEMP_DIR}" -x -z -f "$(download_ranger)"
Review Comment:
If I understand correctly, the `exit 1` inside `$(download_ranger)` only
stops the subshell. Could we capture the download result separately and stop
setup if it fails?
##########
hadoop-ozone/dist/src/main/compose/common/s3a-test.sh:
##########
@@ -17,16 +17,35 @@
# This script runs S3A contract tests against various bucket types on
# a Docker Compose-based Ozone cluster.
-# Requires HADOOP_AWS_DIR to point the directory containing hadoop-aws sources.
+#
+# If you have Hadoop sources locally, set HADOOP_AWS_DIR to the directory
containing `hadoop-aws`.
+# Otherwise it will try to download Hadoop ${HADOOP_VERSION}, defaults to
${hadoop.version}.
-if [[ -z ${HADOOP_AWS_DIR} ]] || [[ ! -e ${HADOOP_AWS_DIR} ]]; then
- echo "Skipping S3A tests due to missing HADOOP_AWS_DIR (directory with
hadoop-aws sources)" >&2
- exit
+if [[ -z "${HADOOP_AWS_DIR:-}" ]]; then
+ export HADOOP_AWS_DIR="${TEMP_DIR:-/tmp}"
Review Comment:
How about using `/tmp/hadoop-aws-${HADOOP_VERSION}` here?
##########
hadoop-ozone/dist/src/main/compose/testlib.sh:
##########
@@ -695,24 +704,67 @@ download_if_not_exists() {
local f="$2"
if [[ -e "${f}" ]]; then
- echo "${f} already downloaded"
+ echo "${f} already downloaded" >&2
else
- echo "Downloading ${f} from ${url}"
- curl --fail --location --output "${f}" --show-error --silent "${url}" ||
rm -fv "${f}"
+ download_with_retry "${url}" "${f}" >&2 || rm -fv "${f}"
fi
}
+import_keys() {
+ local url="$1"
+
+ keys=$(mktemp)
+ download_with_retry "$url" "$keys"
+ gpg --import "$keys"
+ rm -f "$keys"
+}
+
download_and_verify_apache_release() {
local remote_path="$1"
local f="$(basename "${remote_path}")"
local
base_url="${APACHE_MIRROR_URL:-https://www.apache.org/dyn/closer.lua?action=download&filename=}";
local
checksum_base_url="${APACHE_OFFICIAL_URL:-https://downloads.apache.org/}";
- local download_dir="${DOWNLOAD_DIR:-/tmp}"
- download_if_not_exists "${base_url}${remote_path}" "${download_dir}/${f}"
+ download_if_not_exists "${base_url}${remote_path}" "${DOWNLOAD_DIR}/${f}" >&2
if [[ "${SKIP_APACHE_VERIFY_DOWNLOAD}" != "true" ]]; then
- download_if_not_exists "${checksum_base_url}${remote_path}.asc"
"${download_dir}/${f}.asc"
- gpg --verify "${download_dir}/${f}.asc" "${download_dir}/${f}" || exit 1
+ download_if_not_exists "${checksum_base_url}${remote_path}.asc"
"${DOWNLOAD_DIR}/${f}.asc" >&2
+ gpg --verify "${DOWNLOAD_DIR}/${f}.asc" "${DOWNLOAD_DIR}/${f}" >&2 || exit
1
fi
+ echo "${DOWNLOAD_DIR}/${f}"
+}
+
+# retry at two levels:
+# 1. curl --retry 3
+# 2. re-run curl 3x
+download_with_retry() {
+ local url=$1
+ local output=$2
+
+ local rc=1
+
+ mkdir -p "$(dirname "${output}")"
+
+ for i in {1..3}; do
+ echo "Downloading ${output} from ${url}, attempt ${i}"
+ if curl --continue-at - --fail --location -o "${output}" --retry 3
--show-error --silent "${url}"; then
Review Comment:
nit: Perhaps we could also add `--connect-timeout` here?
##########
.github/workflows/populate-cache.yml:
##########
@@ -59,30 +62,77 @@ jobs:
key: maven-repo-${{ hashFiles('**/pom.xml') }}
- name: Setup Java
- if: steps.restore-cache.outputs.cache-hit != 'true'
uses: actions/setup-java@v6
with:
distribution: 'temurin'
java-version: ${{ env.TEST_JAVA_VERSION }}
- - name: Get NodeJS version
- id: nodejs-version
- if: steps.restore-cache.outputs.cache-hit != 'true'
- run: echo "nodejs-version=$(mvn help:evaluate
-Dexpression=nodejs.version -q -DforceStdout -Dscan=false)" >> $GITHUB_OUTPUT
+ - name: Get versions
+ id: versions
+ run: |
+ NODEJS_VERSION=$(mvn help:evaluate -N -Dexpression=nodejs.version -q
-DforceStdout -Dscan=false)
+ echo "NODEJS_VERSION=$NODEJS_VERSION" >> $GITHUB_ENV
+ HADOOP_VERSION=$(mvn help:evaluate -N -Dexpression=hadoop.version -q
-DforceStdout -Dscan=false)
+ echo "HADOOP_VERSION=$HADOOP_VERSION" >> $GITHUB_ENV
+ RANGER_VERSION=$(mvn help:evaluate -N -Dexpression=ranger.version -q
-DforceStdout -Dscan=false)
+ echo "RANGER_VERSION=$RANGER_VERSION" >> $GITHUB_ENV
+
+ - name: Restore Hadoop
+ id: restore-hadoop
+ uses: apache/infrastructure-actions/stash/restore@restore/v1
+ with:
+ clean: true
+ path: ~/.cache/ozone
+ key: hadoop-${{ env.HADOOP_VERSION }}
+
+ - name: Get Hadoop
+ if: steps.restore-hadoop.outputs.stash-hit != 'true'
+ run: |
+ source hadoop-ozone/dist/src/main/compose/testlib.sh
+ import_keys https://downloads.apache.org/hadoop/common/KEYS
+ download_hadoop
+
+ - name: Stash Hadoop
+ if: steps.restore-hadoop.outputs.stash-hit != 'true'
+ uses: apache/infrastructure-actions/stash/save@save/v1
+ with:
+ path: ~/.cache/ozone
+ key: hadoop-${{ env.HADOOP_VERSION }}
+
+ - name: Restore Ranger
+ id: restore-ranger
+ uses: apache/infrastructure-actions/stash/restore@restore/v1
+ with:
+ clean: true
+ path: ~/.cache/ozone
+ key: ranger-${{ env.RANGER_VERSION }}
+
+ - name: Get Ranger
+ if: steps.restore-ranger.outputs.stash-hit != 'true'
+ run: |
+ source hadoop-ozone/dist/src/main/compose/testlib.sh
+ import_keys https://downloads.apache.org/ranger/KEYS
Review Comment:
Just curious, is `import_keys` needed here? I might be missing something,
but CI skips signature checks by default, so I think these keys may not be used.
##########
hadoop-ozone/dist/src/main/compose/testlib.sh:
##########
@@ -695,24 +704,67 @@ download_if_not_exists() {
local f="$2"
if [[ -e "${f}" ]]; then
- echo "${f} already downloaded"
+ echo "${f} already downloaded" >&2
else
- echo "Downloading ${f} from ${url}"
- curl --fail --location --output "${f}" --show-error --silent "${url}" ||
rm -fv "${f}"
+ download_with_retry "${url}" "${f}" >&2 || rm -fv "${f}"
Review Comment:
If the download fails, this still returns success, because `rm` succeeds.
Could we return an error when the download fails?
--
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]
