)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4829793869cc81e71e93ff08f8114cb2b48fdd9b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"docs: Don\u0027t configure \u0027[scheduler] discover_hosts_in_cells_interval\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This option has essentially been superseded by the \u0027--by-service\u0027 flag"},{"line_number":10,"context_line":"for the \u0027nova-manage discover_hosts\u0027 command. As such, we should not be"},{"line_number":11,"context_line":"recommending users use this option in our documentation."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_2cc61517","line":9,"range":{"start_line":9,"start_character":51,"end_line":9,"end_character":70},"updated":"2018-10-10 14:40:41.000000000","message":"Umm, you don\u0027t even refer to this in the install guide you\u0027re changing....","commit_id":"8b57a50bbd80f3d61aecc42e792e0e2608eb676e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48ba17773a56ff6eb19aa43cad23dbc89814968f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"docs: Don\u0027t configure \u0027[scheduler] discover_hosts_in_cells_interval\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This option has essentially been superseded by the \u0027--by-service\u0027 flag"},{"line_number":10,"context_line":"for the \u0027nova-manage discover_hosts\u0027 command. As such, we should not be"},{"line_number":11,"context_line":"recommending users use this option in our documentation."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_7cd8387f","line":9,"range":{"start_line":9,"start_character":51,"end_line":9,"end_character":70},"in_reply_to":"3f79a3b5_2cc61517","updated":"2018-10-10 16:14:41.000000000","message":"Two things going on here:\n\na) apparently this option is a hack and deployment tooling should be doing the work, not nova. This is based on dansmith\u0027s commands on a patch I proposed against DevStack [1]:\n\n\u003e [The option is] definitely a hack, IMHO. We added it for ironic because they had a bunch of sequencing problems without it. Then with it, they still had problems because of the inherent raciness of needing to basically wait 2*interval before trying anything that assumed the hosts were up. After adding the by-service approach, they were finally able to have a repeatable process, and IIRC, tripleo removed the use of the periodic as a result.\n\nb) the option was only added in the first place because deployment tooling wasn\u0027t able to handle this for ironic nodes. This has since been resolved by the \u0027--by-service\u0027 flag. This is based on dansmith\u0027s comments in bug 1755602 [2]:\n\n\u003e Right now, when nova puppet or ansible modules are running to set up nova-compute, they run discover_hosts after starting the service. However, if using ironic, and ironic isn\u0027t running, configured, or populated with hosts, then discover_hosts will not create any host mappings. That means that you can\u0027t create an empty nova deployment properly configured, pointing to ironic, and then start enrolling ironic nodes. It also means that you must run discover_hosts after adding ironic nodes, which is mixing nova and ironic tasks, and has been the source of much complaining from people using ironic.\n\u003e If we allow discovery by service, then you can set up nova fully during the nova phase, and then move on to ironic without having to come back. A much better workflow.\n\nNone of the sections I\u0027ve removed mentioned anything about Ironic - they simply said you can do this thing. Given that we\u0027re now going to start discouraging this thing, removing references to it makes sense. I could add a note stating that your deployment tooling should run this command but we\u0027re already in-effect saying this:\n\n\u003e When you add new compute nodes, you must run ``nova-manage cell_v2 discover_hosts`` on the controller node...\n\nSo unless you want \u0027s/you/you or your deployment tooling/\u0027 then this is good as-is, IMO.\n\n[1] https://review.openstack.org/#/c/608703/\n[2] https://bugs.launchpad.net/nova/+bug/1755602/comments/3","commit_id":"8b57a50bbd80f3d61aecc42e792e0e2608eb676e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"94dc7e4a4c469a66b1b364d36c349a13f959324b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"docs: Don\u0027t configure \u0027[scheduler] discover_hosts_in_cells_interval\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This option has essentially been superseded by the \u0027--by-service\u0027 flag"},{"line_number":10,"context_line":"for the \u0027nova-manage discover_hosts\u0027 command. As such, we should not be"},{"line_number":11,"context_line":"recommending users use this option in our documentation."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_fc5d287c","line":9,"range":{"start_line":9,"start_character":51,"end_line":9,"end_character":70},"in_reply_to":"3f79a3b5_5c6e9c64","updated":"2018-10-10 16:33:40.000000000","message":"Fair fair. Definitely crossed wires :) Moving on","commit_id":"8b57a50bbd80f3d61aecc42e792e0e2608eb676e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"39fccd52bded0ddc899a1cceada1d4945abfa55f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"docs: Don\u0027t configure \u0027[scheduler] discover_hosts_in_cells_interval\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This option has essentially been superseded by the \u0027--by-service\u0027 flag"},{"line_number":10,"context_line":"for the \u0027nova-manage discover_hosts\u0027 command. As such, we should not be"},{"line_number":11,"context_line":"recommending users use this option in our documentation."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_5c6e9c64","line":9,"range":{"start_line":9,"start_character":51,"end_line":9,"end_character":70},"in_reply_to":"3f79a3b5_7cd8387f","updated":"2018-10-10 16:26:41.000000000","message":"\u003e b) the option was only added in the first place because deployment\n \u003e tooling wasn\u0027t able to handle this for ironic nodes. This has since\n \u003e been resolved by the \u0027--by-service\u0027 flag. This is based on\n \u003e dansmith\u0027s comments in bug 1755602 [2]:\n\nThe ironic and tripleo case was resolved by --by-service, yes. The thing is potentially useful for non-ironic cases where they don\u0027t have infrastructure to do this, or because they just prefer the original behavior of nova where compute nodes auto-register. I definitely think this is a poor alternative to making your CMS handle this for you and that nova shouldn\u0027t have to automate this for you. However, it was kindof a behavior regression in the first place, this is built and works, and if you prefer the overhead of constantly polling to the need to orchestrate this yourself, it\u0027s useful. If it were a big maintenance burden or something, I\u0027d be on the wagon to get this removed, but it\u0027s just not.\n\n \u003e None of the sections I\u0027ve removed mentioned anything about Ironic -\n \u003e they simply said you can do this thing. Given that we\u0027re now going\n \u003e to start discouraging this thing, removing references to it makes\n \u003e sense. I could add a note stating that your deployment tooling\n \u003e should run this command but we\u0027re already in-effect saying this:\n\nRight, which is the point I think. It\u0027s not ironic specific, although ironic was a solid example of when it was useful for almost everyone, regardless of their deployment mechanism. But, it\u0027s not just for ironic, and ideally the deployment tooling would do it, but if not, then this is your alternative. So I think the existing wording is fine as-is.","commit_id":"8b57a50bbd80f3d61aecc42e792e0e2608eb676e"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f0c0a209bed1a7954902880fd6b01b340ae5be99","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"docs: Don\u0027t configure \u0027[scheduler] discover_hosts_in_cells_interval\u0027"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This option has essentially been superseded by the \u0027--by-service\u0027 flag"},{"line_number":10,"context_line":"for the \u0027nova-manage discover_hosts\u0027 command. As such, we should not be"},{"line_number":11,"context_line":"recommending users use this option in our documentation."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_bc3c9092","line":9,"range":{"start_line":9,"start_character":51,"end_line":9,"end_character":70},"in_reply_to":"3f79a3b5_7cd8387f","updated":"2018-10-10 16:16:00.000000000","message":"I can put that entire explanation in the commit message, if you like. Going to wait til the comments on the next patch are resolved first though","commit_id":"8b57a50bbd80f3d61aecc42e792e0e2608eb676e"}]}