Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-09
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 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:32 openstackgerrit Merged openstack/nova stable/queens: Add functional recreate test for bug 1764556 https://review.opendev.org/673824
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 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:16 mriedem i see takashin has a patch up for your bug https://bugs.launchpad.net/nova/+bug/1833278
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
17:59:33 mriedem we've added checks that get backported, like for the ironic flavor migratoins in pike
18:00:03 dansmith if we're going to require db sync first, why did we go with raw sqla access then?
18:00:18 mriedem but i don't think we've ever said this is an end-all perfect tool, it's part of the whole box - blocker migratoins, online data migratoins, runtime data migrations, and the checks to automate release notes
18:00:21 dansmith I mean, that was the whole point of doing that, AFAIK, so we could talk to the schema independent of the models
18:00:31 dansmith sure
18:00:36 mriedem i'm not saying db sync is required,
18:00:51 mriedem i'm saying that's how we test it, and how it behaves before db sync is undefined
18:01:02 dansmith ack,
18:01:05 mriedem if we want to support both ways, we need to test it both ways
18:01:12 dansmith but I think the question is about intent and assumptions and what-not,
18:01:19 dansmith especially in the face of that patch to require it
18:01:38 mriedem yup, which is a result of a bug that mgoddard opened
18:01:48 mriedem to i guess make us shit or get off the pot
18:02:01 mriedem i prefer to wallow thank you very much
18:02:30 mriedem and i think he did that b/c he's getting pushback in kolla about adding the checks in
18:02:41 mriedem https://review.opendev.org/#/c/665643/
18:03:50 dansmith his patch actually doesn't make it required, so much as it just checks that you've done it I guess
18:03:51 dansmith which is less concerning
18:05:27 dansmith if the expectation of the tool is to tell you go/no-go for deploying new code, then it's probably fine to have that be an ERROR and not a WARN
18:05:37 mriedem https://review.opendev.org/#/c/667047/10/nova/cmd/status.py@395 is a fail
18:05:44 dansmith right, but ^
18:05:54 mriedem and https://review.opendev.org/#/c/667047/10/nova/cmd/status.py@370
18:06:13 dansmith it's still legit to run that before you do db sync to catch other soft changes knowing that the error is expected before you've done anything
18:07:00 mriedem maybe, but then you have to build some logic into your ansible that detects which check failed and ignore this one if you know you're doing before-sync checks
18:07:04 dansmith we could have a pre-upgrade command in that tool which doesn't run stuff like that, but does do things like sanity checks,
18:07:16 mriedem which gets back to something mnaser originally asked for which was more machine-readable output
18:07:41 dansmith we could also have the pre-upgrade run any of the checks from the previous release as error->fail, and any of the current release checks as error->warn or something
18:07:44 mriedem separating the commands would make more sense imo
18:07:47 dansmith and just move them each release
18:08:03 mriedem then you have a defined behavior and we'd test that in grenade
18:08:13 dansmith yeah
18:08:19 mnaser i did this a really long time ago but yeah -- https://review.opendev.org/#/c/576944/
18:12:04 openstack Launchpad bug 1839560 in OpenStack Compute (nova) "ironic: moving node to maintenance makes it unusable afterwards" [High,In progress] - Assigned to Matt Riedemann (mriedem)
18:12:04 mnaser also just a friendly bringing up of this fun one https://bugs.launchpad.net/nova/+bug/1839560

Earlier   Later