From 75273302ed84d37f2742714b4ed0af073cf8deb3 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Fri, 13 Mar 2020 17:57:20 +0000 Subject: [PATCH] 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 --- .../library/virtualbmc_domain.py | 163 ++++++++++++++++++ .../roles/virtualbmc-domain/tasks/main.yml | 78 ++------- .../virtualbmc-module-f9d40eb9198ca8c0.yaml | 4 + 3 files changed, 178 insertions(+), 67 deletions(-) create mode 100644 ansible/roles/virtualbmc-domain/library/virtualbmc_domain.py create mode 100644 releasenotes/notes/virtualbmc-module-f9d40eb9198ca8c0.yaml diff --git a/ansible/roles/virtualbmc-domain/library/virtualbmc_domain.py b/ansible/roles/virtualbmc-domain/library/virtualbmc_domain.py new file mode 100644 index 0000000..fff183f --- /dev/null +++ b/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() diff --git a/ansible/roles/virtualbmc-domain/tasks/main.yml b/ansible/roles/virtualbmc-domain/tasks/main.yml index b0f210c..c18ba66 100644 --- a/ansible/roles/virtualbmc-domain/tasks/main.yml +++ b/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 diff --git a/releasenotes/notes/virtualbmc-module-f9d40eb9198ca8c0.yaml b/releasenotes/notes/virtualbmc-module-f9d40eb9198ca8c0.yaml new file mode 100644 index 0000000..e744b32 --- /dev/null +++ b/releasenotes/notes/virtualbmc-module-f9d40eb9198ca8c0.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Improves reliability and performance of VirtualBMC domain management.