feat(coder-utils): nest logs under module_directory/logs (#870)

Author: matifali

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"
@@ -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.

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~}

registry/coder/modules/coder-utils/main.tftest.hcl

@@ -510,22 +510,71 @@ 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"
+  }
+
+  # 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"
+  }
+
+  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"
   }
 }