From 7ad354c2afa20d5a2e209bf19ec58543f36c2242 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Thu, 31 Jan 2019 14:39:00 +0000 Subject: [PATCH] Use node state rather than 'cmd' There are two cases when we may want to delete things: 1. if a desired node configuration or spec changes, we delete existing node resources. 2. if we use the teardown.yml playbook, cmd == teardown, delete everything, including global state. In both cases, Tenks state (state.yml), all nodes to be deleted are given a 'state' of absent'. We should therefore use this rather than the 'cmd' variable which only works in the latter case. Change-Id: Icc033340c7fd882e61d90e3d086f7ba4a5b673bf Story: 2004907 Task: 29257 --- ansible/node_bmc.yml | 9 ++++----- ansible/node_instantiation.yml | 5 +---- ansible/node_networking.yml | 2 +- ansible/roles/ironic-enrolment/tasks/main.yml | 4 +--- ansible/roles/virtualbmc-domain/tasks/main.yml | 8 ++++---- releasenotes/notes/fix-reconfigure-1d1b11a57f4d1827.yaml | 6 ++++++ 6 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/fix-reconfigure-1d1b11a57f4d1827.yaml diff --git a/ansible/node_bmc.yml b/ansible/node_bmc.yml index ead201a..044ebe5 100644 --- a/ansible/node_bmc.yml +++ b/ansible/node_bmc.yml @@ -35,16 +35,15 @@ include_role: name: virtualbmc-domain vars: - vbmc_domain: "{{ domain }}" + vbmc_domain: "{{ domain.name }}" vbmc_libvirt_uri: "{{ libvirt_local_uri }}" vbmc_ipmi_address: "{{ ipmi_address }}" vbmc_ipmi_username: "{{ ipmi_username }}" vbmc_ipmi_password: "{{ ipmi_password }}" - vbmc_ipmi_port: "{{ ipmi_port_range_start + port_offset }}" + vbmc_ipmi_port: "{{ domain.ipmi_port }}" vbmc_virtualenv_path: "{{ virtualenv_path }}" vbmc_log_directory: "{{ log_directory }}" - vbmc_state: "{{ 'absent' if cmd == 'teardown' else 'present' }}" - loop: "{{ vbmc_nodes | map(attribute='name') | sort | list }}" + vbmc_state: "{{ domain.get('state', 'present') }}" + loop: "{{ vbmc_nodes | sort(attribute='name') | list }}" loop_control: loop_var: domain - index_var: port_offset diff --git a/ansible/node_instantiation.yml b/ansible/node_instantiation.yml index 720e102..3cefffb 100644 --- a/ansible/node_instantiation.yml +++ b/ansible/node_instantiation.yml @@ -23,10 +23,7 @@ # basis to account for existing state, rather than for all nodes # here. libvirt_vms: >- - {{ nodes | map('combine', - {'state': ('absent' if cmd == 'teardown' - else 'present')}) - | map('set_libvirt_interfaces') + {{ nodes | map('set_libvirt_interfaces') | map('set_libvirt_volume_pool') | map('set_libvirt_start_params') | list }} diff --git a/ansible/node_networking.yml b/ansible/node_networking.yml index 81c6640..4a1d84e 100644 --- a/ansible/node_networking.yml +++ b/ansible/node_networking.yml @@ -18,7 +18,7 @@ veth_pair_ovs_bridge: "{{ physnet.1 | bridge_name }}" veth_pair_ovs_link_name: "{{ physnet.0 | ovs_link_name(physnet.1) }}" veth_pair_source_link_name: "{{ physnet.0 | source_link_name(physnet.1) }}" - veth_pair_state: "{{ 'absent' if cmd == 'teardown' else 'present' }}" + veth_pair_state: "{{ physnet.0.get('state', 'present') }}" # Loop over each physical network for each node allocated to this host. # Allocations are stored in localhost's vars. loop: >- diff --git a/ansible/roles/ironic-enrolment/tasks/main.yml b/ansible/roles/ironic-enrolment/tasks/main.yml index 581ead1..6d05613 100644 --- a/ansible/roles/ironic-enrolment/tasks/main.yml +++ b/ansible/roles/ironic-enrolment/tasks/main.yml @@ -94,8 +94,7 @@ include_tasks: node.yml vars: node: "{{ ironic_node }}" - ipmi_port: >- - {{ hostvars[ironic_hypervisor].ipmi_port_range_start + port_offset }} + ipmi_port: "{{ ironic_node.ipmi_port }}" ironic_deploy_kernel_uuid: >- {{ deploy_image_ids.results.0.stdout | default(ironic_deploy_kernel) }} ironic_deploy_ramdisk_uuid: >- @@ -103,7 +102,6 @@ loop: "{{ ironic_nodes | sort(attribute='name') }}" loop_control: loop_var: ironic_node - index_var: port_offset # If no ironic_config options were set, this means that enrolment should not # be performed. when: "'ironic_config' in ironic_node" diff --git a/ansible/roles/virtualbmc-domain/tasks/main.yml b/ansible/roles/virtualbmc-domain/tasks/main.yml index 08f986a..d6581fc 100644 --- a/ansible/roles/virtualbmc-domain/tasks/main.yml +++ b/ansible/roles/virtualbmc-domain/tasks/main.yml @@ -7,7 +7,7 @@ '{{ vbmc_virtualenv_path }}/bin/vbmc' --no-daemon {% if vbmc_log_directory is not none %} - --log-file '{{ vbmc_log_directory }}/vbmc-{{ domain }}.log' + --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 @@ -15,7 +15,7 @@ # involve minimal downtime. - name: Ensure domain is stopped and deleted in VBMC command: >- - {{ vbmc_cmd }} {{ item }} '{{ domain }}' + {{ vbmc_cmd }} {{ item }} '{{ vbmc_domain }}' loop: - stop - delete @@ -40,7 +40,7 @@ # the checks. - name: Ensure domain is added to VBMC command: >- - {{ vbmc_cmd }} add '{{ domain }}' + {{ vbmc_cmd }} add '{{ vbmc_domain }}' --port {{ vbmc_ipmi_port }} --username '{{ vbmc_ipmi_username }}' --password '{{ vbmc_ipmi_password }}' @@ -53,7 +53,7 @@ - name: Ensure domain is started in VBMC command: > - {{ vbmc_cmd }} start '{{ domain }}' + {{ 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. diff --git a/releasenotes/notes/fix-reconfigure-1d1b11a57f4d1827.yaml b/releasenotes/notes/fix-reconfigure-1d1b11a57f4d1827.yaml new file mode 100644 index 0000000..379adcf --- /dev/null +++ b/releasenotes/notes/fix-reconfigure-1d1b11a57f4d1827.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where VMs, virtual ethernet pairs and virtual BMC state is + not cleaned up on a reconfiguration. See `story 2004907 + `__ for details.