review comments

Author: Aashiq-J

README.md

@@ -132,7 +132,6 @@ No modules.
 | <a name="input_cluster_shield_requests_cpu"></a> [cluster\_shield\_requests\_cpu](#input\_cluster\_shield\_requests\_cpu) | Specify CPU resource requests for the cluster shield pods. | `string` | `"500m"` | no |
 | <a name="input_cluster_shield_requests_memory"></a> [cluster\_shield\_requests\_memory](#input\_cluster\_shield\_requests\_memory) | Specify memory resource requests for the cluster shield pods. | `string` | `"512Mi"` | no |
 | <a name="input_container_filter"></a> [container\_filter](#input\_container\_filter) | Customize the agent to exclude containers from metrics collection. For more info, see https://cloud.ibm.com/docs/monitoring?topic=monitoring-change_kube_agent#change_kube_agent_filter_data | <pre>list(object({<br/>    type      = string<br/>    parameter = string<br/>    name      = string<br/>  }))</pre> | `[]` | no |
-| <a name="input_create_priority_class"></a> [create\_priority\_class](#input\_create\_priority\_class) | Whether to create a priority class for the sysdig agent daemonset. | `bool` | `false` | no |
 | <a name="input_deployment_tag"></a> [deployment\_tag](#input\_deployment\_tag) | Sets a global tag that will be included in the components. It represents the mechanism from where the components have been installed (terraform, local...). | `string` | `"terraform"` | no |
 | <a name="input_enable_host_scanner"></a> [enable\_host\_scanner](#input\_enable\_host\_scanner) | Enable host scanning to detect vulnerabilities and identify the resolution priority based on available fixed versions and severity. Requires a Security and Compliance Center Workload Protection instance to view results. | `bool` | `true` | no |
 | <a name="input_enable_kspm_analyzer"></a> [enable\_kspm\_analyzer](#input\_enable\_kspm\_analyzer) | Enable Kubernetes Security Posture Management (KSPM) analyzer. Requires a Security and Compliance Center Workload Protection instance to view results. | `bool` | `true` | no |
@@ -144,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 maximum number of nodes that can have an extra DaemonSet pod during a rolling update. Accepts absolute number or percentage (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. 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_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 sysdig 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 sysdig 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. | `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_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,8 +145,7 @@ module "monitoring_agents" {
   is_vpc_cluster            = var.is_vpc_cluster
   access_key                = module.cloud_monitoring.access_key
   instance_region           = var.region
-  create_priority_class     = true
-  priority_class_name       = "sysdig-daemonset-priority"
+  # priority_class_name       = "sysdig-daemonset-priority"
   prometheus_config = {
     scrape_configs = [
       {

ibm_catalog.json

@@ -265,9 +265,6 @@
             {
               "key": "agent_limits_memory"
             },
-            {
-              "key": "create_priority_class"
-            },
             {
               "key": "max_surge"
             },

main.tf

@@ -252,9 +252,11 @@ resource "helm_release" "cloud_monitoring_agent" {
 %{for line in split("\n", yamlencode(var.prometheus_config))~}
       ${line}
 %{endfor~}
-  "createPriorityClass": ${var.create_priority_class}
-  "priorityClassName": ${var.priority_class_name == null ? "null" : var.priority_class_name}
+%{if var.priority_class_name != null}
+  "createPriorityClass": true
+  "priorityClassName": ${var.priority_class_name}
   "priorityClassValue": ${var.priority_class_value}
+%{endif}
   "daemonset":
     "updateStrategy":
       "type": "RollingUpdate"

solutions/fully-configurable/main.tf

@@ -55,7 +55,6 @@ module "monitoring_agent" {
   cluster_shield_limits_memory    = var.cluster_shield_limits_memory
   max_unavailable                 = var.max_unavailable
   max_surge                       = var.max_surge
-  create_priority_class           = var.create_priority_class
   priority_class_name             = var.priority_class_name
   priority_class_value            = var.priority_class_value
 }

solutions/fully-configurable/variables.tf

@@ -249,30 +249,26 @@ variable "max_unavailable" {
   type        = string
   description = "The maximum number of pods that can be unavailable during a DaemonSet rolling update. Accepts absolute number or percentage (e.g., '1' or '10%')."
   default     = "1"
+  nullable    = false
 }
 
 variable "max_surge" {
   type        = string
-  description = "The maximum number of nodes that can have an extra DaemonSet pod during a rolling update. Accepts absolute number or percentage (e.g., '1' or '10%')."
+  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%')."
   default     = null
 }
 
-variable "create_priority_class" {
-  type        = bool
-  description = "Whether to create a priority class for the sysdig agent daemonset."
-  default     = false
-}
-
 variable "priority_class_name" {
   type        = string
-  description = "The priority class name for the PriorityClasses assigned to the sysdig daemonset."
+  description = "The priority class name for the PriorityClasses assigned to the monitoring agent daemonset."
   default     = null
 }
 
 variable "priority_class_value" {
   type        = number
-  description = "The numerical priority assigned to PriorityClass, which determines the importance of sysdig 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."
   default     = 10
+  nullable    = false
 }
 
 ##############################################################################

variables.tf

@@ -250,6 +250,7 @@ variable "max_unavailable" {
   type        = string
   description = "The maximum number of pods that can be unavailable during a DaemonSet rolling update. Accepts absolute number or percentage (e.g., '1' or '10%')."
   default     = "1"
+  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%')"
@@ -258,7 +259,7 @@ variable "max_unavailable" {
 
 variable "max_surge" {
   type        = string
-  description = "The maximum number of nodes that can have an extra DaemonSet pod during a rolling update. Accepts absolute number or percentage (e.g., '1' or '10%')."
+  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%')."
   default     = null
   validation {
     condition = (
@@ -269,26 +270,16 @@ variable "max_surge" {
   }
 }
 
-variable "create_priority_class" {
-  type        = bool
-  description = "Whether to create a priority class for the sysdig agent daemonset."
-  default     = false
-}
-
 variable "priority_class_name" {
   type        = string
-  description = "The priority class name for the PriorityClasses assigned to the sysdig daemonset."
+  description = "The priority class name for the PriorityClasses assigned to the monitoring agent daemonset."
   default     = null
-
-  validation {
-    condition     = var.create_priority_class ? var.priority_class_name == null ? false : true : true
-    error_message = "When 'create_priority_class' is set to true, a value for 'priority_class_name' should be passed."
-  }
 }
 
 variable "priority_class_value" {
   type        = number
-  description = "The numerical priority assigned to PriorityClass, which determines the importance of sysdig daemonset pod within the cluster for both scheduling and eviction decisions."
+  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."
   default     = 10
 }