| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-13 | |||
| 17:45:03 | kashyap | superdan: Ah, noted. | |
| 17:45:09 | superdan | kashyap: I spent an epic hour or so with sahid getting it all figured out myself | |
| 17:45:45 | kashyap | lyarwood: or any "Stable People": Want to put this out of its misery: https://review.openstack.org/#/c/561168/ | |
| 17:47:35 | kashyap | superdan: FWIW, so many TODOs in already one patch: https://review.openstack.org/#/c/484997/16/nova/virt/libvirt/vif.py? | |
| 17:47:45 | kashyap | A bit moderation would be nice | |
| 17:48:16 | superdan | kashyap: yeah, he's going to move some of the stuff out to another patch, which will pull some TODOs with it | |
| 17:48:27 | superdan | kashyap: they're not TODOs about the patch itself, but some stupid stuff that was in the file before this | |
| 17:48:42 | superdan | like "I noticed this was already wrong when I was in here" | |
| 17:48:44 | kashyap | Ah, I see. | |
| 17:48:49 | superdan | which should be in another patch, | |
| 17:48:57 | superdan | especially because it took me days to suss that out :) | |
| 17:48:58 | kashyap | Right. "One logical change per patch" | |
| 17:49:40 | jgwentworth | jackie-truong: is this the tempest test where we can see the cert validation working end-to-end? https://review.openstack.org/#/c/526485 | |
| 17:50:04 | superdan | jgwentworth: it's linked in the bottom patch | |
| 17:50:08 | superdan | I don't think that's it | |
| 17:50:26 | superdan | jgwentworth: https://review.openstack.org/#/c/515210/ | |
| 17:50:31 | jgwentworth | linked in the comments? cause I don't see anything in the commit message | |
| 17:50:36 | superdan | yeah comments | |
| 17:50:47 | jgwentworth | k, thanks | |
| 17:50:57 | kashyap | superdan: Also, it's never a "mess" (as you put it there) to do connsciousness stream write-ups on reviews are perfectly "legal", as you know :P | |
| 17:51:16 | superdan | I just rechecked it a bit ago after pushing up the split, but it wasn't a change, so the last run of that should be fine for validation | |
| 17:51:19 | jgwentworth | oh, k. I did see that patch but missed that it's tempest via pluginness | |
| 17:51:23 | superdan | kashyap: it's a total mess | |
| 17:51:53 | kashyap | If you say so... | |
| 17:52:50 | cfriesen | do we do anything to prevent a port deletion of a port that is connected to an instance? or should I be asking the neutron folks? | |
| 17:53:26 | kashyap | Unrelated: Are Gerrit review comments look-up-able? As in: I know that dansmith said $unique-phrase on a change, putting that phrase in double quotes in Google will yield the right URL? | |
| 17:53:54 | superdan | what did I say? | |
| 17:53:59 | jgwentworth | yes they are. can't remember how off the top of my head | |
| 17:54:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add config option to block image-backed servers with 0 root disk flavor https://review.openstack.org/561284 | |
| 17:54:13 | mriedem | jgwentworth: mnaser: ^ | |
| 17:54:32 | jgwentworth | kashyap: https://review.openstack.org/Documentation/user-search.html#comment | |
| 17:54:33 | kashyap | superdan: Heh, not what you said, but I just took you as an example. Any unique phrase that I know of | |
| 17:54:41 | mriedem | 95% of that patch is test and docs | |
| 17:55:04 | jgwentworth | mriedem: whew, thanks | |
| 17:55:30 | kashyap | jgwentworth: Thanks. So that is searching in review.openstack.org itself. | |
| 17:55:56 | arvindn05 | mriedem: morning :) | |
| 17:56:02 | jgwentworth | kashyap: yeah, like in the serach box you do comment:"unique phrase" | |
| 17:56:08 | jgwentworth | *search | |
| 17:56:11 | kashyap | Thanks. That's useful | |
| 17:56:22 | arvindn05 | mriedem: looks like bauza does not like special case logic in scheduler :( | |
| 17:57:35 | openstack | Launchpad bug 1750623 in OpenStack Compute (nova) queens "rebuild to same host with different image shouldn't check with placement" [Medium,In progress] - Assigned to Hongbin Lu (hongbin.lu) | |
| 17:57:35 | arvindn05 | mriedem: i did mention to him that we plan to have the logic in the scheduler already as part of the bug fix for https://bugs.launchpad.net/nova/+bug/1750623 | |
| 18:00:19 | arvindn05 | bauzas: can we discuss your comments on https://review.openstack.org/#/c/560718/ - Handle rebuild of instance with new image? | |
| 18:03:20 | mriedem | arvindn05: bauzas is in france so probably done for the day (it's 8pm there) | |
| 18:03:47 | mriedem | jgwentworth: mnaser: as soon as i pushed that up for review and stepped away for a minute, i thought that is probably better as a policy rule rather than a config option... | |
| 18:04:05 | mnaser | thats an interesting approach | |
| 18:04:20 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Parse forbidden traits in query strings https://review.openstack.org/556819 | |
| 18:04:20 | mriedem | because if you don't allow users to upload images, then you don't need the warning spam in the nova-api logs, | |
| 18:04:22 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Filter resource providers by forbidden traits in db https://review.openstack.org/556472 | |
| 18:04:23 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Filter allocation candidates by forbidden traits in db https://review.openstack.org/556660 | |
| 18:04:24 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Support forbidden traits in API https://review.openstack.org/556820 | |
| 18:04:25 | jgwentworth | I could see that | |
| 18:04:38 | mriedem | and if you want to play with this feature but only for admins, then you can do that with policy but the config option is either on or off | |
| 18:05:20 | jgwentworth | ah, for the dev/test scenario. yeah | |
| 18:05:21 | mriedem | well, we might have the warning in the logs either way since we don't know if glance is configured to allow image uploads | |
| 18:05:23 | mriedem | for tenants | |
| 18:05:44 | mriedem | the warning is really my way of trying to get people to realize this is a thing if they aren't reading release notes | |
| 18:11:46 | mnaser | i like the idea of making it a policy flag thing, in case you want to allow specific tenants that rely on old behaviour (but you know won't be breaking things) | |
| 18:12:30 | mriedem | yeah me too, it's more flexible | |
| 18:12:37 | mriedem | i'll wait a bit before making changes | |
| 18:14:59 | openstackgerrit | Merged openstack/nova master: Remove the branch specifier from the nova-multiattach job https://review.openstack.org/560930 | |
| 18:15:23 | openstackgerrit | Merged openstack/nova master: libvirt: move version to string in utils https://review.openstack.org/560455 | |
| 18:15:38 | openstackgerrit | Merged openstack/nova master: libvirt: refactor get_base_config to accept host arg https://review.openstack.org/560456 | |
| 18:15:59 | mriedem | superdan: can you fast approve this on stable? https://review.openstack.org/#/c/560931/ | |
| 18:16:05 | mriedem | need that to get the multiattach job voting again on master | |
| 18:16:10 | superdan | can I or will I? | |
| 18:16:17 | mriedem | can you will it | |
| 18:21:13 | kashyap | superdan: I know you did a more robust review, I hope my -1 here is still in the spirit of it: https://review.openstack.org/#/c/484997/16/nova/conf/libvirt.py | |
| 18:24:54 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Parse forbidden traits in query strings https://review.openstack.org/556819 | |
| 18:24:55 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Filter resource providers by forbidden traits in db https://review.openstack.org/556472 | |
| 18:24:56 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Filter allocation candidates by forbidden traits in db https://review.openstack.org/556660 | |
| 18:24:57 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Support forbidden traits in API https://review.openstack.org/556820 | |
| 18:28:44 | mriedem | jgwentworth: mlavalle: posted http://forumtopics.openstack.org/cfp/details/124 | |
| 18:29:15 | jgwentworth | coolness. thanks. superdan, want to post the cells one? | |
| 18:29:49 | mlavalle | mriedem: thanks | |
| 18:30:33 | openstackgerrit | Hongbin Lu proposed openstack/nova stable/pike: Skip placement on rebuild in same host https://review.openstack.org/561015 | |
| 18:31:25 | mriedem | mlavalle: ops might want to ask about routed networsk too, i don't know how much you get for questions about that | |
| 18:32:17 | mlavalle | mriedem: I am not getting many questions lately | |
| 18:38:09 | superdan | jgwentworth: okay | |
| 18:45:51 | jgwentworth | thankee | |
| 18:46:25 | superdan | jgwentworth: http://forumtopics.openstack.org/cfp/details/125 | |
| 18:46:42 | jgwentworth | woot | |
| 18:49:13 | cfriesen | got a weird setup. we had in instance on a host in Newton, then deleted the host, then upgraded to Pike (which added uuids to the Service records but apparently not deleted ones), then created the compute node again with the same name. Now when we try to get that instance from the db it chokes, I suspect because it pulls in the deleted host record which has no uuid. | |
| 18:49:34 | cfriesen | should we have applied UUIDs to deleted hosts as well in the Pike online data migration? | |
| 18:49:39 | jgwentworth | there's a bug for that | |
| 18:50:58 | openstack | Launchpad bug 1746509 in OpenStack Compute (nova) pike "TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction" [High,Fix committed] - Assigned to melanie witt (melwitt) | |
| 18:50:58 | jgwentworth | cfriesen: https://bugs.launchpad.net/nova/+bug/1746509 | |
| 18:51:36 | jgwentworth | did you do your pike upgrade before that merged or was released? | |
| 18:51:46 | openstackgerrit | Merged openstack/nova master: Add host field to InstanceActionEvent https://review.openstack.org/555146 | |
| 18:53:13 | cfriesen | jgwentworth: cool, looks interesting. I need to dig into this a bit more to get the details. | |
| 18:54:09 | jgwentworth | k. yeah, this was specifically having to do with an upgrade scenario where there were already existing service records with no UUID | |
| 18:54:22 | jgwentworth | and instance list failed | |
| 18:54:35 | cfriesen | jgwentworth: do you know why we didn't just apply uuids to deleted services for consistency? | |
| 18:55:26 | jgwentworth | cfriesen: we were but on-the-fly upon access. so the bug unfortunately was shown *during* the generation of service UUID for one that did not yet have a UUID :/ | |
| 18:55:35 | cfriesen | ah, got it. | |
| 18:56:02 | jgwentworth | because of nested database transaction contexts. unrelated to the generation but nothing hit the path until the generation was added to the instance read flow | |
| 18:56:38 | cfriesen | jg: I was thinking in service_uuids_online_data_migration() | |
| 18:56:44 | cfriesen | jgwentworth: ^ | |
| 18:57:22 | jgwentworth | yeah, I figured that's what you meant. was not done in a batch like that and I don't know why not | |
| 18:57:49 | jgwentworth | superdan might know. why we didn't do service UUID as a batch online data migration? | |