)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"470d6a4e585fd4f608b4463aa3ae1e132209e420","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When running in a container, the host must already have the modules"},{"line_number":10,"context_line":"inserted in its kernel."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ic61ad57557be5dd1319db70bfb50c23836874028"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a1ff146_be08dfda","line":12,"updated":"2017-03-16 03:10:26.000000000","message":"Shouldn\u0027t there be a bug created and a bug ID associated with this here?","commit_id":"13262e6924a208b19cd57dec76f71c8b4bc33574"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"b8d432529f531ea802cecc00020c4b215161c88d","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When running in a container, the host must already have the modules"},{"line_number":10,"context_line":"inserted in its kernel."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ic61ad57557be5dd1319db70bfb50c23836874028"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3a1ff146_88e84212","line":12,"in_reply_to":"3a1ff146_be08dfda","updated":"2017-03-16 09:30:40.000000000","message":"I don\u0027t know, I have never contributed to Ironic before.","commit_id":"13262e6924a208b19cd57dec76f71c8b4bc33574"}],"devstack/lib/ironic":[{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"f42ca2f15f2ccd8d11c99f16682de228655ef4fc","unresolved":false,"context_lines":[{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"function configure_iptables {"},{"line_number":1666,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1667,"context_line":"    if ! running_in_container; then"},{"line_number":1668,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1669,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1670,"context_line":"    fi"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3a1ff146_7372184b","line":1667,"updated":"2017-03-16 08:44:17.000000000","message":"What if those modules are not loaded in host OS? We should fail here, as tftp will not work in this case.\n\nInstead of wrapping modprobe to run only on host OS, I suggest here to check that module is already loaded and do not try to load it second time, if not loaded - die.","commit_id":"13262e6924a208b19cd57dec76f71c8b4bc33574"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"a1de248801384625d4951bdb28a696f65ad3623d","unresolved":false,"context_lines":[{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"function configure_iptables {"},{"line_number":1666,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1667,"context_line":"    if ! running_in_container; then"},{"line_number":1668,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1669,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1670,"context_line":"    fi"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3a1ff146_a8025e09","line":1667,"in_reply_to":"3a1ff146_28458e16","updated":"2017-03-16 09:56:11.000000000","message":"Hi Julian,\n\nI think Vasyl points was not about loading it twice but about the modules not being loaded at all. So his suggestion was to run \"lsmod\" and check if the modules \"nf_conntrack_tftp\" and \"nf_nat_tftp\" is on the list and if not we should stop the devstack setup here (die), probably with a message telling the user to load those modules at the host OS.\n\nAn alternative to \"lsmod\" would be to just call \"modinfo \u003cmodule name\u003e\" and check it\u0027s return code (for both modules).\n\nAnd yes, I do understand other DevStack plugins might not do it but, that\u0027s not a reason why we wouldn\u0027t try to make it a bit better for the users here in the Ironic pluging IMHO.\n\nWhat do you think ?","commit_id":"13262e6924a208b19cd57dec76f71c8b4bc33574"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"b8d432529f531ea802cecc00020c4b215161c88d","unresolved":false,"context_lines":[{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"function configure_iptables {"},{"line_number":1666,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1667,"context_line":"    if ! running_in_container; then"},{"line_number":1668,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1669,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1670,"context_line":"    fi"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3a1ff146_28458e16","line":1667,"in_reply_to":"3a1ff146_7372184b","updated":"2017-03-16 09:30:40.000000000","message":"This is consistent with all the other callsites of running_in_container - I think it\u0027s fine to do the same, at least for now. I don\u0027t have time to do anything else here and I figured I\u0027d be helping someone ... it\u0027s not making it worse than it is already which is a hard fail.\n\nAlso FWIW, loading a second time is a no-op, it\u0027s safe.","commit_id":"13262e6924a208b19cd57dec76f71c8b4bc33574"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"38bd452c8ff8381b7f1b5af99a6e5cb57f100c42","unresolved":false,"context_lines":[{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"function configure_iptables {"},{"line_number":1666,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1667,"context_line":"    if ! running_in_container; then"},{"line_number":1668,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1669,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1670,"context_line":"    fi"}],"source_content_type":"application/x-shellscript","patch_set":1,"id":"3a1ff146_59ca9a6f","line":1667,"in_reply_to":"3a1ff146_a8025e09","updated":"2017-03-16 10:52:44.000000000","message":"function safe_load_modules {\n  for module_name in $@; do\n    if ! lsmod |grep -q $module_name; then\n      echo \"Loading module $module_name\";\n      sudo modprobe $module_name\n    else\n      echo \"Module $module_name already loaded.\"\n    fi\n  done\n}\n\nsafe_load_module nf_conntrack_tftp nf_nat_tftp\n\nShould work with containers.","commit_id":"13262e6924a208b19cd57dec76f71c8b4bc33574"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"b252e42cae453660e68da81e555da277e64ef6e0","unresolved":false,"context_lines":[{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"function configure_iptables {"},{"line_number":1672,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1673,"context_line":"    if ! running_in_container; then"},{"line_number":1674,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1675,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1676,"context_line":"    else"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"1a1ced50_b74919d3","line":1673,"updated":"2017-03-17 09:18:07.000000000","message":"So there is a patch to devstack that should generalize this function to support wide range of container apps, not only LXC as it is at the moment. https://review.openstack.org/#/c/446217\n\nI\u0027m ok with this approach than. Thanks!","commit_id":"1de3681b13b5b857ce036660e5a44b730f09d042"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"35a1a071e6e2892b232fad591c74d11e7e825d7e","unresolved":false,"context_lines":[{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"function configure_iptables {"},{"line_number":1672,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1673,"context_line":"    if ! running_in_container; then"},{"line_number":1674,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1675,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1676,"context_line":"    else"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"1a1ced50_8c821951","line":1673,"in_reply_to":"1a1ced50_1804bebe","updated":"2017-03-20 10:17:07.000000000","message":"Nice, yeah ++ to mention it","commit_id":"1de3681b13b5b857ce036660e5a44b730f09d042"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d4dd27ee6f824d5deb3fa467ab707ecc18e33cd4","unresolved":false,"context_lines":[{"line_number":1670,"context_line":""},{"line_number":1671,"context_line":"function configure_iptables {"},{"line_number":1672,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1673,"context_line":"    if ! running_in_container; then"},{"line_number":1674,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1675,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1676,"context_line":"    else"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"1a1ced50_1804bebe","line":1673,"in_reply_to":"1a1ced50_b74919d3","updated":"2017-03-18 01:51:23.000000000","message":"Yes, that\u0027s my patch :)\nI probably should have mentioned it.","commit_id":"1de3681b13b5b857ce036660e5a44b730f09d042"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"d3c3aa59af49768d3e773a210889133b3bf8a455","unresolved":false,"context_lines":[{"line_number":1671,"context_line":"function configure_iptables {"},{"line_number":1672,"context_line":"    # enable tftp natting for allowing connections to HOST_IP\u0027s tftp server"},{"line_number":1673,"context_line":"    if ! running_in_container; then"},{"line_number":1674,"context_line":"        sudo modprobe nf_conntrack_tftp"},{"line_number":1675,"context_line":"        sudo modprobe nf_nat_tftp"},{"line_number":1676,"context_line":"    else"},{"line_number":1677,"context_line":"        die_if_module_not_loaded nf_conntrack_tftp"}],"source_content_type":"application/x-shellscript","patch_set":2,"id":"1a1ced50_36f8d97c","line":1674,"updated":"2017-03-16 23:03:32.000000000","message":"I still prefer to check if module is loaded before trigger modprobe. It will be general fix, that will work with containers also.","commit_id":"1de3681b13b5b857ce036660e5a44b730f09d042"}]}
