| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-06 | |||
| 15:08:40 | fried_rice | mriedem: Care to +W https://review.openstack.org/#/c/555961/ (lower-constraints.txt) quick before we break it again? | |
| 15:08:40 | tssurya | hmm question.. why wasn't this an issue before me adding a new column to cell_mappings ? | |
| 15:09:12 | tssurya | I mean no other column change in api_db before has needed a api sync before db sync situation ? | |
| 15:09:29 | odyssey4me | mriedem yeah, that does seem a bit chicken-and-egg | |
| 15:09:55 | cdent | finucannot: +many on the -W stuff. pain now for less pain later | |
| 15:10:09 | mriedem | tssurya: we just haven't modified the cell_mappings table in forever | |
| 15:10:41 | tssurya | mriedem: cell_mappings yes, what about other tables ? | |
| 15:10:57 | mriedem | db sync doesn't hit other tables | |
| 15:11:01 | mriedem | in the api db | |
| 15:11:11 | jgwentworth | mriedem: interesting, I always thought the order was api_db first in the upgrade docs. I would have thought that one should be done first and I can't immediately think of why it's not | |
| 15:11:17 | superdan | mriedem: oh is it trying to read the cell mappings to do the cell databases before it has sync'd the api db? | |
| 15:11:27 | superdan | yeah I'm not sure why api isn't first | |
| 15:11:32 | finucannot | cdent: Yup, only way to do it, unfortunately | |
| 15:11:35 | mriedem | superdan: nova-manage db sync by default tries to also sync cell0- | |
| 15:11:37 | tssurya | mriedem: ah heh yes, anyways I thought api_sync was run before db sync until now | |
| 15:11:59 | superdan | mriedem: and it looks up the mapping for it before it has sync'd the api db? | |
| 15:12:18 | tssurya | superdan: yes | |
| 15:12:32 | mriedem | yeah | |
| 15:12:58 | superdan | er, I guess t's just that we call db sync before api_db sync in grenade you mean | |
| 15:13:27 | mriedem | right | |
| 15:13:31 | mriedem | that's one possible fix | |
| 15:13:31 | superdan | mriedem: tbh, I imagine that it's second because the main db sync was there first, and we were adding stuff, so appending it to the end | |
| 15:13:52 | superdan | I don't think that was an intentional decision | |
| 15:14:00 | mriedem | superdan: maybe https://review.openstack.org/#/c/190399/ | |
| 15:14:15 | mriedem | wow mitaka | |
| 15:14:23 | superdan | I'm kinda surprised actually since you would have to sync the api db at day zero to get cell0 to work at all, unless it's just using config for that one | |
| 15:14:49 | mriedem | ocata->pike would use devstack to setup ocata first though wouldn't it? | |
| 15:14:55 | mriedem | and devstack does the base install with the api db sync first | |
| 15:15:11 | superdan | ah, right, right | |
| 15:15:17 | mriedem | https://github.com/openstack-dev/devstack/blob/master/lib/nova#L724 | |
| 15:15:31 | mriedem | then later: https://github.com/openstack-dev/devstack/blob/master/lib/nova#L741 | |
| 15:15:44 | superdan | yeah I was thinking about my superconductor changes and was like "I was sure api goes first" | |
| 15:15:51 | superdan | but this is grenade, which matters less | |
| 15:15:55 | superdan | because devstack has already run | |
| 15:15:57 | superdan | so that makes sense | |
| 15:16:27 | mriedem | fried_rice: someone is goign to have to tl;dr that for me - will that lower-constraints.txt file get synced automagically by the reqs bot? | |
| 15:16:44 | superdan | someone else should write the grenade patch so I can +2 it | |
| 15:16:47 | fried_rice | mriedem: No, it's manual. | |
| 15:17:02 | fried_rice | mriedem: There was a ML thread by dhellmann | |
| 15:17:11 | mriedem | yeah i saw the novel | |
| 15:17:13 | tssurya | superdan: I can.. | |
| 15:17:14 | mriedem | again, tl;dr | |
| 15:17:28 | mriedem | fried_rice: also, is that job supposed to actually run tests? http://logs.openstack.org/61/555961/3/check/openstack-tox-lower-constraints/91b6d17/job-output.txt.gz#_2018-04-06_13_19_18_865706 | |
| 15:17:30 | mriedem | b/c it doesn't | |
| 15:17:35 | superdan | and we should change those docs, I imagine they're in that order for the same reason | |
| 15:18:23 | mriedem | fried_rice: so -1 on that | |
| 15:19:24 | superdan | tssurya: okay I'll tweak the doc | |
| 15:19:29 | fried_rice | mriedem: Okay. The requirements-check job sure works (cause that's what held up PS1 and PS2). But yeah, I don't know anything beyond that. | |
| 15:19:36 | tssurya | although mriedem says he prefers a "Add a from-queens script" to swapping | |
| 15:19:52 | mriedem | no i don't prefer that | |
| 15:20:04 | mriedem | i said, "Option #1 seems a bit shady, but option #2 seems excessive, especially if we're going to be changing things in the API DB in future releases." | |
| 15:20:19 | mriedem | in other words, i'm hedging | |
| 15:20:35 | tssurya | okay sorry so its the shady one then | |
| 15:21:16 | superdan | I don't think it's shady, I think it's been wrong for a long time, hidden by the fact that we run devstack (in the proper order) first before grenade | |
| 15:21:18 | mriedem | if we added a from-queens script, per grenade guidelines, we'd also need a nova release note in rocky saying you need to run nova-manage api_db sync before nova-manage db sync | |
| 15:21:20 | jgwentworth | heh, I think it was shady assuming there was a good reason for the current ordering, which there apparently isn't | |
| 15:21:31 | superdan | yeah | |
| 15:21:43 | odyssey4me | mriedem alright, I've pushed up an OSA patch to test out the shady option: https://review.openstack.org/#/c/559354/1/tasks/nova_db_setup.yml | |
| 15:21:54 | tssurya | superdan : should I do the docs in the same one then ? | |
| 15:22:03 | odyssey4me | that'll make api_db sync before before db | |
| 15:22:06 | superdan | tssurya: I'm doing the doc patch, if you can do grenade | |
| 15:22:16 | tssurya | superdan: okay :) | |
| 15:22:28 | odyssey4me | go.... TEAM! ;) | |
| 15:22:36 | mriedem | odyssey4me: that will affect both base install and upgrade? | |
| 15:22:46 | odyssey4me | mriedem yep | |
| 15:23:08 | jgwentworth | \o/ | |
| 15:24:28 | fried_rice | mriedem: Looking over dhellmann's email again, it's possible that the no-op-ness of that job is intentional; to be switched on at some point in the future once the dust has settled. This is just setting up the framework. | |
| 15:24:56 | mriedem | fried_rice: well the commit message is clearly misleading then | |
| 15:25:17 | fried_rice | mriedem: But... I guess I'll let dhellmann respond to your -1. I was just trying to take over since he's got a few hundred of these up in the air. | |
| 15:25:29 | jgwentworth | so we set our constraints manually and the job makes sure all of the components work together with the versions they've required? | |
| 15:25:34 | odyssey4me | mriedem if install must do a different order to upgrade, then that's not awesome - but we can adjust... but as it stands now it looks like our api_db sync happened first on upgrade only, but the db sync didn't happen before the cell mapping... so I can adjust the sequence to just move when the db sync happens if that's more sensible | |
| 15:25:59 | mriedem | odyssey4me: this might be fine, we'll see; i was wondering why there were redundant api db syncs in there before | |
| 15:26:32 | mriedem | i guess the first one was trying to be sly | |
| 15:26:40 | mriedem | and a one time only thing on install | |
| 15:27:39 | odyssey4me | yeah, I believe there was a sequence difference between new environment vs upgrade | |
| 15:28:05 | mriedem | odyssey4me: left a comment, i think this might fail | |
| 15:28:08 | mriedem | we'll see | |
| 15:28:23 | openstackgerrit | Dan Smith proposed openstack/nova master: Clarify/correct the ordering of API and Cell database schema updates https://review.openstack.org/559357 | |
| 15:28:26 | superdan | ooh, look how many words! | |
| 15:29:39 | jgwentworth | you dun good | |
| 15:30:46 | odyssey4me | mriedem question to clarify before I revise | |
| 15:30:51 | odyssey4me | (in review) | |
| 15:33:50 | tssurya | https://review.openstack.org/#/c/559358/ | |
| 15:35:45 | superdan | tssurya: comments on the commit message | |
| 15:35:53 | tssurya | looking | |
| 15:36:02 | superdan | also probably need jgwentworth's sign-off on the final version before we put it in | |
| 15:36:14 | superdan | we usually try to require that for things like this in grenade | |
| 15:36:55 | tssurya | sure :) | |
| 15:37:04 | jgwentworth | I shall ack it | |
| 15:38:37 | superdan | tssurya: if my friday-speak wasn't clear enough on that review let me know and I'll try to plain it up a little | |
| 15:39:50 | sususuryashines | superdan: yes please | |
| 15:40:04 | jgwentworth | hah, good friday nick | |
| 15:40:14 | sususuryashines | for starters did you only want a change in explanation or the title also seems odd to you ? | |
| 15:40:30 | superdan | no, the title seems okay | |
| 15:40:33 | sususuryashines | jgwentworth: heh thanks ;) | |
| 15:40:39 | superdan | let me just write what I think it should be | |
| 15:41:55 | mriedem | sususuryashines: you could just lift from this https://review.openstack.org/#/c/559357/ | |
| 15:42:47 | sususuryashines | mriedem: yes I thought so at first, but that commit message is a little different from superdan's recommendations on my commit message. | |
| 15:43:27 | mriedem | it's different in that it doesn't have all the meh and stuff :) | |
| 15:44:23 | superdan | sususuryashines: left a suggested block of text | |