<div dir="ltr"><div>Thanks for bringing this up, Alex</div><div><br></div><div>I was thinking if we can the third option - to have small "single responsibility" roles for every action. For example:</div><div>tripleo-undercloud-install</div><div>tripleo-undercloud-backup</div><div>tripleo-undercloud-upgrade</div><div><br></div><div>And then no one needs to dig into roles to check what actions are supported, but just "ls roles/". Also these roles usually have nothing in common but name, and if they are quite isolated, I think it's better to have them defined separately.</div><div>From cons I can count: more roles and might be some level of duplication in variables.<br></div><div>For pros it's more readable playbook and clear actions:</div><div><br></div><div>- hosts: undercloud<br>  gather_facts: true<br>  collections:<br>    - tripleo.operator<br>  vars:<br>    tripleo_undercloud_debug: true<br>  tasks:<br><br>    - name: Install undercloud<br>      import_role:<br>        name: undercloud-install<br><br>    - name: Upgrade undercloud<br>      import_role:<br>        name: undercloud-upgrade<br></div><div><br></div><div>Thanks<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 9, 2020 at 12:22 AM Alex Schultz <<a href="mailto:aschultz@redhat.com">aschultz@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">[Hello folks,<br>
<br>
I've begun the basic start of the tripleo-operator-ansible collection<br>
work[0].  At the start of this work, I've chosen the undercloud<br>
installation[1] as the first role to use to figure out how we the end<br>
user's to consume these roles.  I wanted to bring up this initial<br>
implementation so that we can discuss how folks will include these<br>
roles.  The initial implementation is a wrapper around the<br>
tripleoclient command as run via openstackclient.  This means that the<br>
'tripleo-undercloud' role provides implementations for 'openstack<br>
undercloud backup', 'openstack undercloud install', and 'openstack<br>
undercloud upgrade'.<br>
<br>
In terms of naming conventions, I'm proposing that we would name the<br>
roles "tripleo-<command-base>" with the last part of the command<br>
action being an "action". Examples:<br>
<br>
"openstack undercloud *" -><br>
role: tripleo-undercloud<br>
action: (backup|install|upgrade)<br>
<br>
"openstack undercloud minion *" -><br>
role: tripleo-undercloud-minion<br>
action: (install|upgrade)<br>
<br>
"openstack overcloud *" -><br>
role: tripleo-overcloud<br>
action: (deploy|delete|export)<br>
<br>
"openstack overcloud node *" -><br>
role: tripleo-overcloud-node<br>
action: (import|introspect|provision|unprovision)<br>
<br>
In terms of end user interface, I've got two proposals out in terms of<br>
possible implementations.<br>
<br>
Tasks from method:<br>
The initial commit propose that we would require the end user to use<br>
an include_role/tasks_from call to perform the desired action.  For<br>
example:<br>
<br>
    - hosts: undercloud<br>
      gather_facts: true<br>
      tasks:<br>
        - name: Install undercloud<br>
          collections:<br>
            - tripleo.operator<br>
          import_role:<br>
            name: tripleo-undercloud<br>
            tasks_from: install<br>
          vars:<br>
            tripleo_undercloud_debug: true<br>
<br>
Variable switch method:<br>
I've also proposed an alternative implementation[2] that would use<br>
include_role but require the end user to set a specific variable to<br>
change if the role runs 'install', 'backup' or 'upgrade'. With this<br>
patch the playbook would look something like:<br>
<br>
    - hosts: undercloud<br>
      gather_facts: true<br>
      tasks:<br>
        - name: Install undercloud<br>
          collections:<br>
            - tripleo.operator<br>
          import_role:<br>
            name: tripleo-undercloud<br>
          vars:<br>
            tripleo_undercloud_action: install<br>
            tripleo_undercloud_debug: true<br>
<br>
I would like to solicit feedback on which one of these is the<br>
preferred integration method when calling these roles. I have two<br>
patches up in tripleo-quickstart-extras to show how these calls could<br>
be run. The "Tasks from method" can be viewed here[3]. The "Variable<br>
switch method" can be viewed here[4].  I can see pros and cons for<br>
both methods.<br>
<br>
My take would be:<br>
<br>
Tasks from method:<br>
Pros:<br>
 - action is a bit more explicit<br>
 - dynamic logic left up to the playbook/consumer.<br>
 - May not have a 'default' action (as main.yml is empty, though it<br>
could be implemented).<br>
 - tasks_from would be a global implementation across all roles rather<br>
than having a changing variable name.<br>
<br>
Cons:<br>
 - internal task file names must be known by the consumer (though IMHO<br>
this is no different than the variable name + values in the other<br>
implementation)<br>
 - role/action inclusions is not dynamic in the role (it can be in the playbook)<br>
<br>
Variable switch method:<br>
Pros:<br>
 - inclusion of the role by default runs an install<br>
 - action can be dynamically changed from the calling playbook via an<br>
ansible var<br>
 - structure of the task files is internal to the role and the user of<br>
the role need not know the filenames/structure.<br>
<br>
Cons:<br>
 - calling playbook is not explicit in that the action can be switched<br>
dynamically (e.g. intentionally or accidentally because it is dynamic)<br>
 - implementer must know to configure a variable called<br>
`tripleo_undercloud_action` to switch between install/backup/upgrade<br>
actions<br>
 - variable names are likely different depending on the role<br>
<br>
My personal preference might be to use the "Tasks from method" because<br>
it would lend itself to the same implementation across all roles and<br>
the dynamic logic is left to the playbook rather than internally in<br>
the role. For example, we'd end up with something like:<br>
<br>
    - hosts: undercloud<br>
      gather_facts: true<br>
      collections:<br>
        - tripleo.operator<br>
      tasks:<br>
        - name: Install undercloud<br>
          import_role:<br>
            name: tripleo-undercloud<br>
            tasks_from: install<br>
          vars:<br>
            tripleo_undercloud_debug: true<br>
        - name: Upload images<br>
          import_role:<br>
            name: tripleo-overcloud-images<br>
            tasks_from: upload<br>
          vars:<br>
            tripleo_overcloud_images_debug: true<br>
        - name: Import nodes<br>
          import_role:<br>
            name: tripleo-overcloud-node<br>
            tasks_from: import<br>
          vars:<br>
            tripleo_overcloud_node_debug: true<br>
            tripleo_overcloud_node_import_file: instack.json<br>
        - name: Introspect nodes<br>
          import_role:<br>
            name: tripleo-overcloud-node<br>
            tasks_from: introspect<br>
          vars:<br>
            tripleo_overcloud_node_debug: true<br>
            tripleo_overcloud_node_introspect_all_manageable: True<br>
            tripleo_overcloud_node_introspect_provide: True<br>
        - name: Overcloud deploy<br>
          import_role:<br>
            name: tripleo-overcloud<br>
            tasks_from: deploy<br>
          vars:<br>
            tripleo_overcloud_debug: true<br>
            tripleo_overcloud_deploy_environment_files:<br>
              - /home/stack/params.yaml<br>
<br>
The same general tasks performed via the "Variable switch method"<br>
would look something like:<br>
<br>
    - hosts: undercloud<br>
      gather_facts: true<br>
      collections:<br>
        - tripleo.operator<br>
      tasks:<br>
        - name: Install undercloud<br>
          import_role:<br>
            name: tripleo-undercloud<br>
          vars:<br>
            tripleo_undercloud_action: install<br>
            tripleo_undercloud_debug: true<br>
        - name: Upload images<br>
          import_role:<br>
            name: tripleo-overcloud-images<br>
          vars:<br>
            tripleo_overcloud_images_action: upload<br>
            tripleo_overcloud_images_debug: true<br>
        - name: Import nodes<br>
          import_role:<br>
            name: tripleo-overcloud-node<br>
          vars:<br>
            tripleo_overcloud_node_action: import<br>
            tripleo_overcloud_node_debug: true<br>
            tripleo_overcloud_node_import_file: instack.json<br>
        - name: Introspect nodes<br>
          import_role:<br>
            name: tripleo-overcloud-node<br>
          vars:<br>
            tripleo_overcloud_node_action: introspect<br>
            tripleo_overcloud_node_debug: true<br>
            tripleo_overcloud_node_introspect_all_manageable: True<br>
            tripleo_overcloud_node_introspect_provide: True<br>
        - name: Overcloud deploy<br>
          import_role:<br>
            name: tripleo-overcloud<br>
          vars:<br>
            tripleo_overcloud_action: deploy<br>
            tripleo_overcloud_debug: true<br>
            tripleo_overcloud_deploy_environment_files:<br>
              - /home/stack/params.yaml<br>
<br>
Thoughts?<br>
<br>
Thanks,<br>
-Alex<br>
<br>
[0] <a href="https://blueprints.launchpad.net/tripleo/+spec/tripleo-operator-ansible" rel="noreferrer" target="_blank">https://blueprints.launchpad.net/tripleo/+spec/tripleo-operator-ansible</a><br>
[1] <a href="https://review.opendev.org/#/c/699311/" rel="noreferrer" target="_blank">https://review.opendev.org/#/c/699311/</a><br>
[2] <a href="https://review.opendev.org/#/c/701628/" rel="noreferrer" target="_blank">https://review.opendev.org/#/c/701628/</a><br>
[3] <a href="https://review.opendev.org/#/c/701034/" rel="noreferrer" target="_blank">https://review.opendev.org/#/c/701034/</a><br>
[4] <a href="https://review.opendev.org/#/c/701628/" rel="noreferrer" target="_blank">https://review.opendev.org/#/c/701628/</a><br>
<br>
<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Best regards<br></div>Sagi Shnaidman<br></div></div></div>