Refactor virtualbmc-domain tasks into a module

Virtualbmc domain creation can be unreliable, in particular when domains
already exist. This is in part due to the vbmc stop command not functioning
properly [1]. By moving the domain management into a Python module we can
better control the process of creation, and improve performance.

[1] https://storyboard.openstack.org/#!/story/2003534

Change-Id: I52cb08cd0d300630cb6341f50eb0484c1d16daa4

ansible/roles/virtualbmc-domain/library/virtualbmc_domain.py

@@ -0,0 +1,163 @@
+#!/usr/bin/env python3
+
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from ansible.module_utils.basic import AnsibleModule # noqa
+
+import json
+import os.path
+import time
+
+
+DOCUMENTATION = '''
+---
+module: virtualbmc_domain
+short_description: Manages domains in VirtualBMC.
+'''
+
+
+RETRIES = 60
+INTERVAL = 0.5
+
+
+def _vbmc_command(module, args):
+    path_prefix = ("%s/bin" % module.params["virtualenv"]
+                   if module.params["virtualenv"] else None)
+    cmd = ["vbmc", "--no-daemon"]
+    if module.params["log_directory"]:
+        log_file = os.path.join(module.params["log_directory"],
+                                "vbmc-%s.log" % module.params["domain"])
+        cmd += ["--log-file", log_file]
+    cmd += args
+    result = module.run_command(cmd, check_rc=True, path_prefix=path_prefix)
+    rc, out, err = result
+    return out
+
+
+def _get_domain(module, allow_missing=False):
+    domain_name = module.params["domain"]
+    if allow_missing:
+        # Use a list to avoid failing if the domain does not exist.
+        domains = _vbmc_command(module, ["list", "-f", "json"])
+        domains = json.loads(domains)
+        # vbmc list returns a list of dicts. Transform into a dict of dicts
+        # keyed by domain name.
+        domains = {d["Domain name"]: d for d in domains}
+        try:
+            return domains[domain_name]
+        except KeyError:
+            return None
+    else:
+        domain = _vbmc_command(module, ["show", domain_name, "-f", "json"])
+        domain = json.loads(domain)
+        domain = {field["Property"]: field["Value"] for field in domain}
+        return domain
+
+
+def _add_domain(module):
+    domain_name = module.params["domain"]
+    args = [
+        "add", domain_name,
+        "--address", module.params["ipmi_address"],
+        "--port", str(module.params["ipmi_port"]),
+        "--username", module.params["ipmi_username"],
+        "--password", module.params["ipmi_password"],
+    ]
+    if module.params["libvirt_uri"]:
+        args += ["--libvirt-uri", module.params["libvirt_uri"]]
+    _vbmc_command(module, args)
+
+
+def _wait_for_existence(module, exists):
+    """Wait for the domain to exist or cease existing."""
+    for _ in range(RETRIES):
+        domain = _get_domain(module, allow_missing=True)
+        if (exists and domain) or (not exists and not domain):
+            return
+    time.sleep(INTERVAL)
+    action = "added" if exists else "deleted"
+    module.fail_json(msg="Timed out waiting for domain %s to be %s" %
+                     (module.params['domain'], action))
+
+
+def _wait_for_status(module, status):
+    """Wait for the domain to reach a particular status."""
+    for _ in range(RETRIES):
+        domain = _get_domain(module)
+        if domain["status"] == status:
+            return
+        time.sleep(INTERVAL)
+    module.fail_json(msg="Timed out waiting for domain %s to reach status "
+                         "%s" % (module.params['domain'], status))
+
+
+def _virtualbmc_domain(module):
+    """Configure a VirtualBMC domain."""
+    changed = False
+    domain_name = module.params["domain"]
+
+    # Even if the domain 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.
+    domain = _get_domain(module, allow_missing=True)
+    if domain and domain["Status"] == "running":
+        if not module.check_mode:
+            module.debug("Stopping domain %s" % domain_name)
+            _vbmc_command(module, ["stop", domain_name])
+            _wait_for_status(module, "down")
+        changed = True
+
+    if domain:
+        if not module.check_mode:
+            module.debug("Deleting domain %s" % domain_name)
+            _vbmc_command(module, ["delete", domain_name])
+            _wait_for_existence(module, False)
+        changed = True
+
+    if module.params['state'] == 'present':
+        if not module.check_mode:
+            module.debug("Adding domain %s" % domain_name)
+            _add_domain(module)
+            _wait_for_existence(module, True)
+
+            module.debug("Starting domain %s" % domain_name)
+            _vbmc_command(module, ["start", domain_name])
+            _wait_for_status(module, "running")
+        changed = True
+
+    return {"changed": changed}
+
+
+def main():
+    module = AnsibleModule(
+        argument_spec=dict(
+            domain=dict(type='str', required=True),
+            ipmi_address=dict(type='str', required=True),
+            ipmi_port=dict(type='int', required=True),
+            ipmi_username=dict(type='str', required=True),
+            ipmi_password=dict(type='str', required=True, no_log=True),
+            libvirt_uri=dict(type='str'),
+            log_directory=dict(type='str'),
+            state=dict(type=str, default='present',
+                       choices=['present', 'absent']),
+            virtualenv=dict(type='str'),
+        ),
+        supports_check_mode=True,
+    )
+
+    result = _virtualbmc_domain(module)
+    module.exit_json(**result)
+
+
+if __name__ == '__main__':
+    main()

ansible/roles/virtualbmc-domain/tasks/main.yml

@@ -1,69 +1,13 @@
 ---
-- name: Set VBMC command string
-  vars:
-    vbmc_path: >-
-      {{ vbmc_virtualenv_path ~ '/bin/vbmc'
-         if vbmc_virtualenv_path
-         else '/usr/local/bin/vbmc' }}
-  set_fact:
-    # vbmcd should already be running, so --no-daemon stops vbmc from spawning
-    # another instance of the daemon.
-    vbmc_cmd: >-
-      '{{ vbmc_path }}'
-        --no-daemon
-        {% if vbmc_log_directory is not none %}
-        --log-file '{{ vbmc_log_directory }}/vbmc-{{ vbmc_domain }}.log'
-        {% endif %}
-
-# Even if the domain 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 domain is stopped and deleted in VBMC
-  command: >-
-    {{ vbmc_cmd }} {{ item }} '{{ vbmc_domain }}'
-  loop:
-    - stop
-    - delete
-  register: res
-  changed_when: res.rc == 0
-  failed_when:
-    - res.rc != 0
-    - "'No domain with matching name' not in res.stderr"
+- name: Ensure domain {{ vbmc_domain }} is configured
+  virtualbmc_domain:
+    domain: "{{ vbmc_domain }}"
+    ipmi_address: "{{ vbmc_ipmi_address }}"
+    ipmi_port: "{{ vbmc_ipmi_port }}"
+    ipmi_username: "{{ vbmc_ipmi_username }}"
+    ipmi_password: "{{ vbmc_ipmi_password }}"
+    libvirt_uri: "{{ vbmc_libvirt_uri | default(omit, true) }}"
+    log_directory: "{{ vbmc_log_directory | default(omit, true) }}"
+    state: "{{ vbmc_state }}"
+    virtualenv: "{{ vbmc_virtualenv_path | default(omit, true) }}"
   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_address }}"
-    port: "{{ vbmc_ipmi_port }}"
-    state: stopped
-    timeout: 15
-
-# These tasks will trigger ansible lint rule ANSIBLE0012 because they are not
-# idempotent (we always delete and recreate the domain). Use a tag to suppress
-# the checks.
-- name: Ensure domain is added to VBMC
-  command: >-
-    {{ vbmc_cmd }} add '{{ vbmc_domain }}'
-      --port {{ vbmc_ipmi_port }}
-      --username '{{ vbmc_ipmi_username }}'
-      --password '{{ vbmc_ipmi_password }}'
-      --address {{ vbmc_ipmi_address }}
-    {% if vbmc_libvirt_uri %} --libvirt-uri '{{ vbmc_libvirt_uri }}'{% endif %}
-  when: vbmc_state == 'present'
-  become: true
-  tags:
-    - skip_ansible_lint
-
-- name: Ensure domain is started in VBMC
-  command: >
-    {{ vbmc_cmd }} start '{{ vbmc_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
-  when: vbmc_state == 'present'
-  become: true
-  tags:
-    - skip_ansible_lint

releasenotes/notes/virtualbmc-module-f9d40eb9198ca8c0.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Improves reliability and performance of VirtualBMC domain management.