From c6b36533ca79434f8e9a5529ed515646d1d98f23 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 27 Jan 2025 14:52:45 +0100 Subject: [PATCH 01/12] add resources_monitoring logic --- docs/resources/agent.md | 27 ++++++++++++++++++ provider/agent.go | 62 +++++++++++++++++++++++++++++++++++++++++ provider/agent_test.go | 53 +++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) diff --git a/docs/resources/agent.md b/docs/resources/agent.md index 8c786d6e..ec3f8781 100644 --- a/docs/resources/agent.md +++ b/docs/resources/agent.md @@ -79,6 +79,7 @@ resource "kubernetes_pod" "dev" { - `metadata` (Block List) Each `metadata` block defines a single item consisting of a key/value pair. This feature is in alpha and may break in future releases. (see [below for nested schema](#nestedblock--metadata)) - `motd_file` (String) The path to a file within the workspace containing a message to display to users when they login via SSH. A typical value would be `"/etc/motd"`. - `order` (Number) The order determines the position of agents in the UI presentation. The lowest order is shown first and agents with equal order are sorted by name (ascending order). +- `resources_monitoring` (Block Set) The resources monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring)) - `shutdown_script` (String) A script to run before the agent is stopped. The script should exit when it is done to signal that the workspace can be stopped. This option is an alias for defining a `coder_script` resource with `run_on_stop` set to `true`. - `startup_script` (String) A script to run after the agent starts. The script should exit when it is done to signal that the agent is ready. This option is an alias for defining a `coder_script` resource with `run_on_start` set to `true`. - `startup_script_behavior` (String) This option sets the behavior of the `startup_script`. When set to `"blocking"`, the `startup_script` must exit before the workspace is ready. When set to `"non-blocking"`, the `startup_script` may run in the background and the workspace will be ready immediately. Default is `"non-blocking"`, although `"blocking"` is recommended. This option is an alias for defining a `coder_script` resource with `start_blocks_login` set to `true` (blocking). @@ -116,3 +117,29 @@ Optional: - `display_name` (String) The user-facing name of this value. - `order` (Number) The order determines the position of agent metadata in the UI presentation. The lowest order is shown first and metadata with equal order are sorted by key (ascending order). - `timeout` (Number) The maximum time the command is allowed to run in seconds. + + + +### Nested Schema for `resources_monitoring` + +Optional: + +- `memory` (Block Set) The memory monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--memory)) +- `volumes` (Block Set) The volumes monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--volumes)) + + +### Nested Schema for `resources_monitoring.memory` + +Optional: + +- `enabled` (Boolean) Enable memory monitoring for this agent. +- `threshold` (Number) The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100. + + + +### Nested Schema for `resources_monitoring.volumes` + +Optional: + +- `enabled` (Boolean) Enable volume monitoring for this agent. +- `threshold` (Number) The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100. diff --git a/provider/agent.go b/provider/agent.go index ac012bf1..d2b9131f 100644 --- a/provider/agent.go +++ b/provider/agent.go @@ -259,6 +259,68 @@ func agentResource() *schema.Resource { ForceNew: true, Optional: true, }, + "resources_monitoring": { + Type: schema.TypeSet, + Description: "The resources monitoring configuration for this agent.", + ForceNew: true, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "memory": { + Type: schema.TypeSet, + Description: "The memory monitoring configuration for this agent.", + ForceNew: true, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "enabled": { + Type: schema.TypeBool, + Description: "Enable memory monitoring for this agent.", + ForceNew: true, + Optional: true, + Default: true, + }, + "threshold": { + Type: schema.TypeInt, + Description: "The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", + ForceNew: true, + Optional: true, + }, + }, + }, + }, + "volume": { + Type: schema.TypeSet, + Description: "The volumes monitoring configuration for this agent.", + ForceNew: true, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "path": { + Type: schema.TypeString, + Description: "The path of the volume to monitor.", + ForceNew: true, + Optional: true, + }, + "enabled": { + Type: schema.TypeBool, + Description: "Enable volume monitoring for this agent.", + ForceNew: true, + Optional: true, + Default: true, + }, + "threshold": { + Type: schema.TypeInt, + Description: "The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", + ForceNew: true, + Optional: true, + }, + }, + }, + }, + }, + }, + }, }, CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i any) error { if !rd.HasChange("metadata") { diff --git a/provider/agent_test.go b/provider/agent_test.go index d40caf56..801d316a 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -211,6 +211,59 @@ func TestAgent_Metadata(t *testing.T) { }) } +func TestAgent_ResourcesMonitoring(t *testing.T) { + t.Parallel() + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + memory { + enabled = true + threshold = 80 + } + volume { + path = "/volume1" + enabled = true + threshold = 80 + } + volume { + path = "/volume2" + enabled = true + threshold = 100 + } + } + }`, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + + resource := state.Modules[0].Resources["coder_agent.dev"] + require.NotNil(t, resource) + + t.Logf("resource: %v", resource.Primary.Attributes) + + attr := resource.Primary.Attributes + require.Equal(t, "1", attr["resources_monitoring.#"]) + require.Equal(t, "1", attr["resources_monitoring.0.memory.#"]) + require.Equal(t, "2", attr["resources_monitoring.0.volume.#"]) + require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"]) + require.Equal(t, "/volume1", attr["resources_monitoring.0.volume.0.path"]) + require.Equal(t, "100", attr["resources_monitoring.0.volume.1.threshold"]) + require.Equal(t, "/volume2", attr["resources_monitoring.0.volume.1.path"]) + return nil + }, + }}, + }) +} + func TestAgent_MetadataDuplicateKeys(t *testing.T) { t.Parallel() resource.Test(t, resource.TestCase{ From 8c6ffc1ce09ac55b4bd6735bb03ac06a4b5bdea2 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 27 Jan 2025 14:54:51 +0100 Subject: [PATCH 02/12] add resources_monitoring logic --- .../coder_resources_monitoring/data-source.tf | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 examples/data-sources/coder_resources_monitoring/data-source.tf diff --git a/examples/data-sources/coder_resources_monitoring/data-source.tf b/examples/data-sources/coder_resources_monitoring/data-source.tf new file mode 100644 index 00000000..e65ea7a6 --- /dev/null +++ b/examples/data-sources/coder_resources_monitoring/data-source.tf @@ -0,0 +1,27 @@ +provider "coder" {} + +data "coder_provisioner" "dev" {} + +data "coder_workspace" "dev" {} + +resource "coder_agent" "main" { + arch = data.coder_provisioner.dev.arch + os = data.coder_provisioner.dev.os + dir = "/workspace" + resources_monitoring { + memory { + enabled = true + threshold = 80 + } + volume { + path = "/volume1" + enabled = true + threshold = 80 + } + volume { + path = "/volume2" + enabled = true + threshold = 100 + } + } +} \ No newline at end of file From 2e613353bec0693af178b9fa7fd1b1eb9df1c80c Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 27 Jan 2025 14:58:29 +0100 Subject: [PATCH 03/12] improve testing logic --- provider/agent.go | 12 +++++------- provider/agent_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/provider/agent.go b/provider/agent.go index d2b9131f..1d927863 100644 --- a/provider/agent.go +++ b/provider/agent.go @@ -277,14 +277,13 @@ func agentResource() *schema.Resource { Type: schema.TypeBool, Description: "Enable memory monitoring for this agent.", ForceNew: true, - Optional: true, - Default: true, + Required: true, }, "threshold": { Type: schema.TypeInt, Description: "The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", ForceNew: true, - Optional: true, + Required: true, }, }, }, @@ -300,20 +299,19 @@ func agentResource() *schema.Resource { Type: schema.TypeString, Description: "The path of the volume to monitor.", ForceNew: true, - Optional: true, + Required: true, }, "enabled": { Type: schema.TypeBool, Description: "Enable volume monitoring for this agent.", ForceNew: true, - Optional: true, - Default: true, + Required: true, }, "threshold": { Type: schema.TypeInt, Description: "The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", ForceNew: true, - Optional: true, + Required: true, }, }, }, diff --git a/provider/agent_test.go b/provider/agent_test.go index 801d316a..c782f137 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -264,6 +264,45 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { }) } +func TestAgent_ResourcesMonitoring_OnlyMemory(t *testing.T) { + t.Parallel() + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + memory { + enabled = true + threshold = 80 + } + } + }`, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + + resource := state.Modules[0].Resources["coder_agent.dev"] + require.NotNil(t, resource) + + t.Logf("resource: %v", resource.Primary.Attributes) + + attr := resource.Primary.Attributes + require.Equal(t, "1", attr["resources_monitoring.#"]) + require.Equal(t, "1", attr["resources_monitoring.0.memory.#"]) + require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"]) + return nil + }, + }}, + }) +} + func TestAgent_MetadataDuplicateKeys(t *testing.T) { t.Parallel() resource.Test(t, resource.TestCase{ From bfe8f567d6880a89b5b7d3245809fb78a179538b Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 27 Jan 2025 15:02:26 +0100 Subject: [PATCH 04/12] improve testing logic --- .../coder_resources_monitoring/data-source.tf | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/examples/data-sources/coder_resources_monitoring/data-source.tf b/examples/data-sources/coder_resources_monitoring/data-source.tf index e65ea7a6..2a78a87a 100644 --- a/examples/data-sources/coder_resources_monitoring/data-source.tf +++ b/examples/data-sources/coder_resources_monitoring/data-source.tf @@ -8,20 +8,20 @@ resource "coder_agent" "main" { arch = data.coder_provisioner.dev.arch os = data.coder_provisioner.dev.os dir = "/workspace" - resources_monitoring { - memory { - enabled = true - threshold = 80 - } - volume { - path = "/volume1" - enabled = true - threshold = 80 - } - volume { - path = "/volume2" - enabled = true - threshold = 100 - } - } + resources_monitoring { + memory { + enabled = true + threshold = 80 + } + volume { + path = "/volume1" + enabled = true + threshold = 80 + } + volume { + path = "/volume2" + enabled = true + threshold = 100 + } + } } \ No newline at end of file From 760b90587fd245aeae10421ab424069708bab6b0 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Mon, 27 Jan 2025 15:07:09 +0100 Subject: [PATCH 05/12] gen doc --- docs/resources/agent.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/resources/agent.md b/docs/resources/agent.md index ec3f8781..46eeb26f 100644 --- a/docs/resources/agent.md +++ b/docs/resources/agent.md @@ -125,21 +125,22 @@ Optional: Optional: - `memory` (Block Set) The memory monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--memory)) -- `volumes` (Block Set) The volumes monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--volumes)) +- `volume` (Block Set) The volumes monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--volume)) ### Nested Schema for `resources_monitoring.memory` -Optional: +Required: - `enabled` (Boolean) Enable memory monitoring for this agent. - `threshold` (Number) The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100. - -### Nested Schema for `resources_monitoring.volumes` + +### Nested Schema for `resources_monitoring.volume` -Optional: +Required: - `enabled` (Boolean) Enable volume monitoring for this agent. +- `path` (String) The path of the volume to monitor. - `threshold` (Number) The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100. From ba0cbb34b2d172866e529dbcf57594fa6a8c0b3b Mon Sep 17 00:00:00 2001 From: defelmnq Date: Tue, 4 Feb 2025 01:52:55 +0100 Subject: [PATCH 06/12] improved testing and validation --- .../coder_resources_monitoring/data-source.tf | 1 - provider/agent.go | 89 ++++++++-- provider/agent_test.go | 164 ++++++++++++------ provider/examples_test.go | 1 + 4 files changed, 183 insertions(+), 72 deletions(-) diff --git a/examples/data-sources/coder_resources_monitoring/data-source.tf b/examples/data-sources/coder_resources_monitoring/data-source.tf index 2a78a87a..94bc55ed 100644 --- a/examples/data-sources/coder_resources_monitoring/data-source.tf +++ b/examples/data-sources/coder_resources_monitoring/data-source.tf @@ -7,7 +7,6 @@ data "coder_workspace" "dev" {} resource "coder_agent" "main" { arch = data.coder_provisioner.dev.arch os = data.coder_provisioner.dev.os - dir = "/workspace" resources_monitoring { memory { enabled = true diff --git a/provider/agent.go b/provider/agent.go index 1d927863..be24f791 100644 --- a/provider/agent.go +++ b/provider/agent.go @@ -3,10 +3,12 @@ package provider import ( "context" "fmt" + "path/filepath" "reflect" "strings" "github.com/google/uuid" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -264,6 +266,7 @@ func agentResource() *schema.Resource { Description: "The resources monitoring configuration for this agent.", ForceNew: true, Optional: true, + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "memory": { @@ -271,6 +274,7 @@ func agentResource() *schema.Resource { Description: "The memory monitoring configuration for this agent.", ForceNew: true, Optional: true, + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "enabled": { @@ -284,6 +288,12 @@ func agentResource() *schema.Resource { Description: "The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", ForceNew: true, Required: true, + ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics { + if i.(int) < 0 || i.(int) > 100 { + return diag.Errorf("volume threshold must be between 0 and 100") + } + return nil + }, }, }, }, @@ -300,6 +310,17 @@ func agentResource() *schema.Resource { Description: "The path of the volume to monitor.", ForceNew: true, Required: true, + ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics { + if i.(string) == "" { + return diag.Errorf("volume path must not be empty") + } + + if !filepath.IsAbs(i.(string)) { + return diag.Errorf("volume path must be an absolute path") + } + + return nil + }, }, "enabled": { Type: schema.TypeBool, @@ -312,6 +333,12 @@ func agentResource() *schema.Resource { Description: "The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", ForceNew: true, Required: true, + ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics { + if i.(int) < 0 || i.(int) > 100 { + return diag.Errorf("volume threshold must be between 0 and 100") + } + return nil + }, }, }, }, @@ -321,29 +348,61 @@ func agentResource() *schema.Resource { }, }, CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i any) error { - if !rd.HasChange("metadata") { - return nil + if rd.HasChange("metadata") { + keys := map[string]bool{} + metadata, ok := rd.Get("metadata").([]any) + if !ok { + return xerrors.Errorf("unexpected type %T for metadata, expected []any", rd.Get("metadata")) + } + for _, t := range metadata { + obj, ok := t.(map[string]any) + if !ok { + return xerrors.Errorf("unexpected type %T for metadata, expected map[string]any", t) + } + key, ok := obj["key"].(string) + if !ok { + return xerrors.Errorf("unexpected type %T for metadata key, expected string", obj["key"]) + } + if keys[key] { + return xerrors.Errorf("duplicate agent metadata key %q", key) + } + keys[key] = true + } } - keys := map[string]bool{} - metadata, ok := rd.Get("metadata").([]any) - if !ok { - return xerrors.Errorf("unexpected type %T for metadata, expected []any", rd.Get("metadata")) - } - for _, t := range metadata { - obj, ok := t.(map[string]any) + if rd.HasChange("resources_monitoring") { + monitors, ok := rd.Get("resources_monitoring").(*schema.Set) if !ok { - return xerrors.Errorf("unexpected type %T for metadata, expected map[string]any", t) + return xerrors.Errorf("unexpected type %T for resources_monitoring.0.volume, expected []any", rd.Get("resources_monitoring.0.volume")) } - key, ok := obj["key"].(string) + + monitor := monitors.List()[0].(map[string]any) + + volumes, ok := monitor["volume"].(*schema.Set) if !ok { - return xerrors.Errorf("unexpected type %T for metadata key, expected string", obj["key"]) + return xerrors.Errorf("unexpected type %T for resources_monitoring.0.volume, expected []any", monitor["volume"]) } - if keys[key] { - return xerrors.Errorf("duplicate agent metadata key %q", key) + + paths := map[string]bool{} + for _, volume := range volumes.List() { + obj, ok := volume.(map[string]any) + if !ok { + return xerrors.Errorf("unexpected type %T for volume, expected map[string]any", volume) + } + + // print path for debug purpose + + path, ok := obj["path"].(string) + if !ok { + return xerrors.Errorf("unexpected type %T for volume path, expected string", obj["path"]) + } + if paths[path] { + return xerrors.Errorf("duplicate volume path %q", path) + } + paths[path] = true } - keys[key] = true } + return nil }, } diff --git a/provider/agent_test.go b/provider/agent_test.go index c782f137..02f20654 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -213,11 +213,13 @@ func TestAgent_Metadata(t *testing.T) { func TestAgent_ResourcesMonitoring(t *testing.T) { t.Parallel() - resource.Test(t, resource.TestCase{ - ProviderFactories: coderFactory(), - IsUnitTest: true, - Steps: []resource.TestStep{{ - Config: ` + + t.Run("OK", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` provider "coder" { url = "https://example.com" } @@ -241,65 +243,115 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { } } }`, - Check: func(state *terraform.State) error { - require.Len(t, state.Modules, 1) - require.Len(t, state.Modules[0].Resources, 1) - - resource := state.Modules[0].Resources["coder_agent.dev"] - require.NotNil(t, resource) + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) - t.Logf("resource: %v", resource.Primary.Attributes) + resource := state.Modules[0].Resources["coder_agent.dev"] + require.NotNil(t, resource) - attr := resource.Primary.Attributes - require.Equal(t, "1", attr["resources_monitoring.#"]) - require.Equal(t, "1", attr["resources_monitoring.0.memory.#"]) - require.Equal(t, "2", attr["resources_monitoring.0.volume.#"]) - require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"]) - require.Equal(t, "/volume1", attr["resources_monitoring.0.volume.0.path"]) - require.Equal(t, "100", attr["resources_monitoring.0.volume.1.threshold"]) - require.Equal(t, "/volume2", attr["resources_monitoring.0.volume.1.path"]) - return nil - }, - }}, + attr := resource.Primary.Attributes + require.Equal(t, "1", attr["resources_monitoring.#"]) + require.Equal(t, "1", attr["resources_monitoring.0.memory.#"]) + require.Equal(t, "2", attr["resources_monitoring.0.volume.#"]) + require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"]) + require.Equal(t, "/volume1", attr["resources_monitoring.0.volume.0.path"]) + require.Equal(t, "100", attr["resources_monitoring.0.volume.1.threshold"]) + require.Equal(t, "/volume2", attr["resources_monitoring.0.volume.1.path"]) + return nil + }, + }}, + }) }) -} -func TestAgent_ResourcesMonitoring_OnlyMemory(t *testing.T) { - t.Parallel() - resource.Test(t, resource.TestCase{ - ProviderFactories: coderFactory(), - IsUnitTest: true, - Steps: []resource.TestStep{{ - Config: ` - provider "coder" { - url = "https://example.com" - } - resource "coder_agent" "dev" { - os = "linux" - arch = "amd64" - resources_monitoring { - memory { - enabled = true - threshold = 80 - } + t.Run("OnlyMemory", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" } - }`, - Check: func(state *terraform.State) error { - require.Len(t, state.Modules, 1) - require.Len(t, state.Modules[0].Resources, 1) + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + memory { + enabled = true + threshold = 80 + } + } + }`, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) - resource := state.Modules[0].Resources["coder_agent.dev"] - require.NotNil(t, resource) + resource := state.Modules[0].Resources["coder_agent.dev"] + require.NotNil(t, resource) - t.Logf("resource: %v", resource.Primary.Attributes) + attr := resource.Primary.Attributes + require.Equal(t, "1", attr["resources_monitoring.#"]) + require.Equal(t, "1", attr["resources_monitoring.0.memory.#"]) + require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"]) + return nil + }, + }}, + }) + }) - attr := resource.Primary.Attributes - require.Equal(t, "1", attr["resources_monitoring.#"]) - require.Equal(t, "1", attr["resources_monitoring.0.memory.#"]) - require.Equal(t, "80", attr["resources_monitoring.0.memory.0.threshold"]) - return nil - }, - }}, + t.Run("InvalidThreshold", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + memory { + enabled = true + threshold = 101 + } + } + }`, + ExpectError: regexp.MustCompile("threshold must be between 0 and 100"), + }}, + }) + }) + + t.Run("DuplicatePaths", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + volume { + path = "/volume1" + enabled = true + threshold = 80 + } + volume { + path = "/volume1" + enabled = true + threshold = 100 + } + } + }`, + ExpectError: regexp.MustCompile("duplicate volume path"), + }}, + }) }) } diff --git a/provider/examples_test.go b/provider/examples_test.go index c6931ae3..1d17b1ba 100644 --- a/provider/examples_test.go +++ b/provider/examples_test.go @@ -15,6 +15,7 @@ func TestExamples(t *testing.T) { for _, testDir := range []string{ "coder_parameter", "coder_workspace_tags", + "coder_resources_monitoring", } { t.Run(testDir, func(t *testing.T) { testDir := testDir From d8844b8fe3b493cf5e77ead71fd1f92a945ffbda Mon Sep 17 00:00:00 2001 From: defelmnq Date: Tue, 4 Feb 2025 02:00:30 +0100 Subject: [PATCH 07/12] improved testing and validation --- provider/agent_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/provider/agent_test.go b/provider/agent_test.go index 02f20654..f4821b14 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -299,6 +299,33 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { }}, }) }) + t.Run("MultipleMemory", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + memory { + enabled = true + threshold = 80 + } + memory { + enabled = true + threshold = 90 + } + } + }`, + ExpectError: regexp.MustCompile(`No more than 1 "memory" blocks are allowed`), + }}, + }) + }) t.Run("InvalidThreshold", func(t *testing.T) { resource.Test(t, resource.TestCase{ From 3cf722946acc973617e64f87bedd1adf2d5cba9c Mon Sep 17 00:00:00 2001 From: defelmnq Date: Tue, 4 Feb 2025 02:01:57 +0100 Subject: [PATCH 08/12] improved testing and validation --- provider/agent_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/provider/agent_test.go b/provider/agent_test.go index f4821b14..0ca049f3 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -380,6 +380,30 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { }}, }) }) + + t.Run("NoPath", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + volume { + enabled = true + threshold = 80 + } + } + }`, + ExpectError: regexp.MustCompile(`The argument "path" is required, but no definition was found.`), + }}, + }) + }) } func TestAgent_MetadataDuplicateKeys(t *testing.T) { From f81c6f5807921b83168fd8d95c20bee82ba6d804 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Tue, 4 Feb 2025 02:06:22 +0100 Subject: [PATCH 09/12] regenerate documentation with validation --- docs/resources/agent.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/resources/agent.md b/docs/resources/agent.md index 46eeb26f..7c28b1f4 100644 --- a/docs/resources/agent.md +++ b/docs/resources/agent.md @@ -79,7 +79,7 @@ resource "kubernetes_pod" "dev" { - `metadata` (Block List) Each `metadata` block defines a single item consisting of a key/value pair. This feature is in alpha and may break in future releases. (see [below for nested schema](#nestedblock--metadata)) - `motd_file` (String) The path to a file within the workspace containing a message to display to users when they login via SSH. A typical value would be `"/etc/motd"`. - `order` (Number) The order determines the position of agents in the UI presentation. The lowest order is shown first and agents with equal order are sorted by name (ascending order). -- `resources_monitoring` (Block Set) The resources monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring)) +- `resources_monitoring` (Block Set, Max: 1) The resources monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring)) - `shutdown_script` (String) A script to run before the agent is stopped. The script should exit when it is done to signal that the workspace can be stopped. This option is an alias for defining a `coder_script` resource with `run_on_stop` set to `true`. - `startup_script` (String) A script to run after the agent starts. The script should exit when it is done to signal that the agent is ready. This option is an alias for defining a `coder_script` resource with `run_on_start` set to `true`. - `startup_script_behavior` (String) This option sets the behavior of the `startup_script`. When set to `"blocking"`, the `startup_script` must exit before the workspace is ready. When set to `"non-blocking"`, the `startup_script` may run in the background and the workspace will be ready immediately. Default is `"non-blocking"`, although `"blocking"` is recommended. This option is an alias for defining a `coder_script` resource with `start_blocks_login` set to `true` (blocking). @@ -124,7 +124,7 @@ Optional: Optional: -- `memory` (Block Set) The memory monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--memory)) +- `memory` (Block Set, Max: 1) The memory monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--memory)) - `volume` (Block Set) The volumes monitoring configuration for this agent. (see [below for nested schema](#nestedblock--resources_monitoring--volume)) From 6329d031bbb983bff6bb1849707611e60e08d900 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Thu, 6 Feb 2025 19:00:16 +0100 Subject: [PATCH 10/12] add more test cases --- provider/agent_test.go | 100 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/provider/agent_test.go b/provider/agent_test.go index 0ca049f3..cedae8a1 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -404,6 +404,106 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { }}, }) }) + + t.Run("NonAbsPath", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + volume { + path = "tmp" + enabled = true + threshold = 80 + } + } + }`, + Check: nil, + ExpectError: regexp.MustCompile(`volume path must be an absolute path`), + }}, + }) + }) + + t.Run("EmptyPath", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + volume { + path = "" + enabled = true + threshold = 80 + } + } + }`, + Check: nil, + ExpectError: regexp.MustCompile(`volume path must not be empty`), + }}, + }) + }) + + t.Run("ThresholdMissing", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + volume { + path = "/volume1" + enabled = true + } + } + }`, + Check: nil, + ExpectError: regexp.MustCompile(`The argument "threshold" is required, but no definition was found.`), + }}, + }) + }) + t.Run("EnabledMissing", func(t *testing.T) { + resource.Test(t, resource.TestCase{ + ProviderFactories: coderFactory(), + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: ` + provider "coder" { + url = "https://example.com" + } + resource "coder_agent" "dev" { + os = "linux" + arch = "amd64" + resources_monitoring { + memory { + threshold = 80 + } + } + }`, + Check: nil, + ExpectError: regexp.MustCompile(`The argument "enabled" is required, but no definition was found.`), + }}, + }) + }) } func TestAgent_MetadataDuplicateKeys(t *testing.T) { From 0a78f08f76dc97d271865667bd729a4dee6dc588 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Thu, 6 Feb 2025 19:55:58 +0100 Subject: [PATCH 11/12] add validation methods --- provider/agent.go | 30 ++++++++++-------------------- provider/agent_test.go | 3 ++- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/provider/agent.go b/provider/agent.go index be24f791..5a52b481 100644 --- a/provider/agent.go +++ b/provider/agent.go @@ -284,16 +284,11 @@ func agentResource() *schema.Resource { Required: true, }, "threshold": { - Type: schema.TypeInt, - Description: "The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", - ForceNew: true, - Required: true, - ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics { - if i.(int) < 0 || i.(int) > 100 { - return diag.Errorf("volume threshold must be between 0 and 100") - } - return nil - }, + Type: schema.TypeInt, + Description: "The memory usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", + ForceNew: true, + Required: true, + ValidateFunc: validation.IntBetween(0, 100), }, }, }, @@ -329,16 +324,11 @@ func agentResource() *schema.Resource { Required: true, }, "threshold": { - Type: schema.TypeInt, - Description: "The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", - ForceNew: true, - Required: true, - ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics { - if i.(int) < 0 || i.(int) > 100 { - return diag.Errorf("volume threshold must be between 0 and 100") - } - return nil - }, + Type: schema.TypeInt, + Description: "The volume usage threshold in percentage at which to trigger an alert. Value should be between 0 and 100.", + ForceNew: true, + Required: true, + ValidateFunc: validation.IntBetween(0, 100), }, }, }, diff --git a/provider/agent_test.go b/provider/agent_test.go index cedae8a1..42e37eb9 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -346,7 +346,8 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { } } }`, - ExpectError: regexp.MustCompile("threshold must be between 0 and 100"), + Check: nil, + ExpectError: regexp.MustCompile("Error running pre-apply refresh"), }}, }) }) From 1045b170f5d4e16f6fc2a7ad400c87e43c3619d9 Mon Sep 17 00:00:00 2001 From: defelmnq Date: Fri, 7 Feb 2025 06:19:27 +0100 Subject: [PATCH 12/12] improve test verbose --- provider/agent.go | 6 +++++- provider/agent_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/provider/agent.go b/provider/agent.go index 5a52b481..3ddae235 100644 --- a/provider/agent.go +++ b/provider/agent.go @@ -306,7 +306,11 @@ func agentResource() *schema.Resource { ForceNew: true, Required: true, ValidateDiagFunc: func(i interface{}, s cty.Path) diag.Diagnostics { - if i.(string) == "" { + path, ok := i.(string) + if !ok { + return diag.Errorf("volume path must be a string") + } + if path == "" { return diag.Errorf("volume path must not be empty") } diff --git a/provider/agent_test.go b/provider/agent_test.go index 42e37eb9..a45ac86a 100644 --- a/provider/agent_test.go +++ b/provider/agent_test.go @@ -347,7 +347,7 @@ func TestAgent_ResourcesMonitoring(t *testing.T) { } }`, Check: nil, - ExpectError: regexp.MustCompile("Error running pre-apply refresh"), + ExpectError: regexp.MustCompile(`expected resources_monitoring\.0\.memory\.0\.threshold to be in the range \(0 - 100\), got 101`), }}, }) })