)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bb1c46b7be830fc95140f739288f20a7744f0e9e","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the new style native QEMU LUKS decryption or original decryption method"},{"line_number":12,"context_line":"using os-brick encrytors is used."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"While this works well in most deployments some issues have been observed"},{"line_number":15,"context_line":"in Kolla based environments where the Libvirt secrets are not fully"},{"line_number":16,"context_line":"persisted between host reboots or container upgrades. This can lead to"},{"line_number":17,"context_line":"_detach_encryptor attempting to build an encryptor which will fail if"},{"line_number":18,"context_line":"the associated connection_info for the volume does not contain a"},{"line_number":19,"context_line":"device_path, such as in the case for rbd volumes."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This change adds a simple conditional to _detach_encryptor to ensure we"},{"line_number":22,"context_line":"return when device_path is not present in connection_info and native"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_9c008c36","line":19,"range":{"start_line":14,"start_character":0,"end_line":19,"end_character":49},"updated":"2019-04-06 00:28:02.000000000","message":"So if I\u0027m reading this correctly, and after skimming the discussion in the bug, it sounds like kolla has a bug (which tripleo has worked around) and this is working around the issue in nova - but is this doing the right thing? I mean, are we properly cleaning up the encryptor in this case when disconnecting the volume? We\u0027re avoiding an error but should we be allowing the error? Or is there something else going on where disconnecting the volume with native QEMU LUKS encryption does the cleanup for us so we can side-step this?","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f0b0d3ada64e61f14f679a5987ded343d023ef7e","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the new style native QEMU LUKS decryption or original decryption method"},{"line_number":12,"context_line":"using os-brick encrytors is used."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"While this works well in most deployments some issues have been observed"},{"line_number":15,"context_line":"in Kolla based environments where the Libvirt secrets are not fully"},{"line_number":16,"context_line":"persisted between host reboots or container upgrades. This can lead to"},{"line_number":17,"context_line":"_detach_encryptor attempting to build an encryptor which will fail if"},{"line_number":18,"context_line":"the associated connection_info for the volume does not contain a"},{"line_number":19,"context_line":"device_path, such as in the case for rbd volumes."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This change adds a simple conditional to _detach_encryptor to ensure we"},{"line_number":22,"context_line":"return when device_path is not present in connection_info and native"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_7b410b0a","line":19,"range":{"start_line":14,"start_character":0,"end_line":19,"end_character":49},"in_reply_to":"5fc1f717_9c008c36","updated":"2019-04-08 16:22:38.000000000","message":"\u003e So if I\u0027m reading this correctly, and after skimming the discussion\n \u003e in the bug, it sounds like kolla has a bug (which tripleo has\n \u003e worked around) and this is working around the issue in nova - but\n \u003e is this doing the right thing? \n\nYes, I\u0027ll respin and call this out more but in the specific case this fix is trying to handle there\u0027s no need for us to call the os-brick encryptors as we can be sure they were not used to attach the volume in the first place.\n\n \u003e I mean, are we properly cleaning up the encryptor in this case when \n \u003e disconnecting the volume? We\u0027re avoiding an error but should we be \n \u003e allowing the error? \n\nNo we shouldn\u0027t allow the lack of device_path in the connection_info here to cause an error to be thrown. The fact that we don\u0027t have device_path shows that the volume was never decrypted using the os-brick encryptors so we can safely return here.\n\n \u003e Or is there something else going on where disconnecting the volume\n \u003e with native QEMU LUKS encryption does the cleanup for us so we can\n \u003e side-step this?\n\nYes, with native QEMU LUKS encryption it\u0027s all on the fly within the QEMU processes itself leaving only the Libvirt secrets to clean up on the host when disconnecting the volume.\n\nIf these have already gone and native QEMU LUKS encryption is supported then we could just return early, the only reason I haven\u0027t in this change is to handle the following update case:\n\n- Attach an encrypted volume using os-brick encryptors\n- Update Libvirt/QEMU to the versions required by native LUKS encryption\n- Attempt to detach/shutdown/hard reboot the instance\n\nAs this wouldn\u0027t find the secrets and without calling os-brick would cause the underlying volume disconnect to fail. Again that call to os-brick is idempotent *if* the provided connection_info has a device_path.\n\nSo yeah, encryption is still a mess ;)","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bb1c46b7be830fc95140f739288f20a7744f0e9e","unresolved":false,"context_lines":[{"line_number":8345,"context_line":"    def test_detach_encryptor_native_luks_device_id_secret_missing(self,"},{"line_number":8346,"context_line":"            mock_get_encryptor, mock_use_native_luks, mock_find_secret):"},{"line_number":8347,"context_line":"        \"\"\"Assert that the encryptor is not built when native LUKS is"},{"line_number":8348,"context_line":"        available, the associated volume secret is missing and device_id is"},{"line_number":8349,"context_line":"        also missing from the connection_info."},{"line_number":8350,"context_line":"        \"\"\""},{"line_number":8351,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_9cd72cc0","line":8348,"range":{"start_line":8348,"start_character":63,"end_line":8348,"end_character":72},"updated":"2019-04-06 00:28:02.000000000","message":"device_path","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f0b0d3ada64e61f14f679a5987ded343d023ef7e","unresolved":false,"context_lines":[{"line_number":8345,"context_line":"    def test_detach_encryptor_native_luks_device_id_secret_missing(self,"},{"line_number":8346,"context_line":"            mock_get_encryptor, mock_use_native_luks, mock_find_secret):"},{"line_number":8347,"context_line":"        \"\"\"Assert that the encryptor is not built when native LUKS is"},{"line_number":8348,"context_line":"        available, the associated volume secret is missing and device_id is"},{"line_number":8349,"context_line":"        also missing from the connection_info."},{"line_number":8350,"context_line":"        \"\"\""},{"line_number":8351,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_bb6453ba","line":8348,"range":{"start_line":8348,"start_character":63,"end_line":8348,"end_character":72},"in_reply_to":"5fc1f717_9cd72cc0","updated":"2019-04-08 16:22:38.000000000","message":"ack thanks!","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bb1c46b7be830fc95140f739288f20a7744f0e9e","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        # VolumeEncryptionNotSupported being thrown when we incorrectly build"},{"line_number":1462,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1463,"context_line":"        if (encryption and self._use_native_luks(encryption) and"},{"line_number":1464,"context_line":"            not connection_info.get(\u0027data\u0027).get(\u0027device_path\u0027)):"},{"line_number":1465,"context_line":"                return"},{"line_number":1466,"context_line":"        if encryption:"},{"line_number":1467,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_9c696c26","line":1464,"range":{"start_line":1464,"start_character":31,"end_line":1464,"end_character":43},"updated":"2019-04-06 00:28:02.000000000","message":"I know this should always be in here, but I\u0027m still always nervous about this random dict from whatever vendor storage backend is being used in cinder, and would opt to be safe and default to {} if \u0027data\u0027 isn\u0027t in there. Although it looks like we have plenty of libvirt drivers that just blindly access connection_info[\u0027data\u0027] so whatever. A bit nitty but then I\u0027d change this to connection_info[\u0027data\u0027] to be explicit, but it doesn\u0027t really matter.","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3ad53b8707538b5e1e6ab047e86ddf69d0a2ce07","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        # VolumeEncryptionNotSupported being thrown when we incorrectly build"},{"line_number":1462,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1463,"context_line":"        if (encryption and self._use_native_luks(encryption) and"},{"line_number":1464,"context_line":"            not connection_info.get(\u0027data\u0027).get(\u0027device_path\u0027)):"},{"line_number":1465,"context_line":"                return"},{"line_number":1466,"context_line":"        if encryption:"},{"line_number":1467,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_dbd59fe7","line":1464,"range":{"start_line":1464,"start_character":31,"end_line":1464,"end_character":43},"in_reply_to":"5fc1f717_5b7f6f16","updated":"2019-04-08 16:25:31.000000000","message":"Sorry I see what you mean now, we could end up skipping this if \u0027data\u0027 wasn\u0027t present in the connection_info we had to hand so I may as well just use connection_info[\u0027data\u0027].get(\u0027device_path\u0027) and hit the KeyError.","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f0b0d3ada64e61f14f679a5987ded343d023ef7e","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        # VolumeEncryptionNotSupported being thrown when we incorrectly build"},{"line_number":1462,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1463,"context_line":"        if (encryption and self._use_native_luks(encryption) and"},{"line_number":1464,"context_line":"            not connection_info.get(\u0027data\u0027).get(\u0027device_path\u0027)):"},{"line_number":1465,"context_line":"                return"},{"line_number":1466,"context_line":"        if encryption:"},{"line_number":1467,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_5b7f6f16","line":1464,"range":{"start_line":1464,"start_character":31,"end_line":1464,"end_character":43},"in_reply_to":"5fc1f717_9c696c26","updated":"2019-04-08 16:22:38.000000000","message":"I\u0027m sure a dashing young gentleman talked about o.vo\u0027ing this multiple times at multiple PTGs? /s\n\nI thought {}.get(\u0027data\u0027) was safe at least in the context of this conditional \n as it just defaults to None if \u0027data\u0027 isn\u0027t a key within connection_info? [1][2]\n\n[1] https://docs.python.org/2/library/stdtypes.html#dict.get\n[2] https://docs.python.org/3/library/stdtypes.html#dict.get","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"24766953f3c87b3ce96b96e8d7c82738ebb35757","unresolved":false,"context_lines":[{"line_number":1461,"context_line":"        # VolumeEncryptionNotSupported being thrown when we incorrectly build"},{"line_number":1462,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1463,"context_line":"        if (encryption and self._use_native_luks(encryption) and"},{"line_number":1464,"context_line":"            not connection_info.get(\u0027data\u0027).get(\u0027device_path\u0027)):"},{"line_number":1465,"context_line":"                return"},{"line_number":1466,"context_line":"        if encryption:"},{"line_number":1467,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fce034c_13892ec9","line":1464,"range":{"start_line":1464,"start_character":31,"end_line":1464,"end_character":43},"in_reply_to":"5fc1f717_dbd59fe7","updated":"2019-04-15 13:27:19.000000000","message":"\u003e Sorry I see what you mean now, we could end up skipping this if\n \u003e \u0027data\u0027 wasn\u0027t present in the connection_info we had to hand so I\n \u003e may as well just use connection_info[\u0027data\u0027].get(\u0027device_path\u0027) and\n \u003e hit the KeyError.\n\nYup.","commit_id":"8c0c5432a00adbde921ca688aa05b8cc749f2b59"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"257db51b2a215bc560c98df7ef78eefba59a1bb0","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1470,"context_line":"        if (encryption and self._use_native_luks(encryption) and"},{"line_number":1471,"context_line":"            not connection_info[\u0027data\u0027].get(\u0027device_path\u0027)):"},{"line_number":1472,"context_line":"                return"},{"line_number":1473,"context_line":"        if encryption:"},{"line_number":1474,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1475,"context_line":"                                                   encryption)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_686151d3","line":1472,"range":{"start_line":1472,"start_character":16,"end_line":1472,"end_character":22},"updated":"2019-04-28 17:45:22.000000000","message":"http://logs.openstack.org/51/649951/2/gate/openstack-tox-pep8/b079f29/job-output.txt.gz - passes locally but fails in the gate, I\u0027ll correct, rebase and try again.","commit_id":"86bcc835bc66cb81c254580e1be3922f8e2a4ccc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6a5f2534120a2a1f831e2945faa15c04d9e395c1","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"        # the encryptor below due to the secrets not being present above."},{"line_number":1470,"context_line":"        if (encryption and self._use_native_luks(encryption) and"},{"line_number":1471,"context_line":"            not connection_info[\u0027data\u0027].get(\u0027device_path\u0027)):"},{"line_number":1472,"context_line":"                return"},{"line_number":1473,"context_line":"        if encryption:"},{"line_number":1474,"context_line":"            encryptor \u003d self._get_volume_encryptor(connection_info,"},{"line_number":1475,"context_line":"                                                   encryption)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_88b3654d","line":1472,"range":{"start_line":1472,"start_character":16,"end_line":1472,"end_character":22},"in_reply_to":"ffb9cba7_686151d3","updated":"2019-04-28 17:47:32.000000000","message":"http://logs.openstack.org/51/649951/2/gate/openstack-tox-pep8/b079f29/job-output.txt.gz#_2019-04-26_22_20_49_213593 - is the specific failure sorry.","commit_id":"86bcc835bc66cb81c254580e1be3922f8e2a4ccc"}]}