)]}'
{"playbooks/ceph-health.yaml":[{"author":{"_account_id":18002,"name":"John Fulton","email":"fulton@redhat.com","username":"fultonj"},"change_message_id":"cd73cffac274c81cfb2c397b5d6c3e1ceaa47364","unresolved":false,"context_lines":[{"line_number":10,"context_line":"      groups:"},{"line_number":11,"context_line":"        - post-deployment"},{"line_number":12,"context_line":"        - post-ceph"},{"line_number":13,"context_line":"    tripleo_delegate_to: \"{{ groups[\u0027Controller\u0027] | default([]) }}\""},{"line_number":14,"context_line":"    osd_percentage_min: 0"},{"line_number":15,"context_line":"  tasks:"},{"line_number":16,"context_line":"    - include_role:"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f4c43b2_4af4c1cf","line":13,"updated":"2020-04-17 15:47:51.000000000","message":"The CephMon service could be composed anywhere. E.g. this will fail on DCN deployments.\n\nSee this example:\n\nhttps://github.com/openstack/tripleo-heat-templates/commit/fdb204b9633d8da8742b95c5bede313110adc7e2\n\nIn that case I use:\n\ndelegate_to: \"{{ groups[\u0027ceph_mon\u0027][0] }}\"\n\nSo that I specifically target the ceph_mon group. I think you should be able to do that here too.","commit_id":"571277ab0d138397996a343768003477b49f33e4"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"940733b7f8d3ac4f43128fb12264e3032c9a3b6c","unresolved":false,"context_lines":[{"line_number":10,"context_line":"      groups:"},{"line_number":11,"context_line":"        - post-deployment"},{"line_number":12,"context_line":"        - post-ceph"},{"line_number":13,"context_line":"    tripleo_delegate_to: \"{{ groups[\u0027Controller\u0027] | default([]) }}\""},{"line_number":14,"context_line":"    osd_percentage_min: 0"},{"line_number":15,"context_line":"  tasks:"},{"line_number":16,"context_line":"    - include_role:"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f4c43b2_6a609d02","line":13,"in_reply_to":"3f4c43b2_4af4c1cf","updated":"2020-04-17 16:08:39.000000000","message":"Hey John, thanks for the review.\nLet me understand what could be the best value.\nAs you can see here [1] I removed the delegate_to in favour of the tripleo_delegate_to wrapper, so in tht instead of using the delegate_to keyword, we can rely on the tripleo wrapper around it.\nThat value allows to properly execute the included role in on the specified hostgroup.\nHere, in the playbook, I see the default target is Controller (host: Controller), for this reason I though indicating groups[\u0027Controller\u0027] should have been the right default value.\nBut I understand it can generate regressions for DCN use cases. I\u0027m thinking about changing this value, but this depends on the inventory.\nDo you have an example of dcn use case, so that I can fix the playbook default accordingly?\n\n\n[1] https://review.opendev.org/#/c/718012/2/deployment/ceph-ansible/ceph-base.yaml@687","commit_id":"571277ab0d138397996a343768003477b49f33e4"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"0147d42280533799f24651a3536c2c0fcaaa38b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"- hosts: ceph_mon"},{"line_number":3,"context_line":"  vars:"},{"line_number":4,"context_line":"    metadata:"},{"line_number":5,"context_line":"      name: Check the status of the ceph cluster"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"3f4c43b2_a4ebd8d9","line":2,"range":{"start_line":2,"start_character":9,"end_line":2,"end_character":17},"updated":"2020-04-20 14:01:37.000000000","message":"Changing this to ceph_mon.\nAs fultonj pointed out, mons can be in Controllers, but if we\u0027re using composable roles they can be deployed on other nodes.\nIf ceph is deployed, a tripleo-inventory ceph-ansible compatible is always there and we have ceph_mon group as [1] suggests.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":18002,"name":"John Fulton","email":"fulton@redhat.com","username":"fultonj"},"change_message_id":"94b157ff3fb84e5f96ec69e736ee9ff1f80330d2","unresolved":false,"context_lines":[{"line_number":10,"context_line":"      groups:"},{"line_number":11,"context_line":"        - post-deployment"},{"line_number":12,"context_line":"        - post-ceph"},{"line_number":13,"context_line":"    tripleo_delegate_to: \"{{ groups[\u0027ceph_mon\u0027] | default([]) }}\""},{"line_number":14,"context_line":"    osd_percentage_min: 0"},{"line_number":15,"context_line":"  tasks:"},{"line_number":16,"context_line":"    - include_role:"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_de9ac16c","line":13,"updated":"2020-04-20 16:57:47.000000000","message":"thanks","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"}],"roles/ceph/tasks/ceph-health.yaml":[{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"73dcb2b99cac438fd955e8a40bae48eac10cc5ef","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  ignore_errors: true"},{"line_number":6,"context_line":"  register: ceph_mon_enabled"},{"line_number":7,"context_line":"  changed_when: false"},{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to[0] }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to[0] }}\""}],"source_content_type":"text/x-yaml","patch_set":2,"id":"df33271e_5cda9e4a","line":8,"updated":"2020-04-09 21:16:36.000000000","message":"I don\u0027t think this works if we ever get the default of [].  Also \"{{ tripleo_delegate_to | first }}\" is prefered however this also suffers from a templating error if tripleo_delegate_to is []. Not certain you can do this","commit_id":"243de87cf2db4e248ab4a73658a46c23df015efa"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"ef4f72b4de7e1dcc51b02d84626faaa480c7ebc3","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  ignore_errors: true"},{"line_number":6,"context_line":"  register: ceph_mon_enabled"},{"line_number":7,"context_line":"  changed_when: false"},{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to[0] }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to[0] }}\""}],"source_content_type":"text/x-yaml","patch_set":2,"id":"df33271e_3cad1a9b","line":8,"updated":"2020-04-09 21:20:04.000000000","message":"In some testing it looks like this might work:\n\n  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\"","commit_id":"243de87cf2db4e248ab4a73658a46c23df015efa"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"7854b48a6746f8e40bf03a66e25858dd444ed36e","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  ignore_errors: true"},{"line_number":6,"context_line":"  register: ceph_mon_enabled"},{"line_number":7,"context_line":"  changed_when: false"},{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to[0] }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to[0] }}\""}],"source_content_type":"text/x-yaml","patch_set":2,"id":"df33271e_9656becd","line":8,"in_reply_to":"df33271e_3cad1a9b","updated":"2020-04-10 07:45:37.000000000","message":"Thank for the review Alex! \nYeah, my first attempt shouldn\u0027t work if the variable is not set and/or the mons are not present in the inventory.\nI think using the first filter is the best approach to select the first mon from the group, and the omit filter can ensure the task won\u0027t fail if no mons are included in the inventory.\nWe\u0027ll test it but overall the approach you\u0027re suggesting it\u0027s the best.\nThanks again, I\u0027m going to update the PS with this change.","commit_id":"243de87cf2db4e248ab4a73658a46c23df015efa"},{"author":{"_account_id":26343,"name":"Jose Luis Franco","email":"jfrancoa@redhat.com","username":"jfrancoa"},"change_message_id":"d53691018d226ad0108917de866a5b8dfe546b6c","unresolved":false,"context_lines":[{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":12,"context_line":"  block:"},{"line_number":13,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":14,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3f4c43b2_b8a84d77","line":11,"updated":"2020-04-16 21:21:59.000000000","message":"It keeps on failing... \n\nTASK [tripleo-ceph-run-ansible : search output of ceph-ansible run(s) non-zero return codes] ***                                                                             \nThursday 16 April 2020  13:13:50 -0400 (0:03:13.918)       0:06:38.773 ********                                                                                              \nskipping: [undercloud] \u003d\u003e (item\u003dNone)  \u003d\u003e {\"censored\": \"the output has been hidden due to the fact that \u0027no_log: true\u0027 was specified for this result\", \"changed\": false}     \nskipping: [undercloud] \u003d\u003e {\"censored\": \"the output has been hidden due to the fact that \u0027no_log: true\u0027 was specified for this result\", \"changed\": false}                     \n                                                                                                                                                                             \nTASK [tripleo-ceph-run-ansible : print ceph-ansible output in case of failure] ***                                                                                           \nThursday 16 April 2020  13:13:51 -0400 (0:00:00.993)       0:06:39.766 ********                                                                                              \nskipping: [undercloud] \u003d\u003e {}                                                                                                                                                 \n                                                                                                                                                                             \nTASK [ceph : Check if ceph_mon is deployed] ************************************                                                                                             \nThursday 16 April 2020  13:13:51 -0400 (0:00:00.725)       0:06:40.492 ********                                                                                              \nok: [undercloud -\u003e 192.168.24.10] \u003d\u003e {\"changed\": false, \"cmd\": \"hiera -c /etc/puppet/hiera.yaml enabled_services | egrep -sq ceph_mon\", \"delta\": \"0:00:01.822391\", \"end\": \"20\n20-04-10 15:48:43.146773\", \"rc\": 0, \"start\": \"2020-04-10 15:48:41.324382\", \"stderr\": \"\", \"stderr_lines\": [], \"stdout\": \"\", \"stdout_lines\": []}                               \n                                                                                                                                                                             \nTASK [ceph : Set container_cli fact from the inventory] ************************                                                                                             \nThursday 16 April 2020  13:13:55 -0400 (0:00:03.113)       0:06:43.606 ********                                                                                              \nok: [undercloud -\u003e 192.168.24.10] \u003d\u003e {\"ansible_facts\": {\"container_cli\": \"podman\"}, \"changed\": false}                                                                        \n                                                                                                                                                                             \nTASK [ceph : Set container filter format] **************************************                                                                                            \nThursday 16 April 2020  13:13:58 -0400 (0:00:03.231)       0:06:46.838 ********                                                                                             \nok: [undercloud -\u003e 192.168.24.10] \u003d\u003e {\"ansible_facts\": {\"container_filter_format\": \"--format \u0027{{ .Names }}\u0027\"}, \"changed\": false}                                            \n                                                                                                                                                                            \nTASK [ceph : Set ceph_mon_container name] **************************************                                                                                            \nThursday 16 April 2020  13:13:59 -0400 (0:00:00.884)       0:06:47.722 ********\nfatal: [undercloud -\u003e 192.168.24.10]: FAILED! \u003d\u003e {\"changed\": false, \"cmd\": \"podman ps --format \u0027{{ .Names }}\u0027 | grep ceph-mon\", \"delta\": \"0:00:00.029022\", \"end\": \"2020-04-1$\n 15:48:48.614535\", \"msg\": \"non-zero return code\", \"rc\": 1, \"start\": \"2020-04-10 15:48:48.585513\", \"stderr\": \"/bin/sh: podman: command not found\", \"stderr_lines\": [\"/bin/sh:\npodman: command not found\"], \"stdout\": \"\", \"stdout_lines\": []}\n\n\n\nEven though it runs now in the right node (controller-0):\n\n(undercloud) [stack@undercloud-0 ~]$ openstack server list\n+--------------------------------------+--------------+--------+------------------------+----------------+------------+                                                     \n| ID                                   | Name         | Status | Networks               | Image          | Flavor     |                                                     \n+--------------------------------------+--------------+--------+------------------------+----------------+------------+                                                     \n| 68a0fe64-f36b-435c-a74a-4d84da29f064 | ceph-2       | ACTIVE | ctlplane\u003d192.168.24.9  | overcloud-full | ceph       |                                                     \n| 1f98db99-5256-41c1-b2f3-ee7911eafa63 | compute-0    | ACTIVE | ctlplane\u003d192.168.24.11 | overcloud-full | compute    |                                                     \n| 54f48f0b-bea1-43d0-b308-f9880c12334d | ceph-0       | ACTIVE | ctlplane\u003d192.168.24.6  | overcloud-full | ceph       |                                                     \n| 55404e5f-724a-46d0-94f7-25b64d5020e5 | controller-0 | ACTIVE | ctlplane\u003d192.168.24.10 | overcloud-full | controller |                                                     \n| 765730a1-9719-4b3f-a307-4efaed61e2b1 | controller-1 | ACTIVE | ctlplane\u003d192.168.24.28 | overcloud-full | controller |                                                     \n| c280905c-5c2b-4113-a82a-ed4486a78cde | controller-2 | ACTIVE | ctlplane\u003d192.168.24.13 | overcloud-full | controller |                                                     \n| 101a54d0-53a3-4431-ad72-f3798dbf1d27 | compute-1    | ACTIVE | ctlplane\u003d192.168.24.17 | overcloud-full | compute    |                                                     \n| 257e2419-baa4-4dc6-bed4-a39bbc5e7b90 | ceph-1       | ACTIVE | ctlplane\u003d192.168.24.14 | overcloud-full | ceph       |                                                     \n+--------------------------------------+--------------+--------+------------------------+----------------+------------+    \n\nHowever, the inventory parameters are not matching the reality... If you want an inventory for debugging the issue, ping me Tomorrow morning.","commit_id":"d7783a4e5f1f116515e6f45f6d3345012167ea22"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"eb162f266df15ed224d254b96352f0da43081c4f","unresolved":false,"context_lines":[{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":12,"context_line":"  block:"},{"line_number":13,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":14,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3f4c43b2_df50980d","line":11,"in_reply_to":"3f4c43b2_b8a84d77","updated":"2020-04-17 09:17:52.000000000","message":"Hi Jose,\nok so basically I think the problem here is different: the delegation is now correct and we\u0027re not on the undercloud anymore, so we can see the right node (\"controller-0\"), but the next task which sets the \"container_cli\" fact still uses hostvars[inventory_hostname]: this points to the undercloud facts (the source host of the playbook) and that\u0027s the reason we\u0027re still seeing \"podman\" instead of \"docker\".","commit_id":"d7783a4e5f1f116515e6f45f6d3345012167ea22"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"1d7f4cbbd67813c175028aa6ac565a618a39e438","unresolved":false,"context_lines":[{"line_number":12,"context_line":"  block:"},{"line_number":13,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":14,"context_line":"      set_fact:"},{"line_number":15,"context_line":"        container_cli: \"{{ hostvars[inventory_hostname].container_cli|default(\u0027podman\u0027) }}\""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"    - name: Set container filter format"},{"line_number":18,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"3f4c43b2_5f65a8e0","line":15,"range":{"start_line":15,"start_character":27,"end_line":15,"end_character":55},"updated":"2020-04-17 09:19:27.000000000","message":"This is wrong and should be set on the delegated host. For standalone deployments, or for scenarios different from upgrades, we can still fallback on the inventory_hostname.","commit_id":"d7783a4e5f1f116515e6f45f6d3345012167ea22"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"b106b995ece7c4ca970aeed87c3564765077f7d7","unresolved":false,"context_lines":[{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":12,"context_line":"  block:"},{"line_number":13,"context_line":"    - name: Check for docker cli"},{"line_number":14,"context_line":"      stat:"},{"line_number":15,"context_line":"        path: \"/var/run/docker.sock\""},{"line_number":16,"context_line":"      register: check_docker_cli"},{"line_number":17,"context_line":"      check_mode: false"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":20,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f4c43b2_44b95778","line":17,"range":{"start_line":13,"start_character":4,"end_line":17,"end_character":23},"updated":"2020-04-17 15:29:59.000000000","message":"So after some testing we figured out the `container_cli` fact is only present at tht level and in the upgrade scenario is always set to \"podman\" because tht moved the value to podman since Rocky.\nFor this reason if the undercloud is update (train) but the overcloud is still waiting for the upgrade process (queens), this value should be computed by executing a query on the overcloud controller (ceph_mon|first).","commit_id":"571277ab0d138397996a343768003477b49f33e4"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"b106b995ece7c4ca970aeed87c3564765077f7d7","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":20,"context_line":"      set_fact:"},{"line_number":21,"context_line":"        container_cli: |-"},{"line_number":22,"context_line":"          {% set container_cli \u003d \u0027podman\u0027 %}"},{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"},{"line_number":25,"context_line":"          {%   endif %}"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"3f4c43b2_647293e5","line":22,"range":{"start_line":22,"start_character":10,"end_line":22,"end_character":44},"updated":"2020-04-17 15:29:59.000000000","message":"we set podman by default, but we can meet the requirements for the upgrade process with an if statement which depends on the previous task and it\u0027s executed only when the docker sock if found on the controller.","commit_id":"571277ab0d138397996a343768003477b49f33e4"},{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"5f64fe282b7ffa61b86623b0caecc8d780051cc7","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  ignore_errors: true"},{"line_number":6,"context_line":"  register: ceph_mon_enabled"},{"line_number":7,"context_line":"  changed_when: false"},{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_e279ce2f","line":8,"range":{"start_line":8,"start_character":0,"end_line":8,"end_character":66},"updated":"2020-04-21 16:07:45.000000000","message":"I may be wrong - but I have a vague memory of delegation with omit not working.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"b99fc8e0fcc5aee4c896353862501ef434c19dcd","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  ignore_errors: true"},{"line_number":6,"context_line":"  register: ceph_mon_enabled"},{"line_number":7,"context_line":"  changed_when: false"},{"line_number":8,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"- when: \"ceph_mon_enabled is succeeded\""},{"line_number":11,"context_line":"  delegate_to: \"{{ tripleo_delegate_to | first | default(omit) }}\""}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_54cdce5b","line":8,"range":{"start_line":8,"start_character":0,"end_line":8,"end_character":66},"in_reply_to":"1f493fa4_e279ce2f","updated":"2020-04-22 09:33:08.000000000","message":"Yeah, you\u0027re right, tripleo_delegate_to is treated as a variable here, so this task will fail if it\u0027s not defined, and omit can be actually avoided.\nThis task has `ignore_errors: true`, this means if it fails looking for a monitor the next block is skipped.\nOf course the tripleo_delegate_to undefined is one of the causes this task can fail, so we\u0027re still covered if the variable is not passed or it\u0027s wrong.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":26343,"name":"Jose Luis Franco","email":"jfrancoa@redhat.com","username":"jfrancoa"},"change_message_id":"fa66ee7abf0adbc447482d4585e0e6c6fc414b56","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":20,"context_line":"      set_fact:"},{"line_number":21,"context_line":"        container_cli: |-"},{"line_number":22,"context_line":"          {% set container_cli \u003d \u0027podman\u0027 %}"},{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"3f4c43b2_ad529bdc","line":21,"updated":"2020-04-20 15:47:02.000000000","message":"I\u0027m wondering if by setting a fact with the very same name as container_cli, you are not overriding the container_cli fact for the whole deployment. As if that\u0027s the case, you might be breaking the deployment tasks which could come after. As we are using this fact only in this validation, can\u0027t we use a different name to avoid conflicts?","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"5f64fe282b7ffa61b86623b0caecc8d780051cc7","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":20,"context_line":"      set_fact:"},{"line_number":21,"context_line":"        container_cli: |-"},{"line_number":22,"context_line":"          {% set container_cli \u003d \u0027podman\u0027 %}"},{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_d6af3392","line":21,"in_reply_to":"1f493fa4_b3dc4347","updated":"2020-04-21 16:07:45.000000000","message":"The THT output generates group_vars, doesn\u0027t it? Although in some locations it sets other things. If anyone has a generated set to look at, then it can be mapped against https://docs.ansible.com/ansible/latest/user_guide/playbooks_variables.html#variable-precedence-where-should-i-put-a-variable to figure out the precedence.\n\nThat said, using a different var name guarantees that everything is OK.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"b99fc8e0fcc5aee4c896353862501ef434c19dcd","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":20,"context_line":"      set_fact:"},{"line_number":21,"context_line":"        container_cli: |-"},{"line_number":22,"context_line":"          {% set container_cli \u003d \u0027podman\u0027 %}"},{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_d42f1ef4","line":21,"in_reply_to":"1f493fa4_d6af3392","updated":"2020-04-22 09:33:08.000000000","message":"Yeah, the goal is to make this validation use the container_cli variable computed according to what is installed on the overcloud, and not relying anymore on the tht variable which cannot cover all the use cases.\nFor this reason the best way to do that is to change the variable name: this is more robust and less error-prone for validation purposes.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"baef0c1bf8afe638ba03616968a345e8ff93fa29","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    - name: Set container_cli fact from the inventory"},{"line_number":20,"context_line":"      set_fact:"},{"line_number":21,"context_line":"        container_cli: |-"},{"line_number":22,"context_line":"          {% set container_cli \u003d \u0027podman\u0027 %}"},{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_b3dc4347","line":21,"in_reply_to":"3f4c43b2_ad529bdc","updated":"2020-04-21 07:10:57.000000000","message":"This is a good point. TBH I\u0027m not sure if this fact exists only in this scope or it\u0027s used in the next deployments (since it\u0027s cached somewhere).\nThe best thing we can do is change the variable name and avoid aliasing.\nIn the validation we shouldn\u0027t rely on the tht defined variable, so we should compute and assign it according to the overcloud facts.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":18002,"name":"John Fulton","email":"fulton@redhat.com","username":"fultonj"},"change_message_id":"94b157ff3fb84e5f96ec69e736ee9ff1f80330d2","unresolved":false,"context_lines":[{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"},{"line_number":25,"context_line":"          {%   endif %}"},{"line_number":26,"context_line":"          {{ container_cli }}"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    - name: Set container filter format"},{"line_number":29,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_3e8e4527","line":26,"updated":"2020-04-20 16:57:47.000000000","message":"Francesco, Please add a when so we only set this fact if it\u0027s not set.\n\nJose, do you think that will address your concern?","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"baef0c1bf8afe638ba03616968a345e8ff93fa29","unresolved":false,"context_lines":[{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"},{"line_number":25,"context_line":"          {%   endif %}"},{"line_number":26,"context_line":"          {{ container_cli }}"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    - name: Set container filter format"},{"line_number":29,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_93bae7db","line":26,"in_reply_to":"1f493fa4_3e8e4527","updated":"2020-04-21 07:10:57.000000000","message":"Hey John, I don\u0027t think so.\ncontainer_cli is always defined at tht level, so this task will always find an already existing container_cli variable.\nFor this reason I think we should change the name of this variable in validation and don\u0027t rely on the original container_cli since it\u0027s global and cannot reflect the whole state of the cluster in all the use cases.\nWe will always have podman as default, looking for docker when the socket exist in the ceph_mon groups.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":26343,"name":"Jose Luis Franco","email":"jfrancoa@redhat.com","username":"jfrancoa"},"change_message_id":"b2a25ea7d4f6134bc098b7eab3ec4af34a48d827","unresolved":false,"context_lines":[{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_cli \u003d \u0027docker\u0027 %}"},{"line_number":25,"context_line":"          {%   endif %}"},{"line_number":26,"context_line":"          {{ container_cli }}"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    - name: Set container filter format"},{"line_number":29,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1f493fa4_336aee7f","line":26,"in_reply_to":"1f493fa4_93bae7db","updated":"2020-04-21 11:58:00.000000000","message":"Agree with Francesco, if we include the conditional then his latest changes won\u0027t work for FFWD. As it will take the value defined from tht (which will point to podman independently that all overcloud nodes are still on Queens, so using docker) and the validation will fail.","commit_id":"db895dfd9afdfaa86ff151403211da716ab4bc5d"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"60b572f043b1ea674cb7c9c68e86559964818e34","unresolved":false,"context_lines":[{"line_number":16,"context_line":"      register: check_docker_cli"},{"line_number":17,"context_line":"      check_mode: false"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    - name: Set container_client fact"},{"line_number":20,"context_line":"      set_fact:"},{"line_number":21,"context_line":"        container_client: |-"},{"line_number":22,"context_line":"          {% set container_cli \u003d \u0027podman\u0027 %}"},{"line_number":23,"context_line":"          {%   if check_docker_cli.stat.exists|bool %}"},{"line_number":24,"context_line":"          {%     set container_client \u003d \u0027docker\u0027 %}"},{"line_number":25,"context_line":"          {%   endif %}"},{"line_number":26,"context_line":"          {{ container_client }}"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    - name: Set container filter format"},{"line_number":29,"context_line":"      set_fact:"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"1f493fa4_f326cb16","line":26,"range":{"start_line":19,"start_character":4,"end_line":26,"end_character":32},"updated":"2020-04-21 07:16:02.000000000","message":"John,\nthis can avoid aliasing with container_cli, which can break the upgrades use cases because it\u0027s a static values and doesn\u0027t change in function of the state of the cluster or the use case.\nAs you can see in defaults/main.yaml has still \"podman\" as default value, or you can ovverride the value running something like:\n\nansible-playbook -i tripleo-ansible-inventory.yaml ceph-health.yaml -e \"container_client\u003dmyclient\"\n\nbut this should avoid aliasing with container_cli variable and avoid us breaking the next phases just because we\u0027re adding extra logic here.","commit_id":"3baced6e89d4d3af7d2190f4c587a4ee05f5977a"}]}