review comments

Author: Aashiq-J

README.md

@@ -143,13 +143,13 @@ No modules.
 | <a name="input_is_vpc_cluster"></a> [is\_vpc\_cluster](#input\_is\_vpc\_cluster) | Specify true if the target cluster is a VPC cluster, false if it is a classic cluster. | `bool` | `true` | no |
 | <a name="input_kernal_module_image_repository"></a> [kernal\_module\_image\_repository](#input\_kernal\_module\_image\_repository) | The image repository to pull the agent kernal module initContainer image from. | `string` | `"agent-kmodule"` | no |
 | <a name="input_kernel_module_image_tag_digest"></a> [kernel\_module\_image\_tag\_digest](#input\_kernel\_module\_image\_tag\_digest) | The image tag or digest to use for the agent kernel module used by the initContainer. If using digest, it must be in the format of `X.Y.Z@sha256:xxxxx` | `string` | `"14.2.0@sha256:3be3966e2bef75364f55d248156a568a222afaba3067f43c5c642c46b690cca9"` | no |
-| <a name="input_max_surge"></a> [max\_surge](#input\_max\_surge) | The number of pods that can be created above the desired amount of daemonset pods during an update. By default, the `max_surge` is set to null. The variable accepts absolute number or percentage value(e.g., '1' or '10%'). | `string` | `null` | no |
+| <a name="input_max_surge"></a> [max\_surge](#input\_max\_surge) | The number of pods that can be created above the desired amount of daemonset pods during an update. If `max_surge` is set to null, the `max_surge` setting is ignored.. The variable accepts absolute number or percentage value(e.g., '1' or '10%'). | `string` | `null` | no |
 | <a name="input_max_unavailable"></a> [max\_unavailable](#input\_max\_unavailable) | The maximum number of pods that can be unavailable during a DaemonSet rolling update. Accepts absolute number or percentage (e.g., '1' or '10%'). | `string` | `"1"` | no |
 | <a name="input_metrics_filter"></a> [metrics\_filter](#input\_metrics\_filter) | To filter custom metrics you can specify which metrics to include and exclude. For more info, see https://cloud.ibm.com/docs/monitoring?topic=monitoring-change_kube_agent#change_kube_agent_inc_exc_metrics | <pre>list(object({<br/>    include = optional(string)<br/>    exclude = optional(string)<br/>  }))</pre> | `[]` | no |
 | <a name="input_name"></a> [name](#input\_name) | The name to give the agent helm release. | `string` | `"sysdig-agent"` | no |
 | <a name="input_namespace"></a> [namespace](#input\_namespace) | Namespace to deploy the agent to. | `string` | `"ibm-observe"` | no |
-| <a name="input_priority_class_name"></a> [priority\_class\_name](#input\_priority\_class\_name) | The priority class name for the PriorityClasses assigned to the monitoring agent daemonset. | `string` | `null` | no |
-| <a name="input_priority_class_value"></a> [priority\_class\_value](#input\_priority\_class\_value) | The numerical priority assigned to PriorityClass, which determines the importance of monitoring agent daemonset pod within the cluster for both scheduling and eviction decisions. | `number` | `10` | no |
+| <a name="input_priority_class_name"></a> [priority\_class\_name](#input\_priority\_class\_name) | The priority class name for the PriorityClasses assigned to the monitoring agent daemonset. If no value is passed, priority class is not used. | `string` | `null` | no |
+| <a name="input_priority_class_value"></a> [priority\_class\_value](#input\_priority\_class\_value) | The numerical priority assigned to PriorityClass, which determines the importance of monitoring agent daemonset pod within the cluster for both scheduling and eviction decisions. The value only applies if a value was passed for `priority_class_name` | `number` | `10` | no |
 | <a name="input_prometheus_config"></a> [prometheus\_config](#input\_prometheus\_config) | Prometheus configuration for the agent. If you want to enable Prometheus configuration provide the prometheus.yaml file content in `hcl` format. [Learn more](https://github.com/terraform-ibm-modules/terraform-ibm-monitoring-agent/blob/main/solutions/fully-configurable/DA-types.md#prometheus_config). | `map(any)` | `{}` | no |
 | <a name="input_tolerations"></a> [tolerations](#input\_tolerations) | List of tolerations to apply to the agent. | <pre>list(object({<br/>    key               = optional(string)<br/>    operator          = optional(string)<br/>    value             = optional(string)<br/>    effect            = optional(string)<br/>    tolerationSeconds = optional(number)<br/>  }))</pre> | <pre>[<br/>  {<br/>    "operator": "Exists"<br/>  },<br/>  {<br/>    "effect": "NoSchedule",<br/>    "key": "node-role.kubernetes.io/master",<br/>    "operator": "Exists"<br/>  }<br/>]</pre> | no |
 | <a name="input_use_private_endpoint"></a> [use\_private\_endpoint](#input\_use\_private\_endpoint) | Whether send data over a private endpoint or not. To use a private endpoint, you must enable virtual routing and forwarding (VRF) for your account. See https://cloud.ibm.com/docs/account?topic=account-vrf-service-endpoint. | `bool` | `true` | no |

examples/obs-agent-iks/main.tf

@@ -145,7 +145,7 @@ module "monitoring_agents" {
   is_vpc_cluster            = var.is_vpc_cluster
   access_key                = module.cloud_monitoring.access_key
   instance_region           = var.region
-  # priority_class_name       = "sysdig-daemonset-priority"
+  priority_class_name       = "sysdig-daemonset-priority"
   prometheus_config = {
     scrape_configs = [
       {

ibm_catalog.json

@@ -266,10 +266,24 @@
               "key": "agent_limits_memory"
             },
             {
-              "key": "max_surge"
+              "key": "max_surge",
+              "value_constraints": [
+                {
+                  "type": "regex",
+                  "description": "The value provided for 'max_surge' must be a positive integer (e.g., '1') or a percentage (e.g., '10%'), or null.",
+                  "value": "^__NULL__$|^\\d+%?$"
+                }
+              ]
             },
             {
-              "key": "max_unavailable"
+              "key": "max_unavailable",
+              "value_constraints": [
+                {
+                  "type": "regex",
+                  "description": "The value provided for 'max_unavailable' must be a positive integer (e.g., '1') or a percentage (e.g., '10%').",
+                  "value": "^\\d+%?$"
+                }
+              ]
             },
             {
               "key": "priority_class_name"

solutions/fully-configurable/variables.tf

@@ -254,21 +254,21 @@ variable "max_unavailable" {
 
 variable "max_surge" {
   type        = string
-  description = "The number of pods that can be created above the desired amount of daemonset pods during an update. By default, the `max_surge` is set to null. The variable accepts absolute number or percentage value(e.g., '1' or '10%')."
+  description = "The number of pods that can be created above the desired amount of daemonset pods during an update. If `max_surge` is set to null, the `max_surge` setting is ignored.. The variable accepts absolute number or percentage value(e.g., '1' or '10%')."
   default     = null
 }
 
 variable "priority_class_name" {
   type        = string
-  description = "The priority class name for the PriorityClasses assigned to the monitoring agent daemonset."
+  description = "The priority class name for the PriorityClasses assigned to the monitoring agent daemonset. If no value is passed, priority class is not used."
   default     = null
 }
 
 variable "priority_class_value" {
   type        = number
-  description = "The numerical priority assigned to PriorityClass, which determines the importance of monitoring agent daemonset pod within the cluster for both scheduling and eviction decisions."
-  default     = 10
   nullable    = false
+  description = "The numerical priority assigned to PriorityClass, which determines the importance of monitoring agent daemonset pod within the cluster for both scheduling and eviction decisions. The value only applies if a value was passed for `priority_class_name`"
+  default     = 10
 }
 
 ##############################################################################

variables.tf

@@ -253,13 +253,13 @@ variable "max_unavailable" {
   nullable    = false
   validation {
     condition     = can(regex("^\\d+%?$", var.max_unavailable))
-    error_message = "max_unavailable must be a positive integer (e.g., '1') or a percentage (e.g., '10%')"
+    error_message = "max_unavailable must be a positive integer (e.g., '1') or a percentage (e.g., '10%')."
   }
 }
 
 variable "max_surge" {
   type        = string
-  description = "The number of pods that can be created above the desired amount of daemonset pods during an update. By default, the `max_surge` is set to null. The variable accepts absolute number or percentage value(e.g., '1' or '10%')."
+  description = "The number of pods that can be created above the desired amount of daemonset pods during an update. If `max_surge` is set to null, the `max_surge` setting is ignored.. The variable accepts absolute number or percentage value(e.g., '1' or '10%')."
   default     = null
   validation {
     condition = (
@@ -272,14 +272,14 @@ variable "max_surge" {
 
 variable "priority_class_name" {
   type        = string
-  description = "The priority class name for the PriorityClasses assigned to the monitoring agent daemonset."
+  description = "The priority class name for the PriorityClasses assigned to the monitoring agent daemonset. If no value is passed, priority class is not used."
   default     = null
 }
 
 variable "priority_class_value" {
   type        = number
   nullable    = false
-  description = "The numerical priority assigned to PriorityClass, which determines the importance of monitoring agent daemonset pod within the cluster for both scheduling and eviction decisions."
+  description = "The numerical priority assigned to PriorityClass, which determines the importance of monitoring agent daemonset pod within the cluster for both scheduling and eviction decisions. The value only applies if a value was passed for `priority_class_name`"
   default     = 10
 }