)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12321,"name":"Ryan Brown","email":"rybrown@redhat.com","username":"ryansb"},"change_message_id":"2c9b0a5b615a940ebe129d06254624e166a7ab5c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ryan S. Brown \u003csb@ryansb.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-09-09 15:39:07 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Stop false positive validation warning"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Per rhbz#1260776, deployments that didn\u0027t specify flavors per role (not"},{"line_number":10,"context_line":"using --compute-flavor flags) would have a false warning about \"nodes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_0ab82c10","line":7,"updated":"2015-09-11 14:04:10.000000000","message":"It\u0027s a false positive, because the error is raised (positive) when everything is ok (false).","commit_id":"9243f819994e0b7aa76cb76d0e013597651a4019"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a7a073dd1fc3d9b4a1702b9cfbf26be6530bb4ed","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ryan S. Brown \u003csb@ryansb.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-09-09 15:39:07 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Stop false positive validation warning"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Per rhbz#1260776, deployments that didn\u0027t specify flavors per role (not"},{"line_number":10,"context_line":"using --compute-flavor flags) would have a false warning about \"nodes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_f74292aa","line":7,"updated":"2015-09-11 08:30:14.000000000","message":"it\u0027s false negative, not false positive :) I mean, a user gets an error when everything is actually ok, right?","commit_id":"9243f819994e0b7aa76cb76d0e013597651a4019"}],"tripleoclient/v1/overcloud_deploy.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a7a073dd1fc3d9b4a1702b9cfbf26be6530bb4ed","unresolved":false,"context_lines":[{"line_number":796,"context_line":"                flavor_profile_map,"},{"line_number":797,"context_line":"                node_profile_map)"},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"        if len(node_profile_map.get(None, [])) \u003e 0 \\"},{"line_number":800,"context_line":"                and any(["},{"line_number":801,"context_line":"                    parsed_args.block_storage_flavor,"},{"line_number":802,"context_line":"                    parsed_args.ceph_storage_flavor,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_971236b3","line":799,"updated":"2015-09-11 08:30:14.000000000","message":"+1, and also:\n\n if node_profile_map.get(None) and any(...):","commit_id":"9243f819994e0b7aa76cb76d0e013597651a4019"},{"author":{"_account_id":7065,"name":"Brad P. Crochet","email":"brad@redhat.com","username":"bcrochet"},"change_message_id":"e55434e77ec16acc35a574942b2ca096593805ab","unresolved":false,"context_lines":[{"line_number":796,"context_line":"                flavor_profile_map,"},{"line_number":797,"context_line":"                node_profile_map)"},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"        if len(node_profile_map.get(None, [])) \u003e 0 \\"},{"line_number":800,"context_line":"                and any(["},{"line_number":801,"context_line":"                    parsed_args.block_storage_flavor,"},{"line_number":802,"context_line":"                    parsed_args.ceph_storage_flavor,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_053fe825","line":799,"updated":"2015-09-10 17:09:13.000000000","message":"Nit: I think I\u0027d rather see and extra set of parens instead of a \u0027\\\u0027","commit_id":"9243f819994e0b7aa76cb76d0e013597651a4019"},{"author":{"_account_id":12321,"name":"Ryan Brown","email":"rybrown@redhat.com","username":"ryansb"},"change_message_id":"2c9b0a5b615a940ebe129d06254624e166a7ab5c","unresolved":false,"context_lines":[{"line_number":796,"context_line":"                flavor_profile_map,"},{"line_number":797,"context_line":"                node_profile_map)"},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"        if len(node_profile_map.get(None, [])) \u003e 0 \\"},{"line_number":800,"context_line":"                and any(["},{"line_number":801,"context_line":"                    parsed_args.block_storage_flavor,"},{"line_number":802,"context_line":"                    parsed_args.ceph_storage_flavor,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_aa37c0ae","line":799,"updated":"2015-09-11 14:04:10.000000000","message":"No problem.","commit_id":"9243f819994e0b7aa76cb76d0e013597651a4019"},{"author":{"_account_id":7582,"name":"Jan Provaznik","email":"jan.provaznik@gmail.com","username":"jprovazn"},"change_message_id":"e5cf37b0c1961646553aec8f79d104eae564a177","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                 parsed_args.ceph_storage_flavor,"},{"line_number":802,"context_line":"                 parsed_args.compute_flavor,"},{"line_number":803,"context_line":"                 parsed_args.control_flavor,"},{"line_number":804,"context_line":"                 parsed_args.swift_storage_flavor])):"},{"line_number":805,"context_line":"            self.predeploy_warnings +\u003d 1"},{"line_number":806,"context_line":"            self.log.warning("},{"line_number":807,"context_line":"                \"There are %d ironic nodes with no profile that will \""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_da4c6444","line":804,"updated":"2015-09-16 09:57:45.000000000","message":"This verification is called both for create and update of a deployment. I wonder if it\u0027s safe to assume that flavor parameters will be always passed when updating an existing deployment (as it\u0027s not necessary because heat remembers previously passed values)? If not, then this warning wouldn\u0027t be shown on updates (where it should probably) because flavor params will not be present.\n\nNit: This check could be still false-positive if there is a nova flavor with unset profile used in the deployment - then a node without profile could be still used by this flavor. But this is probably quite extreme situation which doesn\u0027t have to be addressed.\n\nIt would be really nice to have tests for the pre deployment validation functionality (and potentially extend them with a test for this bug) in future.","commit_id":"2ba3ddd15d248ec95ee8f389b7fc05fe425b7472"},{"author":{"_account_id":7582,"name":"Jan Provaznik","email":"jan.provaznik@gmail.com","username":"jprovazn"},"change_message_id":"09585fe3a399bfdf94aff137f348888a3f2409dc","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                 parsed_args.ceph_storage_flavor,"},{"line_number":802,"context_line":"                 parsed_args.compute_flavor,"},{"line_number":803,"context_line":"                 parsed_args.control_flavor,"},{"line_number":804,"context_line":"                 parsed_args.swift_storage_flavor])):"},{"line_number":805,"context_line":"            self.predeploy_warnings +\u003d 1"},{"line_number":806,"context_line":"            self.log.warning("},{"line_number":807,"context_line":"                \"There are %d ironic nodes with no profile that will \""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_0acd79b6","line":804,"in_reply_to":"ba15a1d1_083fec24","updated":"2015-09-17 12:34:50.000000000","message":"OK, fair enough.","commit_id":"2ba3ddd15d248ec95ee8f389b7fc05fe425b7472"},{"author":{"_account_id":7582,"name":"Jan Provaznik","email":"jan.provaznik@gmail.com","username":"jprovazn"},"change_message_id":"c308b790dc143c8194926108c93fad31514b982b","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                 parsed_args.ceph_storage_flavor,"},{"line_number":802,"context_line":"                 parsed_args.compute_flavor,"},{"line_number":803,"context_line":"                 parsed_args.control_flavor,"},{"line_number":804,"context_line":"                 parsed_args.swift_storage_flavor])):"},{"line_number":805,"context_line":"            self.predeploy_warnings +\u003d 1"},{"line_number":806,"context_line":"            self.log.warning("},{"line_number":807,"context_line":"                \"There are %d ironic nodes with no profile that will \""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_c8181478","line":804,"in_reply_to":"ba15a1d1_4fcdeeb5","updated":"2015-09-16 14:12:23.000000000","message":"\u003e My reasoning was that it is safe to not show this warning when flavors are not passed in on update, because in that case it is likely the user omitted the flavors intentionally, and is happy with the existing settings.\n\nWell, but on update there is probably no reason for user to pass these flavor params - only reason would be if he wants to change flavor for a role (which I think would cause replacing all instances with this flavor so it\u0027s extra dangerous), or he adds nodes of a new role (which is not very common too). \n\nMy thinking was that displaying this warning would make sense on updates where flavor is not explicitly passed (typically scaling up). So the scenario would be: 1) user registers new nodes into ironic and forgets to run profile matching on them, 2) then user scales up nodes, e.g. \"openstack overcloud deploy --compute-scale 10\" (no falvor set as he doesn\u0027t want to change them)\nIn this scenario I think this warning would be quite helpful to point him to the root cause of the issue.\n\nIf this doesn\u0027t make sense or is out of scope of this check or is too difficult to change, I\u0027m fine with removing the -1 as I don\u0027t want to block the improvement.","commit_id":"2ba3ddd15d248ec95ee8f389b7fc05fe425b7472"},{"author":{"_account_id":12321,"name":"Ryan Brown","email":"rybrown@redhat.com","username":"ryansb"},"change_message_id":"ce71a070af81112fc1561f506e963389bf7079ad","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                 parsed_args.ceph_storage_flavor,"},{"line_number":802,"context_line":"                 parsed_args.compute_flavor,"},{"line_number":803,"context_line":"                 parsed_args.control_flavor,"},{"line_number":804,"context_line":"                 parsed_args.swift_storage_flavor])):"},{"line_number":805,"context_line":"            self.predeploy_warnings +\u003d 1"},{"line_number":806,"context_line":"            self.log.warning("},{"line_number":807,"context_line":"                \"There are %d ironic nodes with no profile that will \""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_083fec24","line":804,"in_reply_to":"ba15a1d1_c8181478","updated":"2015-09-16 18:23:33.000000000","message":"That is an excellent point. I think that\u0027s in-scope for validations, but not for this change. If you don\u0027t mind removing your -1, I can post the larger change in another patch.","commit_id":"2ba3ddd15d248ec95ee8f389b7fc05fe425b7472"},{"author":{"_account_id":12321,"name":"Ryan Brown","email":"rybrown@redhat.com","username":"ryansb"},"change_message_id":"d1e90533ca0df99c7b6716c3b04edcbf177a5a50","unresolved":false,"context_lines":[{"line_number":801,"context_line":"                 parsed_args.ceph_storage_flavor,"},{"line_number":802,"context_line":"                 parsed_args.compute_flavor,"},{"line_number":803,"context_line":"                 parsed_args.control_flavor,"},{"line_number":804,"context_line":"                 parsed_args.swift_storage_flavor])):"},{"line_number":805,"context_line":"            self.predeploy_warnings +\u003d 1"},{"line_number":806,"context_line":"            self.log.warning("},{"line_number":807,"context_line":"                \"There are %d ironic nodes with no profile that will \""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_4fcdeeb5","line":804,"in_reply_to":"ba15a1d1_da4c6444","updated":"2015-09-16 13:03:59.000000000","message":"My reasoning was that it is safe to not show this warning when flavors are not passed in on update, because in that case it is likely the user omitted the flavors intentionally, and is happy with the existing settings. \n\nThere are tests for pre-deployment validation, they don\u0027t cover as many cases as I\u0027d like, but I plan on improving them.","commit_id":"2ba3ddd15d248ec95ee8f389b7fc05fe425b7472"}]}