| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-02 | |||
| 10:08:23 | mdbooth | So, anyway, I think the bug here is in LibvirtISCSIVolumeDriver and all other places which mutate connection_info | |
| 10:08:23 | openstackgerrit | Tomi Juvonen proposed openstack/nova-specs master: Add Host Details https://review.openstack.org/428070 | |
| 10:08:30 | mdbooth | They need to stash that metadata somewhere else. | |
| 10:08:52 | mdbooth | I also disapprove of even looking inside connection_info, tbh | |
| 10:09:05 | lyarwood | mdbooth: yeah that's all going to change with cinder v3 anyway | |
| 10:09:15 | mdbooth | If we could have cinder pass it to us as an opaque packed C struct, that would be awesome | |
| 10:09:28 | mdbooth | The only thing you can do with this data is give it back to cinder | |
| 10:09:29 | lyarwood | mdbooth: but I can't see how that relates to this issue tbh | |
| 10:09:59 | mdbooth | It relates because it means we *can't* refetch connection info from cinder after cinder also mutates it | |
| 10:10:29 | mdbooth | Because Nova futzes with it, we're now stuck trying to second guess one or the other. | |
| 10:11:05 | lyarwood | mdbooth: ah right you're talking about the cinder retype situation | |
| 10:11:20 | mdbooth | cinder volume migration | |
| 10:11:31 | lyarwood | mdbooth: yeah same thing | |
| 10:11:49 | mdbooth | volume migration is meaningful to me :) | |
| 10:12:03 | lyarwood | mdbooth: I need to drop now, can we pick this up when I get back? | |
| 10:12:08 | mdbooth | lyarwood: Sure thing. | |
| 10:12:23 | lyarwood | mdbooth: cool | |
| 10:13:21 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Fix access_ip_v4/6 filters params for servers filter https://review.openstack.org/428071 | |
| 10:13:57 | gmann | johnthetubaguy: mriedem alex_xu seems like we missed another bit in filter/sort thing - https://bugs.launchpad.net/nova/+bug/1661195 | |
| 10:13:57 | openstack | Launchpad bug 1661195 in OpenStack Compute (nova) "Servers filter by access_ip_v4 does not filter servers" [Undecided,In progress] - Assigned to Ghanshyam Mann (ghanshyammann) | |
| 10:14:18 | gmann | johnthetubaguy: alex_xu mriedem this is fix - https://review.openstack.org/428071 | |
| 10:14:45 | johnthetubaguy | gmann: oh, you mean we should allow them, because we used to allow them | |
| 10:15:02 | gmann | but different filed name in filter and sort than what API represent is odd to me | |
| 10:15:16 | gmann | johnthetubaguy: i think so, or no ? | |
| 10:15:32 | johnthetubaguy | gmann: don't they now match what the API has? | |
| 10:15:34 | gmann | i may be missed the discussion on that if any | |
| 10:16:15 | gmann | johnthetubaguy: oh so you mean new list will match the what API has either it used to work before or not | |
| 10:17:08 | johnthetubaguy | gmann: the whole plan is the filter name matches what the API names are now, rather than matching the DB | |
| 10:17:29 | gmann | johnthetubaguy: yea | |
| 10:18:08 | gmann | johnthetubaguy: ok, i think m going in opposite way :) | |
| 10:18:15 | gmann | johnthetubaguy: i agree on that point | |
| 10:18:49 | gmann | johnthetubaguy: but bug seems now accessIPv4 work on filter or not as DB fileds are different for those? | |
| 10:19:26 | gmann | johnthetubaguy: and sort has 'access_ip_v4' not accessIPv4 so it is issue on sort list then | |
| 10:19:46 | johnthetubaguy | gmann: so we must have accessipv4 and accessipv6 as thats what in the API, in both filter and sort | |
| 10:20:31 | gmann | johnthetubaguy: yea and those should work as filter. because i doubt currently those works as we did not add translation layer for those | |
| 10:20:32 | johnthetubaguy | gmann: but... for backwards compatibility, we should probably also allow access_ip_v4 and access_ip_v6, and mark them for removal in a future microversion | |
| 10:20:39 | mdbooth | Can anybody confirm that schema migrations are intended to be idempotent? Specifically that I should be able to run db_sync multiple times? | |
| 10:20:44 | johnthetubaguy | gmann: yeah, we should add a test for both | |
| 10:20:47 | johnthetubaguy | mdbooth: thats correct | |
| 10:21:00 | gmann | johnthetubaguy: yea, that was actually i was thinking initially | |
| 10:21:07 | gmann | johnthetubaguy: cool, | |
| 10:21:32 | gmann | johnthetubaguy: ll update | |
| 10:21:36 | johnthetubaguy | gmann: cool | |
| 10:22:07 | johnthetubaguy | gmann: we should give that the tag, and get it on mriedem's etherpad I think | |
| 10:22:25 | johnthetubaguy | RC potential thing | |
| 10:22:36 | mdbooth | johnthetubaguy: Thought so, thanks. I'm seeing an error in 216_havana creating the instances table, specifically line 1046 where it calls instances.create() raises an error containing: (1050, u"Table 'instances' already exists") | |
| 10:22:38 | gmann | johnthetubaguy: but last doubt, is it ok to introduced new param without version up. i mean accessIPv4 are kind of new addition as filter | |
| 10:22:54 | gmann | johnthetubaguy: sure, ll do on etherpad | |
| 10:22:59 | mdbooth | It does already exist. Any idea how that's supposed to work? | |
| 10:24:33 | mdbooth | Or maybe it's a lower level failure... Perhaps sqlalchemy is supposed to transmute it into gold. | |
| 10:25:37 | johnthetubaguy | mriedem: hmm thats odd, thats the our base one that everyone has run again on upgrade for years, its curious that has broken | |
| 10:25:45 | gmann | johnthetubaguy: i remember we did microversion when adding ipv6 as filter | |
| 10:26:33 | mdbooth | johnthetubaguy: Yeah, suspect it's something weird in this environment. | |
| 10:26:39 | mdbooth | File hasn't changed in ages. | |
| 10:26:59 | mdbooth | It could be an interesting candidate for robustification, though. | |
| 10:27:09 | mdbooth | If I can work out why it's broken. | |
| 10:27:12 | johnthetubaguy | gmann: yes, thats a good point, we also have these: https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/servers.py#L354 :S | |
| 10:27:40 | johnthetubaguy | mdbooth: generally we try never to change those files, but yeah | |
| 10:27:59 | johnthetubaguy | flavors was the exception in there, I think | |
| 10:28:22 | gmann | johnthetubaguy: i thought of adding accessIPv4 with version up fir interop things | |
| 10:28:55 | johnthetubaguy | gmann: we haven't yet released the last fix, so I am OK with that for right now | |
| 10:30:35 | johnthetubaguy | mdbooth: maybe you died half way through the migration, and somehow that got committed into the DB? That might confuse things | |
| 10:31:23 | gmann | johnthetubaguy: but accessIPv4 are always new even before filter/sort fix. i means they are totally new to users after Ocata | |
| 10:32:25 | johnthetubaguy | gmann: yeah, I am not sure why we added new ones without a microversion, now you meantion it, I got distracted with killing bad ones | |
| 10:33:33 | gmann | johnthetubaguy: yea, list is too huge we did not notice those | |
| 10:35:10 | gmann | johnthetubaguy: i have added that in etherpad also (so mriedem can mark that rc potential), ll check from home. | |
| 10:36:12 | johnthetubaguy | gmann: thanks | |
| 10:36:39 | johnthetubaguy | gmann: it was such a massive list, there were always going to be niggles, would love to squash then before release, if possible | |
| 10:36:58 | gmann | +1 | |
| 10:48:30 | sfinucan | bauzas: Can I +W this? https://review.openstack.org/#/c/426864/6 | |
| 10:48:47 | sfinucan | I assume it won't do anything silly like tag the release or whatnot | |
| 10:51:34 | johnthetubaguy | sfinucan: that sounds fine to me, we have added lots of preludes by accident in the past | |
| 10:51:56 | sfinucan | johnthetubaguy: Righto - +W it is | |
| 10:52:27 | johnthetubaguy | oh year, I should review that one again | |
| 10:52:30 | bauzas | sfinucan: cool with me | |
| 10:52:36 | bauzas | johnthetubaguy: I'm already on it | |
| 10:53:29 | sfinucan | bauzas: Could you also take a look at https://review.openstack.org/#/c/427499/, seeing as you're far more familiar with that feature | |
| 10:55:29 | bauzas | sfinucan: mmm, I had a concern on the bottom patch | |
| 10:55:46 | bauzas | at least a typo | |
| 10:56:22 | sfinucan | bauzas: Hmm, if you want to fix it now we can just re-approve, right? | |
| 10:56:46 | sfinucan | Also, do you know what "barber pole sorting" is? :D | |
| 10:57:06 | sfinucan | I'm guessing it's Python's 'zip', but that's a guess | |
| 10:58:31 | bauzas | sfinucan: https://en.wikipedia.org/wiki/Barber's_pole#Computer_science | |
| 10:58:56 | sfinucan | bauzas: I don't think that's what he's talking about though | |
| 10:59:06 | sfinucan | That describes a loading bar - he's on about sorting something | |
| 10:59:38 | bauzas | sfinucan: that would mean it takes time IIUC | |
| 10:59:50 | bauzas | sfinucan: oh, you removed +W | |
| 11:00:00 | bauzas | sfinucan: but that doesn't mean the gate jobs will stop | |
| 11:00:07 | bauzas | sfinucan: we have to trigger a new PS | |
| 11:00:12 | sfinucan | bauzas: it doesn't? | |
| 11:00:14 | sfinucan | Oh | |
| 11:00:15 | bauzas | nope | |
| 11:00:20 | sfinucan | Well, should we fix that typo? | |
| 11:00:31 | bauzas | we could do that in a follow-up | |
| 11:00:34 | bauzas | now it's in the gate | |
| 11:00:55 | sfinucan | That sounds fair | |
| 11:00:56 | bauzas | and given we have 3 patches that could be trampled if we provide a new rev | |
| 11:01:06 | bauzas | so, adding the fix at a top patch | |
| 11:01:24 | mdbooth | johnthetubaguy: In case you're interested, looks like the db state was probably corrupt, perhaps due to a previous incomplete cleanup. This was in a CI system. So robustification likely not relevant to anybody. | |
| 11:02:21 | bauzas | sfinucan: restored the +W | |