From c7efb163d499e5618b88d7d1abd3f8762468d064 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Fri, 24 Aug 2018 16:50:05 +0000
Subject: [PATCH 01/10] Use base log directory

Python log handlers in VBMC don't play well with nested log directories,
so just use /var/log/tenks/.
---
 ansible/host_setup.yml         | 4 ++--
 ansible/libvirt_create_vms.yml | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ansible/host_setup.yml b/ansible/host_setup.yml
index 34a8bd9..1216636 100644
--- a/ansible/host_setup.yml
+++ b/ansible/host_setup.yml
@@ -4,9 +4,9 @@
     name: "{{ system_requirements }}"
   become: true
 
-- name: Ensure console log directory exists
+- name: Ensure log directory exists
   file:
-    path: "{{ console_log_directory }}"
+    path: "{{ log_directory }}"
     state: directory
   become: true
 
diff --git a/ansible/libvirt_create_vms.yml b/ansible/libvirt_create_vms.yml
index d6ca822..05c0469 100644
--- a/ansible/libvirt_create_vms.yml
+++ b/ansible/libvirt_create_vms.yml
@@ -3,7 +3,7 @@
   include_role:
     name: stackhpc.libvirt-vm
   vars:
-    libvirt_vm_default_console_log_dir: "{{ console_log_directory }}"
+    libvirt_vm_default_console_log_dir: "{{ log_directory }}"
     # Configure VM definitions for the Libvirt provider.
     libvirt_vms: >-
       {{ vms | map('set_libvirt_interfaces')

From c1cb7b9baf332641bd11417766389bb8cc3c23a5 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Tue, 28 Aug 2018 12:11:11 +0000
Subject: [PATCH 02/10] Use Ansible variables for relative paths

---
 ansible/group_vars/hypervisors | 2 +-
 ansible/host_vars/localhost    | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/ansible/group_vars/hypervisors b/ansible/group_vars/hypervisors
index 3f1d03b..e9bf112 100644
--- a/ansible/group_vars/hypervisors
+++ b/ansible/group_vars/hypervisors
@@ -11,7 +11,7 @@ system_requirements:
 
 # Path to virtualenv used to install Python requirements. If a virtualenv does
 # not exist at this location, one will be created.
-virtualenv_path: ~/tenks-venv
+virtualenv_path: "{{ '/'.join([ansible_env['HOME'], 'tenks-venv']) }}"
 
 python_requirements:
   - virtualbmc
diff --git a/ansible/host_vars/localhost b/ansible/host_vars/localhost
index dd29727..1948434 100644
--- a/ansible/host_vars/localhost
+++ b/ansible/host_vars/localhost
@@ -1,7 +1,6 @@
 ---
-# FIXME(w-miller): set this more cleverly?
-base_path: "~/tenks/"
-allocations_file_path: "{{ base_path + '/allocations.yml' }}"
+allocations_file_path: >-
+    {{ '/'.join([(playbook_dir | dirname), 'allocations.yml']) }}
 
 flavours:
   flav0:

From e5fc211de3decebb9a811c55f157150870d21034 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Tue, 28 Aug 2018 12:12:21 +0000
Subject: [PATCH 03/10] Configure Virtual BMC

* Configure the Virtual BMC daemon in systemd
* Start the Virtual BMC daemon per hypervisor
* Add each hypervisor's VMs to Virtual BMC
---
 ansible/deploy.yml                 |  8 ++++++
 ansible/group_vars/hypervisors     | 13 +++++++++-
 ansible/templates/vbmcd.service.j2 |  7 ++++++
 ansible/virtualbmc.yml             | 40 ++++++++++++++++++++++++++++++
 ansible/vm_virtualbmc.yml          | 38 ++++++++++++++++++++++++++++
 5 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 ansible/templates/vbmcd.service.j2
 create mode 100644 ansible/virtualbmc.yml
 create mode 100644 ansible/vm_virtualbmc.yml

diff --git a/ansible/deploy.yml b/ansible/deploy.yml
index 41ddb6b..e6e503d 100644
--- a/ansible/deploy.yml
+++ b/ansible/deploy.yml
@@ -45,3 +45,11 @@
         vms: >-
           {{ hostvars['localhost'].allocations.result[inventory_hostname]
                | default([]) }}
+
+- hosts: localhost
+  tasks:
+    - include_tasks: virtualbmc.yml
+      vars:
+        vms: >-
+          {{ hostvars['localhost'].allocations.result[inventory_hostname]
+               | default([]) }}
diff --git a/ansible/group_vars/hypervisors b/ansible/group_vars/hypervisors
index e9bf112..3656a47 100644
--- a/ansible/group_vars/hypervisors
+++ b/ansible/group_vars/hypervisors
@@ -35,4 +35,15 @@ veth_vm_ovs_suffix: '-ovs'
 # seem right.
 veth_vm_source_suffix: '-tap'
 
-console_log_directory: /var/log/tenks/console_logs/
+# Directory in which to store Tenks logs.
+log_directory: /var/log/tenks/
+
+# The address on which VBMC will listen for IPMI traffic.
+vbmc_ipmi_listen_address: 0.0.0.0
+# The range of ports available for use by VBMC.
+vbmc_ipmi_port_range_start: 6230
+vbmc_ipmi_port_range_end: 6240
+# The IPMI username that VBMC will use.
+vbmc_ipmi_username: username
+# The IPMI password that VBMC will use.
+vbmc_ipmi_password: password
diff --git a/ansible/templates/vbmcd.service.j2 b/ansible/templates/vbmcd.service.j2
new file mode 100644
index 0000000..cb959e9
--- /dev/null
+++ b/ansible/templates/vbmcd.service.j2
@@ -0,0 +1,7 @@
+[Unit]
+Description=Virtual BMC daemon
+
+[Service]
+Type=simple
+Restart=on-failure
+ExecStart="{{ virtualenv_path }}/bin/vbmcd" --foreground
diff --git a/ansible/virtualbmc.yml b/ansible/virtualbmc.yml
new file mode 100644
index 0000000..b7f5ccf
--- /dev/null
+++ b/ansible/virtualbmc.yml
@@ -0,0 +1,40 @@
+---
+- name: Set service name
+  set_fact:
+    service: vbmcd
+
+- name: Check that enough ports are available for Virtual BMC
+  fail: >
+    {{ vms | count }} VMs were specified to be created, but only
+    {{ vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start }} ports are
+    available for use by Virtual BMC.
+  when: (vms | count) > (vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start)
+
+- name: Ensure Virtual BMC systemd service is configured
+  template:
+    src: templates/{{ item }}.j2
+    dest: /etc/systemd/system/{{ item }}
+    owner: root
+    group: root
+    mode: 0644
+  become: true
+  register: vbmc_service_file
+  loop:
+    - "{{ service }}.service"
+
+- name: Ensure Virtual BMC systemd service is started and enabled
+  systemd:
+    name: "{{ service }}"
+    enabled: yes
+    state: started
+    daemon_reload: "{{ vbmc_service_file.changed }}"
+  become: true
+
+- include_tasks: vm_virtualbmc.yml
+  vars:
+    vm: "{{ vm }}"
+    port: "{{ vbmc_ipmi_port_range_start + port_offset }}"
+  loop: "{{ vms }}"
+  loop_control:
+    loop_var: vm
+    index_var: port_offset
diff --git a/ansible/vm_virtualbmc.yml b/ansible/vm_virtualbmc.yml
new file mode 100644
index 0000000..84ac0bb
--- /dev/null
+++ b/ansible/vm_virtualbmc.yml
@@ -0,0 +1,38 @@
+---
+- name: Set common strings
+  set_fact:
+    # vbmcd should already be running, so --no-daemon stops vbmc from spawning
+    # another instance of the daemon.
+    cmd: "'{{ virtualenv_path }}/bin/vbmc' --no-daemon"
+    log_arg: "--log-file '{{ log_directory }}/vbmc-{{ vm.name }}.log'"
+
+# Even if the VM is present in VBMC, we can't guarantee that it's configured
+# correctly. It's easiest to delete and re-add it; this should involve minimal
+# downtime.
+- name: Ensure VM is stopped and deleted in VBMC
+  command: >-
+    {{ cmd }} {{ item }} '{{ vm.name }}' {{ log_arg }}
+  loop:
+    - stop
+    - delete
+  register: res
+  changed_when: res.rc != 0
+  failed_when:
+    - res.rc != 0
+    - "'No domain with matching name' not in res.stderr"
+  become: true
+
+- name: Ensure VM is added to VBMC
+  command: >-
+    {{ cmd }} add '{{ vm.name }}'
+      --port {{ port }}
+      --username '{{ vbmc_ipmi_username }}'
+      --password '{{ vbmc_ipmi_password }}'
+      --address {{ vbmc_ipmi_listen_address }}
+      {{ log_arg }}
+  become: true
+
+- name: Ensure VM is started in VBMC
+  command: >
+    {{ cmd }} start '{{ vm.name }}' {{ log_arg }}
+  become: true

From fb0a948950a3a1d3eadf477ad28646cb05545ff7 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Tue, 28 Aug 2018 13:24:33 +0000
Subject: [PATCH 04/10] Move Python package vars together

---
 ansible/group_vars/hypervisors | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ansible/group_vars/hypervisors b/ansible/group_vars/hypervisors
index 42b7944..a693e63 100644
--- a/ansible/group_vars/hypervisors
+++ b/ansible/group_vars/hypervisors
@@ -10,6 +10,11 @@ system_requirements:
 # not exist at this location, one will be created.
 virtualenv_path: "{{ '/'.join([ansible_env['HOME'], 'tenks-venv']) }}"
 
+# The URL of the upper constraints file to pass to pip when installing Python
+# packages.
+python_upper_contraints_url: >-
+  https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt
+
 # Naming scheme for bridges created by tenks for physical networks is
 # {{ bridge_prefix + i }}, where `i` is the index of the physical network in
 # physnet_mappings (sorted alphabetically by key).
@@ -41,8 +46,3 @@ vbmc_ipmi_port_range_end: 6240
 vbmc_ipmi_username: username
 # The IPMI password that VBMC will use.
 vbmc_ipmi_password: password
-
-# The URL of the upper constraints file to pass to pip when installing Python
-# packages.
-python_upper_contraints_url: >-
-  https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt

From b36f95a04fff07cee6581009bf024bd0dc32880b Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Tue, 28 Aug 2018 15:13:31 +0000
Subject: [PATCH 05/10] Extract Virtual BMC tasks into a role

The role will:
* Install VBMC requirements in virtualenv
* Set up VBMC daemon
* Add domains to VBMC
---
 ansible/deploy.yml                            | 12 ++++----
 ansible/group_vars/hypervisors                | 12 +-------
 ansible/host_setup.yml                        |  8 ------
 ansible/roles/virtualbmc/README.md            | 25 +++++++++++++++++
 ansible/roles/virtualbmc/defaults/main.yml    | 17 +++++++++++
 .../roles/virtualbmc/files/requirements.txt   |  0
 .../virtualbmc/tasks/domain.yml}              | 12 ++++----
 .../virtualbmc/tasks/main.yml}                | 28 +++++++++++++------
 8 files changed, 75 insertions(+), 39 deletions(-)
 create mode 100644 ansible/roles/virtualbmc/README.md
 create mode 100644 ansible/roles/virtualbmc/defaults/main.yml
 rename venv-requirements.txt => ansible/roles/virtualbmc/files/requirements.txt (100%)
 rename ansible/{vm_virtualbmc.yml => roles/virtualbmc/tasks/domain.yml} (73%)
 rename ansible/{virtualbmc.yml => roles/virtualbmc/tasks/main.yml} (50%)

diff --git a/ansible/deploy.yml b/ansible/deploy.yml
index 84f6ad8..3c09b73 100644
--- a/ansible/deploy.yml
+++ b/ansible/deploy.yml
@@ -52,10 +52,12 @@
           {{ hostvars['localhost'].allocations.result[inventory_hostname]
                | default([]) }}
 
-- hosts: localhost
-  tasks:
-    - include_tasks: virtualbmc.yml
+    - include_role:
+        name: virtualbmc
       vars:
-        vms: >-
+        vbmc_libvirt_domains: >-
           {{ hostvars['localhost'].allocations.result[inventory_hostname]
-               | default([]) }}
+               | default([]) | map(attribute='name') | list }}
+        vbmc_log_directory: "{{ log_directory }}"
+        vbmc_virtualenv_path: "{{ virtualenv_path }}"
+        vbmc_python_upper_contraints_url: "{{ python_upper_constraints_url }}"
diff --git a/ansible/group_vars/hypervisors b/ansible/group_vars/hypervisors
index a693e63..df2eb20 100644
--- a/ansible/group_vars/hypervisors
+++ b/ansible/group_vars/hypervisors
@@ -12,7 +12,7 @@ virtualenv_path: "{{ '/'.join([ansible_env['HOME'], 'tenks-venv']) }}"
 
 # The URL of the upper constraints file to pass to pip when installing Python
 # packages.
-python_upper_contraints_url: >-
+python_upper_constraints_url: >-
   https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints.txt
 
 # Naming scheme for bridges created by tenks for physical networks is
@@ -36,13 +36,3 @@ veth_vm_source_suffix: '-tap'
 
 # Directory in which to store Tenks logs.
 log_directory: /var/log/tenks/
-
-# The address on which VBMC will listen for IPMI traffic.
-vbmc_ipmi_listen_address: 0.0.0.0
-# The range of ports available for use by VBMC.
-vbmc_ipmi_port_range_start: 6230
-vbmc_ipmi_port_range_end: 6240
-# The IPMI username that VBMC will use.
-vbmc_ipmi_username: username
-# The IPMI password that VBMC will use.
-vbmc_ipmi_password: password
diff --git a/ansible/host_setup.yml b/ansible/host_setup.yml
index aa31e86..ca38567 100644
--- a/ansible/host_setup.yml
+++ b/ansible/host_setup.yml
@@ -43,11 +43,3 @@
   loop: "{{ physnet_mappings | dictsort }}"
   loop_control:
     index_var: idx
-
-- name: Ensure Python requirements are installed
-  pip:
-    requirements: >-
-      {{ '/'.join([(playbook_dir | dirname), 'venv-requirements.txt']) }}
-    extra_args: >-
-      -c {{ python_upper_contraints_url }}
-    virtualenv: "{{ virtualenv_path }}"
diff --git a/ansible/roles/virtualbmc/README.md b/ansible/roles/virtualbmc/README.md
new file mode 100644
index 0000000..486a83d
--- /dev/null
+++ b/ansible/roles/virtualbmc/README.md
@@ -0,0 +1,25 @@
+Virtual BMC
+===========
+
+This role sets up Virtual BMC. It will configure the Virtual BMC daemon in
+systemd, and add the specified domains to the daemon.
+
+Requirements
+------------
+
+- systemd
+
+Role Variables
+--------------
+
+- `vbmc_libvirt_domains`: A list of Libvirt domain names to be added to Virtual
+  BMC.
+- `vbmc_ipmi_listen_address`: The address on which Virtual BMC will listen for
+  IPMI traffic.
+- `vbmc_ipmi_port_range_start`, `vbmc_ipmi_port_range_end`: The range of ports
+  available for use by Virtual BMC.
+- `vbmc_ipmi_username`: The IPMI username that Virtual BMC will use.
+- `vbmc_ipmi_password`: The IPMI password that Virtual BMC will use.
+- `vbmc_log_directory`: The directory in which to store Virtual BMC logs.
+- `vbmc_virtualenv_path`: The path to the virtualenv in which to install
+  Virtual BMC.
diff --git a/ansible/roles/virtualbmc/defaults/main.yml b/ansible/roles/virtualbmc/defaults/main.yml
new file mode 100644
index 0000000..6560a2d
--- /dev/null
+++ b/ansible/roles/virtualbmc/defaults/main.yml
@@ -0,0 +1,17 @@
+---
+# A list of Libvirt domain names to be added to VBMC.
+vbmc_libvirt_domains: []
+# The address on which VBMC will listen for IPMI traffic.
+vbmc_ipmi_listen_address: 0.0.0.0
+# The range of ports available for use by VBMC.
+vbmc_ipmi_port_range_start: 6230
+vbmc_ipmi_port_range_end: 6240
+# The IPMI username that VBMC will use.
+vbmc_ipmi_username: username
+# The IPMI password that VBMC will use.
+vbmc_ipmi_password: password
+
+# The directory in which to store VBMC logs.
+vbmc_log_directory:
+# The path to the virtualenv in which to install Virtual BMC.
+vbmc_virtualenv_path:
diff --git a/venv-requirements.txt b/ansible/roles/virtualbmc/files/requirements.txt
similarity index 100%
rename from venv-requirements.txt
rename to ansible/roles/virtualbmc/files/requirements.txt
diff --git a/ansible/vm_virtualbmc.yml b/ansible/roles/virtualbmc/tasks/domain.yml
similarity index 73%
rename from ansible/vm_virtualbmc.yml
rename to ansible/roles/virtualbmc/tasks/domain.yml
index 84ac0bb..773c2e6 100644
--- a/ansible/vm_virtualbmc.yml
+++ b/ansible/roles/virtualbmc/tasks/domain.yml
@@ -3,15 +3,15 @@
   set_fact:
     # vbmcd should already be running, so --no-daemon stops vbmc from spawning
     # another instance of the daemon.
-    cmd: "'{{ virtualenv_path }}/bin/vbmc' --no-daemon"
-    log_arg: "--log-file '{{ log_directory }}/vbmc-{{ vm.name }}.log'"
+    cmd: "'{{ vbmc_virtualenv_path }}/bin/vbmc' --no-daemon"
+    log_arg: "--log-file '{{ vbmc_log_directory }}/vbmc-{{ domain }}.log'"
 
 # Even if the VM is present in VBMC, we can't guarantee that it's configured
 # correctly. It's easiest to delete and re-add it; this should involve minimal
 # downtime.
 - name: Ensure VM is stopped and deleted in VBMC
   command: >-
-    {{ cmd }} {{ item }} '{{ vm.name }}' {{ log_arg }}
+    {{ cmd }} {{ item }} '{{ domain }}' {{ log_arg }}
   loop:
     - stop
     - delete
@@ -22,9 +22,9 @@
     - "'No domain with matching name' not in res.stderr"
   become: true
 
-- name: Ensure VM is added to VBMC
+- name: Ensure domain is added to VBMC
   command: >-
-    {{ cmd }} add '{{ vm.name }}'
+    {{ cmd }} add '{{ domain }}'
       --port {{ port }}
       --username '{{ vbmc_ipmi_username }}'
       --password '{{ vbmc_ipmi_password }}'
@@ -34,5 +34,5 @@
 
 - name: Ensure VM is started in VBMC
   command: >
-    {{ cmd }} start '{{ vm.name }}' {{ log_arg }}
+    {{ cmd }} start '{{ domain }}' {{ log_arg }}
   become: true
diff --git a/ansible/virtualbmc.yml b/ansible/roles/virtualbmc/tasks/main.yml
similarity index 50%
rename from ansible/virtualbmc.yml
rename to ansible/roles/virtualbmc/tasks/main.yml
index b7f5ccf..ee1a21e 100644
--- a/ansible/virtualbmc.yml
+++ b/ansible/roles/virtualbmc/tasks/main.yml
@@ -4,11 +4,21 @@
     service: vbmcd
 
 - name: Check that enough ports are available for Virtual BMC
-  fail: >
-    {{ vms | count }} VMs were specified to be created, but only
-    {{ vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start }} ports are
-    available for use by Virtual BMC.
-  when: (vms | count) > (vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start)
+  fail:
+    msg: >
+      {{ vbmc_libvirt_domains | count }} VMs were specified to be created, but
+      only {{ vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start }} ports
+      are available for use by Virtual BMC.
+  when: >-
+      (vbmc_libvirt_domains | count) >
+        (vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start)
+
+- name: Ensure Python requirements are installed
+  pip:
+    requirements: "{{ '/'.join([role_path, 'files', 'requirements.txt']) }}"
+    extra_args: >-
+      -c {{ vbmc_python_upper_contraints_url }}
+    virtualenv: "{{ vbmc_virtualenv_path }}"
 
 - name: Ensure Virtual BMC systemd service is configured
   template:
@@ -30,11 +40,11 @@
     daemon_reload: "{{ vbmc_service_file.changed }}"
   become: true
 
-- include_tasks: vm_virtualbmc.yml
+- include_tasks: domain.yml
   vars:
-    vm: "{{ vm }}"
+    domain: "{{ domain }}"
     port: "{{ vbmc_ipmi_port_range_start + port_offset }}"
-  loop: "{{ vms }}"
+  loop: "{{ vbmc_libvirt_domains }}"
   loop_control:
-    loop_var: vm
+    loop_var: domain
     index_var: port_offset

From 7e0cc7e16ba311b73a09fdcea884997abfc55583 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Tue, 28 Aug 2018 15:16:25 +0000
Subject: [PATCH 06/10] Fix typo in scheduling plugin

---
 ansible/action_plugins/tenks_schedule.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ansible/action_plugins/tenks_schedule.py b/ansible/action_plugins/tenks_schedule.py
index 90161ac..d74057a 100644
--- a/ansible/action_plugins/tenks_schedule.py
+++ b/ansible/action_plugins/tenks_schedule.py
@@ -49,7 +49,7 @@ class ActionModule(ActionBase):
         vms = []
         idx = 0
         for typ, cnt in six.iteritems(task_vars['specs']):
-            for _ in six.range(cnt):
+            for _ in six.moves.range(cnt):
                 vm = deepcopy(task_vars['vm_types'][typ])
                 # Sequentially number the VM and volume names.
                 vm['name'] = "%s%d" % (task_vars['vm_name_prefix'], idx)

From 855a2c2c7c87db81a9ab5babe864fea50ab26fe9 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Wed, 29 Aug 2018 10:06:25 +0000
Subject: [PATCH 07/10] Split Virtual BMC role into two

Create a role to configure a host for Virtual BMC, and another to add
domains to it.

Also add some checking to make the roles more robust.

Conflicts:
	ansible/deploy.yml
	ansible/libvirt_create_vms.yml
	ansible/roles/virtualbmc/tasks/main.yml
---
 ansible/deploy.yml                            | 47 ++++++++++++-----
 ansible/group_vars/libvirt                    |  4 ++
 ansible/roles/virtualbmc-daemon/README.md     | 17 +++++++
 .../roles/virtualbmc-daemon/defaults/main.yml |  6 +++
 .../files/requirements.txt                    |  0
 .../roles/virtualbmc-daemon/tasks/main.yml    | 31 ++++++++++++
 .../templates/vbmcd.service.j2                |  2 +-
 ansible/roles/virtualbmc-domain/README.md     | 27 ++++++++++
 .../roles/virtualbmc-domain/defaults/main.yml | 16 ++++++
 .../tasks/main.yml}                           | 32 +++++++++---
 ansible/roles/virtualbmc/README.md            | 25 ----------
 ansible/roles/virtualbmc/defaults/main.yml    | 17 -------
 ansible/roles/virtualbmc/tasks/main.yml       | 50 -------------------
 13 files changed, 161 insertions(+), 113 deletions(-)
 create mode 100644 ansible/roles/virtualbmc-daemon/README.md
 create mode 100644 ansible/roles/virtualbmc-daemon/defaults/main.yml
 rename ansible/roles/{virtualbmc => virtualbmc-daemon}/files/requirements.txt (100%)
 create mode 100644 ansible/roles/virtualbmc-daemon/tasks/main.yml
 rename ansible/{ => roles/virtualbmc-daemon}/templates/vbmcd.service.j2 (55%)
 create mode 100644 ansible/roles/virtualbmc-domain/README.md
 create mode 100644 ansible/roles/virtualbmc-domain/defaults/main.yml
 rename ansible/roles/{virtualbmc/tasks/domain.yml => virtualbmc-domain/tasks/main.yml} (50%)
 delete mode 100644 ansible/roles/virtualbmc/README.md
 delete mode 100644 ansible/roles/virtualbmc/defaults/main.yml
 delete mode 100644 ansible/roles/virtualbmc/tasks/main.yml

diff --git a/ansible/deploy.yml b/ansible/deploy.yml
index 3c09b73..489f59c 100644
--- a/ansible/deploy.yml
+++ b/ansible/deploy.yml
@@ -44,20 +44,43 @@
              | default([]) | subelements('physical_networks') }}
 
 - hosts: libvirt
+  vars:
+    # Allocations are stored in the localhost's vars.
+    nodes: >-
+      {{ hostvars['localhost'].allocations.result[inventory_hostname]
+           | default([]) }}
   tasks:
-    - include_tasks: libvirt_create_vms.yml
+    - name: Create Libvirt VMs
+      include_tasks: libvirt_create_vms.yml
       vars:
-        # Allocations are stored in the localhost's vars.
-        vms: >-
-          {{ hostvars['localhost'].allocations.result[inventory_hostname]
-               | default([]) }}
+        vms: "{{ nodes }}"
 
-    - include_role:
-        name: virtualbmc
+    - name: Check that enough ports are available for Virtual BMC
+      fail:
+        msg: >
+          {{ nodes | count }} nodes were specified to be added to Virtual BMC,
+          but only {{ ipmi_port_range_end - ipmi_port_range_start }} ports are
+          available for use by Virtual BMC.
+      when: >-
+          (nodes | count) > (ipmi_port_range_end - ipmi_port_range_start)
+
+    - name: Set up Virtual BMC daemon
+      include_role:
+        name: virtualbmc-daemon
       vars:
-        vbmc_libvirt_domains: >-
-          {{ hostvars['localhost'].allocations.result[inventory_hostname]
-               | default([]) | map(attribute='name') | list }}
-        vbmc_log_directory: "{{ log_directory }}"
+        vbmcd_virtualenv_path: "{{ virtualenv_path }}"
+        vbmcd_python_upper_contraints_url: "{{ python_upper_constraints_url }}"
+      when: (nodes | count) > 0
+
+    - name: Register domains with Virtual BMC
+      include_role:
+        name: virtualbmc-domain
+      vars:
+        vbmc_domain: "{{ domain }}"
+        vbmc_ipmi_port: "{{ ipmi_port_range_start + port_offset }}"
         vbmc_virtualenv_path: "{{ virtualenv_path }}"
-        vbmc_python_upper_contraints_url: "{{ python_upper_constraints_url }}"
+        vbmc_log_directory: "{{ log_directory }}"
+      loop: "{{ nodes | map(attribute='name') | list }}"
+      loop_control:
+        loop_var: domain
+        index_var: port_offset
diff --git a/ansible/group_vars/libvirt b/ansible/group_vars/libvirt
index 694a712..0e797d2 100644
--- a/ansible/group_vars/libvirt
+++ b/ansible/group_vars/libvirt
@@ -11,3 +11,7 @@ libvirt_pool_group: "{{ ansible_user_id }}"
 # By default, allow QEMU without hardware virtualisation since this is a
 # development tool.
 libvirt_require_vt: false
+
+# The range of ports available for use for node IPMI traffic.
+ipmi_port_range_start: 6230
+ipmi_port_range_end: 6240
diff --git a/ansible/roles/virtualbmc-daemon/README.md b/ansible/roles/virtualbmc-daemon/README.md
new file mode 100644
index 0000000..9c6f976
--- /dev/null
+++ b/ansible/roles/virtualbmc-daemon/README.md
@@ -0,0 +1,17 @@
+Virtual BMC Daemon
+==================
+
+This role sets up the Virtual BMC daemon in systemd.
+
+Requirements
+------------
+
+- systemd
+
+Role Variables
+--------------
+
+- `vbmcd_virtualenv_path`: The path to the virtualenv in which to install
+  Virtual BMC.
+- `vbmcd_python_upper_constraints_url`: The URL of the upper constraints file
+  to pass to pip when installing Python packages.
diff --git a/ansible/roles/virtualbmc-daemon/defaults/main.yml b/ansible/roles/virtualbmc-daemon/defaults/main.yml
new file mode 100644
index 0000000..1bca599
--- /dev/null
+++ b/ansible/roles/virtualbmc-daemon/defaults/main.yml
@@ -0,0 +1,6 @@
+---
+# The path to the virtualenv in which to install Virtual BMC.
+vbmcd_virtualenv_path:
+# The URL of the upper constraints file to pass to pip when installing Python
+# packages.
+vbmcd_python_upper_constraints_url:
diff --git a/ansible/roles/virtualbmc/files/requirements.txt b/ansible/roles/virtualbmc-daemon/files/requirements.txt
similarity index 100%
rename from ansible/roles/virtualbmc/files/requirements.txt
rename to ansible/roles/virtualbmc-daemon/files/requirements.txt
diff --git a/ansible/roles/virtualbmc-daemon/tasks/main.yml b/ansible/roles/virtualbmc-daemon/tasks/main.yml
new file mode 100644
index 0000000..6f57966
--- /dev/null
+++ b/ansible/roles/virtualbmc-daemon/tasks/main.yml
@@ -0,0 +1,31 @@
+---
+- name: Set service name
+  set_fact:
+    service: vbmcd
+
+- name: Ensure Python requirements are installed
+  pip:
+    requirements: "{{ '/'.join([role_path, 'files', 'requirements.txt']) }}"
+    extra_args: >-
+      -c {{ vbmcd_python_upper_contraints_url }}
+    virtualenv: "{{ vbmcd_virtualenv_path }}"
+
+- name: Ensure Virtual BMC systemd service is configured
+  template:
+    src: templates/{{ item }}.j2
+    dest: /etc/systemd/system/{{ item }}
+    owner: root
+    group: root
+    mode: 0644
+  become: true
+  register: service_file
+  loop:
+    - "{{ service }}.service"
+
+- name: Ensure Virtual BMC systemd service is started and enabled
+  systemd:
+    name: "{{ service }}"
+    enabled: yes
+    state: started
+    daemon_reload: "{{ service_file.changed }}"
+  become: true
diff --git a/ansible/templates/vbmcd.service.j2 b/ansible/roles/virtualbmc-daemon/templates/vbmcd.service.j2
similarity index 55%
rename from ansible/templates/vbmcd.service.j2
rename to ansible/roles/virtualbmc-daemon/templates/vbmcd.service.j2
index cb959e9..00b0133 100644
--- a/ansible/templates/vbmcd.service.j2
+++ b/ansible/roles/virtualbmc-daemon/templates/vbmcd.service.j2
@@ -4,4 +4,4 @@ Description=Virtual BMC daemon
 [Service]
 Type=simple
 Restart=on-failure
-ExecStart="{{ virtualenv_path }}/bin/vbmcd" --foreground
+ExecStart="{{ vbmcd_virtualenv_path }}/bin/vbmcd" --foreground
diff --git a/ansible/roles/virtualbmc-domain/README.md b/ansible/roles/virtualbmc-domain/README.md
new file mode 100644
index 0000000..9ce0236
--- /dev/null
+++ b/ansible/roles/virtualbmc-domain/README.md
@@ -0,0 +1,27 @@
+Virtual BMC Domain
+==================
+
+This role ensures a Libvirt domain is added to and started in Virtual BMC.
+
+Requirements
+------------
+
+- Virtual BMC installed in a virtualenv
+- Virtual BMC daemon running
+
+Role Variables
+--------------
+
+- `vbmc_domain`: The name of the Libvirt domain to be added to Virtual BMC.
+- `vbmc_virtualenv_path`: The path to the virtualenv in which Virtual BMC is
+  installed.
+- `vbmc_ipmi_listen_address`: The address on which Virtual BMC will listen for
+  IPMI traffic. Default is 0.0.0.0.
+- `vbmc_ipmi_port`: The port on which Virtual BMC will listen for IPMI traffic.
+  Default is 6230.
+- `vbmc_ipmi_username`: The IPMI username that Virtual BMC will use. Default is
+  'username'.
+- `vbmc_ipmi_password`: The IPMI password that Virtual BMC will use. Default is
+  'password'.
+- `vbmc_log_directory`: The directory in which to store Virtual BMC logs. If
+   `None`, output will not be logged to a file. Default is `None`.
diff --git a/ansible/roles/virtualbmc-domain/defaults/main.yml b/ansible/roles/virtualbmc-domain/defaults/main.yml
new file mode 100644
index 0000000..25f4416
--- /dev/null
+++ b/ansible/roles/virtualbmc-domain/defaults/main.yml
@@ -0,0 +1,16 @@
+---
+# The address on which VBMC will listen for IPMI traffic for this domain.
+vbmc_ipmi_listen_address: 0.0.0.0
+# The port on which VBMC will listen for IPMI traffic for this domain.
+vbmc_ipmi_port: 6230
+# The IPMI username that VBMC will use.
+vbmc_ipmi_username: username
+# The IPMI password that VBMC will use.
+vbmc_ipmi_password: password
+
+# The name of the Libvirt domain to be added to Virtual BMC.
+vbmc_domain:
+# The directory in which to store VBMC logs.
+vbmc_log_directory:
+# The path to the virtualenv in which Virtual BMC is installed.
+vbmc_virtualenv_path:
diff --git a/ansible/roles/virtualbmc/tasks/domain.yml b/ansible/roles/virtualbmc-domain/tasks/main.yml
similarity index 50%
rename from ansible/roles/virtualbmc/tasks/domain.yml
rename to ansible/roles/virtualbmc-domain/tasks/main.yml
index 773c2e6..c85af50 100644
--- a/ansible/roles/virtualbmc/tasks/domain.yml
+++ b/ansible/roles/virtualbmc-domain/tasks/main.yml
@@ -1,38 +1,54 @@
 ---
-- name: Set common strings
+- name: Set VBMC command string
   set_fact:
     # vbmcd should already be running, so --no-daemon stops vbmc from spawning
     # another instance of the daemon.
-    cmd: "'{{ vbmc_virtualenv_path }}/bin/vbmc' --no-daemon"
-    log_arg: "--log-file '{{ vbmc_log_directory }}/vbmc-{{ domain }}.log'"
+    cmd: >-
+      '{{ vbmc_virtualenv_path }}/bin/vbmc'
+        --no-daemon
+        {% if vbmc_log_directory is not none %}
+        --log-file '{{ vbmc_log_directory }}/vbmc-{{ domain }}.log'
+        {% endif %}
 
 # Even if the VM is present in VBMC, we can't guarantee that it's configured
 # correctly. It's easiest to delete and re-add it; this should involve minimal
 # downtime.
 - name: Ensure VM is stopped and deleted in VBMC
   command: >-
-    {{ cmd }} {{ item }} '{{ domain }}' {{ log_arg }}
+    {{ cmd }} {{ item }} '{{ domain }}'
   loop:
     - stop
     - delete
   register: res
-  changed_when: res.rc != 0
+  changed_when: res.rc == 0
   failed_when:
     - res.rc != 0
     - "'No domain with matching name' not in res.stderr"
   become: true
 
+# The commands above tend to return before the daemon has completed the action.
+# Check here to be safe.
+- name: Wait to ensure socket is closed
+  wait_for:
+    host: "{{ vbmc_ipmi_listen_address }}"
+    port: "{{ vbmc_ipmi_port }}"
+    state: stopped
+    timeout: 15
+
 - name: Ensure domain is added to VBMC
   command: >-
     {{ cmd }} add '{{ domain }}'
-      --port {{ port }}
+      --port {{ vbmc_ipmi_port }}
       --username '{{ vbmc_ipmi_username }}'
       --password '{{ vbmc_ipmi_password }}'
       --address {{ vbmc_ipmi_listen_address }}
-      {{ log_arg }}
   become: true
 
 - name: Ensure VM is started in VBMC
   command: >
-    {{ cmd }} start '{{ domain }}' {{ log_arg }}
+    {{ cmd }} start '{{ domain }}'
+  register: res
+  # Retry a few times in case the VBMC daemon has been slow to process the last
+  # few commands.
+  until: res is succeeded
   become: true
diff --git a/ansible/roles/virtualbmc/README.md b/ansible/roles/virtualbmc/README.md
deleted file mode 100644
index 486a83d..0000000
--- a/ansible/roles/virtualbmc/README.md
+++ /dev/null
@@ -1,25 +0,0 @@
-Virtual BMC
-===========
-
-This role sets up Virtual BMC. It will configure the Virtual BMC daemon in
-systemd, and add the specified domains to the daemon.
-
-Requirements
-------------
-
-- systemd
-
-Role Variables
---------------
-
-- `vbmc_libvirt_domains`: A list of Libvirt domain names to be added to Virtual
-  BMC.
-- `vbmc_ipmi_listen_address`: The address on which Virtual BMC will listen for
-  IPMI traffic.
-- `vbmc_ipmi_port_range_start`, `vbmc_ipmi_port_range_end`: The range of ports
-  available for use by Virtual BMC.
-- `vbmc_ipmi_username`: The IPMI username that Virtual BMC will use.
-- `vbmc_ipmi_password`: The IPMI password that Virtual BMC will use.
-- `vbmc_log_directory`: The directory in which to store Virtual BMC logs.
-- `vbmc_virtualenv_path`: The path to the virtualenv in which to install
-  Virtual BMC.
diff --git a/ansible/roles/virtualbmc/defaults/main.yml b/ansible/roles/virtualbmc/defaults/main.yml
deleted file mode 100644
index 6560a2d..0000000
--- a/ansible/roles/virtualbmc/defaults/main.yml
+++ /dev/null
@@ -1,17 +0,0 @@
----
-# A list of Libvirt domain names to be added to VBMC.
-vbmc_libvirt_domains: []
-# The address on which VBMC will listen for IPMI traffic.
-vbmc_ipmi_listen_address: 0.0.0.0
-# The range of ports available for use by VBMC.
-vbmc_ipmi_port_range_start: 6230
-vbmc_ipmi_port_range_end: 6240
-# The IPMI username that VBMC will use.
-vbmc_ipmi_username: username
-# The IPMI password that VBMC will use.
-vbmc_ipmi_password: password
-
-# The directory in which to store VBMC logs.
-vbmc_log_directory:
-# The path to the virtualenv in which to install Virtual BMC.
-vbmc_virtualenv_path:
diff --git a/ansible/roles/virtualbmc/tasks/main.yml b/ansible/roles/virtualbmc/tasks/main.yml
deleted file mode 100644
index ee1a21e..0000000
--- a/ansible/roles/virtualbmc/tasks/main.yml
+++ /dev/null
@@ -1,50 +0,0 @@
----
-- name: Set service name
-  set_fact:
-    service: vbmcd
-
-- name: Check that enough ports are available for Virtual BMC
-  fail:
-    msg: >
-      {{ vbmc_libvirt_domains | count }} VMs were specified to be created, but
-      only {{ vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start }} ports
-      are available for use by Virtual BMC.
-  when: >-
-      (vbmc_libvirt_domains | count) >
-        (vbmc_ipmi_port_range_end - vbmc_ipmi_port_range_start)
-
-- name: Ensure Python requirements are installed
-  pip:
-    requirements: "{{ '/'.join([role_path, 'files', 'requirements.txt']) }}"
-    extra_args: >-
-      -c {{ vbmc_python_upper_contraints_url }}
-    virtualenv: "{{ vbmc_virtualenv_path }}"
-
-- name: Ensure Virtual BMC systemd service is configured
-  template:
-    src: templates/{{ item }}.j2
-    dest: /etc/systemd/system/{{ item }}
-    owner: root
-    group: root
-    mode: 0644
-  become: true
-  register: vbmc_service_file
-  loop:
-    - "{{ service }}.service"
-
-- name: Ensure Virtual BMC systemd service is started and enabled
-  systemd:
-    name: "{{ service }}"
-    enabled: yes
-    state: started
-    daemon_reload: "{{ vbmc_service_file.changed }}"
-  become: true
-
-- include_tasks: domain.yml
-  vars:
-    domain: "{{ domain }}"
-    port: "{{ vbmc_ipmi_port_range_start + port_offset }}"
-  loop: "{{ vbmc_libvirt_domains }}"
-  loop_control:
-    loop_var: domain
-    index_var: port_offset

From e31a313f3cec941ca49469df921f28c106b598ed Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Thu, 30 Aug 2018 08:09:54 +0000
Subject: [PATCH 08/10] Check IPMI port count before creating VMs

---
 ansible/deploy.yml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/ansible/deploy.yml b/ansible/deploy.yml
index 489f59c..55507f8 100644
--- a/ansible/deploy.yml
+++ b/ansible/deploy.yml
@@ -50,11 +50,6 @@
       {{ hostvars['localhost'].allocations.result[inventory_hostname]
            | default([]) }}
   tasks:
-    - name: Create Libvirt VMs
-      include_tasks: libvirt_create_vms.yml
-      vars:
-        vms: "{{ nodes }}"
-
     - name: Check that enough ports are available for Virtual BMC
       fail:
         msg: >
@@ -64,6 +59,11 @@
       when: >-
           (nodes | count) > (ipmi_port_range_end - ipmi_port_range_start)
 
+    - name: Create Libvirt VMs
+      include_tasks: libvirt_create_vms.yml
+      vars:
+        vms: "{{ nodes }}"
+
     - name: Set up Virtual BMC daemon
       include_role:
         name: virtualbmc-daemon

From 2cc0bb6da4264b3883511d75c24404cdd7b0e080 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Thu, 30 Aug 2018 08:12:24 +0000
Subject: [PATCH 09/10] Set up VBMCD with Libvirt host setup

---
 ansible/deploy.yml | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/ansible/deploy.yml b/ansible/deploy.yml
index 55507f8..ef52770 100644
--- a/ansible/deploy.yml
+++ b/ansible/deploy.yml
@@ -5,7 +5,8 @@
 
 - hosts: libvirt
   tasks:
-    - include_role:
+    - name: Configure host for Libvirt
+      include_role:
         name: stackhpc.libvirt-host
       vars:
         libvirt_host_pools:
@@ -18,6 +19,13 @@
             group: "{{ libvirt_pool_group }}"
         libvirt_host_require_vt: "{{ libvirt_require_vt }}"
 
+    - name: Set up Virtual BMC daemon
+      include_role:
+        name: virtualbmc-daemon
+      vars:
+        vbmcd_virtualenv_path: "{{ virtualenv_path }}"
+        vbmcd_python_upper_contraints_url: "{{ python_upper_constraints_url }}"
+
 # Ensure we have facts about all hypervisors before scheduling begins.
 - hosts: hypervisors
   gather_facts: true
@@ -64,14 +72,6 @@
       vars:
         vms: "{{ nodes }}"
 
-    - name: Set up Virtual BMC daemon
-      include_role:
-        name: virtualbmc-daemon
-      vars:
-        vbmcd_virtualenv_path: "{{ virtualenv_path }}"
-        vbmcd_python_upper_contraints_url: "{{ python_upper_constraints_url }}"
-      when: (nodes | count) > 0
-
     - name: Register domains with Virtual BMC
       include_role:
         name: virtualbmc-domain

From 0d88c38c1b22b4421e2b4bea611d132339906c65 Mon Sep 17 00:00:00 2001
From: Will Miller <willm@stackhpc.com>
Date: Thu, 30 Aug 2018 09:15:13 +0000
Subject: [PATCH 10/10] Move VBMCD service name into vars file

---
 ansible/roles/virtualbmc-daemon/tasks/main.yml | 4 ----
 ansible/roles/virtualbmc-daemon/vars/main.yml  | 3 +++
 2 files changed, 3 insertions(+), 4 deletions(-)
 create mode 100644 ansible/roles/virtualbmc-daemon/vars/main.yml

diff --git a/ansible/roles/virtualbmc-daemon/tasks/main.yml b/ansible/roles/virtualbmc-daemon/tasks/main.yml
index 6f57966..5dd895c 100644
--- a/ansible/roles/virtualbmc-daemon/tasks/main.yml
+++ b/ansible/roles/virtualbmc-daemon/tasks/main.yml
@@ -1,8 +1,4 @@
 ---
-- name: Set service name
-  set_fact:
-    service: vbmcd
-
 - name: Ensure Python requirements are installed
   pip:
     requirements: "{{ '/'.join([role_path, 'files', 'requirements.txt']) }}"
diff --git a/ansible/roles/virtualbmc-daemon/vars/main.yml b/ansible/roles/virtualbmc-daemon/vars/main.yml
new file mode 100644
index 0000000..176ea55
--- /dev/null
+++ b/ansible/roles/virtualbmc-daemon/vars/main.yml
@@ -0,0 +1,3 @@
+---
+# The name of the Virtual BMC daemon systemd service.
+service: vbmcd