| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-09 | |||
| 16:29:05 | melwitt | heh. well, there are some quirks with counting quota usage from placement today which need consumer types to be resolved. but counting from placement is opt-in, and I'd be looking to operators to know how urgently they want the quirks resolved | |
| 16:30:08 | melwitt | and AFAIK only CERN is planning to opt-in to counting quota usage from placement, the majority of folks will be running the default I expect | |
| 16:31:01 | melwitt | so, I consider it as medium priority unless operators signal otherwise | |
| 16:31:39 | cdent | I think in that case we can let CERN drive the bus then, and if tssurya needs an assist from other placement folk we can work that out. if this never ending performance improvement train in placement ever stops there may be some cycles spare | |
| 16:31:45 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add 'Instance.get_vifs' helper method https://review.opendev.org/675665 | |
| 16:32:23 | melwitt | cdent: sounds reasonable to me | |
| 16:34:14 | dansmith | mriedem: yep, piled on | |
| 16:37:09 | melwitt | cdent: to be clear, on my side my plan is to start working on integration whenever the placement side is ready. we can't do it this cycle, it's too late, but whenever it's ready I would write up a spec to consume it | |
| 16:38:31 | cdent | melwitt: one factor is that none of the regular active contributors to placement has any particular need for the feature, so it's good (and appropriate) that tssurya is willing and able to work on it | |
| 16:40:08 | melwitt | cdent: yeah, makes sense. just wanted to be clear that it wouldn't get dropped on the floor once it's done. I'd write up the spec and do the integration once ready, but that said it's medium priority in my mind unless CERN turns up the priority | |
| 16:40:57 | cdent | ✔ | |
| 16:42:07 | openstackgerrit | Merged openstack/nova stable/pike: Replace non-nova server fault message https://review.opendev.org/674877 | |
| 17:12:58 | mgoddard | hi mriedem, quick question about upgrade checks | |
| 17:13:20 | efried | mgoddard: I think Matt's at lunch fyi | |
| 17:13:32 | mgoddard | efried: oh ok, thanks | |
| 17:13:57 | efried | dansmith knows things about upgrades tho | |
| 17:21:09 | mgoddard | here's the question, perhaps someone else can answer | |
| 17:21:44 | openstackgerrit | Merged openstack/nova stable/queens: Delete resource providers for all nodes when deleting compute service https://review.opendev.org/666862 | |
| 17:21:51 | mgoddard | it's my understanding that nova-status upgrade check should be run after DB sync and data migrations | |
| 17:21:55 | openstackgerrit | Merged openstack/nova master: Filter UnsupportedServiceVersion warning https://review.opendev.org/674515 | |
| 17:22:15 | mgoddard | could it be harmful to run it before starting an upgrade (using the new code)? | |
| 17:22:20 | dansmith | no, | |
| 17:22:28 | dansmith | it's expected to run before the upgrade to tell you what to look out for, | |
| 17:22:36 | dansmith | and after to make sure everything went green | |
| 17:23:16 | mgoddard | that's how I wanted it to work, but when I've discussed it with mriedem previously I came away with the above impression | |
| 17:23:32 | dansmith | nova-status does not *do* anything, so no damage could occur | |
| 17:24:52 | mgoddard | no, but I don't want it to fail due to running it at the wrong time | |
| 17:24:53 | mgoddard | Install new code | |
| 17:24:55 | mgoddard | Sync the database schema for new models (nova-manage api_db sync; nova-manage db sync) | |
| 17:24:57 | mgoddard | Run the online data migrations (nova-manage db online_data_migrations) | |
| 17:24:59 | mgoddard | Run the upgrade check (nova-status upgrade check) | |
| 17:25:01 | mgoddard | Restart services with new code | |
| 17:25:03 | mgoddard | from https://docs.openstack.org/nova/latest/reference/upgrade-checks.html | |
| 17:25:18 | dansmith | sure | |
| 17:25:42 | dansmith | are you worried about it "failing" because you're scripting this and you want to know if you can always run it and expect a zero return or what? | |
| 17:25:54 | mgoddard | right | |
| 17:26:10 | dansmith | okay, well, it's probably not something you can do blindly regardless, | |
| 17:26:34 | dansmith | because sometimes the checks are things like "yo dawg, you need to modify your flavors to remove X which we've deprecated" or whatever | |
| 17:26:36 | mgoddard | can I safely run it before doing any part of the upgrade (including db sync)? | |
| 17:26:42 | dansmith | which isn't going to get "fixed" by running them | |
| 17:26:59 | dansmith | again, define "safely" | |
| 17:27:10 | dansmith | if you mean "without breaking stuff" then ... of course | |
| 17:27:17 | mgoddard | I don't mind if it tells me to do something that I do need to do | |
| 17:27:29 | dansmith | nova-status is read-only | |
| 17:27:32 | dansmith | it doesn't make changes | |
| 17:27:45 | dansmith | you can run it any time you want and the worst thing that will happen is it will complain about something, end of story | |
| 17:27:48 | mgoddard | but if it fails because I'm running it at the wrong point then that's bad | |
| 17:28:20 | dansmith | you know that it will *usually* not return zero right? | |
| 17:28:26 | dansmith | so I mean, again, define "fail" | |
| 17:28:37 | mgoddard | yeah, we allow 0 or 1 | |
| 17:29:05 | dansmith | IIRC, the point of it not using the db models is so it can run entirely against the old schema and do its checks, | |
| 17:29:10 | mgoddard | let me try to rephrase | |
| 17:29:17 | dansmith | so barring bugs, I think you will get constrained results even before db sync | |
| 17:29:44 | mgoddard | in order to get the 'right' answer out of it, should I run upgrade checks only after doing the DB sync and migrations? | |
| 17:30:02 | dansmith | I don't think it's that simple, | |
| 17:30:16 | dansmith | because I think that we've had some checks which were "hey before you do anything you need to manually change X" | |
| 17:31:00 | dansmith | mriedem has done 95% of the work on this and definitely has the current scope/intent in his head more than me, so I guess I'll just shut up, | |
| 17:31:01 | mgoddard | so it could be useful to run both before DB sync, and after? | |
| 17:31:09 | dansmith | but I think you're kinda assuming more about it than you should | |
| 17:31:27 | mgoddard | perhaps | |
| 17:32:32 | openstackgerrit | Merged openstack/nova stable/queens: Add functional recreate test for bug 1764556 https://review.opendev.org/673824 | |
| 17:32:32 | openstack | bug 1764556 in OpenStack Compute (nova) queens ""nova list" fails with exception.ServiceNotFound if service is deleted and has no UUID" [Medium,In progress] https://launchpad.net/bugs/1764556 - Assigned to Matt Riedemann (mriedem) | |
| 17:32:38 | openstackgerrit | Merged openstack/nova stable/queens: Add functional regression test for bug 1778305 https://review.opendev.org/673825 | |
| 17:32:39 | openstack | bug 1778305 in OpenStack Compute (nova) queens "Nova may erronously look up service version of a deleted service, when hostname have been reused" [Medium,In progress] https://launchpad.net/bugs/1778305 - Assigned to Matt Riedemann (mriedem) | |
| 17:34:45 | mgoddard | thanks for the input dansmith, I'll let mriedem weigh in if he disagrees otherwise I'll go with an early check | |
| 17:35:11 | mgoddard | and possibly a second after the sync/migration | |
| 17:35:28 | dansmith | just to be clear, | |
| 17:36:01 | dansmith | well, nevermind | |
| 17:39:16 | melwitt | mriedem: question in https://review.opendev.org/674908 | |
| 17:49:04 | openstackgerrit | Merged openstack/nova stable/queens: Don't generate service UUID for deleted services https://review.opendev.org/673827 | |
| 17:49:37 | mriedem | mgoddard: grenade runs the db syncs before the upgrade status check and fails if the return code is >= 2: https://github.com/openstack/grenade/blob/00ad9a89684360ca53d7cdda62763de37ba98510/projects/60_nova/upgrade.sh#L94 - which aligns with the doc (that's probably part of where i got that for writing the doc), | |
| 17:49:48 | mriedem | running nova-status upgrade check *before* db sync is _undefined_ | |
| 17:49:50 | mriedem | we don't test it that way | |
| 17:50:13 | mriedem | we don't use the versioned object model code, we do straight sqlalchemy db execution stuff, | |
| 17:50:33 | mriedem | but i wouldn't be surprised if at some point someone writes an upgrade check that is looking at maybe a column that doesn't exist until you've sync'ed, idk | |
| 17:50:53 | mriedem | but like dansmith said, i think we try to avoid that and could justify saying something like that is a bug | |
| 17:52:13 | dansmith | mriedem: for things where we need to warn the person that a blocker migration is going to hork on their db state, they'd need to run it ahead of time right? | |
| 17:52:16 | mriedem | i see takashin has a patch up for your bug https://bugs.launchpad.net/nova/+bug/1833278 | |
| 17:52:16 | openstack | Launchpad bug 1833278 in OpenStack Compute (nova) "nova-status upgrade check should fail if db sync has not been performed" [Medium,In progress] - Assigned to Takashi NATSUME (natsume-takashi) | |
| 17:52:23 | dansmith | meaning, if we're about to drop a column or add a constraint | |
| 17:52:27 | dansmith | I thought we had one of those early on | |
| 17:52:29 | mriedem | dansmith: takashi's patch would break that if we did | |
| 17:53:03 | dansmith | I really thought one of the first uses for nova-status was to warn people before we added a constraint, but I don't remember the details | |
| 17:53:18 | mriedem | earliest checks were for cells v2 and placement | |
| 17:53:19 | mriedem | in ocata | |
| 17:53:49 | mriedem | https://github.com/openstack/nova/blob/stable/ocata/nova/cmd/status.py#L358 | |
| 17:53:53 | melwitt | dansmith: I thought the same thing. like, "you have homework to do before running db migrations" | |
| 17:54:33 | melwitt | or maybe it's before online data migrations? | |
| 17:54:53 | dansmith | it tells you if you *haven't* run online migrations, | |
| 17:55:18 | mriedem | right for some definitely - there was a check for missing request specs which was solved with an online data migration, | |
| 17:55:22 | dansmith | but I really thought we were prescribing running it before to handle things like column drops, constraints, etc | |
| 17:55:26 | mriedem | and couldn't be done with a blocker migratoin b/c the records are in different dbs | |
| 17:55:45 | melwitt | yeah ok | |
| 17:57:02 | mriedem | as far as i know we haven't dropped a column or added a new constraint in a loong time | |
| 17:57:25 | mriedem | so haven't really had to think about the ordering of that plus the upgrade check | |
| 17:58:28 | dansmith | mriedem: but, if people are only ever running this after db sync, we can't rely on this to help us in that regard | |
| 17:58:49 | dansmith | and basically have to stick to blocker migrations | |
| 17:58:58 | mriedem | or you add the check in the n-1 release | |
| 17:59:18 | mriedem | "i see you've got stuff in column x which is going to be dropped in the next release, you should do y to drain it" | |
| 17:59:21 | mriedem | or whatever | |