From e4ad94c1eadcb2d054d52174d0b4c30a5c63c78b Mon Sep 17 00:00:00 2001
From: Riccardo Pittau <elfosardo@gmail.com>
Date: Wed, 2 Sep 2020 15:14:53 +0200
Subject: [PATCH] Follow up on "Fix error 601"

Fixing some nits from the precedent patch.

Change-Id: I4eca6c25d44d3dfebb7ee99bcf206c2a9f8e923b
---
 .../example-deploy-all-available-nodes.yaml   | 12 +++++++----
 playbooks/redeploy-dynamic.yaml               | 21 ++++++++++++-------
 .../tasks/ssh_public_key_path.yaml            |  2 +-
 .../tasks/main.yml                            |  2 +-
 .../tasks/prepare_libvirt.yml                 |  4 ++--
 .../tasks/bootstrap.yml                       | 10 ++++-----
 6 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/playbooks/example-deploy-all-available-nodes.yaml b/playbooks/example-deploy-all-available-nodes.yaml
index 2bed57af7..36d8c6325 100644
--- a/playbooks/example-deploy-all-available-nodes.yaml
+++ b/playbooks/example-deploy-all-available-nodes.yaml
@@ -26,7 +26,11 @@
   become: no
   connection: local
   roles:
-    - { role: bifrost-configdrives-dynamic,
-        when: provision_state == "available" and maintenance | bool }
-    - { role: bifrost-deploy-nodes-dynamic,
-        when: provision_state == "available" and maintenance | bool }
+    - role: bifrost-configdrives-dynamic
+      when:
+        - provision_state == "available"
+        - maintenance | bool
+    - role: bifrost-deploy-nodes-dynamic
+      when:
+        - provision_state == "available"
+        - maintenance | bool
diff --git a/playbooks/redeploy-dynamic.yaml b/playbooks/redeploy-dynamic.yaml
index 8bffab8f2..e7b32f505 100644
--- a/playbooks/redeploy-dynamic.yaml
+++ b/playbooks/redeploy-dynamic.yaml
@@ -38,11 +38,12 @@
         skip_items: []
       register: node_info
   roles:
-    - { role: bifrost-unprovision-node-dynamic,
-        when: (provision_state == "active"
+    - role: bifrost-unprovision-node-dynamic
+      when:
+        - provision_state == "active"
           or provision_state == "deploy failed"
-          or provision_state == "error")
-          and (not maintenance | bool) }
+          or provision_state == "error"
+        - not maintenance | bool
   post_tasks:
     - name: "Pull ironic facts until provision state available"
       os_ironic_node_info:
@@ -60,10 +61,14 @@
   become: no
   connection: local
   roles:
-    - { role: bifrost-configdrives-dynamic,
-        when: (provision_state == "available") and (not maintenance | bool) }
-    - { role: bifrost-deploy-nodes-dynamic,
-        when: (provision_state == "available") and (not maintenance | bool) }
+    - role: bifrost-configdrives-dynamic
+      when:
+        - provision_state == "available"
+        - not maintenance | bool
+    - role: bifrost-deploy-nodes-dynamic
+      when:
+        - provision_state == "available"
+        - not maintenance | bool
   post_tasks:
     - name: "Pull ironic facts until provision state active"
       os_ironic_node_info:
diff --git a/playbooks/roles/bifrost-configdrives-dynamic/tasks/ssh_public_key_path.yaml b/playbooks/roles/bifrost-configdrives-dynamic/tasks/ssh_public_key_path.yaml
index 1a078ca21..55978d751 100644
--- a/playbooks/roles/bifrost-configdrives-dynamic/tasks/ssh_public_key_path.yaml
+++ b/playbooks/roles/bifrost-configdrives-dynamic/tasks/ssh_public_key_path.yaml
@@ -22,7 +22,7 @@
 - name: "Defined ssh_public_key_path - Error if ssh_public_key_path is not valid"
   fail:
     msg: "ssh_public_key_path is not valid."
-  when: not test_ssh_public_key_path.stat.exists | bool
+  when: not test_ssh_public_key_path.stat.exists
   delegate_to: localhost
 
 - name: "Defined ssh_public_key_path - Read SSH public key in"
diff --git a/playbooks/roles/bifrost-create-bootable-image/tasks/main.yml b/playbooks/roles/bifrost-create-bootable-image/tasks/main.yml
index 3b6fe46dd..0c66f9ed3 100644
--- a/playbooks/roles/bifrost-create-bootable-image/tasks/main.yml
+++ b/playbooks/roles/bifrost-create-bootable-image/tasks/main.yml
@@ -18,4 +18,4 @@
   register: test_image_present
 - name: "Create bootable image"
   include: create_bootable_image.yml
-  when: test_image_present.stat.exists | bool
+  when: test_image_present.stat.exists
diff --git a/playbooks/roles/bifrost-create-vm-nodes/tasks/prepare_libvirt.yml b/playbooks/roles/bifrost-create-vm-nodes/tasks/prepare_libvirt.yml
index ec3abc46d..6982a7487 100644
--- a/playbooks/roles/bifrost-create-vm-nodes/tasks/prepare_libvirt.yml
+++ b/playbooks/roles/bifrost-create-vm-nodes/tasks/prepare_libvirt.yml
@@ -58,8 +58,8 @@
 - name: "Place sgabios.bin"
   command: cp /usr/share/misc/sgabios.bin /usr/share/qemu/sgabios.bin
   when:
-    - not test_sgabios_qemu | bool
-    - test_sgabios_misc | bool
+    - not test_sgabios_qemu.stat.exists
+    - test_sgabios_misc.stat.exists
 
 # NOTE(TheJulia): In order to prevent conflicts, stop
 # dnsmasq to prevent conflicts with libvirt restarting.
diff --git a/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml b/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
index 217f562a1..7729dcb3a 100644
--- a/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
+++ b/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
@@ -161,8 +161,7 @@
 
 - name: "Populate keystone for Bifrost"
   include: keystone_setup.yml
-  when:
-    - enable_keystone | bool
+  when: enable_keystone | bool
 
 # NOTE(pas-ha) needed to e.g. pick up new interfaces after libvirt install
 - name: "Refresh facts"
@@ -189,14 +188,13 @@
   environment: "{{ bifrost_venv_env }}"
   when:
     - ironic.database.host == 'localhost'
-    - test_created_db.changed | bool
+    - test_created_db.changed
 
 - name: "Upgrade ironic DB Schema"
   command: ironic-dbsync --config-file /etc/ironic/ironic.conf upgrade
   environment: "{{ bifrost_venv_env }}"
-  when: >
-      ironic.database.host != 'localhost' or
-      not test_created_db.changed | bool
+  when: ironic.database.host != 'localhost'
+        or not test_created_db.changed
 
 - name: "Create service folder"
   file: