Skip to content

Commit b38d7ab

Browse files
dhusebyDave Grantham
dhuseby
and
Dave Grantham
authored
fix docker image naming and selection (#775)
Signed-off-by: Dave Grantham <dwg@linuxprogrammer.org> Co-authored-by: Dave Grantham <dwg@linuxprogrammer.org>
1 parent f8963df commit b38d7ab

File tree

6 files changed

+58
-84
lines changed

6 files changed

+58
-84
lines changed

‎hole-punch/lib/generate-tests.sh‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ trap 'echo "ERROR in generate-tests.sh at line $LINENO: Command exited with stat
1313
source "${SCRIPT_LIB_DIR}/lib-filter-engine.sh"
1414
source "${SCRIPT_LIB_DIR}/lib-generate-tests.sh"
1515
source "${SCRIPT_LIB_DIR}/lib-image-building.sh"
16+
source "${SCRIPT_LIB_DIR}/lib-image-naming.sh"
1617
source "${SCRIPT_LIB_DIR}/lib-output-formatting.sh"
1718
source "${SCRIPT_LIB_DIR}/lib-test-caching.sh"
1819
source "${SCRIPT_LIB_DIR}/lib-test-filtering.sh"
@@ -575,12 +576,12 @@ generate_tests_worker() {
575576
local dialer_router_commit="${router_image_commit[${dialer_router_id}]:-}"
576577
local listener_router_commit="${router_image_commit[${listener_router_id}]:-}"
577578

578-
# Get the image names
579-
local dialer_image_name=$(get_image_name "implementations" "${dialer_id}")
580-
local listener_image_name=$(get_image_name "implementations" "${listener_id}")
581-
local relay_image_name=$(get_image_name "relays" "${relay_id}")
582-
local dialer_router_image_name=$(get_image_name "routers" "${dialer_router_id}")
583-
local listener_router_image_name=$(get_image_name "routers" "${listener_router_id}")
579+
# Get the image names
580+
local dialer_image_name=$(get_image_name "${TEST_TYPE}" "implementations" "${dialer_id}")
581+
local listener_image_name=$(get_image_name "${TEST_TYPE}" "implementations" "${listener_id}")
582+
local relay_image_name=$(get_image_name "${TEST_TYPE}" "relays" "${relay_id}")
583+
local dialer_router_image_name=$(get_image_name "${TEST_TYPE}" "routers" "${dialer_router_id}")
584+
local listener_router_image_name=$(get_image_name "${TEST_TYPE}" "routers" "${listener_router_id}")
584585

585586
# Process each common transport
586587
for transport in $common_transports; do

‎lib/lib-image-building.sh‎

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,6 @@ docker_image_exists() {
1010
docker image inspect "${image_name}" >/dev/null 2>&1
1111
}
1212

13-
# Calculate the name of a Docker image from the test type, section, and id
14-
get_image_name() {
15-
local test_type=${TEST_TYPE:-}
16-
local section="$1"
17-
local id="$2"
18-
19-
echo "${test_type}-${section}-${id}"
20-
}
21-
2213
# Helper function to build images from a YAML section (implementations or baselines)
2314
build_images_from_section() {
2415
local section="$1" # "implementations", "baselines", "routers", etc
@@ -37,7 +28,7 @@ build_images_from_section() {
3728
IFS='|' read -ra FILTER_PATTERNS <<< "${filter}"
3829
for pattern in "${FILTER_PATTERNS[@]}"; do
3930
case "${impl_id}" in
40-
*"${pattern}"*)
31+
"${pattern}")
4132
match_found=true
4233
break
4334
;;
@@ -48,7 +39,7 @@ build_images_from_section() {
4839
fi
4940
fi
5041

51-
local image_name=$(get_image_name "${section}" "${impl_id}")
42+
local image_name=$(get_image_name "${TEST_TYPE}" "${section}" "${impl_id}")
5243

5344
# Check if image already exists (for local builds only)
5445
if [ "${force_image_rebuild}" != "true" ] && docker_image_exists "${image_name}"; then
@@ -57,12 +48,10 @@ build_images_from_section() {
5748
fi
5849

5950
# Create YAML file for this build
60-
local yaml_file="${CACHE_DIR}/build-yamls/docker-build-perf-${impl_id}.yaml"
51+
local yaml_file="${CACHE_DIR}/build-yamls/docker-build-${TEST_TYPE}-${impl_id}.yaml"
6152

6253
cat > "${yaml_file}" <<EOF
6354
imageName: ${image_name}
64-
imageType: peer
65-
imagePrefix: "${TEST_TYPE}"
6655
sourceType: ${source_type}
6756
cacheDir: ${CACHE_DIR}
6857
forceRebuild: ${force_image_rebuild}
@@ -116,11 +105,12 @@ EOF
116105
local build_context=$(dirname "${dockerfile}")
117106
local patch_path=$(yq eval ".${section}[$i].source.patchPath // \"\"" "${IMAGES_YAML}")
118107
local patch_file=$(yq eval ".${section}[$i].source.patchFile // \"\"" "${IMAGES_YAML}")
108+
local base_image_name=$(get_image_name "${TEST_TYPE}" "${section}" "${base_image}")
119109

120110
cat >> "${yaml_file}" <<EOF
121111
122112
browser:
123-
baseImage: ${base_image}
113+
baseImage: ${base_image_name}
124114
browser: ${browser}
125115
dockerfile: ${dockerfile}
126116
buildContext: ${build_context}
@@ -516,25 +506,22 @@ build_browser_image() {
516506
local browser=$(yq eval '.browser.browser' "${yaml_file}")
517507
local dockerfile=$(yq eval '.browser.dockerfile' "${yaml_file}")
518508
local build_context=$(yq eval '.browser.buildContext' "${yaml_file}")
519-
local image_prefix=$(yq eval '.imagePrefix' "${yaml_file}")
520509
local patch_path=$(yq eval '.browser.patchPath // ""' "${yaml_file}")
521510
local patch_file=$(yq eval '.browser.patchFile // ""' "${yaml_file}")
522511

523-
local base_image_name="${image_prefix}-${base_image}"
524-
525-
print_message "Base: ${base_image} (${base_image_name})"
512+
print_message "Base: ${base_image}"
526513
print_message "Browser: ${browser}"
527514

528515
# Ensure base image exists
529-
if ! docker image inspect "${base_image_name}" &>/dev/null; then
530-
print_error "Base image not found: ${base_image_name}"
516+
if ! docker image inspect "${base_image}" &>/dev/null; then
517+
print_error "Base image not found: ${base_image}"
531518
print_message "Please build ${base_image} first"
532519
return 1
533520
fi
534521

535522
# Tag base image for browser build
536-
print_message "Tagging base image..."
537-
docker tag "${base_image_name}" "node-${base_image}"
523+
#print_message "Tagging base image..."
524+
#docker tag "${base_image_name}" "node-${base_image}"
538525

539526
# If patch specified, create temporary copy of build context
540527
local actual_build_context="${build_context}"
@@ -565,7 +552,7 @@ build_browser_image() {
565552
# Build browser image
566553
print_message "Building browser Docker image..."
567554
# Run docker directly (no eval/pipe) for clean output to preserve aesthetic
568-
if ! docker build -f "${actual_dockerfile}" --build-arg BASE_IMAGE="node-${base_image}" --build-arg BROWSER="${browser}" -t "${image_name}" "${actual_build_context}"; then
555+
if ! docker build -f "${actual_dockerfile}" --build-arg BASE_IMAGE="${base_image}" --build-arg BROWSER="${browser}" -t "${image_name}" "${actual_build_context}"; then
569556
print_error "Docker build failed"
570557
if [ "${cleanup_temp}" == "true" ]; then
571558
rm -rf "${actual_build_context}"

‎lib/lib-image-naming.sh‎

Lines changed: 27 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,46 @@
11
#!/bin/bash
22
# Common library for Docker image naming conventions
3-
# Used by both build-images.sh and create-snapshot.sh
3+
# Used by both image building and snapshot creation
44

5-
# Get the Docker image name for an implementation
6-
# Usage: get_impl_image_name <impl_id> <test_type>
7-
# test_type: "transport", "hole-punch", or "perf"
8-
get_impl_image_name() {
9-
local impl_id="${1}"
10-
local test_type="${2}"
5+
# Get the Docker image name for any entity type
6+
# Usage: get_image_name <test_type> <section> <id>
7+
#
8+
# Parameters:
9+
# test_type: "transport", "hole-punch", or "perf"
10+
# section: "implementations", "baselines", "relays", "routers"
11+
# id: entity identifier (e.g., "rust-v0.56")
12+
#
13+
# Returns: <test_type>-<section>-<id>
14+
#
15+
# Examples:
16+
# get_image_name "perf" "implementations" "rust-v0.56"
17+
# → "perf-implementations-rust-v0.56"
18+
# get_image_name "hole-punch" "relays" "go-relay"
19+
# → "hole-punch-relays-go-relay"
20+
get_image_name() {
21+
local test_type="${1}"
22+
local section="${2}"
23+
local id="${3}"
1124

25+
# Validate test_type
1226
case "${test_type}" in
13-
transport)
14-
echo "transport-interop-${impl_id}"
15-
;;
16-
hole-punch)
17-
echo "hole-punch-peer-${impl_id}"
18-
;;
19-
perf)
20-
echo "perf-${impl_id}"
27+
transport|hole-punch|perf)
2128
;;
2229
*)
2330
echo "Error: Unknown test type: ${test_type}" >&2
2431
return 1
2532
;;
2633
esac
27-
}
28-
29-
# Get the Docker image name for a relay
30-
# Usage: get_relay_image_name <relay_id> <test_type>
31-
get_relay_image_name() {
32-
local relay_id="${1}"
33-
local test_type="${2}"
3434

35-
case "${test_type}" in
36-
hole-punch)
37-
echo "hole-punch-relay-${relay_id}"
35+
# Validate section
36+
case "${section}" in
37+
implementations|baselines|relays|routers)
3838
;;
3939
*)
40-
echo "Error: Relays not supported for test type: ${test_type}" >&2
40+
echo "Error: Unknown section: ${section}" >&2
4141
return 1
4242
;;
4343
esac
44-
}
45-
46-
# Get the Docker image name for a router
47-
# Usage: get_router_image_name <router_id> <test_type>
48-
get_router_image_name() {
49-
local router_id="${1}"
50-
local test_type="${2}"
5144

52-
case "${test_type}" in
53-
hole-punch)
54-
echo "hole-punch-router-${router_id}"
55-
;;
56-
*)
57-
echo "Error: Routers not supported for test type: ${test_type}" >&2
58-
return 1
59-
;;
60-
esac
45+
echo "${test_type}-${section}-${id}"
6146
}
62-

‎lib/lib-snapshot-images.sh‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ save_docker_images_for_tests() {
2222
# Get unique implementations (all test types use .dialer and .listener)
2323
while read -r impl_id; do
2424
if [ -n "$impl_id" ] && [ "$impl_id" != "null" ]; then
25-
local img_name=$(get_impl_image_name "$impl_id" "$test_type")
25+
local img_name=$(get_image_name "$test_type" "implementations" "$impl_id")
2626
unique_images["$img_name"]=1
2727
fi
2828
done < <(yq eval '.tests[] | [.dialer.id, .listener.id][]' "$snapshot_dir/test-matrix.yaml" | sort -u)
@@ -31,7 +31,7 @@ save_docker_images_for_tests() {
3131
if [ "$test_type" == "perf" ]; then
3232
while read -r impl_id; do
3333
if [ -n "$impl_id" ] && [ "$impl_id" != "null" ]; then
34-
local img_name=$(get_impl_image_name "$impl_id" "$test_type")
34+
local img_name=$(get_image_name "$test_type" "baselines" "$impl_id")
3535
unique_images["$img_name"]=1
3636
fi
3737
done < <(yq eval '.baselines[] | [.dialer.id, .listener.id][]' "$snapshot_dir/test-matrix.yaml" 2>/dev/null | sort -u)
@@ -42,15 +42,15 @@ save_docker_images_for_tests() {
4242
# Get unique relays
4343
while read -r relay_id; do
4444
if [ -n "$relay_id" ]; then
45-
local img_name=$(get_relay_image_name "$relay_id" "$test_type")
45+
local img_name=$(get_image_name "$test_type" "relays" "$relay_id")
4646
unique_images["$img_name"]=1
4747
fi
4848
done < <(yq eval '.tests[].relay' "$snapshot_dir/test-matrix.yaml" | sort -u)
4949

5050
# Get unique routers
5151
while read -r router_id; do
5252
if [ -n "$router_id" ]; then
53-
local img_name=$(get_router_image_name "$router_id" "$test_type")
53+
local img_name=$(get_image_name "$test_type" "routers" "$router_id")
5454
unique_images["$img_name"]=1
5555
fi
5656
done < <(yq eval '.tests[] | [.dialerRouter, .listenerRouter][]' "$snapshot_dir/test-matrix.yaml" | sort -u)
@@ -64,12 +64,12 @@ save_docker_images_for_tests() {
6464
local source_type=$(yq eval ".implementations[$i].source.type" images.yaml)
6565

6666
# Check if this implementation is used in tests
67-
if echo "${!unique_images[@]}" | grep -q "transport-interop-${impl_id}"; then
67+
if echo "${!unique_images[@]}" | grep -q "transport-implementations-${impl_id}"; then
6868
# If it's a browser type, add its base image
6969
if [ "$source_type" == "browser" ]; then
7070
local base_image=$(yq eval ".implementations[$i].source.baseImage" images.yaml)
7171
if [ -n "$base_image" ] && [ "$base_image" != "null" ]; then
72-
local base_img_name=$(get_impl_image_name "$base_image" "$test_type")
72+
local base_img_name=$(get_image_name "$test_type" "implementations" "$base_image")
7373
unique_images["$base_img_name"]=1
7474
fi
7575
fi

‎perf/lib/generate-tests.sh‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ trap 'echo "ERROR in generate-tests.sh at line $LINENO: Command exited with stat
1313
source "${SCRIPT_LIB_DIR}/lib-filter-engine.sh"
1414
source "${SCRIPT_LIB_DIR}/lib-generate-tests.sh"
1515
source "${SCRIPT_LIB_DIR}/lib-image-building.sh"
16+
source "${SCRIPT_LIB_DIR}/lib-image-naming.sh"
1617
source "${SCRIPT_LIB_DIR}/lib-output-formatting.sh"
1718
source "${SCRIPT_LIB_DIR}/lib-test-caching.sh"
1819
source "${SCRIPT_LIB_DIR}/lib-test-filtering.sh"
@@ -559,9 +560,9 @@ EOF
559560

560561
# Get commits, if they exist
561562
local dialer_commit=$(get_source_commit "${entity_type}" "${dialer}")
562-
local dialer_image_name=$(get_image_name "${entity_type}" "${dialer}")
563+
local dialer_image_name=$(get_image_name "${TEST_TYPE}" "${entity_type}" "${dialer}")
563564
local listener_commit=$(get_source_commit "${entity_type}" "${listener}")
564-
local listener_image_name=$(get_image_name "${entity_type}" "${dialer}")
565+
local listener_image_name=$(get_image_name "${TEST_TYPE}" "${entity_type}" "${listener}")
565566

566567
cat >> "${TEST_PASS_DIR}/test-matrix.yaml" <<EOF
567568
- id: "${id}"

‎transport/lib/generate-tests.sh‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ trap 'echo "ERROR in generate-tests.sh at line $LINENO: Command exited with stat
1313
source "${SCRIPT_LIB_DIR}/lib-filter-engine.sh"
1414
source "${SCRIPT_LIB_DIR}/lib-generate-tests.sh"
1515
source "${SCRIPT_LIB_DIR}/lib-image-building.sh"
16+
source "${SCRIPT_LIB_DIR}/lib-image-naming.sh"
1617
source "${SCRIPT_LIB_DIR}/lib-output-formatting.sh"
1718
source "${SCRIPT_LIB_DIR}/lib-test-caching.sh"
1819
source "${SCRIPT_LIB_DIR}/lib-test-filtering.sh"
@@ -348,9 +349,9 @@ generate_tests_worker() {
348349
local dialer_commit="${image_commit[${dialer_id}]:-}"
349350
local listener_commit="${image_commit[${listener_id}]:-}"
350351

351-
# Get the image names
352-
local dialer_image_name=$(get_image_name "implementations" "${dialer_id}")
353-
local listener_image_name=$(get_image_name "implementations" "${listener_id}")
352+
# Get the image names
353+
local dialer_image_name=$(get_image_name "${TEST_TYPE}" "implementations" "${dialer_id}")
354+
local listener_image_name=$(get_image_name "${TEST_TYPE}" "implementations" "${listener_id}")
354355

355356
# Process each common transport
356357
for transport in ${common_transports}; do

0 commit comments

Comments
 (0)