)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"548ab9b3672462256b95109ede3d381c343acffa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9061a9b1_525a0a86","updated":"2022-10-31 15:18:59.000000000","message":"I always prefer getting rid of heat terminology so I like the idea.\n\nA minor concern with this change is that this change can result in unexpected behavior if a user \"swaps\" the hostname using the HostnameMap parameter.\n\n```\nparameter_defaults:\n  HostnameMap:\n    overcloud-controller-0: overcloud-controller-2\n    overcloud-controller-2: overcloud-controller-0\n```\n\nHowever I don\u0027t think this usage is quite popular.\n\nAnother use case is assigning the same hostname when replacing a node. However in that case we assume the old host no longer exists in the stack, there should not be a problem.\n\n(eg. when replacing the overcloud-controller-0, with keeping the same host name)\n```\nparameter_defaults:\n  HostnameMap:\n    overcloud-controller-3: overcloud-controller-0\n```\n\nWe are probably able to implement the validation to avoid such situation but I\u0027m ok with leaving that to follow-up.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"5346633103bc83e7e4ab7e1e5b802025f4ea5b21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"557588b9_db68b0eb","updated":"2022-09-12 14:11:52.000000000","message":"Not a -1, but honestly I would deprecate these parameters, standardize on just using --limit everywhere, instead of expanding the scope of how these work.\n\nIf we are going to expand the scope of these, then we should push forward with Brendan\u0027s patches to fix the non-inclusive language:\n\nhttps://review.opendev.org/c/openstack/tripleo-heat-templates/+/819889\n\nThat patch  deprecates these parameters, so it would be my preference to go with that, and then focus on how we use --limit going forward.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"7fa1c6480aebabd4e62ddc426b09c8417d2fed6c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e2780522_7352c19d","in_reply_to":"517e4870_b7075bdc","updated":"2022-09-13 03:47:15.000000000","message":"\u003e b/c otherwise external tasks just run against the UC don\u0027t they?\n\nYes, but some tasks \u0027delegate_to\u0027 overcloud nodes groups. Though we convert DeploymentServerBlacklist to limit variable for ansible, unless we update inventory to filter those nodes from those groups those external tasks won\u0027t respect --limit and fail (where a node is actually down). I also see DeploymentServerBlacklist as persistent thing across multiple commands (i.e in templates) vs --limit which is one command specific.\n\n\u003e this conflicts with what Brendan is trying to do in his patch\n\nAs per non-inclusive language, we can do that in Brendan\u0027s patch (irrespective of these changes), but I don\u0027t think we can deprecate/drop the THT parameter atm.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"c2d1bf135d35fbed766c905c6568b922475b5f4f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a2df5a39_dc133bd3","in_reply_to":"5191984d_5f0e301b","updated":"2022-10-31 15:25:56.000000000","message":"You\u0027re right, it looks like delegate_to does not respect --limit. In that case, I would have changed https://review.opendev.org/c/openstack/tripleo-heat-templates/+/856408/ to intersect with the var play_hosts, which will be the list of hosts for the play, with --limit already applied. So you could do something like:\n\ngroups[cinder_api] | intersect(play_hosts) | first\n\nContinuing to use the blacklist params means that users actually have to specify --limit *and* set the blacklist params if they want this to work. That is confusing, and we in fact have a warning in tripleoclient to say not do that.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"0d1033fa0b5bfb7d750358c45ecd4c100e9ccc88","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c9201438_c026231e","in_reply_to":"557588b9_db68b0eb","updated":"2022-09-12 14:17:34.000000000","message":"Using --limit does not work with external_*_tasks. IMO,we need to support heat parameters in the foreseeable future.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"db33abbab84625aad04b436a5e89a5f11d71eb8d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5191984d_5f0e301b","in_reply_to":"7d6f6958_e83823f2","updated":"2022-10-26 02:52:58.000000000","message":"\u003e it seems to me if you pass the same value for --limit in external tasks\n\nI\u0027m probably missing your point here. Do you mean tasks wih delegate_to would honor -limit? I don\u0027t think that\u0027s the case[3]. We still have issues reported when doing upgrade with some broken nodes even after[1]. That\u0027s why we came up with a solution[2], may not be the best one but works.\n\n[1] https://github.com/openstack/python-tripleoclient/commit/935c6592cdf9a324ad0775d26681bb004ed667a9  \n\n[2] https://review.opendev.org/q/topic:deploy_exclude\n\n[3]\n[stack@undercloud-0 ~]$ cat test-playbook.yaml \n---\n- hosts: Undercloud\n  tasks:\n    - name: get hostname\n      command: echo $HOSTNAME\n      delegate_to: controller-0\n\n[stack@undercloud-0 ~]$ ansible-playbook -i inventory.yaml --limit \u0027!controller-0\u0027 test-playbook.yaml \n\nPLAY [Undercloud] *****************************************************************************************************************************************************************\n\nTASK [Gathering Facts] ************************************************************************************************************************************************************\nok: [undercloud]\n\nTASK [get /etc/hosts] *************************************************************************************************************************************************************\nchanged: [undercloud -\u003e controller-0]\n\nPLAY RECAP ************************************************************************************************************************************************************************\nundercloud                 : ok\u003d2    changed\u003d1    unreachable\u003d0    failed\u003d0    skipped\u003d0    rescued\u003d0    ignored\u003d0","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"c1fa34fce32257d4384817e9154ba760342648d2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7d6f6958_e83823f2","in_reply_to":"9561d16a_8f8d7478","updated":"2022-10-25 00:21:30.000000000","message":"it seems to me if you pass the same value for --limit in external tasks, the right nodes will automatically be excluded by ansible. Unless there are different inventories being used, with different node names. But if that were the case, then I suspect the blacklist wouldn\u0027t work either.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"dd8d430bf683c496c3e82aee040573adf7ac112b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"79e7a564_1446c91e","in_reply_to":"a2df5a39_dc133bd3","updated":"2022-10-31 18:47:49.000000000","message":"\u003e groups[cinder_api] | intersect(play_hosts) | first\n\nThat would not work as play_hosts (hosts for the play) for external tasks is [\u0027undercloud\u0027] and intersection would give nothing. If we use \u0027difference\u0027 instead of \u0027intersect\u0027 it would not honor --limit.\n\nWhat we instead need is a filter that parses \u0027ansible_limit\u0027 special variable and gives us nodes/groups that are to be excluded from delegate_to. The other option is we get rid of all delegate_to tasks.\n\n\n\u003e Continuing to use the blacklist params means that users actually have to specify \u003e--limit *and* set the blacklist params if they want this to work\n\n\nRight, that\u0027s true for \u0027overcloud deploy (if --limit is used for scale-out)/upgrade run and there are broken nodes that needs to be excluded, but not for \u0027overcloud external update/upgrade\u0027 where --limit is kind of useless atm.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":33080,"name":"Mikolaj Ciecierski","display_name":"Mikolaj Ciecierski","email":"mciecier@redhat.com","username":"mciecierski"},"change_message_id":"32338d6e593131e79533f73eb0f4136410338413","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5ec5decf_097dff62","in_reply_to":"a2df5a39_dc133bd3","updated":"2022-10-31 15:57:53.000000000","message":"Could you elaborate more on why users have to both specify --limit *and* set the blacklist params?","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"76709ef9f6f2373ca0ab9caf8e2e8e68639a346c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"517e4870_b7075bdc","in_reply_to":"c9201438_c026231e","updated":"2022-09-12 20:57:45.000000000","message":"do you mean external tasks that call ansible-playbook themselves? b/c otherwise external tasks just run against the UC don\u0027t they?\n\nWe\u0027d have to make them work, just like we did the blacklist parameters. The ansible_limit variable exists, which should be the value of whatever was passed for --limit. We ought to be able to use that when a call to ansible-playbook is constructed.\n\nOtherwise, if we are going to stick with parameters, this conflicts with what Brendan is trying to do in his patch. If we aren\u0027t going to deprecate these, then we should at least fix the non-inclusive language.","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"},{"author":{"_account_id":33080,"name":"Mikolaj Ciecierski","display_name":"Mikolaj Ciecierski","email":"mciecier@redhat.com","username":"mciecierski"},"change_message_id":"37a504f17c9275cbcf9d168a42cb2fb6f1bbb5b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9561d16a_8f8d7478","in_reply_to":"e2780522_7352c19d","updated":"2022-09-13 08:09:22.000000000","message":"I agree with Rabi point of view, that --limit and DeploymentServerBlacklist have different purpose and one cannot be replaced by the other. \n\nRegarding using ansible_limit variable in external_*_tasks. This seems more complicated than just using it directly in ansible tasks. If we want to stick to supporting the same limit value patterns as there are implemented in ansible call[1] we would need to do some pre-processing of whatever is passed to limit in tripleo-pythonclient and construct inventory on the fly, inmho.\n\n[1]https://docs.ansible.com/ansible/latest/user_guide/intro_patterns.html#common-patterns","commit_id":"3e41f36368a6e9fa75b66f417a0cf1e5e9ca19c8"}]}