Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-26
16:18:27 openstackgerrit Matt Riedemann proposed openstack/nova master: Use the RequestSpec when getting scheduler_hints in compute https://review.openstack.org/515461
16:18:28 mriedem bauzas: here you go ^ i'm helping!
16:22:46 openstackgerrit Matt Riedemann proposed openstack/nova master: Use the RequestSpec when getting scheduler_hints in compute https://review.openstack.org/515461
16:31:37 cdent jaypipes: thanks for the review on the stack leading to POST /allocations. No, nobody suggested I split things up, I did that to try and keep things a bit smaller, led in part by the stack that is adding last-modified to lots of things, all under the same microversion
16:32:16 jaypipes cdent: sorry, I may have missed the last-modified thing... I don't see that anywhere in this patch series?
16:32:20 cfriesen In _post_live_migration() at https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6075 why are we calling self._update_scheduler_instance_info() rather than self._delete_scheduler_instance_info() ?
16:32:39 cdent jaypipes: it’s a different series, but has the same split behavior of docs/reno coming later
16:33:16 jaypipes cdent: ack. I'd like to see all public API-modified changes in a single commit please. even if it leads to large commits.
16:33:22 cfriesen One of our devs thinks that this is causing issues with the server group affinity filter by causing the scheduler to get out-of-date with the compute nodes.
16:33:41 jaypipes cdent: just enables us to keep our "this single commit changes the API and in these ways" behaviour.
16:33:49 jaypipes dansmith: thoughts on ^?
16:34:00 cdent I thought we had relaxed some of that in favor of being able to review things a bit more discreetly?
16:34:17 sean-k-mooney cfriesen: oh hi since your about do you know if the patches to fix live migration with vnic=macvtap were ever merged
16:34:17 jaypipes cdent: I'm not aware of such a relaxation...
16:34:26 dansmith yeah, we can't
16:34:31 jaypipes cdent: but of course, I very easily could be wrong :)
16:34:32 cdent it’s doable of course, but will require a fair bit of reordering on the last-modified stuff
16:34:37 cdent which is fine
16:34:45 dansmith the only way around it is adding dead code in a commit, then turning it on later when you add the microversion
16:34:57 dansmith but we can't add real functionality separate from the microversion itself
16:35:14 cdent I’m not _not_ adding the microversion, just not doccing it
16:35:31 cdent which perhaps should be considered the same thing? /me shrugs
16:35:40 dansmith oh, I thought it was asserted that you were
16:35:45 cfriesen sean-k-mooney: I don't think so....see https://review.openstack.org/#/c/136077/ and https://review.openstack.org/#/c/251387/
16:36:26 cdent dansmith: no I’m starting a microversion and then adding different pieces of functionality or docs that are within that microversion in subsequent patches
16:36:30 cdent so for example
16:36:42 cdent https://review.openstack.org/#/c/495380/
16:36:46 dansmith cdent: that's the same thing as adding the functionality before the microversion,
16:37:00 sean-k-mooney cfriesen: ok cool ill assume its stil broken then. ill try to make sure we can handel it properly in the multiple port binding work im doing
16:37:01 dansmith although with the latter you can keep it dead until the microversion which is what you'd have to do
16:37:22 sean-k-mooney cfriesen: i might not actully fix it but ill make sure not to break it more
16:37:29 cfriesen sean-k-mooney: :)
16:37:35 cdent dansmith: I’m not sure I’m fully parsing “that's the same thing as adding the functionality before the microversion,"
16:37:46 dansmith cdent: opening a microversion in one commit and extending it in another defeats the point of it, in that clients can't depend on the functionality you're adding when the microversion is exposed
16:38:25 dansmith jaypipes: that's what you're trying to say right? ^
16:38:35 jaypipes dansmith: yeah
16:38:37 cdent right, that’s where I guess I was misinformed/misunderstood on some of the relaxation with regard to CD
16:39:00 cdent ain’t no thing, I’ll just fix it
16:39:23 cdent but first I must go home
16:41:01 openstackgerrit Merged openstack/nova stable/pike: Fix AttributeError in BlockDeviceMapping.obj_load_attr https://review.openstack.org/515404
16:41:15 edleafe dansmith: so is it frowned upon to create separate patches to create the base for the version change in digestible chunks, and then turn it on with a single patch?
16:41:47 dansmith edleafe: that's what I was saying above: if it makes real sense to do that, I think it's cool, you just need to make sure it's dead until the microversion gets added
16:41:51 jaypipes edleafe: yes. AFAIK, we've never done/allowed that.
16:42:13 edleafe Heh, I'll leave it to you two to fight it out
16:42:16 dansmith jaypipes: we have definitely done that in the past, I'm quite sure.. as long as it's dead
16:42:45 dansmith jaypipes: cdent was doing the opposite though AIUI, which is definitely not okay
16:43:07 jaypipes edleafe: we've not allowed adding code that adds the microversion in one patch and modifies code that touches that microversion in a later patch.
16:43:15 dansmith right, exactly that
16:43:21 sean-k-mooney dansmith: do you know why we never versioned the base LiveMigrationData object. im fixing that in https://review.openstack.org/#/c/515423/1/nova/objects/migrate_data.py but not sure why it was not done in the first place when changes to the base require version bumps to all the childeren anyway.
16:43:27 jaypipes sorry, I guess I misread edleafe's comment.
16:43:31 edleafe jaypipes: sure, but that's not what I asked
16:43:36 edleafe sorta jinx
16:43:43 dansmith sean-k-mooney: because it's required to be implemented per virt driver, so it's not possible to use the base object
16:43:47 dansmith sean-k-mooney: so please don't "fix" that :)
16:44:03 jaypipes edleafe: ok. so is it clear now then?
16:44:22 sean-k-mooney dansmith: but that breaks the versioning of all child objects if we dont...
16:44:32 dansmith sean-k-mooney: breaks how?
16:44:33 edleafe jaypipes: yes; it seemed earlier you were stating the opposite
16:44:54 dansmith sean-k-mooney: adding something to the base object requires a bump in all the subclasses.. that's intentional, not broken :)
16:44:59 jaypipes edleafe: ok, not sure why/how I was unclear earlier but as long as I'm clear now, ok.
16:45:22 dansmith sean-k-mooney: it's like an abc. just a template for the real implementations and a common superclass parent
16:45:29 dansmith jaypipes: I was confused about what you said initially too, so..
16:45:33 sean-k-mooney dansmith: yes it does but not versioning the base mean we have to copy the make compatible function to all the childern
16:46:23 dansmith sean-k-mooney: I'm not sure what or why you say that
16:46:37 dansmith oh I see what you mean
16:46:57 dansmith sean-k-mooney: then yeah, that's what it means
16:47:15 dansmith the versions are all different, so you have to handle the version each one took that new change in
16:47:19 dansmith registering doesn't help AFAIK
16:48:12 mriedem heh, wtf, this dict is already a primitive :) https://github.com/openstack/nova/blob/d36dcd52c24c32418fd358d245688c86664025d5/nova/scheduler/utils.py#L83
16:48:13 sean-k-mooney ill be off until wednesday. but let me know if you would perfer me to duplicate the removal of the vifs into all the childeren or if the approch in the patch is ok
16:48:48 dansmith sean-k-mooney: if that's what you have to do then yeah
16:49:10 dansmith sean-k-mooney: you can unify a handler for that in the base class, and then just trigger it at the appropriate version for each child object or something
16:49:36 sean-k-mooney that what i basically did in https://review.openstack.org/#/c/515423/1/nova/objects/migrate_data.py
16:50:18 dansmith sean-k-mooney: ack, will comment
16:51:24 sean-k-mooney dansmith: i don't really mind which way its done i just taught this refactor would be cleaner but im happy to do it another way too.
16:51:34 dansmith okay
17:16:39 openstackgerrit Vladyslav Drok proposed openstack/nova master: rbd: flatten images when unshelving https://review.openstack.org/457886
17:39:58 openstackgerrit Ed Leafe proposed openstack/nova master: Add Selection objects https://review.openstack.org/499239
17:39:58 openstackgerrit Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854
17:39:59 openstackgerrit Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/510159
17:39:59 openstackgerrit Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357
17:40:00 openstackgerrit Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358
17:40:29 edleafe mriedem: jaypipes: ^^ implemented SchedulerLimits for Selection
17:43:10 gibi mriedem: I've left my report about the notification subteam on the today's meeting agenda as I cannot participate.
17:43:28 mriedem gibi: ok, thanks
17:47:46 mriedem edleafe: thanks, soft -1 on the new comment that got added
17:47:54 mriedem since i'm working on making that comment partially untrue
17:53:54 jaypipes edleafe: cheers. will review shortly.
17:59:02 edleafe mriedem: well, since Jay wanted it recorded, I thought I'd include it, but I can easily remove it
17:59:13 mriedem jaypipes: i assumed he meant recorded in gerrit
17:59:33 edleafe mriedem: and Ill remove the mixin
18:00:02 edleafe Hmmm... when I'm researching code behavior, Gerrit isn't where I look
18:01:42 mriedem so leave a generic comment about us having to include limits in this thing because they are per-alternative host which isn't the same thing as limits in reqspec and filter_properties
18:01:42 mriedem idk
18:06:36 edleafe I removed it. It's not like adding it there will suddenly unlock the secrets of Nova to future generations of coders :)
18:07:25 mriedem right - i will still have to re-learn limits everytime i need to look at any of this
18:10:42 cdent jaypipes: since you were in that stack recently, you have time to blip in the spec tune up? https://review.openstack.org/#/c/509136/
18:32:09 openstackgerrit Matt Riedemann proposed openstack/nova master: Use the RequestSpec when getting scheduler_hints in compute https://review.openstack.org/515461
18:32:10 openstackgerrit Matt Riedemann proposed openstack/nova master: Pass RequestSpec to ConductorTaskAPI.build_instances https://review.openstack.org/515495
18:39:41 cfriesen repeat of earlier question since nobody answered it before...In _post_live_migration() at https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6075 why are we calling self._update_scheduler_instance_info() rather than self._delete_scheduler_instance_info() ? Seems logical that we would want to tell the scheduler "hey, this instance isn't here anymore".

Earlier   Later