Skip to content
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

fix(docker-entrypoint): search new directory for socket cleanup #724

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Oct 24, 2024

This adapts the fix from d3411e0 to additionally search $PREFIX/sockets for leftover unix sockets.

See also: Kong/kong#13409 and Kong/kong#13730

KAG-5691

This adapts the fix from d3411e0 to additionally search $PREFIX/sockets
for leftover unix sockets.

See also: Kong/kong#13409
@CLAassistant
Copy link

CLAassistant commented Oct 24, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

@locao locao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call!

@flrgh
Copy link
Contributor Author

flrgh commented Oct 28, 2024

I think this is the correct change, but I want to do some more local testing today to validate.

@@ -46,7 +46,7 @@ if [[ "$1" == "kong" ]]; then

# remove all dangling sockets in $PREFIX dir before starting Kong
LOGGED_SOCKET_WARNING=0
for localfile in "$PREFIX"/*; do
for localfile in "$PREFIX"/* "$PREFIX"/sockets/*; do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copying this exposition from another thread in case it's helpful to other reviewers:

I don't know exactly how this repo interacts with Kong/kong with regards to tags and releases. With that in mind, I figured that adding the new path glob instead of just updating the old one ($PREFIX/* => $PREFIX/sockets/*) and searching both would make this more backwards compatible in case we happen to (re)build any older release trees with the docker entrypoint from this repo.

@flrgh
Copy link
Contributor Author

flrgh commented Oct 28, 2024

Okie doke, I validated the behavior by doing a start + kill -9 + restart across 3.7 and 3.8 images using the entrypoint in the image and the entrypoint from this PR. Results are as expected:

IMAGE ENTRYPOINT OK
kong:3.7-ubuntu image ✔️
kong:3.7-ubuntu branch/PR ✔️
kong:3.8-ubuntu image
kong:3.8-ubuntu branch/PR ✔️
test script
#!/usr/bin/env bash

set -euo pipefail

IMAGES=(
    kong:3.7-ubuntu
    kong:3.8-ubuntu
)

PREFIXES=()

LABEL=michaeltest

cleanup() {
    docker container ls -qa \
        --filter "label=$LABEL" \
    | while read -r id; do
        echo "cleanup: $id"
        docker container rm --force "$id"
    done

    for dir in "${PREFIXES[@]}"; do
        rm -rf "$dir"
    done
}

trap cleanup ERR EXIT

start() {
    local -r img=$1
    shift;

    docker run \
        --detach \
        --label "$LABEL"=1 \
        -e KONG_DATABASE=off \
        -e KONG_PREFIX=/kong_prefix \
        -e KONG_NGINX_MAIN_WORKER_PROCESSES=4 \
        -e KONG_ADMIN_LISTEN=0.0.0.0:9999 \
        -p 9999:9999 \
        "$@" \
        "$img"
}

await_running() {
    for _ in {1..10}; do
        if curl \
            --silent \
            --fail \
            -o /dev/null \
            "http://127.0.0.1:9999/"
        then
            return 0
        fi
        sleep 0.5
    done

    return 1
}

test_image() {
    local -r img=$1
    shift;
    printf 'TEST: %s, ARGS: %s' "$img" "$*"

    prefix=$(mktemp -d)
    PREFIXES+=("$prefix")

    id=$(start "$img" \
        -v "$prefix:/kong_prefix" \
        "$@"
    )
    await_running

    docker container kill -s 9 "$id" >/dev/null

    socks=$(find "$prefix" -type s)
    if [[ -z $socks ]]; then
        printf 'FAIL: no sockets leftover???\n'
        exit 1
    fi
    docker container rm -f "$id" > /dev/null

    id=$(start "$img" \
        -v "$prefix:/kong_prefix" \
        "$@" \
        || true
    )
    if await_running &&
        docker container logs 2>&1 "$id" \
        | grep -q 'found dangling unix sockets'
    then
        printf ' ...SUCCESS\n'
    else
        printf ' ...FAILURE\n'
        docker container logs "$id"
    fi

    cleanup
}

cleanup

for img in "${IMAGES[@]}"; do
    test_image "$img"
    test_image "$img" -v "$PWD/docker-entrypoint.sh:/docker-entrypoint.sh"
done

@flrgh flrgh merged commit 8f0a3c6 into master Oct 31, 2024
6 checks passed
@flrgh flrgh deleted the fix/socket-cleanup branch October 31, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants