)]}'
{"ironic/conductor/base_manager.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"f1c079bb6e96c11bd7d60a9f809bf28cc853e052","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                    # Check maintenance value since it could have changed"},{"line_number":486,"context_line":"                    # after the filtering was done."},{"line_number":487,"context_line":"                    if (desired_maintenance is not None"},{"line_number":488,"context_line":"                            and desired_maintenance !\u003d task.node.maintenance):"},{"line_number":489,"context_line":"                        continue"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"                    if task.node.provision_state not in provision_state:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_1de10e2c","line":488,"updated":"2020-01-20 21:03:03.000000000","message":"In theory, do we really need to?\n\nWith deploy steps, we ultimately have better granularity to where a deploy is, and the next heartbeat would trigger the same action, so could it not be okay to just let the node not move?\n\nAlso, if a node is in maintenance, _should_ we actually change it?","commit_id":"dd18c70543db6f42e531db24606ed8008015828f"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"41a99a71da26a89f79ee51ba726b22bccd46756f","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                    # Check maintenance value since it could have changed"},{"line_number":486,"context_line":"                    # after the filtering was done."},{"line_number":487,"context_line":"                    if (desired_maintenance is not None"},{"line_number":488,"context_line":"                            and desired_maintenance !\u003d task.node.maintenance):"},{"line_number":489,"context_line":"                        continue"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"                    if task.node.provision_state not in provision_state:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_fe7f3e66","line":488,"in_reply_to":"3fa7e38b_1de10e2c","updated":"2020-01-21 15:18:32.000000000","message":"We actually patched this downstream, because we want to move nodes (even in maintenance) from cleaning to cleanfail, if it times out. This is why I was looking at the code here: it doesn\u0027t quite make sense, so I\u0027d like to clean it up.\n\n- If folks are specifying the maintenance value in the filter, it seems like we should honour that. If they aren\u0027t specifying maintenance in the filter, then we shouldn\u0027t be blocking/assuming we know best here.\n\n- if we think we should always block and not do anything if a node is in maintenance, then we should add a comment about that and remove the maintenance stuff from all the filters.\n\nRegardless of what we do, I\u0027ll still need to patch downstream unless we add some sort of config upstream to allow nodes-in-maintenance to be failed. But at least this code will be more understandable. (eg, I removed maintenance\u003dFalse from the filter in the periodic task for cleaning timeout and it didn\u0027t work because of this code.)\n\nAlso, given that some code calls this method without setting maintenance to False, I wonder if they callers meant to operate on all nodes. Eg I think we need to handle nodes-in-maintenance here, otherwise they won\u0027t be associated with the correct conductor, but I could be wrong: https://opendev.org/openstack/ironic/src/commit/8294afa6231629f9734f19ea5b3b0253ee9b8957/ironic/conductor/base_manager.py#L235\n\nRecall: this is a generic method, it shouldn\u0027t know whether it is dealing with deploy steps or anything else.","commit_id":"dd18c70543db6f42e531db24606ed8008015828f"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"b4dca39408a37061d6c07adce711281e9a9e3f6b","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                    # Check maintenance value since it could have changed"},{"line_number":486,"context_line":"                    # after the filtering was done."},{"line_number":487,"context_line":"                    if (desired_maintenance is not None"},{"line_number":488,"context_line":"                            and desired_maintenance !\u003d task.node.maintenance):"},{"line_number":489,"context_line":"                        continue"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"                    if task.node.provision_state not in provision_state:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_6a6574e1","line":488,"in_reply_to":"3fa7e38b_6f42b33e","updated":"2020-04-06 22:49:21.000000000","message":"I picked up on one fine detail, that we\u0027re actually checking desired state in the state when the periodic fired. With that I think I\u0027m good with this.","commit_id":"dd18c70543db6f42e531db24606ed8008015828f"},{"author":{"_account_id":12640,"name":"Arun S A G","email":"sagarun@gmail.com","username":"sagarun"},"change_message_id":"a58ee5cd6684fe8110b6c14cce57d15f1492f694","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                    # Check maintenance value since it could have changed"},{"line_number":486,"context_line":"                    # after the filtering was done."},{"line_number":487,"context_line":"                    if (desired_maintenance is not None"},{"line_number":488,"context_line":"                            and desired_maintenance !\u003d task.node.maintenance):"},{"line_number":489,"context_line":"                        continue"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"                    if task.node.provision_state not in provision_state:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_6f42b33e","line":488,"in_reply_to":"3fa7e38b_fe7f3e66","updated":"2020-01-21 23:46:31.000000000","message":"\u003e Recall: this is a generic method, it shouldn\u0027t know whether it is\n \u003e dealing with deploy steps or anything else.\n\nI agree with ruby on this. This is a generic method and it shouldn\u0027t care if the node is in maintenance or not. It should handle both depending on the caller. The caller of this method should decide whether to include nodes in maintenance or not.","commit_id":"dd18c70543db6f42e531db24606ed8008015828f"}]}