From 25a49624c76dd8133001b5614db0c03cd3c23ef4 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 21 Jan 2023 14:55:41 -0500 Subject: [PATCH 1/7] fix: trigger nginx proxy rebuild early to avoid downtime if deploying large numbers of processes Due to how the nginx proxy implementation works, if an app has either many process types or many containers for a process type other than web, it is possible to see downtime on the web process due to the old web containers rotating out _before_ we reload the nginx config. The correct fix would be to use some sort of dns-based resolution for the nginx config, but pending that change, we reload nginx twice now - ugh - to avoid this. --- .../scheduler-post-deploy-process | 24 +++++++++++++++++++ .../bin/scheduler-deploy-process | 2 ++ 2 files changed, 26 insertions(+) create mode 100755 plugins/nginx-vhosts/scheduler-post-deploy-process diff --git a/plugins/nginx-vhosts/scheduler-post-deploy-process b/plugins/nginx-vhosts/scheduler-post-deploy-process new file mode 100755 index 000000000..1de8c7501 --- /dev/null +++ b/plugins/nginx-vhosts/scheduler-post-deploy-process @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +set -eo pipefail +[[ $DOKKU_TRACE ]] && set -x +source "$PLUGIN_CORE_AVAILABLE_PATH/common/functions" +source "$PLUGIN_AVAILABLE_PATH/nginx-vhosts/functions" + +trigger-nginx-vhosts-scheduler-post-deploy-process() { + declare desc="nginx-vhosts scheduler-post-deploy-process plugin trigger" + declare trigger="scheduler-post-deploy-process" + declare APP="$1" PROCESS_TYPE="$2" + + if [[ "$PROCESS_TYPE" != "web" ]]; then + return + fi + + if [[ "$(plugn trigger proxy-type "$APP")" != "nginx" ]]; then + return + fi + + dokku_log_info1 "Triggering early nginx proxy rebuild" + plugn trigger proxy-build-config "$APP" +} + +trigger-nginx-vhosts-scheduler-post-deploy-process "$@" diff --git a/plugins/scheduler-docker-local/bin/scheduler-deploy-process b/plugins/scheduler-docker-local/bin/scheduler-deploy-process index 32f86f263..27d59889e 100755 --- a/plugins/scheduler-docker-local/bin/scheduler-deploy-process +++ b/plugins/scheduler-docker-local/bin/scheduler-deploy-process @@ -51,6 +51,8 @@ main() { 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 if [[ "$PROC_COUNT" == 0 ]]; then local CONTAINER_IDX_OFFSET=0 From 5719f435511f69baf053b30ac68914b002223216 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 21 Jan 2023 16:43:43 -0500 Subject: [PATCH 2/7] feat: silence env var setting --- plugins/common/common.go | 28 +++++++++++++++++++++++++++- plugins/common/triggers.go | 10 ++++------ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/plugins/common/common.go b/plugins/common/common.go index 091f15451..8e0091438 100644 --- a/plugins/common/common.go +++ b/plugins/common/common.go @@ -74,6 +74,29 @@ func CommandUsage(helpHeader string, helpContent string) { fmt.Println(columnize.Format(content, config)) } +// EnvWrap wraps a func with a setenv call and resets the value at the end +func EnvWrap(fn func() error, environ map[string]string) error { + var oldEnviron map[string]string + for key, value := range environ { + oldEnviron[key] = os.Getenv(key) + if err := os.Setenv(key, value); err != nil { + return err + } + } + + if err := fn(); err != nil { + return err + } + + for key, value := range oldEnviron { + if err := os.Setenv(key, value); err != nil { + return err + } + } + + return nil +} + // GetAppScheduler fetches the scheduler for a given application func GetAppScheduler(appName string) string { appScheduler := "" @@ -308,7 +331,10 @@ func IsDeployed(appName string) bool { deployed = "true" } - CommandPropertySet("common", appName, "deployed", deployed, DefaultProperties, GlobalProperties) + EnvWrap(func() error { + CommandPropertySet("common", appName, "deployed", deployed, DefaultProperties, GlobalProperties) + return nil + }, map[string]string{"DOKKU_QUIET_OUTPUT": "1"}) } return deployed == "true" diff --git a/plugins/common/triggers.go b/plugins/common/triggers.go index e904c525a..813a22d45 100644 --- a/plugins/common/triggers.go +++ b/plugins/common/triggers.go @@ -2,7 +2,6 @@ package common import ( "fmt" - "os" ) // TriggerAppList outputs each app name to stdout on a newline @@ -23,11 +22,10 @@ func TriggerAppList(filtered bool) error { // TriggerCorePostDeploy associates the container with a specified network func TriggerCorePostDeploy(appName string) error { - quiet := os.Getenv("DOKKU_QUIET_OUTPUT") - os.Setenv("DOKKU_QUIET_OUTPUT", "1") - CommandPropertySet("common", appName, "deployed", "true", DefaultProperties, GlobalProperties) - os.Setenv("DOKKU_QUIET_OUTPUT", quiet) - return nil + return EnvWrap(func() error { + CommandPropertySet("common", appName, "deployed", "true", DefaultProperties, GlobalProperties) + return nil + }, map[string]string{"DOKKU_QUIET_OUTPUT": "1"}) } // TriggerInstall runs the install step for the common plugin From 594adf53664e22b361af302ea589877c8fc39d89 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 21 Jan 2023 16:44:21 -0500 Subject: [PATCH 3/7] tests: update count to 2 to handle the fact that now the nginx config is built twice --- tests/unit/nginx-vhosts_11.bats | 4 ++-- tests/unit/nginx-vhosts_12.bats | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/nginx-vhosts_11.bats b/tests/unit/nginx-vhosts_11.bats index dab13fb79..7bf263dfe 100644 --- a/tests/unit/nginx-vhosts_11.bats +++ b/tests/unit/nginx-vhosts_11.bats @@ -178,7 +178,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 2 mv "$CUSTOM_TMP/nginx.conf.sigil" "$CUSTOM_TMP/nginx.conf.sigil-2" @@ -207,7 +207,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 2 popd rm -rf "$TMP" diff --git a/tests/unit/nginx-vhosts_12.bats b/tests/unit/nginx-vhosts_12.bats index 1fd0fd2d3..159526f6b 100644 --- a/tests/unit/nginx-vhosts_12.bats +++ b/tests/unit/nginx-vhosts_12.bats @@ -56,7 +56,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 2 run /bin/bash -c "dokku nginx:set --global nginx-conf-sigil-path dokku/nginx.conf.sigil" echo "output: $output" @@ -103,7 +103,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 2 run /bin/bash -c "dokku nginx:set --global nginx-conf-sigil-path" echo "output: $output" @@ -125,7 +125,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" + assert_output_contains "Overriding default nginx.conf with detected nginx.conf.sigil" 2 run /bin/bash -c "docker image rm dokku-test/$TEST_APP:latest dokku-test/$TEST_APP:v2" echo "output: $output" From 91975c5bfe5ed31a734ac4733d3f067a22e74848 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 21 Jan 2023 16:44:35 -0500 Subject: [PATCH 4/7] chore: set different header level --- plugins/nginx-vhosts/scheduler-post-deploy-process | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/nginx-vhosts/scheduler-post-deploy-process b/plugins/nginx-vhosts/scheduler-post-deploy-process index 1de8c7501..2d2be1d3a 100755 --- a/plugins/nginx-vhosts/scheduler-post-deploy-process +++ b/plugins/nginx-vhosts/scheduler-post-deploy-process @@ -17,7 +17,7 @@ trigger-nginx-vhosts-scheduler-post-deploy-process() { return fi - dokku_log_info1 "Triggering early nginx proxy rebuild" + dokku_log_info2 "Triggering early nginx proxy rebuild" plugn trigger proxy-build-config "$APP" } From 1a498b0b7690673b514e5ed74d958eed42acb042 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sat, 21 Jan 2023 17:26:52 -0500 Subject: [PATCH 5/7] fix: handle nil map issue --- plugins/common/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/common/common.go b/plugins/common/common.go index 8e0091438..cfadb36fd 100644 --- a/plugins/common/common.go +++ b/plugins/common/common.go @@ -76,7 +76,7 @@ func CommandUsage(helpHeader string, helpContent string) { // EnvWrap wraps a func with a setenv call and resets the value at the end func EnvWrap(fn func() error, environ map[string]string) error { - var oldEnviron map[string]string + oldEnviron := map[string]string{} for key, value := range environ { oldEnviron[key] = os.Getenv(key) if err := os.Setenv(key, value); err != nil { From 69c1fe8027abe3fb6afaa94b46307ad61cfeadde Mon Sep 17 00:00:00 2001 From: josegonzalez Date: Mon, 23 Jan 2023 12:22:09 -0500 Subject: [PATCH 6/7] fix: update counts --- tests/unit/nginx-vhosts_6.bats | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/nginx-vhosts_6.bats b/tests/unit/nginx-vhosts_6.bats index 0e01a5b3e..41a69f87f 100644 --- a/tests/unit/nginx-vhosts_6.bats +++ b/tests/unit/nginx-vhosts_6.bats @@ -30,7 +30,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Enabling HSTS" + assert_output_contains "Enabling HSTS" 2 run /bin/bash -c "test -f $HSTS_CONF" echo "output: $output" @@ -113,7 +113,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Enabling HSTS" + assert_output_contains "Enabling HSTS" 2 # disable hsts globally run /bin/bash -c "dokku nginx:set --global hsts false" @@ -139,7 +139,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Enabling HSTS" 1 + assert_output_contains "Enabling HSTS" 2 # set global value to default run /bin/bash -c "dokku nginx:set --global hsts" @@ -152,7 +152,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Enabling HSTS" 1 + assert_output_contains "Enabling HSTS" 2 } @test "(nginx-vhosts) nginx:set bind-address" { From 9d9b880f59c6d3669d008b631a73c02525e5a1e8 Mon Sep 17 00:00:00 2001 From: josegonzalez Date: Mon, 23 Jan 2023 14:36:37 -0500 Subject: [PATCH 7/7] fix: correct counts --- tests/unit/nginx-vhosts_6.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/nginx-vhosts_6.bats b/tests/unit/nginx-vhosts_6.bats index 41a69f87f..5af64cdc9 100644 --- a/tests/unit/nginx-vhosts_6.bats +++ b/tests/unit/nginx-vhosts_6.bats @@ -139,7 +139,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Enabling HSTS" 2 + assert_output_contains "Enabling HSTS" 1 # set global value to default run /bin/bash -c "dokku nginx:set --global hsts" @@ -152,7 +152,7 @@ teardown() { echo "output: $output" echo "status: $status" assert_success - assert_output_contains "Enabling HSTS" 2 + assert_output_contains "Enabling HSTS" 1 } @test "(nginx-vhosts) nginx:set bind-address" {