)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5e32defe833d549ebba96383db3d00632e6b0aed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3c3e5f00_93a1ab16","updated":"2022-03-21 00:29:40.000000000","message":"See inline, I think the fact we might be regressing on the RAM calculations should get us to probe a bit deeper on what is really going on here. ","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"424862f11b78c7f5938ee6e1f86fd77de8b2a27a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"98387fe6_49a4c492","updated":"2022-03-18 12:47:21.000000000","message":"Thx for the fix, I reindented the comment after writing it, which obviously was a bad idea.","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"ec6f803fcaa21d4d971f55a4e17b405c7024831b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c798d5ac_39d6cab6","updated":"2022-04-26 19:03:33.000000000","message":"hrm, same error in boto3?\n\n File \"/home/zuul/src/opendev.org/zuul/nodepool/.tox/py38/lib/python3.8/site-packages/boto3/resources/factory.py\", line 386, in property_loader\n    return self.meta.data.get(name)\n AttributeError: \u0027NoneType\u0027 object has no attribute \u0027get\u0027\n","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"ad72aa32aa0033f335c790528edae9be5f824a00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dcec9cd2_8ebd3c4e","updated":"2022-04-28 03:35:17.000000000","message":"recheck","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"b846a53d1e628dec35e83d16cf532c95f87d767c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ab235e59_3987959d","updated":"2022-04-26 14:08:49.000000000","message":"recheck aws failures, seems unstable testing","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"}],"nodepool/driver/openstack/provider.py":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5e32defe833d549ebba96383db3d00632e6b0aed","unresolved":true,"context_lines":[{"line_number":139,"context_line":"            # In later versions it returns the information we\u0027re looking for."},{"line_number":140,"context_line":"            # If we get the information we want, we do not need to try to"},{"line_number":141,"context_line":"            # lookup the flavor in our list."},{"line_number":142,"context_line":"            if hasattr(server.flavor, \u0027vcpus\u0027):"},{"line_number":143,"context_line":"                flavor \u003d server.flavor"},{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                flavor \u003d flavors.get(server.flavor.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"8ce31579_055d8ced","line":142,"updated":"2022-03-21 00:29:40.000000000","message":"If feel like this doesn\u0027t quite capture what\u0027s going on with the \"vcpus\" probe here.\n\nIt seems that prior to the recent changes, we would get back a flavor object, but that could have \"0\" vcpu\u0027s listed, per [1]:\n\n\"ah, I guess I know now what is happening: vcpus is not set in the tested flavor and as such param is empty. Previously as part of normalization it was set to 0 in such case. This is a tricky one\"\n\nNow, we get back a flavor that doesn\u0027t have a vcpus field if the flavor doesn\u0027t set it.  This raises a few questions:\n\n* The number of VCPU\u0027s doesn\u0027t seem like an optional component of the flavor?\n* How are you supposed to get how many VCPUs a server instance has then?  Is there something else we should use here?\n* If it\u0027s not there, should we assume \"1\"?  This doesn\u0027t seem documented in openstacksdk [2]\n\n[1] https://review.opendev.org/c/openstack/openstacksdk/+/826543\n[2] https://docs.openstack.org/openstacksdk/latest/user/resources/compute/v2/flavor.html#openstack.compute.v2.flavor.Flavor","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"b2b4d16e720bd417f93b59257eb6ff82345b4ad1","unresolved":true,"context_lines":[{"line_number":139,"context_line":"            # In later versions it returns the information we\u0027re looking for."},{"line_number":140,"context_line":"            # If we get the information we want, we do not need to try to"},{"line_number":141,"context_line":"            # lookup the flavor in our list."},{"line_number":142,"context_line":"            if hasattr(server.flavor, \u0027vcpus\u0027):"},{"line_number":143,"context_line":"                flavor \u003d server.flavor"},{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                flavor \u003d flavors.get(server.flavor.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ae403955_e2517edb","line":142,"in_reply_to":"58cfb854_2009f458","updated":"2022-03-22 23:35:05.000000000","message":"Thanks for the details.  \n\nSo in short, the flavor can never not be filled in for an active server with the more recent sdks.  It\u0027s just the \"id\" field changed with sdk 1.0, confusing our check.","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"44573a88e913ed644b5d64157c4a5fdc2c0b59fc","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            # In later versions it returns the information we\u0027re looking for."},{"line_number":140,"context_line":"            # If we get the information we want, we do not need to try to"},{"line_number":141,"context_line":"            # lookup the flavor in our list."},{"line_number":142,"context_line":"            if hasattr(server.flavor, \u0027vcpus\u0027):"},{"line_number":143,"context_line":"                flavor \u003d server.flavor"},{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                flavor \u003d flavors.get(server.flavor.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"58cfb854_2009f458","line":142,"in_reply_to":"8ce31579_055d8ced","updated":"2022-03-21 05:51:55.000000000","message":"The comment in [1] is wrong IMO. Whether we check for \"vcpus\" or \"ram\" or \"swap\" or whatever doesn\u0027t matter, either all of them are there or none. The cases that can happen are:\n\na) Very old sdk or very old nova (not supporting microversion 2.47). In this case \"server.flavor\" will be a dict with only .id and .links and we need to consult our \"flavors\" list in order to get details about the flavor.\n\nb) not-too-old sdk and nova: \"server.flavor\" is a dict containing .vcpus, .ram, etc. and we can simply use these values [3].\n\nc) sdk 1.0: the result from b) will be returned as a Flavor object [2], which synthesizes an .id field from .original_name\n\nThe old code tries to distinguish a) and b) by checking for the presence of .id, which is missing in b). But it mishandles c), trying to use the flavors list with the id seens from the Flavor object, which fails and returns None because \"flavors\" is indexed by the flavor (UU)ID only and not by (original_)name. Also, servers may still be running that were created with a flavor that has since been deleted, so we couldn\u0027t find it in our \"flavors\" list even if we were checking by name instead of id.\nSo by checking for .vcpus we simply bundle b) and c) together, since in both cases we will have both vcpus and ram present in the flavor data.\n\n[3] https://docs.openstack.org/api-ref/compute/?expanded\u003dshow-server-details-detail#show-server-details","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"3698c9dce42318a6618aab44572d4127645d4ae5","unresolved":true,"context_lines":[{"line_number":139,"context_line":"            # In later versions it returns the information we\u0027re looking for."},{"line_number":140,"context_line":"            # If we get the information we want, we do not need to try to"},{"line_number":141,"context_line":"            # lookup the flavor in our list."},{"line_number":142,"context_line":"            if hasattr(server.flavor, \u0027vcpus\u0027):"},{"line_number":143,"context_line":"                flavor \u003d server.flavor"},{"line_number":144,"context_line":"            else:"},{"line_number":145,"context_line":"                flavor \u003d flavors.get(server.flavor.id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7edda257_654cd8f6","line":142,"in_reply_to":"ae403955_e2517edb","updated":"2022-03-23 10:50:56.000000000","message":"At least with recent enough nova, i.e. \u003e\u003d pike, yes. I\u0027m not really sure what sdk 1.0 will produce when running against even older clouds, but I also have no way of testing that and I\u0027m not sure if anyone would still care.\nhttps://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id43\n\nWhat\u0027s really confusing is IMO not the pure existence of the id field, but that it has different content in the server.flavor context compared to the bare flavor context. In essence, these are two similar but not identical objects, so it is questionable for the sdk to use the same object type for both.","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"5e32defe833d549ebba96383db3d00632e6b0aed","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                # If we still haven\u0027t found the flavor, skip handling this"},{"line_number":147,"context_line":"                # server instead of failing completely"},{"line_number":148,"context_line":"                if not flavor:"},{"line_number":149,"context_line":"                    continue"},{"line_number":150,"context_line":"            used_quota.add(QuotaInformation.construct_from_flavor(flavor))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        return used_quota"}],"source_content_type":"text/x-python","patch_set":2,"id":"932a87df_14dc32e2","line":149,"updated":"2022-03-21 00:29:40.000000000","message":"In the old case (where vcpu\u0027s would apparently be \"0\" in some cases?), construct_from_flavor() would still account for the RAM.  I am presuming at it could/was/is set, separate to vcpus?  \n\nIt seems now we\u0027re going to skip that too -- which seems wrong.\n\nI\u0027m not sure what to suggest without knowing answers to the above -- how do we really handle a missing \"vcpu\" field?","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"9c18faf21810336db6c069a668c7aa5d3b4d5816","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                # If we still haven\u0027t found the flavor, skip handling this"},{"line_number":147,"context_line":"                # server instead of failing completely"},{"line_number":148,"context_line":"                if not flavor:"},{"line_number":149,"context_line":"                    continue"},{"line_number":150,"context_line":"            used_quota.add(QuotaInformation.construct_from_flavor(flavor))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        return used_quota"}],"source_content_type":"text/x-python","patch_set":2,"id":"b27b8a88_1fc18758","line":149,"in_reply_to":"61436545_aaf4a973","updated":"2022-04-26 05:14:05.000000000","message":"I\u0027m sorry for not getting back to this.  After coming back to this, and re-reading the code again, it seems fine.  We\u0027ll likely get the info (new sdk) or if we don\u0027t (old sdk, unlikely anyway) we\u0027ll look it up and then, in the even more unlikley case, skip over it.  I think this is fine, thanks","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"3698c9dce42318a6618aab44572d4127645d4ae5","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                # If we still haven\u0027t found the flavor, skip handling this"},{"line_number":147,"context_line":"                # server instead of failing completely"},{"line_number":148,"context_line":"                if not flavor:"},{"line_number":149,"context_line":"                    continue"},{"line_number":150,"context_line":"            used_quota.add(QuotaInformation.construct_from_flavor(flavor))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        return used_quota"}],"source_content_type":"text/x-python","patch_set":2,"id":"61436545_aaf4a973","line":149,"in_reply_to":"8ba48e7c_a9625ea7","updated":"2022-03-23 10:50:56.000000000","message":"I\u0027m not sure what your intention is with this comment, do you want to change the code or only the comment? Feel free to submit a new PS with your idea, that might make discussing it easier.","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"44573a88e913ed644b5d64157c4a5fdc2c0b59fc","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                # If we still haven\u0027t found the flavor, skip handling this"},{"line_number":147,"context_line":"                # server instead of failing completely"},{"line_number":148,"context_line":"                if not flavor:"},{"line_number":149,"context_line":"                    continue"},{"line_number":150,"context_line":"            used_quota.add(QuotaInformation.construct_from_flavor(flavor))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        return used_quota"}],"source_content_type":"text/x-python","patch_set":2,"id":"a0821464_40b12ffe","line":149,"in_reply_to":"932a87df_14dc32e2","updated":"2022-03-21 05:51:55.000000000","message":"Maybe I should make this a different patch then or drop it completely, but this is only going to happen in the case a) above and only in the case that the original flavor has been deleted. The old code would then simply fail, but my idea was that calculating a lower bound for the used quota, skipping servers that we don\u0027t have data for, would be still better than not having any quota information at all.","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"b2b4d16e720bd417f93b59257eb6ff82345b4ad1","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                # If we still haven\u0027t found the flavor, skip handling this"},{"line_number":147,"context_line":"                # server instead of failing completely"},{"line_number":148,"context_line":"                if not flavor:"},{"line_number":149,"context_line":"                    continue"},{"line_number":150,"context_line":"            used_quota.add(QuotaInformation.construct_from_flavor(flavor))"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"        return used_quota"}],"source_content_type":"text/x-python","patch_set":2,"id":"8ba48e7c_a9625ea7","line":149,"in_reply_to":"a0821464_40b12ffe","updated":"2022-03-22 23:35:05.000000000","message":"I think then this would be made clear if we did the continue in the branch where we are looking up the flavor, then?  something ~ like\n\n flavor \u003d flavors.get(server.flavor.id)\n # ... only here with old sdk because all modern sdk\u0027s have returned\n # flavor info directly with the server info call ...\n # ... possible the flavor this server is running is has been deleted\n # and isn\u0027t in our cache, therefore skip if we don\u0027t find ...\n continue\n\n?\n\nI agree that execution of this code path is not particularly important.  More interested in making it very clear for future code explorers what the thinking was without having to go through this again! 😊","commit_id":"ac3dc8d9fe6fde971c8a4442a4bf6272392c4464"}]}