From 11634f9789f3cf3e1af04636559d00c62657960a Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Thu, 23 Apr 2026 06:08:24 +0000 Subject: [PATCH 1/3] feat(coder-utils): nest logs under module_directory/logs Move pre_install.log, install.log, post_install.log, and start.log from the flat module_directory to a logs/ subdirectory. This keeps the scripts directory (install.sh, pre_install.sh, etc.) clean and separates executable artifacts from runtime output. Each coder_script mkdirs the logs/ sub-path before tee runs so the directory exists even when the post_install or start script is the first consumer after install. Breaking for consumers that read pre_install.log et al. directly from module_directory; they must look under module_directory/logs instead. --- registry/coder/modules/coder-utils/README.md | 11 +++ registry/coder/modules/coder-utils/main.tf | 18 +++-- .../coder/modules/coder-utils/main.tftest.hcl | 74 +++++++++++++++++-- 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/registry/coder/modules/coder-utils/README.md b/registry/coder/modules/coder-utils/README.md index 07eb223a6..561a46ac2 100644 --- a/registry/coder/modules/coder-utils/README.md +++ b/registry/coder/modules/coder-utils/README.md @@ -83,3 +83,14 @@ module "coder_utils" { ``` Both variables are optional. `display_name_prefix` defaults to `""` (no prefix), and `icon` defaults to `null` (use the Coder provider's default). + +## Log file locations + +The module writes each script's stdout+stderr to `${module_directory}/logs/`: + +- `pre_install.log` +- `install.log` +- `post_install.log` +- `start.log` + +Each `coder_script` `mkdir -p`s this subdirectory before its `tee` runs, so the first script to execute creates it. diff --git a/registry/coder/modules/coder-utils/main.tf b/registry/coder/modules/coder-utils/main.tf index 126496fb6..7705434e3 100644 --- a/registry/coder/modules/coder-utils/main.tf +++ b/registry/coder/modules/coder-utils/main.tf @@ -51,7 +51,7 @@ variable "agent_name" { variable "module_directory" { type = string - description = "The module's working directory for scripts and logs." + description = "The module's working directory for the install/pre/post/start scripts this module writes. Logs land under a `logs/` subdirectory of this path." } variable "display_name_prefix" { @@ -82,10 +82,12 @@ locals { post_install_path = "${var.module_directory}/post_install.sh" start_path = "${var.module_directory}/start.sh" - pre_install_log_path = "${var.module_directory}/pre_install.log" - install_log_path = "${var.module_directory}/install.log" - post_install_log_path = "${var.module_directory}/post_install.log" - start_log_path = "${var.module_directory}/start.log" + pre_install_log_path = "${local.log_directory}/pre_install.log" + install_log_path = "${local.log_directory}/install.log" + post_install_log_path = "${local.log_directory}/post_install.log" + start_log_path = "${local.log_directory}/start.log" + + log_directory = "${var.module_directory}/logs" install_sync_deps = var.pre_install_script != null ? local.pre_install_script_name : null @@ -110,6 +112,7 @@ resource "coder_script" "pre_install_script" { set -o pipefail mkdir -p ${var.module_directory} + mkdir -p ${local.log_directory} trap 'coder exp sync complete ${local.pre_install_script_name}' EXIT coder exp sync start ${local.pre_install_script_name} @@ -132,6 +135,7 @@ resource "coder_script" "install_script" { set -o pipefail mkdir -p ${var.module_directory} + mkdir -p ${local.log_directory} trap 'coder exp sync complete ${local.install_script_name}' EXIT %{if local.install_sync_deps != null~} @@ -156,6 +160,8 @@ resource "coder_script" "post_install_script" { set -o errexit set -o pipefail + mkdir -p ${local.log_directory} + trap 'coder exp sync complete ${local.post_install_script_name}' EXIT coder exp sync want ${local.post_install_script_name} ${local.install_script_name} coder exp sync start ${local.post_install_script_name} @@ -178,6 +184,8 @@ resource "coder_script" "start_script" { set -o errexit set -o pipefail + mkdir -p ${local.log_directory} + trap 'coder exp sync complete ${local.start_script_name}' EXIT coder exp sync want ${local.start_script_name} ${local.start_sync_deps} diff --git a/registry/coder/modules/coder-utils/main.tftest.hcl b/registry/coder/modules/coder-utils/main.tftest.hcl index 40aeeb41f..d13f1dbcd 100644 --- a/registry/coder/modules/coder-utils/main.tftest.hcl +++ b/registry/coder/modules/coder-utils/main.tftest.hcl @@ -510,22 +510,80 @@ run "test_scripts_tee_stdout_and_log_file" { } assert { - condition = can(regex("pre_install.sh 2>&1 \\| tee .*pre_install.log", coder_script.pre_install_script[0].script)) - error_message = "pre_install wrapper must tee combined output to the log file and stdout" + condition = can(regex("pre_install.sh 2>&1 \\| tee .*logs/pre_install.log", coder_script.pre_install_script[0].script)) + error_message = "pre_install wrapper must tee combined output to the logs/ subdirectory" } assert { - condition = can(regex("install.sh 2>&1 \\| tee .*install.log", coder_script.install_script.script)) - error_message = "install wrapper must tee combined output to the log file and stdout" + condition = can(regex("install.sh 2>&1 \\| tee .*logs/install.log", coder_script.install_script.script)) + error_message = "install wrapper must tee combined output to the logs/ subdirectory" } assert { - condition = can(regex("post_install.sh 2>&1 \\| tee .*post_install.log", coder_script.post_install_script[0].script)) - error_message = "post_install wrapper must tee combined output to the log file and stdout" + condition = can(regex("post_install.sh 2>&1 \\| tee .*logs/post_install.log", coder_script.post_install_script[0].script)) + error_message = "post_install wrapper must tee combined output to the logs/ subdirectory" } assert { - condition = can(regex("start.sh 2>&1 \\| tee .*start.log", coder_script.start_script[0].script)) - error_message = "start wrapper must tee combined output to the log file and stdout" + condition = can(regex("start.sh 2>&1 \\| tee .*logs/start.log", coder_script.start_script[0].script)) + error_message = "start wrapper must tee combined output to the logs/ subdirectory" + } +} + +# Logs unconditionally land under ${module_directory}/logs/. Each script +# mkdirs that path before tee runs so the first script to execute creates it. +run "test_logs_nested_under_module_directory" { + command = plan + + variables { + agent_id = "test-agent-id" + agent_name = "test-agent" + module_directory = ".test-module" + pre_install_script = "echo pre" + install_script = "echo install" + post_install_script = "echo post" + start_script = "echo start" + } + + assert { + condition = can(regex("tee .test-module/logs/pre_install.log", coder_script.pre_install_script[0].script)) + error_message = "pre_install log must land under module_directory/logs" + } + + assert { + condition = can(regex("tee .test-module/logs/install.log", coder_script.install_script.script)) + error_message = "install log must land under module_directory/logs" + } + + assert { + condition = can(regex("tee .test-module/logs/post_install.log", coder_script.post_install_script[0].script)) + error_message = "post_install log must land under module_directory/logs" + } + + assert { + condition = can(regex("tee .test-module/logs/start.log", coder_script.start_script[0].script)) + error_message = "start log must land under module_directory/logs" + } + + # Each script must mkdir -p the logs/ sub-path so tee does not fail + # before install runs. + assert { + condition = can(regex("mkdir -p .test-module/logs", coder_script.pre_install_script[0].script)) + error_message = "pre_install script must mkdir -p the logs/ sub-path" + } + + assert { + condition = can(regex("mkdir -p .test-module/logs", coder_script.install_script.script)) + error_message = "install script must mkdir -p the logs/ sub-path" + } + + assert { + condition = can(regex("mkdir -p .test-module/logs", coder_script.post_install_script[0].script)) + error_message = "post_install script must mkdir -p the logs/ sub-path" + } + + assert { + condition = can(regex("mkdir -p .test-module/logs", coder_script.start_script[0].script)) + error_message = "start script must mkdir -p the logs/ sub-path" } } From b65c93903eb43f2298bda2a5984f7ef2efe21944 Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Thu, 23 Apr 2026 06:18:57 +0000 Subject: [PATCH 2/3] chore(coder-utils): bump version to 1.2.0 --- registry/coder/modules/coder-utils/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/registry/coder/modules/coder-utils/README.md b/registry/coder/modules/coder-utils/README.md index 561a46ac2..813d943b3 100644 --- a/registry/coder/modules/coder-utils/README.md +++ b/registry/coder/modules/coder-utils/README.md @@ -20,7 +20,7 @@ The Coder Utils module is a building block for modules that need to run multiple ```tf module "coder_utils" { source = "registry.coder.com/coder/coder-utils/coder" - version = "1.1.0" + version = "1.2.0" agent_id = coder_agent.main.id agent_name = "myagent" @@ -70,7 +70,7 @@ By default each `coder_script` renders in the Coder UI as plain "Install Script" ```tf module "coder_utils" { source = "registry.coder.com/coder/coder-utils/coder" - version = "1.1.0" + version = "1.2.0" agent_id = coder_agent.main.id agent_name = "myagent" From 70a00934b6701612cdbfef7b99ee981070454449 Mon Sep 17 00:00:00 2001 From: Atif Ali Date: Thu, 23 Apr 2026 06:38:26 +0000 Subject: [PATCH 3/3] fix(coder-utils): drop redundant log_directory mkdir from post_install and start post_install sync-depends on install, and start sync-depends on install (and post_install when present). The install script (and pre_install when configured) mkdirs the logs/ sub-path, so later scripts find it already present. Addresses review feedback on #870. --- registry/coder/modules/coder-utils/main.tf | 4 ---- .../coder/modules/coder-utils/main.tftest.hcl | 15 +++------------ 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/registry/coder/modules/coder-utils/main.tf b/registry/coder/modules/coder-utils/main.tf index 7705434e3..e9abff572 100644 --- a/registry/coder/modules/coder-utils/main.tf +++ b/registry/coder/modules/coder-utils/main.tf @@ -160,8 +160,6 @@ resource "coder_script" "post_install_script" { set -o errexit set -o pipefail - mkdir -p ${local.log_directory} - trap 'coder exp sync complete ${local.post_install_script_name}' EXIT coder exp sync want ${local.post_install_script_name} ${local.install_script_name} coder exp sync start ${local.post_install_script_name} @@ -184,8 +182,6 @@ resource "coder_script" "start_script" { set -o errexit set -o pipefail - mkdir -p ${local.log_directory} - trap 'coder exp sync complete ${local.start_script_name}' EXIT coder exp sync want ${local.start_script_name} ${local.start_sync_deps} diff --git a/registry/coder/modules/coder-utils/main.tftest.hcl b/registry/coder/modules/coder-utils/main.tftest.hcl index d13f1dbcd..a89585de1 100644 --- a/registry/coder/modules/coder-utils/main.tftest.hcl +++ b/registry/coder/modules/coder-utils/main.tftest.hcl @@ -565,8 +565,9 @@ run "test_logs_nested_under_module_directory" { error_message = "start log must land under module_directory/logs" } - # Each script must mkdir -p the logs/ sub-path so tee does not fail - # before install runs. + # Only pre_install and install mkdir the logs/ sub-path. post_install + # and start sync-depend on install so the directory already exists by + # the time they run. assert { condition = can(regex("mkdir -p .test-module/logs", coder_script.pre_install_script[0].script)) error_message = "pre_install script must mkdir -p the logs/ sub-path" @@ -576,14 +577,4 @@ run "test_logs_nested_under_module_directory" { condition = can(regex("mkdir -p .test-module/logs", coder_script.install_script.script)) error_message = "install script must mkdir -p the logs/ sub-path" } - - assert { - condition = can(regex("mkdir -p .test-module/logs", coder_script.post_install_script[0].script)) - error_message = "post_install script must mkdir -p the logs/ sub-path" - } - - assert { - condition = can(regex("mkdir -p .test-module/logs", coder_script.start_script[0].script)) - error_message = "start script must mkdir -p the logs/ sub-path" - } }