docs updates + review suggestions + refactor template override

Author: wtripp180901

ansible/roles/topology/README.md

@@ -3,12 +3,13 @@ topology
 
 Templates out /etc/slurm/topology.conf file based on an Openstack project for use by
 Slurm's [topology/tree plugin.](https://slurm.schedmd.com/topology.html) Models
-project as tree with a heirarchy of:
+cluster as tree with a heirarchy of:
 
-Project -> Availability Zones -> Hypervisors -> VMs
+Top-level inter-rack Switch -> Availability Zones -> Hypervisors -> VMs
 
 Role Variables
 --------------
 
-- `topology_topology_nodes: []`: Required list[str]. List of nodes to include in topology tree. Must be set to include all compute nodes in Slurm cluster. Default `[]`.
-- `topology_topology_override:`: Optional str. If set, will override templating and be provided as custom topology.conf content. Undefined by default.
+- `topology_topology_nodes:`: Required list of strs. List of inventory hostnames of nodes to include in topology tree. Must be set to include all compute nodes in Slurm cluster. Default `[]`.
+- `topology_conf_template`: Optional str. Path to Jinja2 template of topology.conf file. Default
+  `templates/topology.conf.j2`

ansible/roles/topology/defaults/main.yml

@@ -1,5 +1,5 @@
 # Nodes to be included in topology tree, must include all Slurm compute nodes
 topology_topology_nodes: []
 
-# If set, will override topology.conf file auto-detected from OpenStack project
-# topology_topology_override:
+# Override to use custom topology.conf template
+topology_conf_template: templates/topology.conf.j2

ansible/roles/topology/library/map_hosts.py

@@ -12,7 +12,7 @@
 short_description: Creates map of OpenStack VM network topology
 description:
     - Creates map representing the network topology tree of an OpenStack project with a heirarchy
-      of: Availability Zone -> Hypervisors/Baremetal nodes -> VMs/Baremetal instances
+      of: Availability Zone -> Hypervisors -> VMs/Baremetal instances
 options:
     compute_vms:
         description:

ansible/roles/topology/tasks/main.yml

@@ -9,7 +9,7 @@
 - name: Template topology.conf
   become: true
   ansible.builtin.template:
-    src: templates/topology.conf.j2
+    src: "{{ topology_conf_template }}"
     dest: /etc/slurm/topology.conf
     owner: root
     group: root

ansible/roles/topology/templates/topology.conf.j2

@@ -1,13 +1,9 @@
 # topology.conf
 # Switch Configuration
-{% if topology_topology_override is defined %}
-{{ topology_topology_override }}
-{% else %}
 {% for az in _topology.topology.keys() %}
 {% for instance_host in _topology.topology[az].keys() %}
 SwitchName={{ instance_host }} Nodes={{ _topology.topology[az][instance_host] | join(",") }}
 {% endfor %}
 SwitchName={{ az }} Switches={{ _topology.topology[az].keys() | join(",") }}
 {% endfor %}
 SwitchName=master Switches={{ _topology.topology.keys() | join(",") }}
-{% endif %}

ansible/slurm.yml

@@ -65,7 +65,9 @@
         name: topology
       # Gated on topology group having compute nodes but role also
       # needs to run on control and login nodes
-      when: appliances_mode == 'configure' and (groups['topology'] | length) > 0
+      when:
+        - appliances_mode == 'configure'
+        - groups['topology'] | length > 0
     - include_role:
         name: stackhpc.openhpc
         tasks_from: "{{ 'runtime.yml' if appliances_mode == 'configure' else 'main.yml' }}"

environments/common/inventory/groups

@@ -25,7 +25,7 @@ openhpc
 # Do not add hosts here manually - used as part of Packer image build pipeline. See packer/README.md.
 
 [topology]
-# Compute nodes to be included in the Slurm topology plugin's topology tree
+# Compute nodes to be included in the Slurm topology plugin's topology tree. See ansible/roles/topology
 # Should be set to `compute` if enabled
 # Note that this feature currently assumes all compute nodes are VMs, enabling
 # when the cluster contains baremetal compute nodes may lead to unexpected scheduling behaviour

environments/common/layouts/everything

@@ -137,4 +137,8 @@ builder
 compute
 
 [topology:children]
+# Compute nodes to be included in the Slurm topology plugin's topology tree. See ansible/roles/topology
+# Should be set to `compute` if enabled
+# Note that this feature currently assumes all compute nodes are VMs, enabling
+# when the cluster contains baremetal compute nodes may lead to unexpected scheduling behaviour
 compute

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

@@ -150,7 +150,7 @@ variable "match_ironic_node" {
 
 variable "availability_zone" {
     type = string
-    description = "Name of availability zone. If undefined, defaults to 'nova' if match_ironic_node is true, defered to OpenStack otherwise"
+    description = "Name of availability zone. If undefined, defaults to 'nova' if match_ironic_node is true, deferred to OpenStack otherwise"
     default = null
 }
 

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

@@ -79,8 +79,8 @@ variable "login" {
                           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"Name of availability zone. If undefined, defaults to 'nova' 
-                               if match_ironic_node is true, defered to OpenStack otherwise"
+            availability_zone: Name of availability zone. If undefined, defaults to 'nova' 
+                               if match_ironic_node is true, defered to OpenStack otherwise
             gateway_ip: Address to add default route via
             nodename_template: Overrides variable cluster_nodename_template
     EOF
@@ -123,8 +123,8 @@ variable "compute" {
                           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. "Name of availability zone. If undefined, defaults to 'nova'
-                               if match_ironic_node is true, defered to OpenStack otherwise"
+            availability_zone: Name of availability zone. If undefined, defaults to 'nova'
+                               if match_ironic_node is true, defered to OpenStack otherwise
             gateway_ip: Address to add default route via
             nodename_template: Overrides variable cluster_nodename_template