)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c0293a8f3bee6f84d387f4d6c2c1c850672c739","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     licanwei \u003cli.canwei2@zte.com.cn\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-06 16:45:03 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"check instance state"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When receiving Nova notification instance.update,"},{"line_number":10,"context_line":"check the state of instance and don\u0027t update datamodel"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_eecc9bbd","line":7,"updated":"2019-06-06 15:39:32.000000000","message":"nit: \"check instance state for instance.update notification\"","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c22d619cb54c956975e2c59585422b310b62effd","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     licanwei \u003cli.canwei2@zte.com.cn\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-06 16:45:03 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"check instance state"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When receiving Nova notification instance.update,"},{"line_number":10,"context_line":"check the state of instance and don\u0027t update datamodel"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_b906c21b","line":7,"in_reply_to":"9fb8cfa7_eecc9bbd","updated":"2019-06-10 08:20:25.000000000","message":"Done","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c0293a8f3bee6f84d387f4d6c2c1c850672c739","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When receiving Nova notification instance.update,"},{"line_number":10,"context_line":"check the state of instance and don\u0027t update datamodel"},{"line_number":11,"context_line":"if instance state is \u0027building\u0027."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I950eec50d2cee38bd22c47a70ae6f88bbf049080"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_2e0633d0","line":12,"updated":"2019-06-06 15:39:32.000000000","message":"Can you go into more detail about why these should be ignored? Looking at your other recent changes:\n\nhttps://review.opendev.org/#/c/663541/\n\nhttps://review.opendev.org/#/c/663513/\n\nI\u0027m guessing it\u0027s all related to a bug where we\u0027re adding an instance to the nova CDM while it\u0027s building and then during an audit (or something else?) we\u0027re hitting ComputeNotFoundFound errors because the instance isn\u0027t yet mapped to a node (host) in the model.\n\nIt would be nice to have a bug to track these if they are all related to the same thing and use the same topic branch.","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c22d619cb54c956975e2c59585422b310b62effd","unresolved":false,"context_lines":[{"line_number":9,"context_line":"When receiving Nova notification instance.update,"},{"line_number":10,"context_line":"check the state of instance and don\u0027t update datamodel"},{"line_number":11,"context_line":"if instance state is \u0027building\u0027."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I950eec50d2cee38bd22c47a70ae6f88bbf049080"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9fb8cfa7_7914ca44","line":12,"in_reply_to":"9fb8cfa7_2e0633d0","updated":"2019-06-10 08:20:25.000000000","message":"Done","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c0e57a5d16f276bc61652891d9ca85a68aeaae3f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_8807cf82","line":11,"updated":"2019-06-11 13:41:34.000000000","message":"Won\u0027t this also happen the first time the instance is added to the model even during the instance.create.end notification - based on the code I pointed out inline? What is different about the instance.create.end flow besides the host being part of the instance payload since both notifications go through the same instance_update method.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"8a8e5e5b4c74bcd99133f93253abfa62d1e56524","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_71a275fe","line":11,"in_reply_to":"9fb8cfa7_35595734","updated":"2019-06-12 07:03:07.000000000","message":"No. After adding a instance to the model, it\u0027s necessary to map it to the node.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f4572260f2d57829e82f12c5dce6f9c1b0380792","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_74581655","line":11,"in_reply_to":"9fb8cfa7_54c4f2f4","updated":"2019-06-12 19:39:33.000000000","message":"Actually I guess that log is misleading - it\u0027s not processing the event really because no audit has been performed yet:\n\nhttp://logs.openstack.org/32/663332/8/check/watcher-tempest-vm_workload_consolidation/2cfa9a0/controller/logs/screen-watcher-decision-engine.txt.gz#_Jun_12_01_35_05_964247\n\nJun 12 01:35:05.964247 ubuntu-bionic-rax-ord-0007490126 watcher-decision-engine[2298]: INFO watcher.decision_engine.model.notification.nova [None req-d4d5acc5-3d9c-4ea3-a581-b1710f11e2a1 tempest-TestExecuteVmWorkloadBalanceStrategy-720496344 tempest-TestExecuteVmWorkloadBalanceStrategy-720496344] Event \u0027instance.create.end\u0027 received from nova-compute:ubuntu-bionic-rax-ord-0007490126 with metadata {\u0027timestamp\u0027: u\u00272019-06-12 01:35:05.959827\u0027, \u0027message_id\u0027: u\u002716bb32cb-3c5d-4342-bf8a-7fe8b37308f3\u0027}\n\nJun 12 01:35:05.965290 ubuntu-bionic-rax-ord-0007490126 watcher-decision-engine[2298]: DEBUG watcher.decision_engine.model.notification.nova [None req-d4d5acc5-3d9c-4ea3-a581-b1710f11e2a1 tempest-TestExecuteVmWorkloadBalanceStrategy-720496344 tempest-TestExecuteVmWorkloadBalanceStrategy-720496344] Nova CDM has not yet been built; ignoring notifications until an audit is performed. {{(pid\u003d2298) info /opt/stack/watcher/watcher/decision_engine/model/notification/nova.py:287}}\n\nIf we are actually getting a KeyError for each instance.create.end processed after we\u0027ve performed an audit and built the nova CDM, I think that\u0027s a bigger issue (and the KeyError should be dealt with in that case - but maybe that\u0027s a separate patch?).","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c1d542e2713ba230c8b5469fed37bd3835d4c141","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_5416284b","line":11,"in_reply_to":"9fb8cfa7_74581655","updated":"2019-06-13 08:45:17.000000000","message":"You can find the KeyError log at :\nhttp://logs.openstack.org/16/662416/18/check/watcher-tempest-strategies/9c7b6f1/controller/logs/screen-watcher-decision-engine.txt.gz\n\nI don\u0027t think it\u0027s a bigger issue. It\u0027s just a log, we can delete this line to remove it: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/model_root.py#L163","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f4269274e2f9b2e5a8c6e112522a035b1019ec5","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_35595734","line":11,"in_reply_to":"9fb8cfa7_8807cf82","updated":"2019-06-11 16:15:13.000000000","message":"Or is https://review.opendev.org/#/c/663541/ meant to handle that?","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"8a8e5e5b4c74bcd99133f93253abfa62d1e56524","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_ae1bac7e","line":11,"in_reply_to":"9fb8cfa7_8807cf82","updated":"2019-06-12 07:03:07.000000000","message":"Yes, It\u0027s same when adding instance to the model during instance.create.end.\nThis commit just ignores the instance.update with \u0027building\u0027 state.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4e1c600a45e41747c0e1b1e7fb1f458077ec5e2","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In the process of creating an instance, Nova will emit an"},{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_54c4f2f4","line":11,"in_reply_to":"9fb8cfa7_ae1bac7e","updated":"2019-06-12 19:34:22.000000000","message":"\u003e Yes, It\u0027s same when adding instance to the model during instance.create.end.\n\nSo we get a KeyError traceback for every instance.create.end notification when adding the instance to the model? That seems bad if we can avoid it (like I said, we could avoid the KeyError if we know the instance isn\u0027t in the model and just raise InstanceNotFound or something).\n\nAnyway, I must be missing something because I see instance.create.end notifications in the logs here:\n\nhttp://logs.openstack.org/32/663332/8/check/watcher-tempest-vm_workload_consolidation/2cfa9a0/controller/logs/screen-watcher-decision-engine.txt.gz\n\nWithout any KeyError so I guess that isn\u0027t an issue.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c0e57a5d16f276bc61652891d9ca85a68aeaae3f","unresolved":false,"context_lines":[{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1832154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_a831735c","line":13,"updated":"2019-06-11 13:41:34.000000000","message":"It seems to me this is dealing with a symptom of a problem in the ModelRoot code where it\u0027s not checking existence properly and that\u0027s why we get the traceback.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"2579cd1b743ea3ccd0746009c383f40566eeb658","unresolved":false,"context_lines":[{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1832154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_e31b5bc7","line":13,"in_reply_to":"9fb8cfa7_3b88cac4","updated":"2019-06-21 01:39:35.000000000","message":"I\u0027ll submit a separate patch to improve the process of instance.create.end.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"8a8e5e5b4c74bcd99133f93253abfa62d1e56524","unresolved":false,"context_lines":[{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1832154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_b129cd8a","line":13,"in_reply_to":"9fb8cfa7_a831735c","updated":"2019-06-12 07:03:07.000000000","message":"I  don\u0027t think so. Notification instance.update with \u0027building\u0027 state is meaningless for Watcher model and should ignore it.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"d0d8ab3495717ac42c897c11e5012e3a7c06c623","unresolved":false,"context_lines":[{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1832154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_3b88cac4","line":13,"in_reply_to":"9fb8cfa7_a831735c","updated":"2019-06-20 08:52:58.000000000","message":"Yes I think we can improve the way we handle the instance.create.end notifcations to prevent the generation of KeyErrors.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"d0d8ab3495717ac42c897c11e5012e3a7c06c623","unresolved":false,"context_lines":[{"line_number":10,"context_line":"instance.update notification with \u0027building\u0027 state."},{"line_number":11,"context_line":"This will cause a KeyError exception because this instance"},{"line_number":12,"context_line":"isn\u0027t in Watcher datamodel."},{"line_number":13,"context_line":"So we should ignore the notification instance.update with"},{"line_number":14,"context_line":"\u0027building\u0027 state."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1832154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9fb8cfa7_1b8dc6d2","line":13,"in_reply_to":"9fb8cfa7_b129cd8a","updated":"2019-06-20 08:52:58.000000000","message":"But ignoring the instance.update if the state is building seems like a good idea and the improvements to notification handling should be in a separate patch.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"}],"watcher/decision_engine/model/notification/nova.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c0293a8f3bee6f84d387f4d6c2c1c850672c739","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        instance_uuid \u003d instance_data[\u0027uuid\u0027]"},{"line_number":219,"context_line":"        instance_state \u003d instance_data[\u0027state\u0027]"},{"line_number":220,"context_line":"        node_uuid \u003d instance_data.get(\u0027host\u0027)"},{"line_number":221,"context_line":"        # if instance state is building, don\u0027t update data model"},{"line_number":222,"context_line":"        if instance_state \u003d\u003d \u0027building\u0027:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_ae222351","line":221,"updated":"2019-06-06 15:39:32.000000000","message":"Is this because the node_uuid might be None? If so, then why not just check if node_uuid is None here and return? Or is it more about not wanting to potentially do anything with this instance until it\u0027s built?","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c22d619cb54c956975e2c59585422b310b62effd","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        instance_uuid \u003d instance_data[\u0027uuid\u0027]"},{"line_number":219,"context_line":"        instance_state \u003d instance_data[\u0027state\u0027]"},{"line_number":220,"context_line":"        node_uuid \u003d instance_data.get(\u0027host\u0027)"},{"line_number":221,"context_line":"        # if instance state is building, don\u0027t update data model"},{"line_number":222,"context_line":"        if instance_state \u003d\u003d \u0027building\u0027:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_1954aefe","line":221,"in_reply_to":"9fb8cfa7_ae222351","updated":"2019-06-10 08:20:25.000000000","message":"In the process of creating an instance, Nova will emit an instance.update notification with \u0027building\u0027 state before notification instance.create.end. \nI want to ignore this instance.update.","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c0e57a5d16f276bc61652891d9ca85a68aeaae3f","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        \"instance %(instance)s\","},{"line_number":48,"context_line":"                        dict(node\u003dnode_uuid, instance\u003dinstance_uuid))"},{"line_number":49,"context_line":"        try:"},{"line_number":50,"context_line":"            instance \u003d self.cluster_data_model.get_instance_by_uuid("},{"line_number":51,"context_line":"                instance_uuid)"},{"line_number":52,"context_line":"        except exception.InstanceNotFound:"},{"line_number":53,"context_line":"            # The instance didn\u0027t exist yet so we create a new instance object"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_e898cb75","line":50,"range":{"start_line":50,"start_character":47,"end_line":50,"end_character":67},"updated":"2019-06-11 13:41:34.000000000","message":"It looks like this is what causes the KeyError because of this code:\n\nhttps://github.com/openstack/watcher/blob/46a36d1ad73ca9024ee99c8b1c9cd29c3fea14e0/watcher/decision_engine/model/model_root.py#L161\n\nWhy doesn\u0027t _get_by_uuid check if the uuid is in the self.node first to avoid that KeyError, or handle the KeyError and raise ComputeResourceNotFound directly? Then get_instance_by_uuid would catch ComputeResourceNotFound and raise InstanceNotFound and add the instance element below?\n\nAlso, what about instance.create.end is going to avoid that traceback on the first time we add the instance to the model?","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c1d542e2713ba230c8b5469fed37bd3835d4c141","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        \"instance %(instance)s\","},{"line_number":48,"context_line":"                        dict(node\u003dnode_uuid, instance\u003dinstance_uuid))"},{"line_number":49,"context_line":"        try:"},{"line_number":50,"context_line":"            instance \u003d self.cluster_data_model.get_instance_by_uuid("},{"line_number":51,"context_line":"                instance_uuid)"},{"line_number":52,"context_line":"        except exception.InstanceNotFound:"},{"line_number":53,"context_line":"            # The instance didn\u0027t exist yet so we create a new instance object"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_545dc812","line":50,"range":{"start_line":50,"start_character":47,"end_line":50,"end_character":67},"in_reply_to":"9fb8cfa7_34661e2c","updated":"2019-06-13 08:45:17.000000000","message":"I guess you have known the reason :)","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4e1c600a45e41747c0e1b1e7fb1f458077ec5e2","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        \"instance %(instance)s\","},{"line_number":48,"context_line":"                        dict(node\u003dnode_uuid, instance\u003dinstance_uuid))"},{"line_number":49,"context_line":"        try:"},{"line_number":50,"context_line":"            instance \u003d self.cluster_data_model.get_instance_by_uuid("},{"line_number":51,"context_line":"                instance_uuid)"},{"line_number":52,"context_line":"        except exception.InstanceNotFound:"},{"line_number":53,"context_line":"            # The instance didn\u0027t exist yet so we create a new instance object"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_34661e2c","line":50,"range":{"start_line":50,"start_character":47,"end_line":50,"end_character":67},"in_reply_to":"9fb8cfa7_ae640c23","updated":"2019-06-12 19:34:22.000000000","message":"\u003e KeyError has been captured and the log about KeyError is the output\n \u003e of LOG.exception()\n \u003e https://github.com/openstack/watcher/blob/46a36d1ad73ca9024ee99c8b1c9cd29c3fea14e0/watcher/decision_engine/model/model_root.py#L163\n\nYeah I know, I\u0027d just like to avoid KeyError tracebacks in the logs for known/expected situations - and that\u0027s what you\u0027re trying to fix here it sounds like - but I was confused about whether or not we\u0027d also get that during instance.create.end in the \"normal\" case, but looking at the instance.create.end notifications in these logs:\n\nhttp://logs.openstack.org/32/663332/8/check/watcher-tempest-vm_workload_consolidation/2cfa9a0/controller/logs/screen-watcher-decision-engine.txt.gz\n\nI don\u0027t see any errors so we must be OK.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"8a8e5e5b4c74bcd99133f93253abfa62d1e56524","unresolved":false,"context_lines":[{"line_number":47,"context_line":"                        \"instance %(instance)s\","},{"line_number":48,"context_line":"                        dict(node\u003dnode_uuid, instance\u003dinstance_uuid))"},{"line_number":49,"context_line":"        try:"},{"line_number":50,"context_line":"            instance \u003d self.cluster_data_model.get_instance_by_uuid("},{"line_number":51,"context_line":"                instance_uuid)"},{"line_number":52,"context_line":"        except exception.InstanceNotFound:"},{"line_number":53,"context_line":"            # The instance didn\u0027t exist yet so we create a new instance object"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_ae640c23","line":50,"range":{"start_line":50,"start_character":47,"end_line":50,"end_character":67},"in_reply_to":"9fb8cfa7_e898cb75","updated":"2019-06-12 07:03:07.000000000","message":"KeyError has been captured and the log about KeyError is the output of LOG.exception()\nhttps://github.com/openstack/watcher/blob/46a36d1ad73ca9024ee99c8b1c9cd29c3fea14e0/watcher/decision_engine/model/model_root.py#L163","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f4572260f2d57829e82f12c5dce6f9c1b0380792","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        instance_uuid \u003d instance_data[\u0027uuid\u0027]"},{"line_number":219,"context_line":"        instance_state \u003d instance_data[\u0027state\u0027]"},{"line_number":220,"context_line":"        node_uuid \u003d instance_data.get(\u0027host\u0027)"},{"line_number":221,"context_line":"        # if instance state is building, don\u0027t update data model"},{"line_number":222,"context_line":"        if instance_state \u003d\u003d \u0027building\u0027:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_341dbe73","line":221,"updated":"2019-06-12 19:39:33.000000000","message":"Is what really matters if there is a host to map the instance to? Because if so, then an instance in shelved_offloaded state won\u0027t have a host and we\u0027d want to unmap it - do we handle that anywhere?","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"2579cd1b743ea3ccd0746009c383f40566eeb658","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        instance_uuid \u003d instance_data[\u0027uuid\u0027]"},{"line_number":219,"context_line":"        instance_state \u003d instance_data[\u0027state\u0027]"},{"line_number":220,"context_line":"        node_uuid \u003d instance_data.get(\u0027host\u0027)"},{"line_number":221,"context_line":"        # if instance state is building, don\u0027t update data model"},{"line_number":222,"context_line":"        if instance_state \u003d\u003d \u0027building\u0027:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_c3f2d79a","line":221,"in_reply_to":"9fb8cfa7_1b7b86be","updated":"2019-06-21 01:39:35.000000000","message":"It\u0027s the second. yes, For now most strategies check compute nodes first, so the unmappable instances will be excluded.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c1d542e2713ba230c8b5469fed37bd3835d4c141","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        instance_uuid \u003d instance_data[\u0027uuid\u0027]"},{"line_number":219,"context_line":"        instance_state \u003d instance_data[\u0027state\u0027]"},{"line_number":220,"context_line":"        node_uuid \u003d instance_data.get(\u0027host\u0027)"},{"line_number":221,"context_line":"        # if instance state is building, don\u0027t update data model"},{"line_number":222,"context_line":"        if instance_state \u003d\u003d \u0027building\u0027:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_d45bf8f2","line":221,"in_reply_to":"9fb8cfa7_341dbe73","updated":"2019-06-13 08:45:17.000000000","message":"The instance without host will be exclude in strategies.\nNow we handle instance.shelve.end and instance.unshelve.end notifications.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"},{"author":{"_account_id":29911,"name":"Dantali0n","email":"info@dantalion.nl","username":"Dantali0n"},"change_message_id":"d0d8ab3495717ac42c897c11e5012e3a7c06c623","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        instance_uuid \u003d instance_data[\u0027uuid\u0027]"},{"line_number":219,"context_line":"        instance_state \u003d instance_data[\u0027state\u0027]"},{"line_number":220,"context_line":"        node_uuid \u003d instance_data.get(\u0027host\u0027)"},{"line_number":221,"context_line":"        # if instance state is building, don\u0027t update data model"},{"line_number":222,"context_line":"        if instance_state \u003d\u003d \u0027building\u0027:"},{"line_number":223,"context_line":"            return"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fb8cfa7_1b7b86be","line":221,"in_reply_to":"9fb8cfa7_d45bf8f2","updated":"2019-06-20 08:52:58.000000000","message":"Were do un-mappable instances get excluded from the model? Or do you mean strategies exclude them because they typically iterate over the ComputeNodes with an inner loop iterating over the Instances of that ComputeNode.\n\nIf the second is the case that would mean that some strategies depending on how the are written still could access the un-mappable instance.","commit_id":"f9e267fa42f5b8a024edaafd872eeeb9eb9f9b43"}],"watcher/tests/decision_engine/model/notification/test_nova_notifications.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4c0293a8f3bee6f84d387f4d6c2c1c850672c739","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            metadata\u003dself.FAKE_METADATA,"},{"line_number":267,"context_line":"        )"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        self.assertEqual(element.InstanceState.ACTIVE.value, instance0.state)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    @mock.patch.object(nova_helper, \"NovaHelper\")"},{"line_number":272,"context_line":"    def test_nova_instance_update_notfound_still_creates("}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_4ea5a7b4","line":269,"updated":"2019-06-06 15:39:32.000000000","message":"nit: a comment here would be useful, something like \"Assert that the instance state in the model is unchanged since the \u0027building\u0027 state is ignored.\"","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"},{"author":{"_account_id":21692,"name":"licanwei","email":"li.canwei2@zte.com.cn","username":"licanwei"},"change_message_id":"c22d619cb54c956975e2c59585422b310b62effd","unresolved":false,"context_lines":[{"line_number":266,"context_line":"            metadata\u003dself.FAKE_METADATA,"},{"line_number":267,"context_line":"        )"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        self.assertEqual(element.InstanceState.ACTIVE.value, instance0.state)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    @mock.patch.object(nova_helper, \"NovaHelper\")"},{"line_number":272,"context_line":"    def test_nova_instance_update_notfound_still_creates("}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_99117e55","line":269,"in_reply_to":"9fb8cfa7_4ea5a7b4","updated":"2019-06-10 08:20:25.000000000","message":"Done","commit_id":"ea9adcd7006c9d37bb08db47faa9e6c3cc386b9d"}]}