| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-05 | |||
| 13:32:56 | openstackgerrit | Merged openstack/nova master: Fix TypeError in nova-manage db archive_deleted_rows https://review.openstack.org/520765 | |
| 13:38:24 | alex_xu | cdent: so "tree=True&resources=...." means that give me the tree which match those resources? | |
| 13:38:51 | stephenfin | cdent: I think it best to ask mriedem that | |
| 13:39:54 | alex_xu | jaypipes: thanks | |
| 13:39:56 | cdent | alex_xu: sort of: for the resource providers that satisfy the resources request, return those resource providers and their trees | |
| 13:40:25 | jaypipes | cdent: what do you mean by "and their trees"? | |
| 13:41:00 | cdent | jaypipes: if rp X has a parent of Y and a sibling of Z, all of X Y and Z would return | |
| 13:41:10 | cdent | (where X satisfies resources) | |
| 13:41:35 | jaypipes | cdent: the problem with that is due to GET /resource_providers not returning information about inventory or allocations, you would have no idea *which* of the resource providers met the resources query. | |
| 13:42:17 | jaypipes | cdent: for GET /allocation_candidates, we will return the provider information (in the provider summaries section) for all providers in the tree of resource providers included in allocation requests. | |
| 13:42:34 | cdent | yeah, I know. It’s how tree “feels” as a param though | |
| 13:42:54 | cdent | the meaning you’ve made is “limit to tree”, yes? | |
| 13:42:59 | jaypipes | cdent: do you prefer "in_tree" then? or something else entirely? | |
| 13:43:21 | cdent | I can’t decide. I’m trying to talk out loud to see if anything reasonable leaks out | |
| 13:43:26 | jaypipes | cdent: the meaning of "tree=X" is "get the root provider UUID of X and return all providers in that tree" | |
| 13:43:56 | jaypipes | cdent: so yeah, "limit to this provider's tree" is what the filter says. | |
| 13:44:16 | jaypipes | cdent: just want to be clear that "X" doesn't need to be the root provider UUID. | |
| 13:44:34 | jaypipes | cdent: it can be any old resource provider UUID. we look up that provider's root UUID. | |
| 13:44:54 | cdent | and if some other parameter (like resources) is present, and X isn’t in the resource satisfyng rps, no resource, right? | |
| 13:45:05 | cdent | s/no resource/no results/ | |
| 13:45:05 | jaypipes | correct | |
| 13:45:09 | efried_cya_wed | I was thinking ?tree=X&resources=Y would mean, "find me only the providers from within tree X that have resources Y" | |
| 13:45:19 | cdent | efried_cya_wed: it is not wed, go away | |
| 13:45:23 | efried_cya_wed | I.e. explicitly *not* the whole tree. | |
| 13:45:24 | jaypipes | efried_cya_wed: that is precisely what it means. | |
| 13:45:46 | jaypipes | efried_cya_wed: filters are "ANDed" together... | |
| 13:46:12 | cdent | I reckon in_tree is better | |
| 13:46:15 | cdent | but not hugely so | |
| 13:46:54 | alex_xu | cdent: X isn't in the resource satisfying rps, there may have result, for the case, the other rps match the resource in the tree | |
| 13:48:10 | jaypipes | alex_xu: yes, that's true. if X is a grandchild and Y is a child, and Y has all the resources needed, then Y would be returned, yes. | |
| 13:48:45 | alex_xu | jaypipes: yea | |
| 13:50:00 | jaypipes | alex_xu: we could call the filter 木 :) | |
| 13:50:12 | cdent | so a) in_tree is beginning to sound better to me, b) what’s the use case? when does a client want to do this? | |
| 13:51:31 | jaypipes | cdent: this is primarily going to be called by the scheduler report client's get_providers_in_tree() method which will populate a ProviderTree structure that is passed to the virt driver to populate | |
| 13:51:45 | alex_xu | jaypipes: you mean tree? it should be 树，木 is wood :) | |
| 13:51:59 | cdent | jaypipes: so in that case only the tree param is used, yes? | |
| 13:52:12 | jaypipes | alex_xu: crap! there's like 15 symbols that are "tree" in Google translate ;) | |
| 13:52:18 | alex_xu | haha | |
| 13:53:15 | jaypipes | cdent: yeah, in that case, only the tree filter is used | |
| 13:53:40 | alex_xu | the chinese version 'GET /资源_提供者?树=...&资源=...' | |
| 13:59:23 | alex_xu | jaypipes: I guess cdent is asking the use case of tree+resources | |
| 13:59:53 | cdent | not really. I agree that if we have filters, they should all be allowed and all should be and-ed | |
| 13:59:59 | cdent | I also agree that some combinations are weird | |
| 14:00:11 | cdent | but as long as we are and-ing correctly it is okay | |
| 14:00:28 | cdent | I don’t want us to be saying that some filter combinations are disallowed | |
| 14:00:54 | efried_cya_wed | ++ | |
| 14:01:21 | efried_cya_wed | Realistically, there are combinations consumers won't use because they don't make any sense. And that should be fine. | |
| 14:02:00 | alex_xu | cdent: +1 | |
| 14:17:03 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: [placement] Fix foreign key constraint error https://review.openstack.org/525620 | |
| 14:20:48 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 14:20:49 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: libvirt: Allow multiple volume attachments https://review.openstack.org/267587 | |
| 14:20:49 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Add a new check to volume attach https://review.openstack.org/525622 | |
| 14:21:54 | ildikov | mriedem: first attempt ^^ | |
| 14:22:36 | mriedem | ack | |
| 14:22:51 | ildikov | mriedem: I might've bumped the service version too early... :/ | |
| 14:23:53 | mriedem | yup | |
| 14:23:55 | mriedem | https://review.openstack.org/#/c/525622/1/nova/objects/service.py shouldn't be in there | |
| 14:24:15 | ildikov | yeah, I realized 5 minutes ago... | |
| 14:24:18 | mriedem | what is this? https://review.openstack.org/#/c/525622/1/nova/volume/cinder.py | |
| 14:24:36 | ildikov | checking the cinder microversion | |
| 14:24:56 | ildikov | or well, making it possible to do so | |
| 14:25:14 | ildikov | independently from the attachment_* calls | |
| 14:25:26 | mriedem | i don't think we want/need that in this patch, | |
| 14:25:37 | mriedem | what i was thinking was in the change that introduces _check_volume_already_attached_to_instance, | |
| 14:25:46 | mriedem | we'd just implement the usage of that in swap_volume, | |
| 14:25:56 | mriedem | because in swap volume we know if we're doing old or new flow based on the bdm.attachment_id, | |
| 14:25:59 | mriedem | in attach_volume we don't | |
| 14:26:10 | mriedem | so we need to leave the attach_volume changes for the last patch that adds the new flow | |
| 14:27:24 | ildikov | ah, ok | |
| 14:27:46 | ildikov | so you wanted to split out like 10 lines of code change? | |
| 14:28:07 | mriedem | it's more than that | |
| 14:28:17 | mriedem | it's the new method, plus the usage in swap_volume, plus tests | |
| 14:28:18 | ildikov | ok, 20 | |
| 14:28:51 | mriedem | ok - just leave it all in a 2K LOC change and we won't merge any of it if that's what you want | |
| 14:29:09 | ildikov | Jesus, Mary, St Joseph and the camel | |
| 14:29:19 | ildikov | sigh, no, I'll go and start over | |
| 14:29:52 | mriedem | i'm trying to help you split the things out that can be split out to make the main end patch more manageable for reviewers, | |
| 14:29:58 | mriedem | if we don't want to do that, then i'll give up | |
| 14:31:16 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Fix ValueError if invalid max_rows passed to db purge https://review.openstack.org/525628 | |
| 14:31:16 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Fix TypeError in nova-manage db archive_deleted_rows https://review.openstack.org/525629 | |
| 14:31:46 | ildikov | no, don't, I'm sorry, flying freaks me out, so I'm a few levels up regarding stress right now | |
| 14:32:56 | ildikov | and I've just uploaded the 167th revision which gives my stomach an extra bump... :/ :) | |
| 14:33:07 | ildikov | will ping you when I have a next version | |
| 14:33:25 | mriedem | ok | |
| 14:38:56 | sambetts | jaypipes: Is there a bug in Nova tracking the placement race condition we identified at the PTG between nova releasing the allocation and the ironic virt driver setting the number of resources available to zero? | |
| 14:40:06 | jaypipes | sambetts: not sure I follow you... | |
| 14:42:43 | sambetts | jaypipes: the race condition where on "nova delete" of an instance the allocation in placement is released so the node becomes free again, but its not actually free because Ironic is cleaning the node, so we set the resources to zero but for a brief period of time the node in placement can be reallocated because the resources are updated in a timed loop | |
| 14:44:34 | jaypipes | sambetts: but the node is not "available" according to the Ironic virt driver when it's being cleaned and therefore will not appear to the scheduler as passing the compute filter. | |
| 14:46:10 | sambetts | jaypipes: its only not avaiable because we set the avaiable resources for that node to zero, but the avaiable resources isn't refreshed instantly after an instance is deleted, but the allocation in placement is freed | |
| 14:46:45 | sambetts | so the node in placement can be reallocated until the resource tracker updates the avaiable resources | |
| 14:47:25 | jaypipes | sambetts: the allocation is deleted properly (the instance is no longer consuming resources on that node). it is the node itself that is marked as not available and therefore won't be scheduled to. | |
| 14:49:35 | sambetts | jaypipes: I'm not sure what you mean the node is marked as not avaiable, as far as I'm aware that happens by setting the resources to zero and then that causes the code to remove it from placement so it can't be scheduled too, is that what you are refering too | |
| 14:49:45 | sambetts | ? | |
| 14:49:47 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Add regression test for rebuilding a volume-backed server https://review.openstack.org/525632 | |
| 14:49:48 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Only query BDMs once in API during rebuild https://review.openstack.org/525633 | |
| 14:49:48 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Get original image_id from volume for volume-backed instance rebuild https://review.openstack.org/525634 | |
| 14:51:19 | jaypipes | sambetts: I'm referring to the scheduler, not placement. the scheduler checks to see whether a compute node/service is up and available to take requests before it attempts to schedule an instance to it. that check will return False for an Ironic node that is cleaning state. | |
| 14:51:23 | openstackgerrit | Merged openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/525399 | |
| 14:52:25 | sambetts | jaypipes: is that new? I've never heard of that before | |
| 14:52:54 | jaypipes | sambetts: so placement might return that Ironic node to the scheduler as having space (now that the allocation was deleted), but the scheduler won't pick it until the node is available. | |