| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-09 | |||
| 16:32:18 | jaypipes | edleafe: I can try to give it a shot if you'd like, but wouldn't be able to get to it until tomorrow likely | |
| 16:32:24 | jaypipes | edleafe: yeah, lemme grab an example | |
| 16:32:29 | edleafe | ok, let me do some googling | |
| 16:35:05 | edleafe | so it's not the server_default stuff; it's changing the nullable for the project_id and user_id fields that sqlite doesn't support | |
| 16:37:59 | jaypipes | edleafe: no | |
| 16:38:20 | jaypipes | edleafe: it's the addition of the generation column to the consumers table as a NOT NULL field that fails. | |
| 16:38:42 | jaypipes | edleafe: because that create_column() ends up being translated to an ALTER TABLE consumers ADD COLUMN generation NOT NULL | |
| 16:39:00 | jaypipes | edleafe: and SQLite's ALTER TABLE ... ADD COLUMN doesn't support a DEFAULT clause :( | |
| 16:39:49 | jaypipes | edleafe: so you need to do a CREATE TABLE consumers_new with the new schema, then do a SELECT into the new table, then do a DROP TABLE consumers; RENAME TABLE consumers_new TO consumers | |
| 16:39:56 | jaypipes | edleafe: like I said, it's a pain in the ass. | |
| 16:40:16 | jaypipes | edleafe: and I'm wondering if we need to do it for SQLite anyway since it's dev-only DB. | |
| 16:40:19 | edleafe | from the sqlite docs: "Only the RENAME TABLE and ADD COLUMN variants of the ALTER TABLE command are supported. Other kinds of ALTER TABLE operations such as DROP COLUMN, ALTER COLUMN, ADD CONSTRAINT, and so forth are omitted." | |
| 16:40:40 | jaypipes | edleafe: yes, but the DEFAULT clause of ADD COLUMN isn't supported, IIRC | |
| 16:41:20 | edleafe | jaypipes: sure, but neither would the ALTER COLUMN calls for those other two fields, right? | |
| 16:41:24 | jaypipes | so you can't do: ALTER TABLE consumers ADD COLUMN generation INTEGER NOT NULL DEFAULT 0 | |
| 16:41:48 | openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | |
| 16:41:57 | jaypipes | edleafe: yes, likely. it's probably that those errors haven't (yet) come up due to the failure of the consumers.generation thing first. | |
| 16:42:03 | edleafe | jaypipes: ok, I'll give it a shot. It would be *so* much easier in SQL than sqla. | |
| 16:42:38 | edleafe | jaypipes: although I did lift the nullable change from nova/db/sqlalchemy/migrate_repo/versions/267_instance_uuid_non_nullable.py | |
| 16:42:54 | jaypipes | edleafe: so you'll need to add a file in the migrate_repo called 059_add_consumer_generation.sql I believe. | |
| 16:43:25 | edleafe | and that made it in | |
| 16:43:38 | jaypipes | edleafe: and then guard the 059_add_consumer_generation.py file with the whole dialect != 'sqlite' thing | |
| 16:44:54 | jaypipes | edleafe: alternately, you could just execute raw sql statements like is done here: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api_migrations/migrate_repo/versions/044_placement_add_projects_users.py#L63-L77 | |
| 16:44:56 | edleafe | jaypipes: so the .sql file will be executed automatically? Or do I call it from the .py? | |
| 16:45:13 | jaypipes | edleafe: pretty sure it would get executed automatically, but lemme check | |
| 16:45:19 | edleafe | ah, I like that better | |
| 16:48:37 | jaypipes | edleafe: so apparently ALTER TABLE ... ADD COLUMN in SQLite *does* support the DEFAULT clause. | |
| 16:48:47 | jaypipes | edleafe: so I'm not sure why it's not being generated here. | |
| 16:57:19 | jaypipes | edleafe: ok, I'm a bit stumped... may need to reach out to zzzeek_ on this one. can you try setting default="0" instead of default=0 in both the migration and models? | |
| 16:58:22 | jaypipes | edleafe: from the error message it kinda seems like the difference that oslo.db's test_models_in_sync thing is seeing has to do with '0' vs. a sqlalchemy.sql.elements.TextClause objectTextClause | |
| 16:58:43 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Cleanup RP and HM records while deleting a compute service. https://review.openstack.org/554920 | |
| 17:02:10 | dansmith | mriedem: I think we already have a place where we need placement configured for nova-api | |
| 17:02:18 | dansmith | although I think we're not actually doing the thing that would require it | |
| 17:02:27 | dansmith | which is local delete nuking allocations | |
| 17:04:39 | edleafe | jaypipes: no dice on default="0" | |
| 17:04:43 | edleafe | jaypipes: same error | |
| 17:05:34 | jaypipes | :( | |
| 17:12:31 | mriedem | dansmith: yeah that's a latent bug, which i've had sitting in my todo list, | |
| 17:12:36 | mriedem | i'll tackle it at 3pm on friday :) | |
| 17:12:58 | dansmith | that's fine, just don't expect me to be around for it :) | |
| 17:13:14 | mriedem | you said you'd always be there for me | |
| 17:14:16 | mriedem | avolkov: some easy changes to make in https://review.openstack.org/#/c/511183/ - just got feedback from dtroyer too | |
| 17:14:35 | mriedem | although i know it's late in the day for you | |
| 17:23:21 | mriedem | dansmith: when you get a sec, there is a mini debate in my patch to wait for vif plugged events before starting live migration that could use your input https://review.openstack.org/#/c/558001/ - some of it around whether or not we should even fail if we timeout, which thinking back on sahid's recent patch for linuxbridge, that doesn't fail on timout, it just logs a warning and continues | |
| 17:23:34 | openstackgerrit | Merged openstack/nova-specs master: mirror nova host aggregates to placement API https://review.openstack.org/545057 | |
| 17:23:51 | dansmith | mriedem: no, I made him fail if it times out | |
| 17:24:09 | mriedem | yeah i see that now https://github.com/openstack/nova/blob/e2d5dc4e2c5f69fcbcb04c29b2ed469725122787/nova/virt/libvirt/driver.py#L7429 | |
| 17:24:21 | mriedem | so it's totally unconditional based on the CONF.vif_plugging_is_fatal option | |
| 17:24:58 | dansmith | if we're expecting an event and don't get it, we should fail just about anything, IMHO, except if we're waiting during delete or something | |
| 17:25:08 | mriedem | that was basically my reply in my patch | |
| 17:25:13 | mriedem | if i set this, i expect an event | |
| 17:25:26 | mriedem | fail if it doesn't happen | |
| 17:25:55 | dansmith | totes | |
| 17:33:30 | mriedem | dansmith: on https://review.openstack.org/#/c/558059/4/nova/compute/manager.py@368 did you want to update that and add yourself as co-author? | |
| 17:33:34 | mriedem | otherwise the change lgtm | |
| 17:33:54 | mriedem | ill leave it up to efried and jaypipes to love up on it otherwise | |
| 17:35:34 | dansmith | mriedem: I don't care, but if you want blame-sharing I'll be glad to | |
| 17:35:47 | mriedem | updated that comment, event.key is probably ok | |
| 17:36:00 | mriedem | so just drop the wip if you don't want co-blame | |
| 17:36:20 | dansmith | done | |
| 17:41:39 | efried | mriedem, dansmith: +2. Who's gonna be the second +2? | |
| 17:42:00 | mriedem | i only mentioned two people to love up on it | |
| 17:50:08 | zzzeek_ | jaypipes: server_default in SQLAlchemy for a DEFAULT clause | |
| 17:50:36 | dansmith | ah, now that does sound familiar | |
| 18:09:22 | edleafe | zzzeek_: Hey, can you take a look at a migration issue I'm having? | |
| 18:09:47 | jaypipes | zzzeek_: we've tried adding and removing server_default with no luck... | |
| 18:10:35 | edleafe | jaypipes: I tried the recreate, import, drop, rename dance, but it seems to have a problem with the autoincrement | |
| 18:11:01 | mriedem | dansmith: just need a simple unit test add to https://review.openstack.org/#/c/539590/ and i'm +2 | |
| 18:11:14 | mriedem | i know that's an older series | |
| 18:14:41 | dansmith | oh yeah, forgot about that | |
| 18:16:41 | hanish_ | hi all, i am trying to launch vm, but i am facing timed out error in nova conductor, can someone help me on thos | |
| 18:20:27 | esberglu | mriedem: Should I put the new powervm conf group in the features category or upgrade category in the release note? | |
| 18:20:31 | esberglu | All the other conf related stuff is in upgrade, but I haven't found any adding new group, just moving existing groups | |
| 18:20:42 | esberglu | Feature seems more accurate to me | |
| 18:24:13 | openstackgerrit | Merged openstack/nova master: libvirt: Block swapping to an encrypted volume when using QEMU to decrypt https://review.openstack.org/544238 | |
| 18:27:06 | mriedem | esberglu: let me check to see what this is actually doing | |
| 18:28:33 | esberglu | mriedem: Allows a user to specify how much of a physical proc to assign each vCPU | |
| 18:29:40 | mriedem | yeah, don't worry, i've got the fine toothed comb out now | |
| 18:33:36 | openstackgerrit | Merged openstack/nova master: Avoid dumping stack on BuildAbortException https://review.openstack.org/559751 | |
| 18:34:00 | mriedem | esberglu: comments inline | |
| 18:35:27 | mriedem | esberglu: what's with the flavor extra spec mention? | |
| 18:35:35 | mriedem | is that something yet to be added? | |
| 18:40:14 | esberglu | mriedem: Added that flavor extra spec mention after comments from edmondsw | |
| 18:40:56 | edmondsw | what commit? | |
| 18:41:21 | esberglu | edmondsw: https://review.openstack.org/#/c/554688/3/nova/conf/powervm.py@30 | |
| 18:42:45 | mriedem | i'm ok with blaming things on edmondsw | |
| 18:42:51 | edmondsw | works for me :) | |
| 18:43:24 | edmondsw | yeah, we shouldn't mention that if it's only an OOT thing | |
| 18:43:38 | mriedem | i don't even see it in the out of tree driver | |
| 18:43:39 | edmondsw | I didn't realize that was only OOT | |
| 18:43:47 | edmondsw | looking... | |
| 18:44:00 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Log a warning and add nova-status check for old API service versions https://review.openstack.org/557506 | |
| 18:44:10 | mriedem | https://github.com/openstack/nova-powervm/search?utf8=%E2%9C%93&q=proc_unit_factor&type= | |
| 18:44:55 | zzzeek_ | jaypipes edleafe I can look later, though Im not sure SQLite even supports ALTER TABLE for a server default if that's your target DB | |
| 18:44:56 | mriedem | i found https://github.com/openstack/nova-powervm/commit/4bad1ec1654b757c70100fbcc6f51505719d79d9 - and this setting is in the pypowervm lib | |
| 18:45:36 | esberglu | proc_units_factor not proc_unit_factor | |
| 18:45:51 | zzzeek_ | jaypipes edleafe oh but if this is nova / sqlalchemy-migrate then that's another wrinkle | |
| 18:46:01 | edleafe | zzzeek_: it's in one of the functional tests | |
| 18:46:08 | edmondsw | mriedem esberglu misnamed: https://github.com/openstack/nova-powervm/blob/1dc302b7198ad749dc14cc69cf8f2e45e929d132/nova_powervm/virt/powervm/vm.py#L248-L250 | |
| 18:46:26 | zzzeek_ | edleafe: add me to the review and ill try to look later | |