Validate nodegroup names (#793)

* validate nodename groups

* add validation for nodegroup name clashes

* add validation for nodegroup name clashes

* fix linter whinges

* extend validation to cover additional_nodegroups

* fix TF linting

* fixup logic

* fix logic

* fix linter

Author: sjpb

environments/.stackhpc/tofu/ARCUS.tfvars

@@ -1,4 +1,4 @@
-cluster_net = "portal-internal"
-cluster_subnet = "portal-internal"
+cluster_net         = "portal-internal"
+cluster_subnet      = "portal-internal"
 control_node_flavor = "vm.ska.cpu.general.eighth"
-other_node_flavor = "vm.ska.cpu.general.small"
+other_node_flavor   = "vm.ska.cpu.general.small"

environments/.stackhpc/tofu/LEAFCLOUD-dev.tfvars

@@ -1,10 +1,10 @@
 cluster_networks = [
-    {
-        network = "stackhpc-dev"
-        subnet = "stackhpc-dev"
-    }
+  {
+    network = "stackhpc-dev"
+    subnet  = "stackhpc-dev"
+  }
 ]
 control_node_flavor = "ec1.medium" # small ran out of memory, medium gets down to ~100Mi mem free on deployment
-other_node_flavor = "en1.xsmall"
-state_volume_type = "unencrypted"
-home_volume_type = "unencrypted"
+other_node_flavor   = "en1.xsmall"
+state_volume_type   = "unencrypted"
+home_volume_type    = "unencrypted"

environments/.stackhpc/tofu/LEAFCLOUD.tfvars

@@ -1,10 +1,10 @@
 cluster_networks = [
-    {
-        network = "slurmapp-ci"
-        subnet = "slurmapp-ci"
-    }
+  {
+    network = "slurmapp-ci"
+    subnet  = "slurmapp-ci"
+  }
 ]
 control_node_flavor = "ec1.medium" # small ran out of memory, medium gets down to ~100Mi mem free on deployment
-other_node_flavor = "en1.xsmall"
-state_volume_type = "unencrypted"
-home_volume_type = "unencrypted"
+other_node_flavor   = "en1.xsmall"
+state_volume_type   = "unencrypted"
+home_volume_type    = "unencrypted"

environments/.stackhpc/tofu/SMS.tfvars

@@ -1,8 +1,8 @@
 cluster_networks = [
-    {
-        network = "stackhpc-ipv4-geneve"
-        subnet = "stackhpc-ipv4-geneve-subnet"
-    }
+  {
+    network = "stackhpc-ipv4-geneve"
+    subnet  = "stackhpc-ipv4-geneve-subnet"
+  }
 ]
 control_node_flavor = "general.v1.small"
-other_node_flavor = "general.v1.small"
+other_node_flavor   = "general.v1.small"

environments/.stackhpc/tofu/main.tf

@@ -76,7 +76,7 @@ module "cluster" {
   control_node_flavor = var.control_node_flavor
 
   login = {
-    login = {
+    head = {
       nodes  = ["login-0"]
       flavor = var.other_node_flavor
     }

environments/site/tofu/variables.tf

@@ -52,7 +52,8 @@ variable "login" {
     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.
+    Keys are names of groups, and cannot be 'login', 'compute', 'control', or
+    keys in the compute or additional_nodegroups variables.
     Values are a mapping as follows:
 
     Required:
@@ -88,6 +89,25 @@ variable "login" {
   EOF
 
   type = any
+  validation {
+    condition     = length(setintersection(keys(var.login), ["login", "compute", "control"])) == 0
+    error_message = <<-EOF
+      Login nodegroup names cannot be 'login', 'compute' or 'control'. Invalid var.login key(s): ${join(", ", setintersection(keys(var.login), ["login", "compute", "control"]))}.
+    EOF
+  }
+  validation {
+    condition = length(distinct(concat(keys(var.login), keys(var.compute), keys(var.additional_nodegroups)))) == length(concat(keys(var.login), keys(var.compute), keys(var.additional_nodegroups)))
+    error_message = <<-EOF
+      Nodegroup names must be unique. Shared key(s) found in variables login, compute and/or additional_nodegroups: ${
+    join(", ", setunion(
+      setintersection(keys(var.login), keys(var.compute)),
+      setintersection(keys(var.compute), keys(var.additional_nodegroups)),
+      setintersection(keys(var.additional_nodegroups), keys(var.login))
+    ))
+  }
+    EOF
+
+}
 }
 
 variable "cluster_image_id" {
@@ -101,7 +121,8 @@ variable "compute" {
     Mapping defining homogenous groups of compute nodes. Groups are used
     in Slurm partition definitions.
 
-    Keys are names of groups.
+    Keys are names of groups, and cannot be 'compute', 'login', 'control', 'default'
+    or keys in the login or additional_nodegroups variables.
     Values are a mapping as follows:
 
     Required:
@@ -139,6 +160,12 @@ variable "compute" {
   EOF
 
   type = any # can't do any better; TF type constraints can't cope with heterogeneous inner mappings
+  validation {
+    condition     = length(setintersection(keys(var.compute), ["login", "compute", "control", "default"])) == 0
+    error_message = <<-EOF
+      Compute nodegroup names cannot be 'compute', 'default', 'login' or 'control'. Invalid var.compute key(s): ${join(", ", setintersection(keys(var.compute), ["login", "compute", "control", "default"]))}.
+    EOF
+  }
 }
 
 # tflint-ignore: terraform_typed_variables
@@ -149,7 +176,8 @@ variable "additional_nodegroups" {
     These nodes are not in the compute or login inventory groups so they
     will not run slurmd.
 
-    Keys are names of groups.
+    Keys are names of groups and cannot be 'login', 'compute, 'control', or
+    keys in the login or additional_nodegroups variables.
     Values are a mapping as for the "login" variable, with the addition of
     the optional entry:
         
@@ -162,6 +190,13 @@ variable "additional_nodegroups" {
     - $cluster_name + '_' + $group_name
     - 'additional'
   EOF
+  type        = any # can't do any better; TF type constraints can't cope with heterogeneous inner mappings
+  validation {
+    condition     = length(setintersection(keys(var.additional_nodegroups), ["login", "compute", "control"])) == 0
+    error_message = <<-EOF
+      Additional nodegroup names cannot be 'compute', 'login' or 'control'. Invalid var.additional_nodegroups key(s): ${join(", ", setintersection(keys(var.additional_nodegroups), ["login", "compute", "control"]))}.
+    EOF
+  }
 }
 
 variable "environment_root" {