From 2e842ab3f5c7b737b547f4c2c718f96294f582d5 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Mon, 17 Feb 2020 09:22:25 +0000
Subject: [PATCH] Remove activate-virtualenv and deactivate-virtualenv roles

These roles are no longer necessary now that Ansible supports setting
ansible_python_interpreter via a task- or role-scoped variable.

Change-Id: I4121d01dc83ac028350d4d98d3e1158e15fdfd63
Story: 2006574
Task: 38824
---
 ansible/baremetal-compute-inspect.yml         |  11 +-
 ansible/baremetal-compute-manage.yml          |  11 +-
 ansible/baremetal-compute-provide.yml         |  11 +-
 ansible/kayobe-target-venv.yml                |  15 +-
 ansible/pip.yml                               |   2 +
 ansible/provision-net.yml                     |  12 +-
 .../activate-virtualenv/defaults/main.yml     |   3 -
 .../roles/activate-virtualenv/tasks/main.yml  |  10 --
 .../deactivate-virtualenv/defaults/main.yml   |   5 -
 .../deactivate-virtualenv/tasks/main.yml      |   6 -
 ansible/roles/ipa-images/tasks/main.yml       | 133 +++++++++---------
 .../ironic-inspector-rules/tasks/main.yml     |  12 +-
 ansible/roles/pip/tasks/pip_local_mirror.yml  |  20 ---
 13 files changed, 81 insertions(+), 170 deletions(-)
 delete mode 100644 ansible/roles/activate-virtualenv/defaults/main.yml
 delete mode 100644 ansible/roles/activate-virtualenv/tasks/main.yml
 delete mode 100644 ansible/roles/deactivate-virtualenv/defaults/main.yml
 delete mode 100644 ansible/roles/deactivate-virtualenv/tasks/main.yml

diff --git a/ansible/baremetal-compute-inspect.yml b/ansible/baremetal-compute-inspect.yml
index 0425c3cf6..416f70ae3 100644
--- a/ansible/baremetal-compute-inspect.yml
+++ b/ansible/baremetal-compute-inspect.yml
@@ -18,14 +18,13 @@
       os_openstacksdk_state: latest
       os_openstacksdk_upper_constraints_file: "{{ pip_upper_constraints_file }}"
 
-    - role: activate-virtualenv
-      activate_virtualenv_path: "{{ venv }}"
-
 - name: Ensure baremetal compute nodes are inspected in ironic
   hosts: baremetal-compute
   gather_facts: False
   vars:
     controller_host: "{{ groups['controllers'][0] }}"
+    venv: "{{ virtualenv_path }}/openstacksdk"
+    ansible_python_interpreter: "{{ venv }}/python"
     # Whether to wait for the state transition to complete.
     baremetal_compute_wait: True
     # Time to wait for state transition to complete, if baremetal_compute_wait
@@ -46,9 +45,3 @@
         # NOTE: Without this, the controller's ansible_host variable will not
         # be respected when using delegate_to.
         ansible_host: "{{ hostvars[controller_host].ansible_host | default(controller_host) }}"
-
-- name: Deactivate the virtual environment on the controller
-  hosts: controllers[0]
-  gather_facts: False
-  roles:
-    - role: deactivate-virtualenv
diff --git a/ansible/baremetal-compute-manage.yml b/ansible/baremetal-compute-manage.yml
index 43c19dc1c..eddf81f8a 100644
--- a/ansible/baremetal-compute-manage.yml
+++ b/ansible/baremetal-compute-manage.yml
@@ -18,13 +18,12 @@
       os_openstacksdk_state: latest
       os_openstacksdk_upper_constraints_file: "{{ pip_upper_constraints_file }}"
 
-    - role: activate-virtualenv
-      activate_virtualenv_path: "{{ venv }}"
-
 - name: Ensure baremetal compute nodes are manageable in ironic
   hosts: baremetal-compute
   gather_facts: False
   vars:
+    venv: "{{ virtualenv_path }}/openstacksdk"
+    ansible_python_interpreter: "{{ venv }}/python"
     # Whether to wait for the state transition to complete.
     baremetal_compute_wait: True
     # Time to wait for state transition to complete, if baremetal_compute_wait
@@ -41,9 +40,3 @@
       os_ironic_state_wait: "{{ baremetal_compute_wait }}"
       os_ironic_state_timeout: "{{ baremetal_compute_timeout }}"
       os_ironic_state_delegate_to: "{{ groups['controllers'][0] }}"
-
-- name: Ensure baremetal compute nodes are available in ironic
-  hosts: controllers[0]
-  gather_facts: False
-  roles:
-    - role: deactivate-virtualenv
diff --git a/ansible/baremetal-compute-provide.yml b/ansible/baremetal-compute-provide.yml
index 7c01d5a90..62333c02d 100644
--- a/ansible/baremetal-compute-provide.yml
+++ b/ansible/baremetal-compute-provide.yml
@@ -18,13 +18,12 @@
       os_openstacksdk_state: latest
       os_openstacksdk_upper_constraints_file: "{{ pip_upper_constraints_file }}"
 
-    - role: activate-virtualenv
-      activate_virtualenv_path: "{{ venv }}"
-
 - name: Ensure baremetal compute nodes are available in ironic
   hosts: baremetal-compute
   gather_facts: False
   vars:
+    venv: "{{ virtualenv_path }}/openstacksdk"
+    ansible_python_interpreter: "{{ venv }}/python"
     # Whether to wait for the state transition to complete.
     baremetal_compute_wait: True
     # Time to wait for state transition to complete, if baremetal_compute_wait
@@ -41,9 +40,3 @@
       os_ironic_state_wait: "{{ baremetal_compute_wait }}"
       os_ironic_state_timeout: "{{ baremetal_compute_timeout }}"
       os_ironic_state_delegate_to: "{{ groups['controllers'][0] }}"
-
-- name: Ensure baremetal compute nodes are available in ironic
-  hosts: controllers[0]
-  gather_facts: False
-  roles:
-    - role: deactivate-virtualenv
diff --git a/ansible/kayobe-target-venv.yml b/ansible/kayobe-target-venv.yml
index 344f2bf9e..dbbc536e7 100644
--- a/ansible/kayobe-target-venv.yml
+++ b/ansible/kayobe-target-venv.yml
@@ -17,11 +17,6 @@
         - not ansible_python_interpreter.startswith('/usr/bin')
 
     - block:
-        # This will cause ansible to use the system python interpreter.
-        - name: Deactivate the virtualenv
-          include_role:
-            name: deactivate-virtualenv
-
         - name: Gather facts
           setup:
 
@@ -66,12 +61,10 @@
             # Site packages are required for using the yum and selinux python
             # modules, which are not available via PyPI.
             virtualenv_site_packages: True
-
-        - name: Activate the virtualenv
-          include_role:
-            name: activate-virtualenv
-          vars:
-            activate_virtualenv_path: "{{ virtualenv }}"
+      vars:
+        # Use the system python interpreter since the virtualenv might not
+        # exist.
+        ansible_python_interpreter: /usr/libexec/platform-python
       when: virtualenv is defined
 
     - block:
diff --git a/ansible/pip.yml b/ansible/pip.yml
index 24aea98da..4b5803230 100644
--- a/ansible/pip.yml
+++ b/ansible/pip.yml
@@ -3,6 +3,8 @@
   hosts: seed-hypervisor:seed:overcloud
   tags:
     - pip
+  vars:
+    ansible_python_interpreter: /usr/libexec/platform-python
   roles:
     - role: pip
   gather_facts: false
diff --git a/ansible/provision-net.yml b/ansible/provision-net.yml
index 6548e9d32..9332067b7 100644
--- a/ansible/provision-net.yml
+++ b/ansible/provision-net.yml
@@ -78,11 +78,6 @@
     # so during this transition we manually update them here if necessary.
     # TODO(mgoddard): Remove this code after a suitable transition period.
 
-    - import_role:
-        role: activate-virtualenv
-      vars:
-        activate_virtualenv_path: "{{ venv }}"
-
     - name: Ensure python-openstackclient is installed
       pip:
         name: python-openstackclient
@@ -106,6 +101,8 @@
           changed_when: true
           when: openstack_networks[0].shared
           environment: "{{ openstack_auth_env }}"
+      vars:
+        ansible_python_interpreter: "{{ venv }}/bin/python"
       when: not provision_net.shared | bool
 
     - block:
@@ -123,9 +120,8 @@
           changed_when: true
           when: openstack_networks[0].shared
           environment: "{{ openstack_auth_env }}"
+      vars:
+        ansible_python_interpreter: "{{ venv }}/bin/python"
       when:
         - cleaning_net_name != provision_wl_net_name
         - not cleaning_net.shared | bool
-
-    - import_role:
-        role: deactivate-virtualenv
diff --git a/ansible/roles/activate-virtualenv/defaults/main.yml b/ansible/roles/activate-virtualenv/defaults/main.yml
deleted file mode 100644
index 88ddb6608..000000000
--- a/ansible/roles/activate-virtualenv/defaults/main.yml
+++ /dev/null
@@ -1,3 +0,0 @@
----
-# Path to a virtualenv to activate.
-activate_virtualenv_path:
diff --git a/ansible/roles/activate-virtualenv/tasks/main.yml b/ansible/roles/activate-virtualenv/tasks/main.yml
deleted file mode 100644
index fce768d6d..000000000
--- a/ansible/roles/activate-virtualenv/tasks/main.yml
+++ /dev/null
@@ -1,10 +0,0 @@
----
-- name: Set a fact containing the current python interpreter
-  set_fact:
-    activate_virtualenv_current_python_interpreter: "{{ ansible_python_interpreter | default('/usr/bin/python') }}"
-
-# Note that setting this via a play or task variable seems to not
-# evaluate the Jinja variable reference, so we use set_fact.
-- name: Update the Ansible python interpreter fact to point to the virtualenv
-  set_fact:
-    ansible_python_interpreter: "{{ activate_virtualenv_path }}/bin/python"
diff --git a/ansible/roles/deactivate-virtualenv/defaults/main.yml b/ansible/roles/deactivate-virtualenv/defaults/main.yml
deleted file mode 100644
index 726f74f2c..000000000
--- a/ansible/roles/deactivate-virtualenv/defaults/main.yml
+++ /dev/null
@@ -1,5 +0,0 @@
----
-# Path to a python interpreter to set as the ansible_python_interpreter
-# variable. The default uses a variable set by the activate-virtualenv role
-# containing the original python interpreter before entering the virtualenv.
-deactivate_virtualenv_python_interpreter: "{{ activate_virtualenv_current_python_interpreter | default('/usr/bin/python') }}"
diff --git a/ansible/roles/deactivate-virtualenv/tasks/main.yml b/ansible/roles/deactivate-virtualenv/tasks/main.yml
deleted file mode 100644
index 36fc4d9c2..000000000
--- a/ansible/roles/deactivate-virtualenv/tasks/main.yml
+++ /dev/null
@@ -1,6 +0,0 @@
----
-# This variable is unset before we set it, and it does not appear to be
-# possible to unset a variable in Ansible.
-- name: Set a fact to reset the Ansible python interpreter
-  set_fact:
-    ansible_python_interpreter: "{{ deactivate_virtualenv_python_interpreter }}"
diff --git a/ansible/roles/ipa-images/tasks/main.yml b/ansible/roles/ipa-images/tasks/main.yml
index 370e85b48..6884af5ff 100644
--- a/ansible/roles/ipa-images/tasks/main.yml
+++ b/ansible/roles/ipa-images/tasks/main.yml
@@ -50,12 +50,6 @@
   when:
     - not item.exists
 
-- name: Activate the virtualenv
-  include_role:
-    name: activate-virtualenv
-  vars:
-    activate_virtualenv_path: "{{ ipa_images_venv }}"
-
 - name: Ensure we have python-ironicclient installed
   pip:
     name: python-ironicclient
@@ -66,72 +60,71 @@
 # To support updating the IPA image, we check the MD5 sum of the cached image
 # files, and compare with the images in Glance (if there are any).
 
-- name: Gather facts about Ironic Python Agent (IPA) kernel image
-  os_image_facts:
-    auth_type: "{{ ipa_images_openstack_auth_type }}"
-    auth: "{{ ipa_images_openstack_auth }}"
-    cacert: "{{ ipa_images_openstack_cacert | default(omit, true) }}"
-    interface: "{{ ipa_images_openstack_interface | default(omit, true) }}"
-    image: "{{ ipa_images_kernel_name }}"
+- block:
+    - name: Gather facts about Ironic Python Agent (IPA) kernel image
+      os_image_facts:
+        auth_type: "{{ ipa_images_openstack_auth_type }}"
+        auth: "{{ ipa_images_openstack_auth }}"
+        cacert: "{{ ipa_images_openstack_cacert | default(omit, true) }}"
+        interface: "{{ ipa_images_openstack_interface | default(omit, true) }}"
+        image: "{{ ipa_images_kernel_name }}"
 
-- name: Set a fact containing the Ironic Python Agent (IPA) kernel image
-  set_fact:
-    ipa_images_kernel_openstack_image: "{{ openstack_image if openstack_image else {} }}"
+    - name: Set a fact containing the Ironic Python Agent (IPA) kernel image
+      set_fact:
+        ipa_images_kernel_openstack_image: "{{ openstack_image if openstack_image else {} }}"
 
-- name: Gather facts about Ironic Python Agent (IPA) ramdisk image
-  os_image_facts:
-    auth_type: "{{ ipa_images_openstack_auth_type }}"
-    auth: "{{ ipa_images_openstack_auth }}"
-    cacert: "{{ ipa_images_openstack_cacert | default(omit, true) }}"
-    interface: "{{ ipa_images_openstack_interface | default(omit, true) }}"
-    image: "{{ ipa_images_ramdisk_name }}"
+    - name: Gather facts about Ironic Python Agent (IPA) ramdisk image
+      os_image_facts:
+        auth_type: "{{ ipa_images_openstack_auth_type }}"
+        auth: "{{ ipa_images_openstack_auth }}"
+        cacert: "{{ ipa_images_openstack_cacert | default(omit, true) }}"
+        interface: "{{ ipa_images_openstack_interface | default(omit, true) }}"
+        image: "{{ ipa_images_ramdisk_name }}"
 
-- name: Set a fact containing the Ironic Python Agent (IPA) ramdisk image
-  set_fact:
-    ipa_images_ramdisk_openstack_image: "{{ openstack_image if openstack_image else {} }}"
+    - name: Set a fact containing the Ironic Python Agent (IPA) ramdisk image
+      set_fact:
+        ipa_images_ramdisk_openstack_image: "{{ openstack_image if openstack_image else {} }}"
 
-# The os_image module will get confused if there are multiple images with the
-# same name, so rename the old images. They will still be accessible via UUID.
-- name: Ensure old Ironic Python Agent (IPA) images are renamed
-  command: >
-    {{ ipa_images_venv }}/bin/openstack image set {{ item.name }} --name {{ item.name }}.{{ extension }}
+    # The os_image module will get confused if there are multiple images with the
+    # same name, so rename the old images. They will still be accessible via UUID.
+    - name: Ensure old Ironic Python Agent (IPA) images are renamed
+      command: >
+        {{ ipa_images_venv }}/bin/openstack image set {{ item.name }} --name {{ item.name }}.{{ extension }}
+      vars:
+        extension: "{{ item.created_at | replace(':', '-') }}~"
+      with_items:
+        - name: "{{ ipa_images_kernel_name }}"
+          created_at: "{{ ipa_images_kernel_openstack_image.created_at | default }}"
+          checksum: "{{ ipa_images_checksum.results[0].stat.checksum }}"
+          glance_checksum: "{{ ipa_images_kernel_openstack_image.checksum | default }}"
+        - name: "{{ ipa_images_ramdisk_name }}"
+          created_at: "{{ ipa_images_ramdisk_openstack_image.created_at | default }}"
+          checksum: "{{ ipa_images_checksum.results[1].stat.checksum }}"
+          glance_checksum: "{{ ipa_images_ramdisk_openstack_image.checksum | default }}"
+      when:
+        - item.glance_checksum
+        - item.checksum != item.glance_checksum
+      environment: "{{ ipa_images_openstack_auth_env }}"
+
+    - name: Ensure Ironic Python Agent (IPA) images are registered with Glance
+      os_image:
+        auth_type: "{{ ipa_images_openstack_auth_type }}"
+        auth: "{{ ipa_images_openstack_auth }}"
+        cacert: "{{ ipa_images_openstack_cacert | default(omit, true) }}"
+        interface: "{{ ipa_images_openstack_interface | default(omit, true) }}"
+        name: "{{ item.name }}"
+        container_format: "{{ item.format }}"
+        disk_format: "{{ item.format }}"
+        state: present
+        filename: "{{ ipa_images_cache_path }}/{{ item.name }}"
+      with_items:
+        - name: "{{ ipa_images_kernel_name }}"
+          format: aki
+        - name: "{{ ipa_images_ramdisk_name }}"
+          format: ari
+      register: ipa_images_new_images
+
+    - include_tasks: set-driver-info.yml
+      when: ipa_images_update_ironic_nodes | bool
   vars:
-    extension: "{{ item.created_at | replace(':', '-') }}~"
-  with_items:
-    - name: "{{ ipa_images_kernel_name }}"
-      created_at: "{{ ipa_images_kernel_openstack_image.created_at | default }}"
-      checksum: "{{ ipa_images_checksum.results[0].stat.checksum }}"
-      glance_checksum: "{{ ipa_images_kernel_openstack_image.checksum | default }}"
-    - name: "{{ ipa_images_ramdisk_name }}"
-      created_at: "{{ ipa_images_ramdisk_openstack_image.created_at | default }}"
-      checksum: "{{ ipa_images_checksum.results[1].stat.checksum }}"
-      glance_checksum: "{{ ipa_images_ramdisk_openstack_image.checksum | default }}"
-  when:
-    - item.glance_checksum
-    - item.checksum != item.glance_checksum
-  environment: "{{ ipa_images_openstack_auth_env }}"
-
-- name: Ensure Ironic Python Agent (IPA) images are registered with Glance
-  os_image:
-    auth_type: "{{ ipa_images_openstack_auth_type }}"
-    auth: "{{ ipa_images_openstack_auth }}"
-    cacert: "{{ ipa_images_openstack_cacert | default(omit, true) }}"
-    interface: "{{ ipa_images_openstack_interface | default(omit, true) }}"
-    name: "{{ item.name }}"
-    container_format: "{{ item.format }}"
-    disk_format: "{{ item.format }}"
-    state: present
-    filename: "{{ ipa_images_cache_path }}/{{ item.name }}"
-  with_items:
-    - name: "{{ ipa_images_kernel_name }}"
-      format: aki
-    - name: "{{ ipa_images_ramdisk_name }}"
-      format: ari
-  register: ipa_images_new_images
-
-- include_tasks: set-driver-info.yml
-  when: ipa_images_update_ironic_nodes | bool
-
-- name: Deactivate the virtualenv
-  include_role:
-    name: deactivate-virtualenv
+    ansible_python_interpreter: "{{ ipa_images_venv }}/bin/python"
diff --git a/ansible/roles/ironic-inspector-rules/tasks/main.yml b/ansible/roles/ironic-inspector-rules/tasks/main.yml
index cd9aeb4d1..47103073a 100644
--- a/ansible/roles/ironic-inspector-rules/tasks/main.yml
+++ b/ansible/roles/ironic-inspector-rules/tasks/main.yml
@@ -9,13 +9,9 @@
   with_items:
     - name: python-ironic-inspector-client
 
-- name: Activate the virtualenv
-  include_role:
-    name: activate-virtualenv
-  vars:
-    activate_virtualenv_path: "{{ ironic_inspector_venv }}"
-
 - name: Ensure introspection rules exist
+  vars:
+    ansible_python_interpreter: "{{ ironic_inspector_venv }}/bin/python"
   os_ironic_inspector_rule:
     auth_type: "{{ ironic_inspector_auth_type }}"
     auth: "{{ ironic_inspector_auth }}"
@@ -28,7 +24,3 @@
     state: present
     inspector_url: "{{ ironic_inspector_url }}"
   with_items: "{{ ironic_inspector_rules }}"
-
-- name: Deactivate the virtualenv
-  include_role:
-    name: deactivate-virtualenv
diff --git a/ansible/roles/pip/tasks/pip_local_mirror.yml b/ansible/roles/pip/tasks/pip_local_mirror.yml
index df6889426..f11c70065 100644
--- a/ansible/roles/pip/tasks/pip_local_mirror.yml
+++ b/ansible/roles/pip/tasks/pip_local_mirror.yml
@@ -1,17 +1,4 @@
 ---
-- name: Set a fact about the virtualenv
-  set_fact:
-    virtualenv: "{{ ansible_python_interpreter | dirname | dirname }}"
-  when:
-    - ansible_python_interpreter is defined
-    - not ansible_python_interpreter.startswith('/bin')
-    - not ansible_python_interpreter.startswith('/usr/bin')
-
-- name: Deactivate the virtualenv
-  include_role:
-    name: deactivate-virtualenv
-  when: virtualenv is defined
-
 - name: Create local .pip directory for {{ user }}
   file:
     path: "~{{ user }}/.pip"
@@ -42,10 +29,3 @@
     dest: "~{{ user}}/.pydistutils.cfg"
   become: True
   become_user: "{{ user }}"
-
-- name: Activate the virtualenv
-  include_role:
-    name: activate-virtualenv
-  vars:
-    activate_virtualenv_path: "{{ virtualenv }}"
-  when: virtualenv is defined