-
-
Notifications
You must be signed in to change notification settings - Fork 314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Global image for external #5553
Changes from all commits
37d0c30
0ae0514
e415d58
5047445
3a1b749
9f3f031
520656f
5f88915
b028d4a
0a3f3db
b1e20d1
a3638b3
b47ef1e
26cd87f
cecf323
82b1c78
e36ff60
172a1f5
0839a27
673b223
849d2bd
b7f25af
14dd897
d8acc30
a1db200
e4b11c4
cb277a7
3f2e90c
a1c93fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# those packages are included in all tests and all distributions | ||
generic_packages="" | ||
# those packages are included in all tests and all ubuntu | ||
ubuntu_packages="" | ||
# those packages are included in all tests and all Red Hat distributions | ||
(fedora|ubi|rhel|centos)_packages="" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
github_url="https://github.com/apache/derby.git" | ||
localPropertyFile="local.properties" | ||
tag_version="trunk" | ||
ubuntu_packages="ant-optional git wget tar" | ||
ubuntu_packages="ant-optional" | ||
generic_packages="git wget tar" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,34 +84,50 @@ print_image_args() { | |
local build=$7 | ||
local base_docker_registry_dir="$8" | ||
|
||
image_name="docker.io/library/eclipse-temurin" | ||
tag="" | ||
if [[ "${package}" == "jre" ]]; then | ||
tag="${version}-jre" | ||
else | ||
tag="${version}-jdk" | ||
fi | ||
image_name="$(getTemurinImageName)" | ||
tag="$(getTemurinImageTag "${version}" "${package}")" | ||
if [[ "${vm}" == "openj9" ]]; then | ||
if [[ "${os}" == "ubuntu" ]]; then | ||
image_name="docker.io/ibm-semeru-runtimes" | ||
tag=open-${tag} | ||
image_name="$(getOpenJ9ImageName)" | ||
tag="$(getOpenJ9ImageTag "${version}" "${package}")" | ||
elif [[ "${os}" == *"ubi"* && "${test}" != *"criu"* ]]; then | ||
if isExternalImageEnabled ; then | ||
echo "openj9 ubi and custom EXTERNAL_AQA_IMAGE are not compatible" | ||
exit 1 | ||
fi | ||
image_name="registry.access.redhat.com/$base_docker_registry_dir" | ||
tag="latest" | ||
EXTERNAL_AQA_IMAGE="${image_name}:${tag}" | ||
else | ||
if isExternalImageEnabled ; then | ||
echo "openj9 ubi and custom EXTERNAL_AQA_IMAGE are not compatible" | ||
exit 1 | ||
fi | ||
# os is ubi, and test is criu | ||
# temporarily all ubi based testing use internal base image | ||
image_name="$DOCKER_REGISTRY_URL/$base_docker_registry_dir" | ||
tag="latest" | ||
EXTERNAL_AQA_IMAGE="${image_name}:${tag}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If EXTERNAL_AQA_IMAGE needs to be set? If yes, should move to line 114 for both temurin and openj9 ? Looks like this is not needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unhappy workaround for a corner case I was unable to manage. The only correct sollution to this is described in middle of initial description of issue #5555
This triple-if is changing to much to be served by the methods in provider.sh (without adding similar triple if there). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To construct the EXTERNAL_AQA_IMAGE artificially and globaly, is quite a good idea, and I was playing with it a lot. The reason why I have not included it, are non deterministic OSes. Ubuntu was hardcoded, to be there for jdk-xy-temurin/openj9, because there is no record of ubuntu in name. From my limited knowledge, this is the only case where it is actually missing. Eg redhat java images still have ubi in name, and so on. If there will be more usage of EXTERNAL_AQA_IMAGE then current baseurl, path, name, os variables, then I guess such a crossroad table would be necessary, but for now I decided to not implement it. |
||
fi | ||
fi | ||
image="${image_name}:${tag}" | ||
|
||
echo -e "ARG IMAGE=${image}" \ | ||
if isExternalImageEnabled ; then | ||
os=$(getImageOs) | ||
echo -e \ | ||
"ARG IMAGE=${image}" \ | ||
"\nARG OS=${os}" \ | ||
"\nARG IMAGE_VERSION=nightly" \ | ||
"\nARG TAG=${tag}" \ | ||
"\nFROM \$IMAGE\n" >> ${file} | ||
else | ||
echo -e \ | ||
"ARG IMAGE=${image}" \ | ||
"\nARG OS=${os}" \ | ||
"\nARG IMAGE_VERSION=nightly" \ | ||
"\nARG TAG=${tag}" \ | ||
"\nFROM \$IMAGE\n" >> ${file} | ||
fi | ||
} | ||
|
||
print_test_tag_arg() { | ||
|
@@ -133,8 +149,6 @@ print_result_comment_arg() { | |
# Select the ubuntu OS packages | ||
print_ubuntu_pkg() { | ||
local file=$1 | ||
local packages=$2 | ||
|
||
echo -e "RUN apt-get update \\" \ | ||
"\n\t&& apt-get install -qq -y --no-install-recommends software-properties-common \\" \ | ||
"\n\t&& apt-get install -qq -y --no-install-recommends gnupg \\" \ | ||
|
@@ -148,8 +162,6 @@ print_ubuntu_pkg() { | |
# Select the ubuntu OS packages | ||
print_ubi_pkg() { | ||
local file=$1 | ||
local packages=$2 | ||
|
||
echo -e "RUN dnf install -y ${packages} \\" \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable |
||
"\n\t&& dnf clean all " >> ${file} | ||
echo -e "\nENV LANG='en_US.UTF-8' LANGUAGE='en_US:en' LC_ALL='en_US.UTF-8'" >> ${file} | ||
|
@@ -212,6 +224,9 @@ print_ant_install() { | |
local file=$1 | ||
local ant_version=$2 | ||
local os=$3 | ||
if isExternalImageEnabled ; then | ||
os=$(getImageOs) | ||
karianna marked this conversation as resolved.
Show resolved
Hide resolved
|
||
fi | ||
|
||
echo -e "ARG ANT_VERSION=${ant_version}" \ | ||
"\nENV ANT_VERSION=\$ANT_VERSION" \ | ||
|
@@ -501,6 +516,9 @@ print_testInfo_env() { | |
local OS=$3 | ||
local version=$4 | ||
local vm=$5 | ||
if isExternalImageEnabled ; then | ||
OS=$(getImageOs) | ||
fi | ||
echo -e "ENV APPLICATION_NAME=${test}" \ | ||
"\nENV APPLICATION_TAG=${test_tag}" \ | ||
"\nENV OS_TAG=${OS}" \ | ||
|
@@ -593,7 +611,6 @@ generate_dockerfile() { | |
base_docker_registry_dir="$9" | ||
check_external_custom_test=$10 | ||
|
||
|
||
if [[ "$check_external_custom_test" == "1" ]]; then | ||
tag_version=${EXTERNAL_REPO_BRANCH} | ||
fi | ||
|
@@ -603,7 +620,7 @@ generate_dockerfile() { | |
else | ||
set_test_info ${test} ${check_external_custom_test} | ||
fi | ||
packages=$(echo ${os}_packages | sed 's/-/_/') | ||
|
||
jhome="/opt/java/openjdk" | ||
|
||
mkdir -p `dirname ${file}` 2>/dev/null | ||
|
@@ -612,9 +629,18 @@ generate_dockerfile() { | |
print_legal ${file}; | ||
print_adopt_test ${file} ${test}; | ||
print_image_args ${file} ${test} ${os} ${version} ${vm} ${package} ${build} "${base_docker_registry_dir}"; | ||
# print_image_args is setting up hackisch EXTERNAL_AQA_IMAGE for some corner cases | ||
osDeducted=$(getImageOs) | ||
print_result_comment_arg ${file}; | ||
print_test_tag_arg ${file} ${test} ${tag_version}; | ||
print_${os}_pkg ${file} "${!packages}"; | ||
if echo ${osDeducted} | grep -i -e ubuntu -e debian ; then | ||
print_ubuntu_pkg ${file} | ||
elif echo ${osDeducted} | grep -i -e ubi -e fedora -e rhel -e centos ; then | ||
print_ubi_pkg ${file} | ||
else | ||
echo "unknown os: ${osDeducted} (${os})" | ||
exit 1 | ||
fi | ||
|
||
if [[ ! -z ${ant_version} ]]; then | ||
print_ant_install ${file} ${ant_version} ${os}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
github_url="https://github.com/elastic/elasticsearch.git" | ||
tag_version="v8.1.2" | ||
test_results="testResults" | ||
ubuntu_packages="git wget unzip" | ||
generic_packages="git wget unzip" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,33 @@ set -e | |
|
||
source $(dirname "$0")/provider.sh | ||
|
||
if [ -z "${EXTRA_DOCKER_ARGS}" ] ; then | ||
echo \ | ||
" # ================= WARNING ================= WARNING ================= WARNING ================= # | ||
# EXTRA_DOCKER_ARGS are not set. You will be testing java which is already in container and not TEST_JDK_HOME | ||
# TEST_JDK_HOME is set to $TEST_JDK_HOME but will not be used. See test_base_functions.sh for order of search | ||
# You should set your's TEST_JDK_HOME to mount to /opt/java/openjdk, eg: # | ||
# export EXTRA_DOCKER_ARGS=\"-v \$TEST_JDK_HOME:/opt/java/openjdk\" # | ||
# ================= WARNING ================= WARNING ================= WARNING ================= #" | ||
else | ||
echo \ | ||
" # =================== Info =================== Info =================== Info =================== # | ||
# EXTRA_DOCKER_ARGS set as \"$EXTRA_DOCKER_ARGS\" # | ||
# =================== Info =================== Info =================== Info =================== #" | ||
if echo "${EXTRA_DOCKER_ARGS}" | grep -q "$TEST_JDK_HOME" ; then | ||
echo \ | ||
" # =================== Info =================== Info =================== Info =================== # | ||
# TEST_JDK_HOME of $TEST_JDK_HOME is used in EXTRA_DOCKER_ARGS # | ||
# =================== Info =================== Info =================== Info =================== #" | ||
else | ||
echo \ | ||
" # ================= WARNING ================= WARNING ================= WARNING ================= # | ||
# TEST_JDK_HOME of $TEST_JDK_HOME is NOT used in EXTRA_DOCKER_ARGS # | ||
# ================= WARNING ================= WARNING ================= WARNING ================= #" | ||
fi | ||
fi | ||
|
||
Comment on lines
+22
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the record, not a great fan of this output. My original review said to change the previous incarnation to look more professional. Not going to block the PR on it, but think that this is excessively noisy and seems like its better suited for a debugging session than a production run. |
||
|
||
tag=nightly | ||
docker_os=ubuntu | ||
build_type=full | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the point of this file? Seem it will never actually be used, as most external application tests vary so much, it does not have value. Drop it. It can be added in a later PR when and if it becomes needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. But before I do, please check: 5f88915#diff-3e2f7b162bce7417355d4fe0f18e6366b69e6abe5e73356f863274d3a4f9c04bR2 Those
generic_packages="git wget unzip tar curl"
are used in 99% of tests (see the rest of 5f88915 and search for -ubuntu_packages or -ubi_packages). I think such base really should be added. (note, this is not disagreeing with removal, just asking for brainstroming. I will remove it solemnly on this comment).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smlambert Even if practically unused, I would liek to keep this file. Is is extremly helpful when new distribution is tried. It simply allows you to declare its specific dependencies on one place, and then you can easily pinpoint it. And if this file remains unused for future, then they can be tuned up for individual suites alter.