)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"635358e28416eef2a4870f520a9d462b6f74234d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"tests: Correctly mock out security groups in NeutronFixture"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This will always return something when running so properly mock things."},{"line_number":10,"context_line":"This requires some minor modifications to tests."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibbee7fd11c1aa254e399d302adbae69126e98262"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3fa7e38b_3ad1bb58","line":9,"range":{"start_line":9,"start_character":10,"end_line":9,"end_character":46},"updated":"2019-10-03 19:11:27.000000000","message":"Hmm, are you sure? What if I\u0027ve got a server with no ports attached, will I still get the default security group back when showing server details? Looks like maybe not:\n\nhttps://github.com/openstack/nova/blob/867401e575d2b27b9bc63ceda41cd85233545cd5/nova/api/openstack/compute/views/servers.py#L627\n\nAnd it is possible to create a server with ports:\n\n  openstack --os-compute-api-version 2.37 --nic none ...\n\nAnyway, it\u0027s not a normal case but I don\u0027t think \"always\" is correct here.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"06a2ee821667367a8ab7360dec8f9aa4bc8c88a1","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"tests: Correctly mock out security groups in NeutronFixture"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This will always return something when running so properly mock things."},{"line_number":10,"context_line":"This requires some minor modifications to tests."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibbee7fd11c1aa254e399d302adbae69126e98262"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3fa7e38b_84c16d31","line":9,"range":{"start_line":9,"start_character":10,"end_line":9,"end_character":46},"in_reply_to":"3fa7e38b_14a36e51","updated":"2019-10-04 10:21:44.000000000","message":"Definitely more context needed here, I agree. I thought it was a bug that security_groups wasn\u0027t included in the response. I\u0027m pretty sure I left comments somewhere on this review to that effect. Will I fix that bug?","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d4684ceaae61aad7a3dec3082d9b30481a279ceb","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"tests: Correctly mock out security groups in NeutronFixture"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This will always return something when running so properly mock things."},{"line_number":10,"context_line":"This requires some minor modifications to tests."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibbee7fd11c1aa254e399d302adbae69126e98262"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3fa7e38b_a3c10631","line":9,"range":{"start_line":9,"start_character":10,"end_line":9,"end_character":46},"in_reply_to":"3fa7e38b_3ad1bb58","updated":"2019-10-03 22:17:25.000000000","message":"I verified in devstack (with neutron). I created a server with no network:\n\n$ openstack --os-compute-api-version 2.37 server create --flavor m1.tiny --image cirros-0.4.0-x86_64-disk --nic none --wait vm-no-net\n\nAnd curled GET /servers/detail and there is no security_groups key in the response:\n\n$ curl -H \"X-Auth-Token: $token\" http://10.128.0.6/compute/v2.1/servers/detail | python -m json.tool | grep security_groups\n  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current\n                                 Dload  Upload   Total   Spent    Left  Speed\n100  1388  100  1388    0     0   8213      0 --:--:-- --:--:-- --:--:--  8213\n\nThis is the full response (unformatted) FWIW:\n\n$ curl -H \"X-Auth-Token: $token\" http://10.128.0.6/compute/v2.1/servers/detail\n{\"servers\": [{\"OS-EXT-STS:task_state\": null, \"addresses\": {}, \"links\": [{\"href\": \"http://10.128.0.6/compute/v2.1/servers/689eaa22-e590-4d8c-b0d4-df46b5aa8a58\", \"rel\": \"self\"}, {\"href\": \"http://10.128.0.6/compute/servers/689eaa22-e590-4d8c-b0d4-df46b5aa8a58\", \"rel\": \"bookmark\"}], \"image\": {\"id\": \"34d1abe0-da88-4493-842f-c1250e131611\", \"links\": [{\"href\": \"http://10.128.0.6/compute/images/34d1abe0-da88-4493-842f-c1250e131611\", \"rel\": \"bookmark\"}]}, \"OS-EXT-STS:vm_state\": \"active\", \"OS-EXT-SRV-ATTR:instance_name\": \"instance-00000001\", \"OS-SRV-USG:launched_at\": \"2019-10-03T22:11:57.000000\", \"flavor\": {\"id\": \"1\", \"links\": [{\"href\": \"http://10.128.0.6/compute/flavors/1\", \"rel\": \"bookmark\"}]}, \"id\": \"689eaa22-e590-4d8c-b0d4-df46b5aa8a58\", \"user_id\": \"9508bb31f1cb4a13b02ba97f8c183cc5\", \"OS-DCF:diskConfig\": \"MANUAL\", \"accessIPv4\": \"\", \"accessIPv6\": \"\", \"progress\": 0, \"OS-EXT-STS:power_state\": 1, \"OS-EXT-AZ:availability_zone\": \"nova\", \"config_drive\": \"\", \"status\": \"ACTIVE\", \"updated\": \"2019-10-03T22:11:57Z\", \"hostId\": \"a383733282a8a361e2bfe3637a1f66d5112f4671ffe958ade006049b\", \"OS-EXT-SRV-ATTR:host\": \"devstack\", \"OS-SRV-USG:terminated_at\": null, \"key_name\": null, \"OS-EXT-SRV-ATTR:hypervisor_hostname\": \"devstack\", \"name\": \"vm-no-net\", \"created\": \"2019-10-03T22:11:52Z\", \"tenant_id\": \"c3b9f8e6d9474d2f94fd0df6141212f2\", \"os-extended-volumes:volumes_attached\": [], \"metadata\": {}}]}","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":26458,"name":"Brin Zhang","email":"zhangbailin@inspur.com","username":"zhangbailin"},"change_message_id":"4cb07809ff8ed061066076bab6697c5f7f418af0","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"tests: Correctly mock out security groups in NeutronFixture"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This will always return something when running so properly mock things."},{"line_number":10,"context_line":"This requires some minor modifications to tests."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibbee7fd11c1aa254e399d302adbae69126e98262"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3fa7e38b_14a36e51","line":9,"range":{"start_line":9,"start_character":10,"end_line":9,"end_character":46},"in_reply_to":"3fa7e38b_3ad1bb58","updated":"2019-10-04 06:43:40.000000000","message":"yeah, if the nic is none when server create, it will be not contained the security_group in the response.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca1a6ee6222377e6fe6daa5c9f876a07be1b8c0f","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"tests: Correctly mock out security groups in NeutronFixture"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This will always return something when running so properly mock things."},{"line_number":10,"context_line":"This requires some minor modifications to tests."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibbee7fd11c1aa254e399d302adbae69126e98262"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3fa7e38b_f52569ac","line":9,"range":{"start_line":9,"start_character":10,"end_line":9,"end_character":46},"in_reply_to":"3fa7e38b_84c16d31","updated":"2019-10-04 13:26:10.000000000","message":"\u003e Definitely more context needed here, I agree. I thought it was a\n \u003e bug that security_groups wasn\u0027t included in the response. I\u0027m\n \u003e pretty sure I left comments somewhere on this review to that\n \u003e effect. Will I fix that bug?\n\nSo this statement in the commit message is wrong, but I\u0027m not sure it\u0027s worth respinning this for just this change.\n\nThe other things in here about updating the api-guide, I opened a bug for that and is already wrong before this, it\u0027s just we didn\u0027t realize until your change here so don\u0027t worry about updating that in this change.\n\nSame with the api-ref saying that security_groups are required in the GET response, that\u0027s latent and can be fixed separately.\n\nThe only other thing in here is that the NeutronFixture is unconditionally returning security groups and I think it should only do that if the server has an info cache with VIFs in it which won\u0027t be the case if the server was created with networks\u003d\u0027none\u0027. Dealing with that could be done in a respin or a follow up. If you respin, then update the commit message to fix the false statement.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"36cd3ce47e65d17573c7ea45930f6c8ee7d0bd0a","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"tests: Correctly mock out security groups in NeutronFixture"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This will always return something when running so properly mock things."},{"line_number":10,"context_line":"This requires some minor modifications to tests."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ibbee7fd11c1aa254e399d302adbae69126e98262"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3fa7e38b_431372b9","line":9,"range":{"start_line":9,"start_character":10,"end_line":9,"end_character":46},"in_reply_to":"3fa7e38b_a3c10631","updated":"2019-10-03 22:22:43.000000000","message":"I reported a docs bug against the API reference since it says that \"security_groups\" is required which it clearly isn\u0027t (even from the samples in this patch):\n\nhttps://bugs.launchpad.net/nova/+bug/1846656","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"}],"nova/tests/fixtures.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"635358e28416eef2a4870f520a9d462b6f74234d","unresolved":false,"context_lines":[{"line_number":1537,"context_line":"            _, context, servers, detailed\u003dFalse):"},{"line_number":1538,"context_line":"        if detailed:"},{"line_number":1539,"context_line":"            raise Exception(\u0027We do not support detailed view\u0027)"},{"line_number":1540,"context_line":"        return {server[\u0027id\u0027]: [{\u0027name\u0027: \u0027default\u0027}] for server in servers}"},{"line_number":1541,"context_line":""},{"line_number":1542,"context_line":"    def _get_first_id_match(self, id, list):"},{"line_number":1543,"context_line":"        filtered_list \u003d [p for p in list if p[\u0027id\u0027] \u003d\u003d id]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_9a50efd6","line":1540,"updated":"2019-10-03 19:11:27.000000000","message":"Based on the comment in the commit message, this would likely be more accurate if we filtered out servers that have an empty network info cache meaning they have no ports attached. Note that we have lots of functional tests that don\u0027t care about networking really so they create a server with networks\u003d\u0027none\u0027.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9ddfb85150a75740e36ee1d5e5046a5749c8c433","unresolved":false,"context_lines":[{"line_number":1537,"context_line":"            _, context, servers, detailed\u003dFalse):"},{"line_number":1538,"context_line":"        if detailed:"},{"line_number":1539,"context_line":"            raise Exception(\u0027We do not support detailed view\u0027)"},{"line_number":1540,"context_line":"        return {server[\u0027id\u0027]: [{\u0027name\u0027: \u0027default\u0027}] for server in servers}"},{"line_number":1541,"context_line":""},{"line_number":1542,"context_line":"    def _get_first_id_match(self, id, list):"},{"line_number":1543,"context_line":"        filtered_list \u003d [p for p in list if p[\u0027id\u0027] \u003d\u003d id]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_06dc9d16","line":1540,"in_reply_to":"3fa7e38b_06a01da5","updated":"2019-10-04 15:23:50.000000000","message":"As noted above, there are several functional tests which explicitly create a server without networks just because they don\u0027t care about networking for the test (speed things up, not deal with the networking fixtures, etc).","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1c76c9ebf9311cb567e475ceeea999ac3d63744d","unresolved":false,"context_lines":[{"line_number":1537,"context_line":"            _, context, servers, detailed\u003dFalse):"},{"line_number":1538,"context_line":"        if detailed:"},{"line_number":1539,"context_line":"            raise Exception(\u0027We do not support detailed view\u0027)"},{"line_number":1540,"context_line":"        return {server[\u0027id\u0027]: [{\u0027name\u0027: \u0027default\u0027}] for server in servers}"},{"line_number":1541,"context_line":""},{"line_number":1542,"context_line":"    def _get_first_id_match(self, id, list):"},{"line_number":1543,"context_line":"        filtered_list \u003d [p for p in list if p[\u0027id\u0027] \u003d\u003d id]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_06a01da5","line":1540,"in_reply_to":"3fa7e38b_9a50efd6","updated":"2019-10-04 15:18:44.000000000","message":"+1. I thik till now we do have network in all POST server request but yes we should take care of this case here.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"238b91b105adf1f4e3a87a5805f654b74c05dfcf","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"                # server is in the down cell."},{"line_number":1262,"context_line":"                self.assertEqual(\u0027UNKNOWN\u0027, server[\u0027status\u0027])"},{"line_number":1263,"context_line":"                self.assertIn(server[\u0027id\u0027], self.down_cell_insts)"},{"line_number":1264,"context_line":"                # the partial construct will have only 6 keys:"},{"line_number":1265,"context_line":"                # created, tenant_id, security_groups, status, id and links."},{"line_number":1266,"context_line":"                self.assertEqual(6, len(server))"},{"line_number":1267,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_1aaf1f95","line":1264,"updated":"2019-10-03 19:21:13.000000000","message":"Hmm, I think this is an unintended side effect but the set of keys you get in a down cell case is actually pretty specific, or supposed to be, it\u0027s in the API guide:\n\nhttps://docs.openstack.org/api-guide/compute/down_cells.html\n\nSo I guess maybe that\u0027s a bug in the docs if you can get that today and it\u0027s just our fixtures that weren\u0027t working properly? Although gmann was asking about this earlier in the week...yeah must be a bug in the down cell docs.\n\nFor GET /servers/detail we start here:\n\nhttps://github.com/openstack/nova/blob/867401e575d2b27b9bc63ceda41cd85233545cd5/nova/api/openstack/compute/views/servers.py#L396\n\ncall show() for each server:\n\nreturn back to detail from here:\n\nhttps://github.com/openstack/nova/blob/867401e575d2b27b9bc63ceda41cd85233545cd5/nova/api/openstack/compute/views/servers.py#L210\n\nand then go on to add the security groups:\n\nhttps://github.com/openstack/nova/blob/867401e575d2b27b9bc63ceda41cd85233545cd5/nova/api/openstack/compute/views/servers.py#L410\n\nAnd because we call neutron to get ports by server id:\n\nhttps://github.com/openstack/nova/blob/867401e575d2b27b9bc63ceda41cd85233545cd5/nova/network/security_group/neutron_driver.py#L317\n\nrather than use the network info cache (which doesn\u0027t contain information about security groups) I guess we do proxy to neutron in the down cell case.\n\nI\u0027ll let Surya know...or open a docs bug, something like that.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"de255c696581ae61b0b1dece5ee71572f11b1fdd","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"                # server is in the down cell."},{"line_number":1262,"context_line":"                self.assertEqual(\u0027UNKNOWN\u0027, server[\u0027status\u0027])"},{"line_number":1263,"context_line":"                self.assertIn(server[\u0027id\u0027], self.down_cell_insts)"},{"line_number":1264,"context_line":"                # the partial construct will have only 6 keys:"},{"line_number":1265,"context_line":"                # created, tenant_id, security_groups, status, id and links."},{"line_number":1266,"context_line":"                self.assertEqual(6, len(server))"},{"line_number":1267,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_dad667c4","line":1264,"in_reply_to":"3fa7e38b_1aaf1f95","updated":"2019-10-03 19:36:47.000000000","message":"+1 on doc update to reflect the actual return value in Detail.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1fe482bc57c8cb7a8ef548ce32e7b569bab8cbbc","unresolved":false,"context_lines":[{"line_number":1261,"context_line":"                # server is in the down cell."},{"line_number":1262,"context_line":"                self.assertEqual(\u0027UNKNOWN\u0027, server[\u0027status\u0027])"},{"line_number":1263,"context_line":"                self.assertIn(server[\u0027id\u0027], self.down_cell_insts)"},{"line_number":1264,"context_line":"                # the partial construct will have only 6 keys:"},{"line_number":1265,"context_line":"                # created, tenant_id, security_groups, status, id and links."},{"line_number":1266,"context_line":"                self.assertEqual(6, len(server))"},{"line_number":1267,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_7a7f33ab","line":1264,"in_reply_to":"3fa7e38b_dad667c4","updated":"2019-10-03 19:42:40.000000000","message":"Bug 1846559 reported for the docs issue.","commit_id":"befa031725fbf50c193cfd97d34e277cedd9f3b5"}]}