)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"91b4b12e5e713d7c1b249a8f55b5114e514c5af7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"7def2b23_29d46a49","updated":"2023-03-22 21:19:36.000000000","message":"Looks good to me. Please read my inline comment","commit_id":"c6d486a171b675818ac83c7c20356a2c7f2bca2b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"32ed45a69cb9236461eedc9164bcda833a9654e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"3306f361_feb1d554","updated":"2023-03-28 10:50:46.000000000","message":"recheck neutron-tempest-plugin-fwaas-2023-1","commit_id":"365373910cef371a79ae9bd8e905ce41c1a7cb0a"}],"neutron_tempest_plugin/scenario/test_security_groups.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b92c9bcd9a7d7e823651dd3775d8892afad524ea","unresolved":true,"context_lines":[{"line_number":211,"context_line":"                    constants.PROTO_NAME_TCP,"},{"line_number":212,"context_line":"                    ssh_client\u003dclient_ssh_client)"},{"line_number":213,"context_line":"                return received_msg and expected_msg in received_msg"},{"line_number":214,"context_line":"            except exceptions.ShellCommandFailed:"},{"line_number":215,"context_line":"                return False"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if self.stateless_sg:"},{"line_number":218,"context_line":"            # In case of stateless SG connectivity will not work without"}],"source_content_type":"text/x-python","patch_set":7,"id":"5649ce76_9c797076","line":215,"range":{"start_line":214,"start_character":12,"end_line":215,"end_character":28},"updated":"2023-03-22 16:59:08.000000000","message":"We are changing the method behaviour here. But don\u0027t get me wrong, I prefer catching the exception and returning False, as you did. But here we are also masking other possible problems rather than the negative nc execution. If the nc fails this is because there is no ssh connection to the VM.","commit_id":"c6d486a171b675818ac83c7c20356a2c7f2bca2b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a68c063a371cafa6e4164112533a396a091ed0f6","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                    constants.PROTO_NAME_TCP,"},{"line_number":212,"context_line":"                    ssh_client\u003dclient_ssh_client)"},{"line_number":213,"context_line":"                return received_msg and expected_msg in received_msg"},{"line_number":214,"context_line":"            except exceptions.ShellCommandFailed:"},{"line_number":215,"context_line":"                return False"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if self.stateless_sg:"},{"line_number":218,"context_line":"            # In case of stateless SG connectivity will not work without"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f0362f9_8b6c8f8c","line":215,"range":{"start_line":214,"start_character":12,"end_line":215,"end_character":28},"in_reply_to":"0f2c1f8b_2327f4f6","updated":"2023-03-23 13:45:34.000000000","message":"OK, now that makes sense: Slawek confirmed that we were NOT using this method for negative cases. Then this change makes sense because \"nc\" is failing in the execution, as expected. This is why we need now to catch this exception.","commit_id":"c6d486a171b675818ac83c7c20356a2c7f2bca2b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"15f4282f0bb78ccb666c6b099dd55e5e53bae092","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                    constants.PROTO_NAME_TCP,"},{"line_number":212,"context_line":"                    ssh_client\u003dclient_ssh_client)"},{"line_number":213,"context_line":"                return received_msg and expected_msg in received_msg"},{"line_number":214,"context_line":"            except exceptions.ShellCommandFailed:"},{"line_number":215,"context_line":"                return False"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if self.stateless_sg:"},{"line_number":218,"context_line":"            # In case of stateless SG connectivity will not work without"}],"source_content_type":"text/x-python","patch_set":7,"id":"0f2c1f8b_2327f4f6","line":215,"range":{"start_line":214,"start_character":12,"end_line":215,"end_character":28},"in_reply_to":"24d17067_b28661e9","updated":"2023-03-23 13:44:11.000000000","message":"If we are not catching other exceptions but the command execution, this is fine.\n\nMy question now is how that was working before? In particular for the negative cases. \"nc\" was returning successfully in the positive and negative cases. In the second case, we were just reading the (not) received packets.","commit_id":"c6d486a171b675818ac83c7c20356a2c7f2bca2b"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"91b4b12e5e713d7c1b249a8f55b5114e514c5af7","unresolved":true,"context_lines":[{"line_number":211,"context_line":"                    constants.PROTO_NAME_TCP,"},{"line_number":212,"context_line":"                    ssh_client\u003dclient_ssh_client)"},{"line_number":213,"context_line":"                return received_msg and expected_msg in received_msg"},{"line_number":214,"context_line":"            except exceptions.ShellCommandFailed:"},{"line_number":215,"context_line":"                return False"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if self.stateless_sg:"},{"line_number":218,"context_line":"            # In case of stateless SG connectivity will not work without"}],"source_content_type":"text/x-python","patch_set":7,"id":"b73f4bb1_9727189c","line":215,"range":{"start_line":214,"start_character":12,"end_line":215,"end_character":28},"in_reply_to":"5649ce76_9c797076","updated":"2023-03-22 21:19:36.000000000","message":"mhhhh, I\u0027m not so sure ralonsoh\u0027s concern is justified. If I\u0027m reading correctly the Tempest code, ultimately the nc command is executed by exec_command in https://github.com/openstack/tempest/blob/master/tempest/lib/common/ssh.py. Specifically, exec_command raises SSHExecCommandFailed here: https://github.com/openstack/tempest/blob/bbb2e044132890cabc823e3c0b583a9462f9c2ae/tempest/lib/common/ssh.py#L237-L240. These lines of code only execute if we actually receive a status from the ssh communication channel: https://github.com/openstack/tempest/blob/bbb2e044132890cabc823e3c0b583a9462f9c2ae/tempest/lib/common/ssh.py#L233. If there is no ssh connection, I thin we get a timeout: https://github.com/openstack/tempest/blob/bbb2e044132890cabc823e3c0b583a9462f9c2ae/tempest/lib/common/ssh.py#L205-L209.\n\nIn neutron_tempest_plugin, when there is a not None and not 0 exit status from the remotely executed command, the SSHExecCommandFailed exception is transformed to ShellCommandFailed here https://github.com/openstack/neutron-tempest-plugin/blob/a8d67e6f3bccf9cd9946b0dcd73eb95c248783e1/neutron_tempest_plugin/common/shell.py#L111-L120 and here https://github.com/openstack/neutron-tempest-plugin/blob/a8d67e6f3bccf9cd9946b0dcd73eb95c248783e1/neutron_tempest_plugin/common/shell.py#L176-L180. \n\nSo I think we are getting what Slawek intended. It looks good to me","commit_id":"c6d486a171b675818ac83c7c20356a2c7f2bca2b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"2ae2d71fa2bc77e94efbc21a0b4099137398c6cc","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                    constants.PROTO_NAME_TCP,"},{"line_number":212,"context_line":"                    ssh_client\u003dclient_ssh_client)"},{"line_number":213,"context_line":"                return received_msg and expected_msg in received_msg"},{"line_number":214,"context_line":"            except exceptions.ShellCommandFailed:"},{"line_number":215,"context_line":"                return False"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if self.stateless_sg:"},{"line_number":218,"context_line":"            # In case of stateless SG connectivity will not work without"}],"source_content_type":"text/x-python","patch_set":7,"id":"24d17067_b28661e9","line":215,"range":{"start_line":214,"start_character":12,"end_line":215,"end_character":28},"in_reply_to":"b73f4bb1_9727189c","updated":"2023-03-23 08:13:22.000000000","message":"It\u0027s exactly how Miguel explained. In case of e.g. SSH connectivity issue one of the exceptions from the tuple: https://github.com/openstack/neutron-tempest-plugin/blob/master/neutron_tempest_plugin/scenario/base.py#L43 will be raised and not catched here. So I hope we should be good here.","commit_id":"c6d486a171b675818ac83c7c20356a2c7f2bca2b"}]}