Merge pull request #145 from amandahla/OCM-24713-node-drain-grace-period

OCM-24713 | feat(machine-pool): add node_drain_grace_period attribute

Author: openshift-merge-bot[bot]

Makefile

@@ -72,12 +72,12 @@ gitleaks: $(GITLEAKS)
 # Intended single OpenShift Prow presubmit after openshift/release switches from verify + verify-gen.
 .PHONY: pre-push-checks
 pre-push-checks: tools
-	@$(MAKE) --no-print-directory verify
-	@$(MAKE) --no-print-directory verify-gen
-	@$(MAKE) --no-print-directory lint
-	@$(MAKE) --no-print-directory unit-tests
 	@$(MAKE) --no-print-directory license-check
 	@$(MAKE) --no-print-directory docs-lint
+	@$(MAKE) --no-print-directory verify-gen
+	@$(MAKE) --no-print-directory unit-tests
+	@$(MAKE) --no-print-directory lint
+	@$(MAKE) --no-print-directory verify
 
 # Prow today (until consolidated): verify-format → make verify, verify-gen → make verify-gen.
 # https://github.com/openshift/release/tree/master/ci-operator/config/terraform-redhat/terraform-rhcs-rosa-hcp

README.md

@@ -157,7 +157,7 @@ We recommend you install the following CLI tools:
 | <a name="input_kubelet_configs"></a> [kubelet\_configs](#input\_kubelet\_configs) | Provides a generic approach to add multiple kubelet configs after the creation of the cluster. This variable allows users to specify configurations for multiple kubelet configs in a flexible and customizable manner, facilitating the management of resources post-cluster deployment. For additional details regarding the variables used, refer to the [kubelet-configs sub-module](./modules/kubelet-configs). For non-primitive variables (such as maps, lists, and objects), supply the JSON-encoded string. | `map(any)` | `{}` | no |
 | <a name="input_log_forwarders"></a> [log\_forwarders](#input\_log\_forwarders) | Provides a typed map to add multiple log forwarders after cluster creation. Each entry maps to one rhcs\_log\_forwarder and must specify exactly one destination (s3 or cloudwatch), plus at least one non-empty applications or groups entry. For additional details, refer to the [log-forwarder sub-module](./modules/log-forwarder). Requires terraform-redhat/rhcs provider version that includes the rhcs\_log\_forwarder resource. | <pre>map(object({<br/>    s3 = optional(object({<br/>      bucket_name   = string<br/>      bucket_prefix = optional(string)<br/>    }))<br/>    cloudwatch = optional(object({<br/>      log_group_name            = string<br/>      log_distribution_role_arn = string<br/>    }))<br/>    applications = optional(list(string))<br/>    groups = optional(list(object({<br/>      id      = string<br/>      version = optional(string)<br/>    })))<br/>  }))</pre> | `{}` | no |
 | <a name="input_machine_cidr"></a> [machine\_cidr](#input\_machine\_cidr) | Block of IP addresses used by OpenShift while installing the cluster, for example "10.0.0.0/16". | `string` | `null` | no |
-| <a name="input_machine_pools"></a> [machine\_pools](#input\_machine\_pools) | Provides a typed map to add multiple machine pools after cluster creation. Each key is an arbitrary label; each value aligns with the [machine-pool](./modules/machine-pool) submodule (required: name, subnet\_id, openshift\_version, aws\_node\_pool). Optional fields match that module's optional inputs; omit autoscaling to use a fixed replica count with autoscaling disabled. | <pre>map(object({<br/>    name              = string<br/>    subnet_id         = string<br/>    openshift_version = string<br/>    aws_node_pool = object({<br/>      instance_type                   = string<br/>      tags                            = map(string)<br/>      additional_security_group_ids   = optional(list(string))<br/>      capacity_reservation_id         = optional(string)<br/>      capacity_reservation_preference = optional(string)<br/>    })<br/>    autoscaling = optional(object({<br/>      enabled      = bool<br/>      min_replicas = number<br/>      max_replicas = number<br/>    }))<br/>    replicas    = optional(number)<br/>    auto_repair = optional(bool)<br/>    taints = optional(list(object({<br/>      key           = string<br/>      value         = string<br/>      schedule_type = string<br/>    })))<br/>    labels                       = optional(map(string))<br/>    tuning_configs               = optional(list(string))<br/>    upgrade_acknowledgements_for = optional(string)<br/>    kubelet_configs              = optional(string)<br/>    ignore_deletion_error        = optional(bool)<br/>  }))</pre> | `{}` | no |
+| <a name="input_machine_pools"></a> [machine\_pools](#input\_machine\_pools) | Provides a typed map to add multiple machine pools after cluster creation. Each key is an arbitrary label; each value aligns with the [machine-pool](./modules/machine-pool) submodule (required: name, subnet\_id, openshift\_version, aws\_node\_pool). Optional fields match that module's optional inputs; omit autoscaling to use a fixed replica count with autoscaling disabled. | <pre>map(object({<br/>    name              = string<br/>    subnet_id         = string<br/>    openshift_version = string<br/>    aws_node_pool = object({<br/>      instance_type                   = string<br/>      tags                            = map(string)<br/>      additional_security_group_ids   = optional(list(string))<br/>      capacity_reservation_id         = optional(string)<br/>      capacity_reservation_preference = optional(string)<br/>      node_drain_grace_period         = optional(number)<br/>    })<br/>    autoscaling = optional(object({<br/>      enabled      = bool<br/>      min_replicas = number<br/>      max_replicas = number<br/>    }))<br/>    replicas    = optional(number)<br/>    auto_repair = optional(bool)<br/>    taints = optional(list(object({<br/>      key           = string<br/>      value         = string<br/>      schedule_type = string<br/>    })))<br/>    labels                       = optional(map(string))<br/>    tuning_configs               = optional(list(string))<br/>    upgrade_acknowledgements_for = optional(string)<br/>    kubelet_configs              = optional(string)<br/>    ignore_deletion_error        = optional(bool)<br/>  }))</pre> | `{}` | no |
 | <a name="input_managed_oidc"></a> [managed\_oidc](#input\_managed\_oidc) | OIDC type managed or unmanaged oidc. Only active when create\_oidc also enabled. This value should not be updated, please create a new resource instead | `bool` | `true` | no |
 | <a name="input_no_proxy"></a> [no\_proxy](#input\_no\_proxy) | A comma-separated list of destination domain names, domains, IP addresses or other network CIDRs to exclude proxying. | `string` | `null` | no |
 | <a name="input_oidc_config_id"></a> [oidc\_config\_id](#input\_oidc\_config\_id) | The unique identifier associated with users authenticated through OpenID Connect (OIDC) within the ROSA cluster. If create\_oidc is false this attribute is required. | `string` | `null` | no |

modules/machine-pool/README.md

@@ -37,13 +37,13 @@ module "mp" {
 | Name | Version |
 | ---- | ------- |
 | <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.0 |
-| <a name="requirement_rhcs"></a> [rhcs](#requirement\_rhcs) | >= 1.7.3 |
+| <a name="requirement_rhcs"></a> [rhcs](#requirement\_rhcs) | >= 1.7.7 |
 
 ## Providers
 
 | Name | Version |
 | ---- | ------- |
-| <a name="provider_rhcs"></a> [rhcs](#provider\_rhcs) | >= 1.7.3 |
+| <a name="provider_rhcs"></a> [rhcs](#provider\_rhcs) | >= 1.7.7 |
 
 ## Modules
 
@@ -61,7 +61,7 @@ No modules.
 | ---- | ----------- | ---- | ------- | :------: |
 | <a name="input_auto_repair"></a> [auto\_repair](#input\_auto\_repair) | Configures auto repair option for the pool. | `bool` | `true` | no |
 | <a name="input_autoscaling"></a> [autoscaling](#input\_autoscaling) | Configures autoscaling for the pool. | <pre>object({<br/>    enabled      = bool<br/>    min_replicas = number<br/>    max_replicas = number<br/>  })</pre> | <pre>{<br/>  "enabled": false,<br/>  "max_replicas": null,<br/>  "min_replicas": null<br/>}</pre> | no |
-| <a name="input_aws_node_pool"></a> [aws\_node\_pool](#input\_aws\_node\_pool) | Configures aws settings for the pool. | <pre>object({<br/>    instance_type                   = string<br/>    tags                            = map(string)<br/>    additional_security_group_ids   = optional(list(string))<br/>    capacity_reservation_id         = optional(string)<br/>    capacity_reservation_preference = optional(string)<br/>  })</pre> | n/a | yes |
+| <a name="input_aws_node_pool"></a> [aws\_node\_pool](#input\_aws\_node\_pool) | Configures aws settings for the pool. | <pre>object({<br/>    instance_type                   = string<br/>    tags                            = map(string)<br/>    additional_security_group_ids   = optional(list(string))<br/>    capacity_reservation_id         = optional(string)<br/>    capacity_reservation_preference = optional(string)<br/>    node_drain_grace_period         = optional(number)<br/>  })</pre> | n/a | yes |
 | <a name="input_cluster_id"></a> [cluster\_id](#input\_cluster\_id) | Identifier of the cluster. | `string` | n/a | yes |
 | <a name="input_ignore_deletion_error"></a> [ignore\_deletion\_error](#input\_ignore\_deletion\_error) | Ignore machine pool deletion error. Assists when cluster resource is managed within the same file for the destroy use case | `bool` | `false` | no |
 | <a name="input_kubelet_configs"></a> [kubelet\_configs](#input\_kubelet\_configs) | Name of the kubelet configs to attach to this machine pool. The kubelet configs must already exist | `string` | `null` | no |

modules/machine-pool/tests/aws_node_pool.tftest.hcl

@@ -13,6 +13,7 @@ mock_provider "rhcs" {
     defaults = {
       aws_node_pool = {
         capacity_reservation_preference = "defined_by_provider"
+        node_drain_grace_period         = null
       }
     }
   }
@@ -153,3 +154,83 @@ run "apply_with_capres_preference" {
     error_message = "Setup run should have capacity_reservation_preference = 'none'."
   }
 }
+
+# Test that invalid node_drain_grace_period > 10080 fails validation
+run "invalid_node_drain_grace_period_fails" {
+  command = plan
+
+  providers = {
+    rhcs = rhcs.no_override
+  }
+
+  variables {
+    cluster_id        = "fake-cluster-123"
+    name              = "test-pool"
+    subnet_id         = "subnet-fake123"
+    openshift_version = "4.15.0"
+
+    aws_node_pool = {
+      instance_type           = "m5.xlarge"
+      tags                    = {}
+      node_drain_grace_period = 10081
+    }
+  }
+
+  expect_failures = [
+    var.aws_node_pool,
+  ]
+}
+
+# Test that valid node_drain_grace_period (60 minutes) passes validation and wires through to the resource
+run "valid_node_drain_grace_period_plan" {
+  command = plan
+
+  providers = {
+    rhcs = rhcs.no_override
+  }
+
+  variables {
+    cluster_id        = "fake-cluster-123"
+    name              = "test-pool"
+    subnet_id         = "subnet-fake123"
+    openshift_version = "4.15.0"
+
+    aws_node_pool = {
+      instance_type           = "m5.xlarge"
+      tags                    = {}
+      node_drain_grace_period = 60
+    }
+  }
+
+  assert {
+    condition     = rhcs_hcp_machine_pool.machine_pool.aws_node_pool.node_drain_grace_period == 60
+    error_message = "Expected node_drain_grace_period to be wired through to rhcs_hcp_machine_pool as 60."
+  }
+}
+
+# Test that null node_drain_grace_period passes validation and wires through to the resource as null
+run "node_drain_grace_period_null_plan" {
+  command = plan
+
+  providers = {
+    rhcs = rhcs.with_override
+  }
+
+  variables {
+    cluster_id        = "fake-cluster-123"
+    name              = "test-pool"
+    subnet_id         = "subnet-fake123"
+    openshift_version = "4.15.0"
+
+    aws_node_pool = {
+      instance_type           = "m5.xlarge"
+      tags                    = {}
+      node_drain_grace_period = null
+    }
+  }
+
+  assert {
+    condition     = rhcs_hcp_machine_pool.machine_pool.aws_node_pool.node_drain_grace_period == null
+    error_message = "Expected null node_drain_grace_period to be wired through to rhcs_hcp_machine_pool as null."
+  }
+}

modules/machine-pool/variables.tf

@@ -63,6 +63,7 @@ variable "aws_node_pool" {
     additional_security_group_ids   = optional(list(string))
     capacity_reservation_id         = optional(string)
     capacity_reservation_preference = optional(string)
+    node_drain_grace_period         = optional(number)
   })
   nullable    = false
   description = "Configures aws settings for the pool."
@@ -74,6 +75,22 @@ variable "aws_node_pool" {
     )
     error_message = "capacity_reservation_preference must be one of: none, open, capacity-reservations-only."
   }
+
+  validation {
+    condition = var.aws_node_pool.node_drain_grace_period == null ? true : (
+      var.aws_node_pool.node_drain_grace_period >= 0 &&
+      var.aws_node_pool.node_drain_grace_period <= 10080
+    )
+    error_message = "node_drain_grace_period must be between 0 and 10080 minutes (7 days)."
+  }
+  validation {
+    condition     = var.aws_node_pool.node_drain_grace_period == null ? true : tonumber(var.aws_node_pool.node_drain_grace_period) == floor(var.aws_node_pool.node_drain_grace_period)
+    error_message = "node_drain_grace_period value should be an integer (minutes)"
+  }
+  validation {
+    condition     = var.aws_node_pool.node_drain_grace_period == null ? true : var.aws_node_pool.node_drain_grace_period >= 0
+    error_message = "node_drain_grace_period value should be a positive integer (minutes)"
+  }
 }
 
 variable "auto_repair" {

modules/machine-pool/versions.tf

@@ -6,7 +6,7 @@ terraform {
 
   required_providers {
     rhcs = {
-      version = ">= 1.7.3"
+      version = ">= 1.7.7"
       source  = "terraform-redhat/rhcs"
     }
   }

variables.tf

@@ -397,6 +397,7 @@ variable "machine_pools" {
       additional_security_group_ids   = optional(list(string))
       capacity_reservation_id         = optional(string)
       capacity_reservation_preference = optional(string)
+      node_drain_grace_period         = optional(number)
     })
     autoscaling = optional(object({
       enabled      = bool