)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"79137159b23e43c962c7352ecac332aadb4f8c21","unresolved":false,"context_lines":[{"line_number":13,"context_line":"and ensures os-net-config and openvswitch are available."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Id9bec1c1226922a7a5143947794a077c16cd499b"},{"line_number":16,"context_line":"Depends-on: https://review.opendev.org/841663"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"2fb16119_9ae4f404","line":16,"range":{"start_line":16,"start_character":1,"end_line":16,"end_character":14},"updated":"2022-05-16 06:33:54.000000000","message":"for things in the same repo we typically just rebase instead of depends-on (it used to not work well with build-test-packages but seems it is OK here/zuul is happy enough)","commit_id":"9de060d9d87e9fd087dc806e46a00a2394a98967"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"1cc530e6962e10c5cbf774f64023dd823fb51ae7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"366c2565_bbe68d35","updated":"2022-05-12 23:55:39.000000000","message":"I wonder if it would make sense to include the tripleo_bootstrap role instead.\nhttps://github.com/openstack/tripleo-ansible/tree/master/tripleo_ansible/roles/tripleo_bootstrap\n","commit_id":"f15d78aa0662a1a471c0120c7295353df07daecd"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"eff487d3e2387c412d51015815e298ab08b81ca9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d35ebc49_6f5606ad","in_reply_to":"366c2565_bbe68d35","updated":"2022-05-13 00:04:48.000000000","message":"You\u0027re probably right. I had a look though that role and it seems like it should work fine. This has been updated to include_role now instead of re-implementing the package install stuff.","commit_id":"f15d78aa0662a1a471c0120c7295353df07daecd"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"c60680846365eb6f5ba9d0964a907b7131d9c943","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"549aeadd_d47c800e","updated":"2022-05-13 00:43:30.000000000","message":"I\u0027m ok with this, although I believe we end up executing the tripleo_bootstrap role 3 times. Once when network is provisioned as part of the node provisioning, then again two times when deploying the overcloud since tripleo_network_config is run again in deploy_step plays.\n\nAn alternative to this would be to document that pre-deployed users should add a playbook which includes the tripleo_bootstrap role, adding it to the \"ansible_playbooks\" interface for each role in baremetal_deployment.yaml.\n\nAdding few people to get more opinions.","commit_id":"2291c543062df323353b5fb4f55b9bbf6971733a"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"b210823bb578840e7586de72541578fc8fbd4076","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f5e90149_20bd47b2","in_reply_to":"0edec109_b27eded7","updated":"2022-05-13 01:48:05.000000000","message":"Ok, how do we feel about doing it this way. We split out the packages tasks in bootstrap so that we can just call that component. Maybe that will be more helpful down the line as well?\n\nSo this change will depend on:\nhttps://review.opendev.org/c/openstack/tripleo-ansible/+/841663","commit_id":"2291c543062df323353b5fb4f55b9bbf6971733a"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"8fd9f3dc2164cfe30ebf9ac9bc529f3589d35051","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0edec109_b27eded7","in_reply_to":"549aeadd_d47c800e","updated":"2022-05-13 00:50:44.000000000","message":"I think at a very minimum we go with patchset 2. Those packages are the only hard dependency on the provisioning process with --network-config. \n\nIt shouldn\u0027t cost us too much time to check for them being installed, and it will prevent failure and make the overall experience smoother.","commit_id":"2291c543062df323353b5fb4f55b9bbf6971733a"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"22033bb086ffdef8c86af8c95f8f22feba69062f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6f11c8c2_25b3885d","in_reply_to":"e425e324_11f700c1","updated":"2022-05-13 02:12:10.000000000","message":"Marking not resolved for now.","commit_id":"2291c543062df323353b5fb4f55b9bbf6971733a"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"06926056badd1edd5aa06d899b6b1a4dc8127319","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e425e324_11f700c1","in_reply_to":"f5e90149_20bd47b2","updated":"2022-05-13 02:11:48.000000000","message":"Hmm, the downside appears to be that we don\u0027t have facts to populate the variable here:\nhttps://review.opendev.org/c/openstack/tripleo-ansible/+/841663/3/tripleo_ansible/roles/tripleo_bootstrap/tasks/packages.yml#33\n\nSo we would also need to run the setup module somewhere here if we wanted to take this approach.","commit_id":"2291c543062df323353b5fb4f55b9bbf6971733a"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"ce39595d0f5017200af62087d81b82d1ff06f647","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b7a61598_a2c5b214","updated":"2022-05-16 01:55:23.000000000","message":"So, we can do it this way. But we will need to conditionally gather facts in the tripleo_bootstrap packages.yml for it to work. I guess its not too bad, just another task to skip during the deploy_steps_tasks though.","commit_id":"9de060d9d87e9fd087dc806e46a00a2394a98967"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"cee43ebdfa51abc0735289ced79ae841d3769cd5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0da1e263_6d1548e7","updated":"2022-05-16 02:05:30.000000000","message":"recheck","commit_id":"9de060d9d87e9fd087dc806e46a00a2394a98967"}],"tripleo_ansible/roles/tripleo_network_config/tasks/main.yml":[{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"b37abf8739089333a7a191653c2fbf4fa4f58680","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"- name: Ensure requirements are satisfied"},{"line_number":37,"context_line":"  include_role:"},{"line_number":38,"context_line":"    name: tripleo_bootstrap"},{"line_number":39,"context_line":"    tasks_from: packages.yml"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"- name: Ensure /var/lib/tripleo-config directory exists"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"4860d79d_e83c4ab0","line":38,"range":{"start_line":38,"start_character":4,"end_line":38,"end_character":27},"updated":"2022-05-23 04:42:27.000000000","message":"Hmm.. One role including another unrelated role looks like an anti-pattern to me. We should have handled it in the playbook or tripleoclient.","commit_id":"9de060d9d87e9fd087dc806e46a00a2394a98967"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"2600444ae9ac715acb1444365625cca64ac089ac","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"- name: Ensure requirements are satisfied"},{"line_number":37,"context_line":"  include_role:"},{"line_number":38,"context_line":"    name: tripleo_bootstrap"},{"line_number":39,"context_line":"    tasks_from: packages.yml"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"- name: Ensure /var/lib/tripleo-config directory exists"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"d6848ea4_7d5c732b","line":38,"range":{"start_line":38,"start_character":4,"end_line":38,"end_character":27},"in_reply_to":"4860d79d_e83c4ab0","updated":"2022-05-23 05:07:54.000000000","message":"I\u0027m not sure I quite understand the problem here. If we handle it in tripleoclient or the playbook, I would need to re implement something that already exists to solve this very problem? \n\nInitially, in patchset 1, I was indeed doing it separately. But having to list dependencies in two separate locations seemed like a duplication of code that could lead to conflicts further down the line? The intention of using the pre-existing role was to maintain consistency with what will happen during overcloud deploy.","commit_id":"9de060d9d87e9fd087dc806e46a00a2394a98967"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"e972b35a3b5c9f46d85de7f50b686a6a92e33a00","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"- name: Ensure requirements are satisfied"},{"line_number":37,"context_line":"  include_role:"},{"line_number":38,"context_line":"    name: tripleo_bootstrap"},{"line_number":39,"context_line":"    tasks_from: packages.yml"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"- name: Ensure /var/lib/tripleo-config directory exists"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"8f0d8fb3_32d10e86","line":38,"range":{"start_line":38,"start_character":4,"end_line":38,"end_character":27},"in_reply_to":"d6848ea4_7d5c732b","updated":"2022-05-23 05:37:18.000000000","message":"AFAIK, role is an independent abstraction. One role including another un-related role, hence create a dependency is an anti-pattern.","commit_id":"9de060d9d87e9fd087dc806e46a00a2394a98967"}]}
