| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-23 | |||
| 13:55:10 | efried | dansmith I took the liberty of adding you to the review ;-) | |
| 13:55:13 | dansmith | mriedem: I just replied to your comments but if you want to chat here or in a hangout now, I'm ready | |
| 13:55:17 | dansmith | efried: heh okay | |
| 13:57:40 | mriedem | dansmith: i've got to take maya to school, will ping when i'm back | |
| 13:57:54 | dansmith | okay | |
| 13:57:58 | efried | mriedem That review you asked me for last night - done. | |
| 14:12:27 | alex_xu | sfinucan: thanks, just +w | |
| 14:12:42 | sfinucan | alex_xu: Ta | |
| 14:20:56 | mriedem | dansmith: in your replies, when you say global context, you mean modifying the single context object that's being passed around everywhere, right? | |
| 14:21:02 | mriedem | it's not an actual 'global' | |
| 14:21:15 | mriedem | target_cell mutates the thing that's passed everywhere | |
| 14:21:15 | dansmith | mriedem: right | |
| 14:21:17 | mriedem | ok | |
| 14:21:18 | dansmith | shared context or something | |
| 14:21:21 | mriedem | right | |
| 14:21:30 | mriedem | "Maybe it's because we targeted the same context that was used in _lookup_instance and so when we target it here, it's modifying instance._context by reference?" | |
| 14:21:33 | mriedem | was my attempt at saying the same | |
| 14:21:41 | dansmith | okay | |
| 14:22:05 | mriedem | dansmith: ok on to https://review.openstack.org/#/c/458634/13/nova/conductor/manager.py@846 | |
| 14:22:13 | mriedem | is that only here because it will be a failure later? | |
| 14:22:17 | mriedem | if not done now? | |
| 14:23:16 | dansmith | mriedem: yeah, but that's the same for the rest of the patch | |
| 14:23:27 | dansmith | it's all calling convention changes so I can change the behavior of the target function and have things continue to work | |
| 14:23:40 | dansmith | I could put any of this in the next patch, but I'm just trying to separate things | |
| 14:24:03 | mriedem | sure, but the difference is the rest of the patch is already using target_cell | |
| 14:24:10 | mriedem | that's why everyone is thrown off on that particular change | |
| 14:24:23 | acormier | Quick ?: Is it possible to configure a subnet of compute nodes in Openstack to use Ceph RBD backend? Or do all nodes have to move over in wholesale? | |
| 14:24:33 | mriedem | subset | |
| 14:24:39 | mriedem | acormier: answer is yes | |
| 14:25:07 | dansmith | mriedem: okay I guess it seems like exactly the same to me | |
| 14:25:18 | dansmith | mriedem: but if you really want it broken out then I will | |
| 14:26:06 | mriedem | i'd prefer it lives in the patch that it's going to be needed otherwise it fails | |
| 14:27:00 | mriedem | dansmith: last thing, so you do plan on changing https://review.openstack.org/#/c/458634/13/nova/objects/host_mapping.py to use the yielded cctxt? | |
| 14:27:04 | mriedem | when creating the host mappings i mean, | |
| 14:27:08 | mriedem | even though the api db connection is the same | |
| 14:27:15 | dansmith | I answered that one didn't I? | |
| 14:27:31 | mriedem | you replied, | |
| 14:27:36 | mriedem | but i'm not sure it answered my question | |
| 14:27:54 | dansmith | okay, yes I've already got it changed locally | |
| 14:27:59 | mriedem | ok | |
| 14:28:08 | mriedem | that wasn't obvious if the patch wasn't WIPed or something | |
| 14:28:19 | mriedem | anywho, that's it | |
| 14:28:34 | mriedem | dansmith: can we still be friends? | |
| 14:29:06 | dansmith | mriedem: so, moving those conductor changes into the patch that changes context.py will make that match go from "one functional change and the associated test changes" to "one real code change, an ancillary code change and several test changes" | |
| 14:29:18 | dansmith | can I just move it into another patch and explain in the commit message or something? | |
| 14:29:23 | mriedem | sure | |
| 14:29:25 | dansmith | I really want that context switch patch to be really tight | |
| 14:29:54 | dansmith | okay, I totally don't understand why that's different than having it in the first patch, but I'll do it anyway :) | |
| 14:29:54 | mriedem | or just leave it and point it out in the commit message in this one | |
| 14:30:03 | mriedem | the fact 3 people had to ask about it, | |
| 14:30:05 | mriedem | means it's not clear | |
| 14:30:13 | mriedem | so just point it out in the commit message | |
| 14:30:18 | dansmith | if that's good enough then I'd like that option | |
| 14:30:25 | mriedem | fine by me | |
| 14:30:43 | mriedem | melwitt: it's 7:30am, i assume you're 2 hours into addressing https://review.openstack.org/#/c/446239/ | |
| 14:30:57 | mriedem | because i want to +2 that one | |
| 14:41:21 | mriedem | alex_xu: replied in https://review.openstack.org/#/c/459569/ | |
| 14:42:48 | mriedem | alex_xu: Kevin_Zheng: jaypipes: why is this a 403 and not a 409? https://review.openstack.org/#/c/464548/5/nova/api/openstack/compute/servers.py | |
| 14:42:58 | mriedem | i thought we normally handled over quota as 409 in the api | |
| 14:43:46 | openstackgerrit | Daniel Pawlik proposed openstack/nova master: Add `hide_hypervisor_id` image property https://review.openstack.org/459753 | |
| 14:44:05 | mriedem | nevermind, i guess at some point those were switched to 403 | |
| 14:44:30 | melwitt | mriedem: yeah, I got most of it done yesterday and I'm stuck on a unit test of all things. hope I'll get past that soon | |
| 14:44:55 | Kevin_Zheng | mriedem: I did a search and I saw it was handled in that way somewhere else | |
| 14:45:05 | mriedem | Kevin_Zheng: yeah me too, i didn't realize that was changed | |
| 14:45:56 | Kevin_Zheng | Btw, I tired to cherry pick it to N but seems conflicting | |
| 14:46:17 | mriedem | then you have to do it manually using git cherry-pick -x | |
| 14:46:35 | mriedem | resolve the merge conflict locally, and leave a note in the commit message about what the conflict was | |
| 14:47:36 | mriedem | lyarwood: when you have a minute i'd like to get this released in ocata since it's an upgrade issue https://review.openstack.org/#/c/466774/ | |
| 14:48:01 | lyarwood | mriedem: looking | |
| 14:48:25 | lyarwood | mriedem: while we are talking about stable/ocata, should we drop Citrix XenServer CI to non-voting until they get it passing? | |
| 14:48:54 | Kevin_Zheng | mriedem: ok I will try tomorrow | |
| 14:49:10 | mriedem | lyarwood: did that last night, emailed the team and bobball is already aware and disabled the failing tests until fixed | |
| 14:49:27 | lyarwood | mriedem: cool, thanks :) | |
| 14:52:56 | fishbone_ | Hello all, I am having a strange issue...After an upgrade from newton to ocata all of my old virtual servers are innaccessible | |
| 14:53:30 | mriedem | lyarwood: the depends-on here seems wrong https://review.openstack.org/#/c/466480/1 | |
| 14:53:40 | fishbone_ | I am able to see them in the database and when runnin openstack server list, however if i run openstack server show ID it will say the server does not exist | |
| 14:54:21 | openstackgerrit | Daniel Pawlik proposed openstack/nova master: Add `hide_hypervisor_id` image property https://review.openstack.org/459753 | |
| 14:54:47 | lyarwood | mriedem: yeah that's wrong, gerritui just copied it across for the original commit, I'll remove it now | |
| 14:55:00 | mriedem | fishbone_: did you run nova-manage cell_v2 map_instances as part of the upgrade? | |
| 14:56:06 | mriedem | fishbone_: check to see if you have instance_mappings in the nova_api db | |
| 14:56:09 | fishbone_ | yes I did, I wonder if the fact that I had servers before doing that is the cause of the issue | |
| 14:56:21 | fishbone_ | Any new servers I create register correctly | |
| 14:56:30 | fishbone_ | oh wait | |
| 14:56:42 | fishbone_ | mriedem: I dont think I ran the one for instances | |
| 14:56:46 | mriedem | map_instances is specifically for mapping existing instances to a cell | |
| 14:56:59 | mriedem | https://docs.openstack.org/developer/nova/man/nova-manage.html#nova-cells-v2 | |
| 14:57:39 | mriedem | fishbone_: note that nova-manage cell_v2 simple_cell_setup will also do map_instances automatically | |
| 14:58:29 | fishbone_ | mriedem: I did not use that command when I performed the upgrade in this case | |
| 14:58:40 | fishbone_ | mriedem: I will try to run that command now | |
| 14:58:50 | mriedem | ok, if you don't use simple_cell_setup, then you need to run map_instances | |
| 14:59:23 | mriedem | from the release note too http://git.openstack.org/cgit/openstack/nova/tree/releasenotes/notes/cells-discover-hosts-06a3079ba687e092.yaml | |
| 15:00:05 | openstackgerrit | Eric Berglund proposed openstack/nova master: Add strict option to discover_hosts https://review.openstack.org/467218 | |
| 15:10:52 | mriedem | melwitt: so can we abandon this? https://review.openstack.org/#/c/462744/ | |
| 15:10:58 | fishbone_ | mriedem: Thanks a lot, that worked, I didn't use the simple cell setup because when I did it wasn't naming the cell appropriately | |
| 15:11:32 | mriedem | fishbone_: yup, which i'm resurrecting in newton https://review.openstack.org/#/c/420461/ | |
| 15:11:37 | mriedem | but that doesn't help you here | |
| 15:11:53 | mriedem | fishbone_: i'm going to start a faq in the nova devref for cells v2 things, i'll put your case in there | |
| 15:12:32 | jaypipes | sdague, mriedem: can you point to docs on how to update the global requirements properly for a new dependency (in this case, os-vif and os-traits) | |
| 15:12:46 | melwitt | mriedem: yeah. I'm disturbed by the fact that the message seems to vanish when not converted to unicode (since that will happen to other people in the wild). I was trying to decide if I wanted to recheck it again to see if I could locate the unconverted messages in the logs | |
| 15:12:47 | jaypipes | sdague, mriedem: reference this patch https://review.openstack.org/#/c/462769/ | |