Skip to content

Sub-block targeting and validation #5405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
SikaGrr wants to merge 3 commits into
base: develop
Choose a base branch
from
+72 −5
Draft

Sub-block targeting and validation #5405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
249a7dd
add subblock variables and validation
SikaGrr Mar 24, 2026
c5008d3
Merge branch 'develop' into origin/subblock
SikaGrr Mar 24, 2026
a1b0a4e
multiple blocks
SikaGrr Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions modules/compute/gke-node-pool/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,18 @@ resource "google_container_node_pool" "node_pool" {
condition = var.spot == true ? (var.reservation_affinity.consume_reservation_type == "NO_RESERVATION") : true
error_message = "Spot consumption option only works with reservation_affinity consume_reservation_type NO_RESERVATION."
}
precondition {
condition = (var.reservation_sub_block == null || var.reservation_sub_block == "") || (var.enable_slice_controller && var.reservation_block != null && var.reservation_block != "")
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Leveraging the is_sub_block_targeting_enabled local variable here simplifies the precondition's condition expression. This makes the validation logic more readable and consistent with other parts of the module where this complex condition is used.

This aligns with the repository's focus on "Maintainability: Code should be easy to understand, modify, and extend." (Repository Style Guide, line 6).

      condition     = (var.reservation_sub_block == null || var.reservation_sub_block == "") || local.is_sub_block_targeting_enabled
References
  1. Code should be easy to understand, modify, and extend. (link)

error_message = "reservation_sub_block can only be used when enable_slice_controller is true and reservation_block is set."
}
precondition {
condition = !var.enable_slice_controller || (var.reservation_sub_block == null || var.reservation_sub_block == "") || alltrue([for sb in data.google_compute_reservation_sub_block.this : (sb.sub_block_count - sb.in_use_count) >= local.requested_nodes])
error_message = "Insufficient capacity in the specified reservation sub-block(s)."
}
precondition {
condition = !var.enable_slice_controller || (var.reservation_sub_block == null || var.reservation_sub_block == "") || alltrue([for sb in data.google_compute_reservation_sub_block.this : sb.health_info[0].health_status == "HEALTHY"])
error_message = "The targeted reservation sub-block(s) are not HEALTHY."
}
}
}

Expand Down
47 changes: 42 additions & 5 deletions modules/compute/gke-node-pool/reservation_definitions.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,36 @@ data "google_compute_reservation" "specific_reservations" {
project = each.value.project
}

data "google_compute_reservation_sub_block" "this" {
for_each = (
(var.enable_slice_controller && var.reservation_block != null && var.reservation_block != "" && var.reservation_sub_block != null && var.reservation_sub_block != "") ?
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using the newly introduced is_sub_block_targeting_enabled local variable here improves readability by abstracting the complex conditional logic. This makes the for_each condition more concise and easier to grasp at a glance.

This aligns with the repository's focus on "Maintainability: Code should be easy to understand, modify, and extend." (Repository Style Guide, line 6).

    local.is_sub_block_targeting_enabled ?
References
  1. Code should be easy to understand, modify, and extend. (link)

{
for pair in flatten([
for zone in try(var.zones, []) : [
for i, reservation_name in try(local.input_reservation_names, []) : {
key : "${local.input_reservation_projects[i]}/${zone}/${reservation_name}"
zone : zone
reservation_name : reservation_name
project : local.input_reservation_projects[i]
}
]
]) :
pair.key => pair
} :
{}
)

project = each.value.project
zone = each.value.zone
reservation_name = each.value.reservation_name
reservation_block = var.reservation_block
name = var.reservation_sub_block
}

locals {
requested_nodes = coalesce(var.static_node_count, var.initial_node_count, var.autoscaling_total_max_nodes, 0)
}
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve readability and reduce repetition, consider extracting the complex conditional logic for enable_slice_controller and reservation block/sub-block variables into a local variable. This makes the code easier to understand and maintain.

This aligns with the repository's focus on "Maintainability: Code should be easy to understand, modify, and extend." (Repository Style Guide, line 6).

  requested_nodes = coalesce(var.static_node_count, var.initial_node_count, var.autoscaling_total_max_nodes, 0)
  is_sub_block_targeting_enabled = var.enable_slice_controller && var.reservation_block != null && var.reservation_block != "" && var.reservation_sub_block != null && var.reservation_sub_block != ""
References
  1. Code should be easy to understand, modify, and extend. (link)


locals {
generated_guest_accelerator = module.gpu.machine_type_guest_accelerator
reservation_resource_api_label = "compute.googleapis.com/reservation-name"
Expand Down Expand Up @@ -96,12 +126,19 @@ locals {
# Build the list of reservation names when var.is_reservation_active is true
active_reservation_values = [
for i, r in local.verified_specific_reservations :
length(local.input_reservation_suffixes[i]) > 0 ?
format("%s%s", r.name, local.input_reservation_suffixes[i]) :
"projects/${r.project}/reservations/${r.name}"
(var.enable_slice_controller && var.reservation_block != null && var.reservation_block != "" && var.reservation_sub_block != null && var.reservation_sub_block != "") ?
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using the is_sub_block_targeting_enabled local variable here enhances readability by simplifying the conditional expression for active_reservation_values. This reduces duplication and makes the intent clearer.

This aligns with the repository's focus on "Maintainability: Code should be easy to understand, modify, and extend." (Repository Style Guide, line 6).

    local.is_sub_block_targeting_enabled ?
References
  1. Code should be easy to understand, modify, and extend. (link)

"projects/${r.project}/zones/${r.zone}/reservations/${r.name}/reservationBlocks/${var.reservation_block}/reservationSubBlocks/${var.reservation_sub_block}" :
(length(local.input_reservation_suffixes[i]) > 0 ?
format("%s%s", r.name, local.input_reservation_suffixes[i]) :
"projects/${r.project}/reservations/${r.name}")
]

# Define a default reservation value if no specific reservations are present
specific_reservation_name = length(local.input_reservation_names) > 0 ? local.input_reservation_names[0] : ""
default_reservation_values = ["projects/${var.project_id}/reservations/${local.specific_reservation_name}"]
specific_reservation_name = length(local.input_reservation_names) > 0 ? local.input_reservation_names[0] : ""
default_reservation_values = [
for zone in try(var.zones, []) :
(var.enable_slice_controller && var.reservation_block != null && var.reservation_block != "" && var.reservation_sub_block != null && var.reservation_sub_block != "") ?
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Replacing the repeated conditional logic with the is_sub_block_targeting_enabled local variable in default_reservation_values improves code clarity and maintainability. This helps in avoiding errors if the underlying logic needs to change in the future.

This aligns with the repository's focus on "Maintainability: Code should be easy to understand, modify, and extend." (Repository Style Guide, line 6).

    local.is_sub_block_targeting_enabled ?
References
  1. Code should be easy to understand, modify, and extend. (link)

"projects/${var.project_id}/zones/${zone}/reservations/${local.specific_reservation_name}/reservationBlocks/${var.reservation_block}/reservationSubBlocks/${var.reservation_sub_block}" :
"projects/${var.project_id}/reservations/${local.specific_reservation_name}"
]
}
18 changes: 18 additions & 0 deletions modules/compute/gke-node-pool/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,24 @@ variable "reservation_affinity" {
}
}

variable "reservation_block" {
description = "The name of the reservation block."
type = string
default = null
}

variable "reservation_sub_block" {
description = "The name of the reservation sub-block."
type = string
default = null
}

variable "enable_slice_controller" {
description = "Enables the GKE Slice Controller for Super-slicing topologies."
type = bool
default = false
}

variable "host_maintenance_interval" {
description = "Specifies the frequency of planned maintenance events."
type = string
Expand Down
Loading