From 43c255436d900cddfe7e672a11ff22956bce9682 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 14 Mar 2024 11:29:37 -0400 Subject: [PATCH 1/4] feat: warn when publish ports if scaling up or zero downtime is enabled Closes #4662 --- .../bin/scheduler-deploy-process | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/plugins/scheduler-docker-local/bin/scheduler-deploy-process b/plugins/scheduler-docker-local/bin/scheduler-deploy-process index fce53481b..64b6e2c7a 100755 --- a/plugins/scheduler-docker-local/bin/scheduler-deploy-process +++ b/plugins/scheduler-docker-local/bin/scheduler-deploy-process @@ -50,6 +50,31 @@ fn-scheduler-deploy-process() { PARALLEL_DEPLOY_COUNT="$(plugn trigger "app-json-process-deploy-parallelism" "$APP" "$PROC_TYPE")" DOKKU_CHECKS_DISABLED="$DOKKU_CHECKS_DISABLED" INJECT_INIT_FLAG="$INJECT_INIT_FLAG" parallel --will-cite --halt soon,fail=1 --jobs "$PARALLEL_DEPLOY_COUNT" --ungroup <"$PROCESS_TMP_FILE" + local DOCKER_ARGS + DOCKER_ARGS=$(: | plugn trigger docker-args-deploy "$APP" "$IMAGE_TAG" "$PROC_TYPE") + DOCKER_ARGS+=$(: | plugn trigger docker-args-process-deploy "$APP" "$IMAGE_SOURCE_TYPE" "$IMAGE_TAG" "$PROC_TYPE") + DOCKER_ARGS=" $DOCKER_ARGS " + declare -a ARG_ARRAY + eval "ARG_ARRAY=($DOCKER_ARGS)" + + local port_published=false + for arg in "${ARG_ARRAY[@]}"; do + if [[ "$arg" == "-p" ]] || [[ "$arg" == "--publish" ]] || [[ "$arg" == "-P" ]] || [[ "$arg" == "--publish-all" ]]; then + port_published=true + break + fi + done + + local warned_on_publish=false + if [[ "$port_published" == "true" ]] && [[ "$PROC_COUNT" -gt 1 ]]; then + warned_on_publish=true + dokku_log_warn "Port publishing may not work as expected with multiple containers. Consider scaling process type $PROC_TYPE to 1." + fi + if [[ "$port_published" == "true" ]] && [[ "$DOKKU_CHECKS_DISABLED" != "true" ]] && [[ "$warned_on_publish" == "false" ]]; then + warned_on_publish=true + dokku_log_warn "Port publishing may not work as expected with zero downtime. Consider disabling zero downtime for process type $PROC_TYPE." + fi + plugn trigger scheduler-post-deploy-process "$APP" "$PROC_TYPE" # cleanup when we scale down From 36ade1ecd0ef7df19215649ba93072b5b3ab4565 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 14 Mar 2024 14:07:08 -0400 Subject: [PATCH 2/4] tests: add test for port publishing warning --- tests/unit/scheduler-docker-local.bats | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/unit/scheduler-docker-local.bats b/tests/unit/scheduler-docker-local.bats index ba3199a3b..573cbb819 100644 --- a/tests/unit/scheduler-docker-local.bats +++ b/tests/unit/scheduler-docker-local.bats @@ -119,3 +119,48 @@ teardown() { assert_success assert_output_contains "docker-init" 0 } + +@test "(scheduler-docker-local) publish ports" { + run create_app + echo "output: $output" + echo "status: $status" + assert_success + + run deploy_app + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Port publishing may not work as expected with multiple containers" 0 + assert_output_contains "Port publishing may not work as expected with zero downtime" 0 + + run /bin/bash -c "dokku ps:scale $TEST_APP web=2" + echo "output: $output" + echo "status: $status" + assert_success + + run deploy_app + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Port publishing may not work as expected with multiple containers" 1 + assert_output_contains "Port publishing may not work as expected with zero downtime" 0 + + run /bin/bash -c "dokku ps:scale --skip-deploy $TEST_APP web=2" + echo "output: $output" + echo "status: $status" + assert_success + + run deploy_app + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Port publishing may not work as expected with multiple containers" 0 + assert_output_contains "Port publishing may not work as expected with zero downtime" 1 + + run /bin/bash -c "dokku checks:disable $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Port publishing may not work as expected with multiple containers" 0 + assert_output_contains "Port publishing may not work as expected with zero downtime" 0 +} From 6e3631afeb80f07cdffe48927311d3befdae5e87 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 14 Mar 2024 14:59:49 -0400 Subject: [PATCH 3/4] fix: correct warning placement and update tests to account for parallel's behavior --- .../bin/scheduler-deploy-process | 26 ++++++------ tests/unit/scheduler-docker-local.bats | 41 ++++++++++++------- 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/plugins/scheduler-docker-local/bin/scheduler-deploy-process b/plugins/scheduler-docker-local/bin/scheduler-deploy-process index 64b6e2c7a..4975a9d04 100755 --- a/plugins/scheduler-docker-local/bin/scheduler-deploy-process +++ b/plugins/scheduler-docker-local/bin/scheduler-deploy-process @@ -47,9 +47,6 @@ fn-scheduler-deploy-process() { fi fi - PARALLEL_DEPLOY_COUNT="$(plugn trigger "app-json-process-deploy-parallelism" "$APP" "$PROC_TYPE")" - DOKKU_CHECKS_DISABLED="$DOKKU_CHECKS_DISABLED" INJECT_INIT_FLAG="$INJECT_INIT_FLAG" parallel --will-cite --halt soon,fail=1 --jobs "$PARALLEL_DEPLOY_COUNT" --ungroup <"$PROCESS_TMP_FILE" - local DOCKER_ARGS DOCKER_ARGS=$(: | plugn trigger docker-args-deploy "$APP" "$IMAGE_TAG" "$PROC_TYPE") DOCKER_ARGS+=$(: | plugn trigger docker-args-process-deploy "$APP" "$IMAGE_SOURCE_TYPE" "$IMAGE_TAG" "$PROC_TYPE") @@ -59,22 +56,27 @@ fn-scheduler-deploy-process() { local port_published=false for arg in "${ARG_ARRAY[@]}"; do - if [[ "$arg" == "-p" ]] || [[ "$arg" == "--publish" ]] || [[ "$arg" == "-P" ]] || [[ "$arg" == "--publish-all" ]]; then + if [[ "$arg" == "-p "* ]] || [[ "$arg" =~ "--publish "* ]] || [[ "$arg" == "-P" ]] || [[ "$arg" =~ "--publish-all"* ]]; then port_published=true break fi done - local warned_on_publish=false - if [[ "$port_published" == "true" ]] && [[ "$PROC_COUNT" -gt 1 ]]; then - warned_on_publish=true - dokku_log_warn "Port publishing may not work as expected with multiple containers. Consider scaling process type $PROC_TYPE to 1." - fi - if [[ "$port_published" == "true" ]] && [[ "$DOKKU_CHECKS_DISABLED" != "true" ]] && [[ "$warned_on_publish" == "false" ]]; then - warned_on_publish=true - dokku_log_warn "Port publishing may not work as expected with zero downtime. Consider disabling zero downtime for process type $PROC_TYPE." + if [[ "$port_published" == "true" ]]; then + local warned_on_publish=false + if [[ "$PROC_COUNT" -gt 1 ]]; then + warned_on_publish=true + dokku_log_warn "Deploys may fail when publishing ports and scaling to multiple containers. Consider scaling process type $PROC_TYPE to 1." + fi + if [[ "$DOKKU_CHECKS_DISABLED" != "true" ]] && [[ "$warned_on_publish" == "false" ]]; then + warned_on_publish=true + dokku_log_warn "Deploys may fail when publishing ports and enabling zero downtime. Consider disabling zero downtime for process type $PROC_TYPE." + fi fi + PARALLEL_DEPLOY_COUNT="$(plugn trigger "app-json-process-deploy-parallelism" "$APP" "$PROC_TYPE")" + DOKKU_CHECKS_DISABLED="$DOKKU_CHECKS_DISABLED" INJECT_INIT_FLAG="$INJECT_INIT_FLAG" parallel --will-cite --halt soon,fail=1 --jobs "$PARALLEL_DEPLOY_COUNT" --ungroup <"$PROCESS_TMP_FILE" + plugn trigger scheduler-post-deploy-process "$APP" "$PROC_TYPE" # cleanup when we scale down diff --git a/tests/unit/scheduler-docker-local.bats b/tests/unit/scheduler-docker-local.bats index 573cbb819..d14ea9ed4 100644 --- a/tests/unit/scheduler-docker-local.bats +++ b/tests/unit/scheduler-docker-local.bats @@ -130,37 +130,48 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Port publishing may not work as expected with multiple containers" 0 - assert_output_contains "Port publishing may not work as expected with zero downtime" 0 + assert_output_contains "Deploys may fail when publishing ports and scaling to multiple containers" 0 + assert_output_contains "Deploys may fail when publishing ports and enabling zero downtime" 0 - run /bin/bash -c "dokku ps:scale $TEST_APP web=2" + run /bin/bash -c "dokku docker-options:add $TEST_APP deploy '--publish 5000:5000'" echo "output: $output" echo "status: $status" assert_success - run deploy_app - echo "output: $output" - echo "status: $status" - assert_success - assert_output_contains "Port publishing may not work as expected with multiple containers" 1 - assert_output_contains "Port publishing may not work as expected with zero downtime" 0 - run /bin/bash -c "dokku ps:scale --skip-deploy $TEST_APP web=2" echo "output: $output" echo "status: $status" assert_success - run deploy_app + # the expected output will be seen twice due to how parallel re-outputs stderr in its own output... + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Deploys may fail when publishing ports and scaling to multiple containers" 2 + assert_output_contains "Deploys may fail when publishing ports and enabling zero downtime" 0 + + run /bin/bash -c "dokku ps:scale --skip-deploy $TEST_APP web=1" echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Port publishing may not work as expected with multiple containers" 0 - assert_output_contains "Port publishing may not work as expected with zero downtime" 1 + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_failure + assert_output_contains "Deploys may fail when publishing ports and scaling to multiple containers" 0 + assert_output_contains "Deploys may fail when publishing ports and enabling zero downtime" 2 run /bin/bash -c "dokku checks:disable $TEST_APP" echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Port publishing may not work as expected with multiple containers" 0 - assert_output_contains "Port publishing may not work as expected with zero downtime" 0 + + run /bin/bash -c "dokku ps:rebuild $TEST_APP" + echo "output: $output" + echo "status: $status" + assert_success + assert_output_contains "Deploys may fail when publishing ports and scaling to multiple containers" 0 + assert_output_contains "Deploys may fail when publishing ports and enabling zero downtime" 0 } From c9b9c2ab2573475a0e617949d7a98106207dcf1e Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 14 Mar 2024 15:02:57 -0400 Subject: [PATCH 4/4] chore: run shfmt --- plugins/scheduler-docker-local/bin/scheduler-deploy-process | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/scheduler-docker-local/bin/scheduler-deploy-process b/plugins/scheduler-docker-local/bin/scheduler-deploy-process index 4975a9d04..b84ef7827 100755 --- a/plugins/scheduler-docker-local/bin/scheduler-deploy-process +++ b/plugins/scheduler-docker-local/bin/scheduler-deploy-process @@ -68,7 +68,7 @@ fn-scheduler-deploy-process() { warned_on_publish=true dokku_log_warn "Deploys may fail when publishing ports and scaling to multiple containers. Consider scaling process type $PROC_TYPE to 1." fi - if [[ "$DOKKU_CHECKS_DISABLED" != "true" ]] && [[ "$warned_on_publish" == "false" ]]; then + if [[ "$DOKKU_CHECKS_DISABLED" != "true" ]] && [[ "$warned_on_publish" == "false" ]]; then warned_on_publish=true dokku_log_warn "Deploys may fail when publishing ports and enabling zero downtime. Consider disabling zero downtime for process type $PROC_TYPE." fi