)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"5c30491dd315e8559db5b1cb98156cc1cc4b6470","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3a3299f6_53860d16","updated":"2022-05-13 12:52:15.000000000","message":"Looks like it is better to fix get_server_ip as there are more tests which use this function (also via get_server_ip from tempest/api/compute/base.py). Or we have to fix all of them... ","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"6fd395417b63e9ee5b987ac49fdbd4ba715208c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"16c0d54d_0ccf3e5f","updated":"2022-05-13 13:38:57.000000000","message":"from the code it looks like that we wait for the server to in \"ACTIVE:\ndef create_test_server\n\n\n       for server in servers:\n            try:\n                waiters.wait_for_server_status(\n                    clients.servers_client, server[\u0027id\u0027], wait_until,\n                    request_id\u003drequest_id)\n\nAt this stage the server state is ACTIVE but you dont have the latest server_client.\n\nin  if CONF.validation.run_validation and validatable:\n-------\u003e \nYou need to refresh server config before calling .\nThat will fix the problem.... because its the root problem\n\n\n","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"ddb8c4d8e014456d18dd27423c3a32bcf850e04a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"aaa56501_6b84ffae","updated":"2022-06-17 23:16:36.000000000","message":"lgtm, i inspected this closely using a breakpoint and the proposed solution will fix the problem of missing addresses","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"5c8cd6a8f9fd01cf6eb34f1d26a13733e080d3c0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e1798812_ab0cfbaf","in_reply_to":"16c0d54d_0ccf3e5f","updated":"2022-05-14 20:58:22.000000000","message":"Hello, Benny !\n\nIf you mean create_test_server in the tempest/common/compute.py - it is so. But this function returns body from the following call:\n\n    body \u003d clients.servers_client.create_server(name\u003dname, imageRef\u003dimage_id,\n                                                flavorRef\u003dflavor,\n                                                **kwargs)\n\n\nSo it doesn\u0027t matter, that we waited server to be in ACTIVE status, we return response without IP address which we got before server became active. The upper function create_test_server from tempest/api/compute/base.py get the body and return it without modification. There is no IP address in this body. If we work with floating - it is ok as we add floating after server is in ACTIVE state. But in case of fixed we have to wait when server will be in ACTIVE state and only after we have to get server info (extra call to nava API), where IP will be present.","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4f5bcc4459694d6f9466f7aea444455a7f7162bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bd74bfd3_fd9c7877","in_reply_to":"e1798812_ab0cfbaf","updated":"2022-06-22 03:34:48.000000000","message":"What Benny is suggesting here is to get the server full response (which include the addresses also) from   waiters.wait_for_server_status( function itself. that function anyways wait for server to be active and have full response.\n\nI am commenting inline where exactly you need to refresh the server.","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"53bc0e1aa70dca4d2aec6eba4eb65c739e33b82e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"eaff0532_d45cd058","updated":"2022-06-30 11:09:46.000000000","message":"recheck","commit_id":"f7bacc091dad7a3a3addaa0b91c0ae3501cff69b"},{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"d9b70fe913ad7390aaffc2b3d6ac4e1e317d98c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"ec302651_af46e0da","updated":"2022-06-30 19:07:22.000000000","message":"recheck","commit_id":"2f13c8f154695bdc276855af241f387e9c2294f1"}],"tempest/api/compute/base.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4f5bcc4459694d6f9466f7aea444455a7f7162bd","unresolved":true,"context_lines":[{"line_number":497,"context_line":"        \"\"\""},{"line_number":498,"context_line":"        return compute.get_server_ip("},{"line_number":499,"context_line":"            server, validation_resources\u003dvalidation_resources,"},{"line_number":500,"context_line":"            clients\u003dcls.os_primary)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @classmethod"},{"line_number":503,"context_line":"    def create_volume(cls, image_ref\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5294343c_9b7d20e3","line":500,"range":{"start_line":500,"start_character":24,"end_line":500,"end_character":34},"updated":"2022-06-22 03:34:48.000000000","message":"this is not correct as server can be created by the other credential for example admin or alt member. In that case os_primary client would not be able to query the requested server.","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"03849990fa93d6078751baf58a7b994712015ad9","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        \"\"\""},{"line_number":498,"context_line":"        return compute.get_server_ip("},{"line_number":499,"context_line":"            server, validation_resources\u003dvalidation_resources,"},{"line_number":500,"context_line":"            clients\u003dcls.os_primary)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @classmethod"},{"line_number":503,"context_line":"    def create_volume(cls, image_ref\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"72e6e76c_6bf369b0","line":500,"range":{"start_line":500,"start_character":24,"end_line":500,"end_character":34},"in_reply_to":"4eac9dc5_4a730b5a","updated":"2022-06-30 16:03:37.000000000","message":"Finally, I explore code more detailed and see that previously proposed solution (clients\u003dcls.os_primary) is good enough:\n\n1. When we execute get_server_ip through BaseV2ComputeTest class object method (in tempest/api/compute/base.py) - usage of os_primary is quite reasonable as all method of this class use these creds.\n2. If other code will  execute get_server_ip directly from tempest/common/compute.py - it won\u0027t set clients at all and all stuff will work as it was before (in compatible way).","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"420462f61cb640f3f556d40d2cd804acc68a5e20","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        \"\"\""},{"line_number":498,"context_line":"        return compute.get_server_ip("},{"line_number":499,"context_line":"            server, validation_resources\u003dvalidation_resources,"},{"line_number":500,"context_line":"            clients\u003dcls.os_primary)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @classmethod"},{"line_number":503,"context_line":"    def create_volume(cls, image_ref\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"4eac9dc5_4a730b5a","line":500,"range":{"start_line":500,"start_character":24,"end_line":500,"end_character":34},"in_reply_to":"5294343c_9b7d20e3","updated":"2022-06-24 11:07:32.000000000","message":"Agree about your arguments. Basically we usually operate with server by the same user, but in general potentially it could be different for some cases.","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27ef0c2e1a4c6f171f8a1a5334ce2c9e581b093a","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        \"\"\""},{"line_number":498,"context_line":"        return compute.get_server_ip("},{"line_number":499,"context_line":"            server, validation_resources\u003dvalidation_resources,"},{"line_number":500,"context_line":"            clients\u003dcls.os_primary)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @classmethod"},{"line_number":503,"context_line":"    def create_volume(cls, image_ref\u003dNone, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fe15d3d9_ea658f2e","line":500,"range":{"start_line":500,"start_character":24,"end_line":500,"end_character":34},"in_reply_to":"72e6e76c_6bf369b0","updated":"2022-07-14 20:19:41.000000000","message":"no its not true that every caller passing server in the get_server_ip() method will create server with cls.os_primary (member role). if we are putting this condition here then we will not be able to test ssh and get_server_ip() for admin created servers or alt member created server.","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"}],"tempest/common/compute.py":[{"author":{"_account_id":22873,"name":"Martin Kopec","email":"mkopec@redhat.com","username":"mkopec"},"change_message_id":"ddb8c4d8e014456d18dd27423c3a32bcf850e04a","unresolved":true,"context_lines":[{"line_number":76,"context_line":"                   \u0027validation_resources cannot be None\u0027)"},{"line_number":77,"context_line":"            raise lib_exc.InvalidParam(invalid_param\u003dmsg)"},{"line_number":78,"context_line":"    elif CONF.validation.connect_method \u003d\u003d \u0027fixed\u0027:"},{"line_number":79,"context_line":"        if clients:"},{"line_number":80,"context_line":"            addresses \u003d clients.servers_client.list_addresses("},{"line_number":81,"context_line":"                server[\u0027id\u0027])[\u0027addresses\u0027][CONF.validation.network_for_ssh]"},{"line_number":82,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"a1781fef_87f0ab07","line":79,"updated":"2022-06-17 23:16:36.000000000","message":"i confirm, server variable doesn\u0027t contain any addresses, i put a breakpoint here and this is an example of what\u0027s inside of server var:\n                                                                                                                             \n{\u0027id\u0027: \u0027bbec2692-3fc2-41aa-a6ec-41a4676a4c5a\u0027, \u0027links\u0027: [{\u0027rel\u0027: \u0027self\u0027, \u0027href\u0027:                                                       \n\u0027http://10.0.147.227/compute/v2.1/servers/bbec2692-3fc2-41aa-a6ec-41a4676a4c5a\u0027}, {\u0027rel\u0027: \u0027bookmark\u0027, \u0027href\u0027:                          \n\u0027http://10.0.147.227/compute/servers/bbec2692-3fc2-41aa-a6ec-41a4676a4c5a\u0027}], \u0027OS-DCF:diskConfig\u0027: \u0027MANUAL\u0027, \u0027security_groups\u0027:        \n[{\u0027name\u0027: \u0027tempest-securitygroup--2101301780\u0027}], \u0027adminPass\u0027: \u0027nEL82EfcTxZt\u0027}\n\nAs we need to return the guest\u0027s ip, we need to query it somehow first and that\u0027s what clients.servers_client.list_addresses call is for","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"4f5bcc4459694d6f9466f7aea444455a7f7162bd","unresolved":true,"context_lines":[{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        for server in servers:"},{"line_number":323,"context_line":"            try:"},{"line_number":324,"context_line":"                waiters.wait_for_server_status("},{"line_number":325,"context_line":"                    clients.servers_client, server[\u0027id\u0027], wait_until,"},{"line_number":326,"context_line":"                    request_id\u003drequest_id)"},{"line_number":327,"context_line":"                if CONF.validation.run_validation and validatable:"},{"line_number":328,"context_line":"                    if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":329,"context_line":"                        _setup_validation_fip("}],"source_content_type":"text/x-python","patch_set":2,"id":"f9c5fd4f_5a38cf8a","line":326,"range":{"start_line":324,"start_character":0,"end_line":326,"end_character":42},"updated":"2022-06-22 03:34:48.000000000","message":"here we can get the server full body having the server addresses etc. We can return the server response (show_server response) from wait_for_server_status and store in body. That way we will always get the server full response if any test requesting server to be active or so. Something like below:\n\n body \u003d waiters.wait_for_server_status(\n            clients.servers_client, server[\u0027id\u0027], wait_until,\n            request_id\u003drequest_id)\n servers \u003d [body]\n server \u003d body","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27ef0c2e1a4c6f171f8a1a5334ce2c9e581b093a","unresolved":false,"context_lines":[{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        for server in servers:"},{"line_number":323,"context_line":"            try:"},{"line_number":324,"context_line":"                waiters.wait_for_server_status("},{"line_number":325,"context_line":"                    clients.servers_client, server[\u0027id\u0027], wait_until,"},{"line_number":326,"context_line":"                    request_id\u003drequest_id)"},{"line_number":327,"context_line":"                if CONF.validation.run_validation and validatable:"},{"line_number":328,"context_line":"                    if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":329,"context_line":"                        _setup_validation_fip("}],"source_content_type":"text/x-python","patch_set":2,"id":"57d300db_53ea7bf7","line":326,"range":{"start_line":324,"start_character":0,"end_line":326,"end_character":42},"in_reply_to":"0bf2c6c2_52b6dfe7","updated":"2022-07-14 20:19:41.000000000","message":"\u003e Well, I like this solution but have some comments:\n\u003e \n\u003e 1. I didnt see return of body in waiters.wait_for_server_status - looks like it returns nothing:\n\u003e \n\u003e https://opendev.org/openstack/tempest/src/branch/master/tempest/common/waiters.py#L35-L64\n\u003e \n\u003e So we possibly have to change return code to return body \n\nYes, that is need to change and that should be fine.\n\n\u003e \n\u003e 2. AFAIK it isn\u0027t good to change servers while iterating over it, and in our case after first iterration servers will be changed only with one first item (e.g. server).\n\nOk, in that case let\u0027s change \u0027server\u0027 only so that in wait_for_server_status() we already fetch the full server response and the same we can use while doing ssh to server.\n\n body \u003d waiters.wait_for_server_status(\n            clients.servers_client, server[\u0027id\u0027], wait_until,\n            request_id\u003drequest_id)\n server \u003d body","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":16198,"name":"Ilya Popov","email":"hebulrih@gmail.com","username":"IPO"},"change_message_id":"420462f61cb640f3f556d40d2cd804acc68a5e20","unresolved":false,"context_lines":[{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        for server in servers:"},{"line_number":323,"context_line":"            try:"},{"line_number":324,"context_line":"                waiters.wait_for_server_status("},{"line_number":325,"context_line":"                    clients.servers_client, server[\u0027id\u0027], wait_until,"},{"line_number":326,"context_line":"                    request_id\u003drequest_id)"},{"line_number":327,"context_line":"                if CONF.validation.run_validation and validatable:"},{"line_number":328,"context_line":"                    if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":329,"context_line":"                        _setup_validation_fip("}],"source_content_type":"text/x-python","patch_set":2,"id":"0bf2c6c2_52b6dfe7","line":326,"range":{"start_line":324,"start_character":0,"end_line":326,"end_character":42},"in_reply_to":"f9c5fd4f_5a38cf8a","updated":"2022-06-24 11:07:32.000000000","message":"Well, I like this solution but have some comments:\n\n1. I didnt see return of body in waiters.wait_for_server_status - looks like it returns nothing:\n\nhttps://opendev.org/openstack/tempest/src/branch/master/tempest/common/waiters.py#L35-L64\n\nSo we possibly have to change return code to return body \n\n2. AFAIK it isn\u0027t good to change servers while iterating over it, and in our case after first iterration servers will be changed only with one first item (e.g. server).","commit_id":"5316d226ce3c366d472a9b50859a05a7ae77d112"},{"author":{"_account_id":8367,"name":"Arx Cruz","email":"arxcruz@redhat.com","username":"arxcruz"},"change_message_id":"e218e1517110568d89d11a95ccaa9db1ba8901b5","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                   \u0027validation_resources cannot be None\u0027)"},{"line_number":77,"context_line":"            raise lib_exc.InvalidParam(invalid_param\u003dmsg)"},{"line_number":78,"context_line":"    elif CONF.validation.connect_method \u003d\u003d \u0027fixed\u0027:"},{"line_number":79,"context_line":"        if clients:"},{"line_number":80,"context_line":"            addresses \u003d clients.servers_client.list_addresses("},{"line_number":81,"context_line":"                server[\u0027id\u0027])[\u0027addresses\u0027][CONF.validation.network_for_ssh]"},{"line_number":82,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5d5fa571_ccac7786","line":79,"updated":"2022-07-01 12:13:42.000000000","message":"I wonder if we could just call server \u003d clients.servers_client.show_server(server[\u0027id\u0027])[\u0027server\u0027] and continue having addresses \u003d server[\u0027addresses\u0027] without the else. I think it would be more clean","commit_id":"2f13c8f154695bdc276855af241f387e9c2294f1"}]}