Support fixed IP addresses for nodes (#643)

* Support fixed IP addresses for control node

The variable control_ip_address was documented but not implemented.
Since we support multiple networks, change it to control_ip_addresses
and implement it.

Closes #642.

* Update environments/skeleton/{{cookiecutter.environment}}/tofu/variables.tf

* tf style tweaks

* update production docs for control_ip_addresses

* fix control IP address logic

* add validation for control_ip_addresses

* remove fixed IPs from production docs - not standard process

* support ip_addresses fo all nodes

* make stackhpc tofu format consistent

---------

Co-authored-by: Steve Brasier <[email protected]>
Co-authored-by: Steve Brasier <[email protected]>

Author: priteau

docs/production.md

@@ -134,23 +134,6 @@ and referenced from the `site` and `production` environments, e.g.:
 
 - Consider whether having (read-only) access to Grafana without login is OK. If not, remove `grafana_auth_anonymous` in `environments/$ENV/inventory/group_vars/all/grafana.yml`
 
-- Modify `environments/site/tofu/nodes.tf` to provide fixed IPs for at least
-  the control node, and (if not using FIPs) the login node(s):
-
-    ```
-    resource "openstack_networking_port_v2" "control" {
-        ...
-        fixed_ip {
-            subnet_id = data.openstack_networking_subnet_v2.cluster_subnet.id
-            ip_address = var.control_ip_address
-        }
-    }
-    ```
-    
-  Note the variable `control_ip_address` is new.
-
-  Using fixed IPs will require either using admin credentials or policy changes.
-
 - If floating IPs are required for login nodes, modify the OpenTofu configurations
   appropriately.
 

environments/.stackhpc/tofu/main.tf

@@ -69,23 +69,23 @@ module "cluster" {
     control_node_flavor = var.control_node_flavor
 
     login = {
-        login: {
-            nodes: ["login-0"]
-            flavor: var.other_node_flavor
+        login = {
+            nodes = ["login-0"]
+            flavor = var.other_node_flavor
         }
     }
     compute = {
-        standard: { # NB: can't call this default!
-            nodes: ["compute-0", "compute-1"]
-            flavor: var.other_node_flavor
-            compute_init_enable: ["compute", "chrony", "etc_hosts", "nfs", "basic_users", "eessi", "tuned", "cacerts"]
-            ignore_image_changes: true
+        standard = { # NB: can't call this default!
+            nodes = ["compute-0", "compute-1"]
+            flavor = var.other_node_flavor
+            compute_init_enable = ["compute", "chrony", "etc_hosts", "nfs", "basic_users", "eessi", "tuned", "cacerts"]
+            ignore_image_changes = true
         }
         # Normally-empty partition for testing:
-        extra: {
-            nodes: []
-            #nodes: ["extra-0", "extra-1"]
-            flavor: var.other_node_flavor
+        extra = {
+            nodes = []
+            #nodes = ["extra-0", "extra-1"]
+            flavor = var.other_node_flavor
         }
     }
 

environments/skeleton/{{cookiecutter.environment}}/tofu/compute.tf

@@ -30,6 +30,7 @@ module "compute" {
   ignore_image_changes = lookup(each.value, "ignore_image_changes", null)
   match_ironic_node = lookup(each.value, "match_ironic_node", null)
   availability_zone = lookup(each.value, "availability_zone", null)
+  ip_addresses = lookup(each.value, "ip_addresses", null)
 
   # computed
   # not using openstack_compute_instance_v2.control.access_ip_v4 to avoid
@@ -55,6 +56,7 @@ module "compute" {
     "extra_volumes",
     "match_ironic_node",
     "availability_zone",
+    "ip_addresses",
     "gateway_ip",
     "nodename_template",
   ]

environments/skeleton/{{cookiecutter.environment}}/tofu/control.tf

@@ -24,7 +24,8 @@ resource "openstack_networking_port_v2" "control" {
   admin_state_up = "true"
 
   fixed_ip {
-    subnet_id = data.openstack_networking_subnet_v2.cluster_subnet[each.key].id
+    subnet_id  = data.openstack_networking_subnet_v2.cluster_subnet[each.key].id
+    ip_address = lookup(var.control_ip_addresses, each.key, null)
   }
 
   no_security_groups = lookup(each.value, "no_security_groups", false)

environments/skeleton/{{cookiecutter.environment}}/tofu/login.tf

@@ -30,6 +30,7 @@ module "login" {
   fip_network = lookup(each.value, "fip_network", null)
   match_ironic_node = lookup(each.value, "match_ironic_node", null)
   availability_zone = lookup(each.value, "availability_zone", null)
+  ip_addresses = lookup(each.value, "ip_addresses", null)
 
   # can't be set for login
   compute_init_enable = []
@@ -59,6 +60,7 @@ module "login" {
     "fip_network",
     "match_ironic_node",
     "availability_zone",
+    "ip_addresses",
     "gateway_ip",
     "nodename_template",
   ]

environments/skeleton/{{cookiecutter.environment}}/tofu/node_group/nodes.tf

@@ -47,22 +47,26 @@ resource "openstack_compute_volume_attach_v2" "compute" {
 resource "openstack_networking_port_v2" "compute" {
 
   for_each = {for item in setproduct(var.nodes, var.networks):
-    "${item[0]}-${item[1].network}" => item[1]
+    "${item[0]}-${item[1].network}" => {
+        node_idx = index(var.nodes, item[0])
+        net = item[1]
+      }
   }
 
   name = "${var.cluster_name}-${each.key}"
-  network_id = data.openstack_networking_network_v2.network[each.value.network].id
+  network_id = data.openstack_networking_network_v2.network[each.value.net.network].id
   admin_state_up = "true"
 
   fixed_ip {
-    subnet_id = data.openstack_networking_subnet_v2.subnet[each.value.network].id
+    subnet_id = data.openstack_networking_subnet_v2.subnet[each.value.net.network].id
+    ip_address = try(var.ip_addresses[each.value.net.network][each.value.node_idx], null)
   }
 
-  no_security_groups = lookup(each.value, "no_security_groups", false)
-  security_group_ids = lookup(each.value, "no_security_groups", false) ? [] : var.security_group_ids
+  no_security_groups = lookup(each.value.net, "no_security_groups", false)
+  security_group_ids = lookup(each.value.net, "no_security_groups", false) ? [] : var.security_group_ids
 
   binding {
-    vnic_type = lookup(var.vnic_types, each.value.network, "normal")
+    vnic_type = lookup(var.vnic_types, each.value.net.network, "normal")
   }
 }
 

environments/skeleton/{{cookiecutter.environment}}/tofu/node_group/variables.tf

@@ -119,6 +119,27 @@ variable "fip_network" {
     nullable = false
 }
 
+variable "ip_addresses" {
+    type = map(list(string))
+    description = <<-EOT
+        Mapping of list of fixed IP addresses for nodes, keyed by network name,
+        in same order as nodes parameter. For any networks not specified here
+        the cloud will select addresses.
+
+        NB: Changing IP addresses after deployment may hit terraform provider bugs.
+    EOT
+    default = {}
+    nullable = false
+    validation {
+      condition = length(setsubtract(keys(var.ip_addresses), var.networks[*].network)) == 0
+      error_message = "Keys in ip_addresses for nodegroup \"${var.group_name}\" must match network names in var.cluster_networks"
+    }
+    validation {
+      condition = alltrue([for v in values(var.ip_addresses): length(v) == length(var.nodes)])
+      error_message = "Values in ip_addresses for nodegroup \"${var.group_name}\" must be a list of the same length as var.nodes"
+    }
+}
+
 variable "match_ironic_node" {
     type = bool
     description = "Whether to launch instances on the Ironic node of the same name as each cluster node"

environments/skeleton/{{cookiecutter.environment}}/tofu/variables.tf

@@ -24,45 +24,67 @@ variable "key_pair" {
     description = "Name of an existing keypair in OpenStack"
 }
 
+variable "control_ip_addresses" {
+    type        = map(string)
+    description = <<-EOT
+        Mapping of fixed IP addresses for control node, keyed by network name.
+        For any networks not specified here the cloud will select an address.
+
+        NB: Changing IP addresses after deployment may hit terraform provider bugs.
+    EOT
+    default     = {}
+    validation {
+        # check all keys are network names in cluster_networks
+        condition = length(setsubtract(keys(var.control_ip_addresses), var.cluster_networks[*].network)) == 0
+        error_message = "Keys in var.control_ip_addresses must match network names in var.cluster_networks"
+    }
+}
+
 variable "control_node_flavor" {
     type = string
     description = "Flavor name for control node"
 }
 
 variable "login" {
-  type = any
-  description = <<-EOF
-    Mapping defining homogenous groups of login nodes. Multiple groups may
-    be useful for e.g. separating nodes for ssh and Open Ondemand usage, or
-    to define login nodes with different capabilities such as high-memory.
+    default = {}
+    description = <<-EOF
+        Mapping defining homogenous groups of login nodes. Multiple groups may
+        be useful for e.g. separating nodes for ssh and Open Ondemand usage, or
+        to define login nodes with different capabilities such as high-memory.
+
+        Keys are names of groups.
+        Values are a mapping as follows:
 
-    Keys are names of groups.
-    Values are a mapping as follows:
+        Required:
+            nodes: List of node names
+            flavor: String flavor name
+        Optional:
+            image_id: Overrides variable cluster_image_id
+            extra_networks: List of mappings in same format as cluster_networks
+            vnic_types: Overrides variable vnic_types
+            volume_backed_instances: Overrides variable volume_backed_instances
+            root_volume_size: Overrides variable root_volume_size
+            extra_volumes: Mapping defining additional volumes to create and attach
+                            Keys are unique volume name.
+                            Values are a mapping with:
+                                size: Size of volume in GB
+                            **NB**: The order in /dev is not guaranteed to match the mapping
+            fip_addresses: List of addresses of floating IPs to associate with
+                           nodes, in the same order as nodes parameter. The
+                           floating IPs must already be allocated to the project.
+            fip_network: Name of network containing ports to attach FIPs to. Only
+                        required if multiple networks are defined.
+            ip_addresses: Mapping of list of fixed IP addresses for nodes, keyed
+                          by network name, in same order as nodes parameter.
+                          For any networks not specified here the cloud will
+                          select addresses.
+            match_ironic_node: Set true to launch instances on the Ironic node of the same name as each cluster node
+            availability_zone: Name of availability zone - ignored unless match_ironic_node is true (default: "nova")
+            gateway_ip: Address to add default route via
+            nodename_template: Overrides variable cluster_nodename_template
+    EOF
 
-    Required:
-        nodes: List of node names
-        flavor: String flavor name
-    Optional:
-        image_id: Overrides variable cluster_image_id
-        extra_networks: List of mappings in same format as cluster_networks
-        vnic_types: Overrides variable vnic_types
-        volume_backed_instances: Overrides variable volume_backed_instances
-        root_volume_size: Overrides variable root_volume_size
-        extra_volumes: Mapping defining additional volumes to create and attach
-                        Keys are unique volume name.
-                        Values are a mapping with:
-                            size: Size of volume in GB
-                        **NB**: The order in /dev is not guaranteed to match the mapping
-        fip_addresses: List of addresses of floating IPs to associate with nodes,
-                       in the same order as nodes parameter. The floating IPs
-                       must already be allocated to the project.
-        fip_network: Name of network containing ports to attach FIPs to. Only
-                     required if multiple networks are defined.
-        match_ironic_node: Set true to launch instances on the Ironic node of the same name as each cluster node
-        availability_zone: Name of availability zone - ignored unless match_ironic_node is true (default: "nova")
-        gateway_ip: Address to add default route via
-        nodename_template: Overrides variable cluster_nodename_template
-  EOF
+    type = any
 }
 
 variable "cluster_image_id" {
@@ -71,7 +93,7 @@ variable "cluster_image_id" {
 }
 
 variable "compute" {
-
+    default = {}
     description = <<-EOF
         Mapping defining homogenous groups of compute nodes. Groups are used
         in Slurm partition definitions.
@@ -95,12 +117,16 @@ variable "compute" {
                            Values are a mapping with:
                                 size: Size of volume in GB
                            **NB**: The order in /dev is not guaranteed to match the mapping
+            ip_addresses: Mapping of list of fixed IP addresses for nodes, keyed
+                          by network name, in same order as nodes parameter.
+                          For any networks not specified here the cloud will
+                          select addresses.
             match_ironic_node: Set true to launch instances on the Ironic node of the same name as each cluster node
             availability_zone: Name of availability zone - ignored unless match_ironic_node is true (default: "nova")
             gateway_ip: Address to add default route via
             nodename_template: Overrides variable cluster_nodename_template
     EOF
-    default = {}
+
     type = any # can't do any better; TF type constraints can't cope with heterogeneous inner mappings
 }