ComputeSriov's Role-specific parameters are applied to Controller during minor update

[Saravanan KR]

Controller Role's tuned_profile is "throughput-performance".
ComputeSriov Role's tuned_profile is "cpu-partitioning".

Variable 'tuned_profile' with value 'cpu-partitioning' is applied to
Controller Role, when import_role is used with vars.

    - import_role:
        name: tuned
      vars:
        tuned_profile: 'cpu-partitioning'

Eventhough 'cpu-partitioning' profile is defined only for the
ComputeSriov role under the condition of 'ComptueSriov' role name,
because of using import_role, the variable 'tuned_profile' with value
'cpu-partitioning' is applied to the whole PLAY itself. As per the
TripleO's Role-specific implementation, the Role-specific variables
should be applied only to the specific TripleO Role, and should not
affect the other Roles.

Firstly, is the this expected ansible behavior for import_role?

https://docs.ansible.com/ansible/latest/modules/import_role_module.html#notes

As per the ansible import_role documentation, the behavior is
expected. "Since Ansible 2.7 variables defined in vars and defaults
for the role are exposed at playbook parsing time. Due to this, these
variables will be accessible to roles and tasks executed before the
location of the import_role task."

For TripleO's Role-specific parameter support, using 'include_role'
gives the expected behavior of apply the variable only to the included
role (of the TripleO's Role) only and not affecting the entier PLAY. I
have create a small gist to understand the behavior.

  - name: play1
    hosts: test
    tasks:
      - debug: var=test_role_var1
      - import_role:
          name: test-role
        vars:
          test_role_var1: 'test_var1_local'

  - name: play2
    hosts: test
      - debug: var=test_role_var1

In this play book, the variable 'test_role_var1' is defined for the
entire PLAY 'play1' and it does not affect 'play2'. Here changing
import_role to include_role, makes the variable 'test_role_var1' to be
defined only inside the 'test-role'. This whole sample code is
availble in this git repo for better understanding.

https://github.com/krsacme/ansible-include-vs-import

But the same import_role is present in deployment too, why is it
affecting minor update only?

The static import will affect only the PLAY wher it is included. And
it does not affect other PLAYs. That is the difference for deployment
and minor update.

Deployment (only relevant content of deploy_steps_playbook.yaml):

    - hosts: Controller:overcloud
      name: Overcloud deploy step tasks for step 0
      tasks:
        - import_tasks: deploy_steps_tasks_step_0.yaml
      tags:
        - step0

Minor Update (only relevant content of update_steps_playbook.yaml):

    - hosts: Controller
      name: Run update
      tasks:
        - include_tasks: update_steps_tasks.yaml
          with_sequence: start=0 end=5
          loop_control:
            loop_var: step
        - import_tasks: Controller/host_prep_tasks.yaml
          when: tripleo_role_name == 'Controller'
        - import_tasks: deploy_steps_tasks_step_0.yaml
          vars:
            step: 0
        - import_tasks: common_deploy_steps_tasks_step_1.yaml

In case of deployment, 'deploy_steps_tasks_step_0.yaml' is used in a
separate PLAY, which will affect only the step0.

In case of minor update, 'deploy_steps_tasks_step_0.yaml' is used
along with 'host_prep_tasks.yaml', where tuned is invoked for all the
roles with repsective tuned profile (incase of controller, it should
be throughput-performance). Because of static import of import_role,
the variable 'tuned_profile' is importated from the ComptueSriov role
inside the 'deploy_steps_tasks_step_0.yaml' file, which is causing
the variable 'tuned_profile' updated as 'cpu-partitioning' for the
whole PLAY itself.

Solution to this minor update problem:

option-1) As like deployment separate out the step0 tasks to a
different play. Though this will solve the problem for now, it may
cause trouble when there is a change in step0 which may support
differnt Role-specific variables.

option-2) Move all import_role to include_role, where Role-specific
parameter are used. This will gives the expected behavior, but may use
static import advantages.

I believe include_role should be the appropriate solution to handle
TripleO's Role-specific parameters.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/701882

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/701882
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=8d6edac63708d0f40e7f7db75f86a2013efbbc39
Submitter: Zuul
Branch: master

commit 8d6edac63708d0f40e7f7db75f86a2013efbbc39
Author: Saravanan KR <email address hidden>
Date: Fri Jan 10 11:58:38 2020 +0530

    Modify import_role to include_role for boot params service

    Using static import_role, make the vars defined under the
    import_role task to be available for the whole PLAY itself,
    which is causing the role-specific parameter to be available
    in other roles. Move to dynamic include_role, which will
    define these variables only for the included ansible role.
    Closes-Bug: #1859129

    Change-Id: I402db858526def9dfd33f954f1ecd885c01f4367

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/train
Review: https://review.opendev.org/702344

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/702344
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=dada512485fc5472eededc8407db657a0ceaeb3c
Submitter: Zuul
Branch: stable/train

commit dada512485fc5472eededc8407db657a0ceaeb3c
Author: Saravanan KR <email address hidden>
Date: Fri Jan 10 11:58:38 2020 +0530

    Modify import_role to include_role for boot params service

    Using static import_role, make the vars defined under the
    import_role task to be available for the whole PLAY itself,
    which is causing the role-specific parameter to be available
    in other roles. Move to dynamic include_role, which will
    define these variables only for the included ansible role.
    Closes-Bug: #1859129

    Change-Id: I402db858526def9dfd33f954f1ecd885c01f4367
    (cherry picked from commit 8d6edac63708d0f40e7f7db75f86a2013efbbc39)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/tripleo-heat-templates 12.1.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/tripleo-heat-templates 11.4.0 release.