| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 20:25:25 | mriedem | i.e. bfv in openstack sucks ux-wise | |
| 20:26:42 | dansmith | yeah | |
| 20:26:52 | mriedem | from what i can tell, we don't even have any direct tests for _merge_bdms_lists | |
| 20:26:59 | dansmith | mriedem: well, I commented and summarized my feelings | |
| 20:27:03 | dansmith | mriedem: heh, nice | |
| 20:27:06 | dansmith | and no tempest test, | |
| 20:27:23 | dansmith | which means it's not part of the are-you-openstack test right? | |
| 20:31:19 | mriedem | interop, no | |
| 20:31:26 | mriedem | most things aren't in interop though fwiw | |
| 20:31:36 | dansmith | I know, I'm just poking | |
| 20:32:44 | mriedem | alright i'll give myself a todo to re-spec-ify this | |
| 20:47:18 | openstackgerrit | Julia Kreger proposed openstack/nova master: WIP: Add microversion to ironic client wrapper call https://review.openstack.org/554762 | |
| 20:49:27 | melwitt | ran into something unexpected today, apparently when you specify a non-existent field when creating a nova object, it doesn't complain about it upon create(), it just silently never applies it https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_instance_list.py#L69 | |
| 20:58:32 | cfriesen | when we evacuate a boot-from-volume instance and it's starting up on the new compute node, would we expect the HTTP API calls to cinder to have the same "req-*" number as the evacuate in nova? | |
| 21:04:58 | dansmith | melwitt: yeah, it's always been that non-field properties are free-form | |
| 21:05:21 | dansmith | that's how we manage local caching of stuff, and some of the other things instance does, for example | |
| 21:05:43 | dansmith | I found it surprising the first time someone else brought it up and was surprised by it :) | |
| 21:05:55 | melwitt | yeah, I guess I haven't used the non-field properties too much. or I keep forgetting about them | |
| 21:06:23 | sean-k-mooney[m] | melwitt: ya that has come up a few times | |
| 21:07:48 | sean-k-mooney[m] | Eveyone finds it surprising as they expect ovo/nova objects to prevent python being python and allowing you to add attributes to objects when ever you feel like it | |
| 21:09:10 | melwitt | I think it was the use of it in the init that threw me off. InstanceMapping(user_id=<id>) looked so official | |
| 21:10:33 | sean-k-mooney[m] | Hum it could still be a bug. Non fields are not serialised, the user id sounds like it should be persistent | |
| 21:11:24 | melwitt | yeah, I'm guessing the use wasn't intentional (it's just in a test) | |
| 21:11:38 | melwitt | I'm trying to recreate a bug and been poking around these functional tests | |
| 21:13:23 | sean-k-mooney[m] | Thats always an interesting experience, your never sure what you will find | |
| 21:13:44 | openstackgerrit | melanie witt proposed openstack/nova stable/pike: DNM: reproducing bug 1746509 https://review.openstack.org/555058 | |
| 21:13:46 | openstack | bug 1722404 in OpenStack Compute (nova) ocata "duplicate for #1746509 Database transactions can fail with "TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction" because of scatter_gather_cells" [Undecided,In progress] https://launchpad.net/bugs/1722404 - Assigned to Matt Riedemann (mriedem) | |
| 21:18:47 | dansmith | melwitt: ah, I'd be fine restricting init to only things in fields, that makes plenty of sense to me | |
| 21:19:04 | dansmith | melwitt: just not instance.does_not_exist = True | |
| 21:19:44 | mriedem | ^ is definitely something that tripped me up with something in the nova-network API code that relies on setting things on an object which aren't in fields, and reads that attribute in the REST API | |
| 21:19:57 | mriedem | can't remember which one specifically, but it was probably me that surprised dan about being surprised | |
| 21:20:07 | dansmith | heh | |
| 21:20:51 | mriedem | unrelated, but coincidental to the service / compute node delete and host discovery stuff lately https://bugs.launchpad.net/nova/+bug/1757207 | |
| 21:20:52 | openstack | Launchpad bug 1757207 in OpenStack Compute (nova) "compute resource providers not equal to compute nodes in deployment" [Undecided,Incomplete] | |
| 21:21:24 | sean-k-mooney[m] | dansmith: personally i think the best solution would be to add a mixin to ovo to all strict dield checks and then use it where it is correct to. | |
| 21:22:01 | sean-k-mooney[m] | *allow strict field checks | |
| 21:22:23 | dansmith | sean-k-mooney[m]: it's just not a thing I care much about, but if it's a mixin, then that's better than changing the behavior of the base object | |
| 21:22:53 | dansmith | they behave like any other python object right now, which seems more like I would expect | |
| 21:23:01 | dansmith | it just only version-tracks the things in fields | |
| 21:23:37 | dansmith | I agree we should only set things in init if they're in fields, and explode otherwise | |
| 21:24:41 | sean-k-mooney[m] | Ya the current behavior has helped us break some circular depency too so the current behavior makes sense as the default. | |
| 21:25:10 | tssurya | mriedem: the new bug is again the same thing right ? because a) RPs are not deleted and b)probably the compute service was not shut down before deleting ? so the compute_node record keeps getting recreated ? | |
| 21:25:48 | mriedem | mikal: nice topic https://review.openstack.org/#/q/topic:bp/execs-ive-had-a-few+(status:open+OR+status:merged) | |
| 21:25:52 | melwitt | dansmith: so it would be a mixin in ovo and then use the mixin only in the nova base object? just making sure I understand the best way to go about it | |
| 21:25:58 | mriedem | tssurya: yes | |
| 21:26:24 | tssurya | mriedem: just saw you updated the bug, cool | |
| 21:26:40 | dansmith | melwitt: well you couldn't do it in the nova base object because we utilize the fact that we can set non-field properties on some of our objects | |
| 21:26:50 | dansmith | melwitt: but you could mix it into certain objects if you want | |
| 21:27:09 | dansmith | melwitt: the init fix though could go to the base object in o.vo I think | |
| 21:27:11 | melwitt | I mean to restrict only when calling __init__ | |
| 21:27:13 | melwitt | oh, I see | |
| 21:27:22 | dansmith | oh no, that can co straight to the core as far as I'm concerned | |
| 21:27:37 | dansmith | although we might break some things by doing that, but they're broken now silently | |
| 21:27:38 | melwitt | the setting of non-field properties hasn't confused me before, it's only the passing them in __init__ that has | |
| 21:27:47 | melwitt | (for whatever that's worth :P) | |
| 21:27:58 | dansmith | I guess some library types might want a warning instead of a failure in a minor/rev release or something | |
| 21:28:01 | dansmith | melwitt: ah okay | |
| 21:28:09 | sean-k-mooney[m] | Melwitt probably 2 mixins. One for init check that could be in base nova object and one for all assignments that would not be in base nova object | |
| 21:28:09 | mikal | mriedem: you're welcome | |
| 21:28:13 | dansmith | like, mark it as deprecated, warn and then fail later | |
| 21:28:39 | dansmith | sean-k-mooney[m]: I dunno, we don't actually set the values from init if they're not in fields, do we? | |
| 21:29:05 | dansmith | ah I guess we do | |
| 21:29:09 | dansmith | well, I dunno, | |
| 21:29:21 | dansmith | a warning initially would be totally fine on the base object IMHO | |
| 21:29:31 | dansmith | and the make it required in a year on a version bump | |
| 21:29:44 | sean-k-mooney[m] | Dansmith apparently in the tests mel found | |
| 21:29:59 | dansmith | sean-k-mooney[m]: yeah, I just didn't think it was actually _setting_ it, but I see it is | |
| 21:30:18 | sean-k-mooney[m] | Yeah more warning in test output :) | |
| 21:30:28 | dansmith | well, then we fix it :) | |
| 21:30:44 | dansmith | we could do it in our nova object to pre-fix everything | |
| 21:31:21 | melwitt | yeah, I'm not clear on if everyone would want to warn/fail if they call init with non-existent fields. maybe someone likes a shortcut like that | |
| 21:31:52 | dansmith | *shrug* I guess | |
| 21:31:58 | sean-k-mooney[m] | Ya im sure you could phase it in gradually | |
| 21:32:13 | dansmith | it was my intent to allow free-form properties, not to allow anything in init(), that's my bias I guess :) | |
| 21:32:56 | melwitt | yeah, I had the same thinking too as a user of objects | |
| 21:33:20 | sean-k-mooney[m] | Melwitt well if they ever call make compatible or serialise the object those non fields are lost so you still have to be careful with thwm or you will get weird bugs | |
| 21:34:09 | melwitt | yeah. I agree it makes sense to make it more intentional when setting free-form properties, i.e. you have to do that outside of init | |
| 21:34:19 | melwitt | being that those will be lost as you said in those situations | |
| 21:35:38 | sean-k-mooney[m] | Yep. We don't really use the free form attributes that often but they can be useful sometimes | |
| 21:42:37 | openstackgerrit | Merged openstack/nova stable/pike: Handle volume-backed instances in IsolatedHostsFilter https://review.openstack.org/543603 | |
| 21:42:44 | openstackgerrit | Merged openstack/nova stable/pike: Fix docs for IsolatedHostsFilter https://review.openstack.org/543604 | |
| 21:42:51 | mriedem | so uh, | |
| 21:42:52 | mriedem | https://bugs.launchpad.net/nova/+bug/1755392 | |
| 21:42:53 | openstackgerrit | Merged openstack/nova master: Remove useless run_periodic_tasks call in ClientRouter https://review.openstack.org/554381 | |
| 21:42:53 | openstack | Launchpad bug 1755392 in OpenStack Compute (nova) "resize the instance is fail" [Undecided,New] | |
| 21:43:06 | mriedem | i thought there was a thing where you couldn't resize/migrate ephemeral/swap disks? | |
| 21:43:09 | mriedem | am i making that up? | |
| 21:45:41 | openstack | Launchpad bug 1755266 in OpenStack Compute (nova) "Instance resize with swap on cinder volume fails" [Undecided,New] | |
| 21:45:41 | mriedem | similar https://bugs.launchpad.net/nova/+bug/1755266 | |
| 22:02:09 | openstackgerrit | Julia Kreger proposed openstack/nova master: WIP: Add microversion to ironic client wrapper call https://review.openstack.org/554762 | |
| 22:02:34 | sean-k-mooney[m] | mriedem: i believe its undefined behavior if you change the number of ephemeral disk on resize. Migration of swap is fine. Resize is not | |
| 22:03:32 | sean-k-mooney[m] | Actually swap resize should be fine since we basically reboot | |
| 22:04:40 | openstackgerrit | Michael Still proposed openstack/nova master: Use a pythonic delete. https://review.openstack.org/554792 | |
| 22:04:40 | openstackgerrit | Michael Still proposed openstack/nova master: Remove duplicative implementation of temporary directories. https://review.openstack.org/554791 | |
| 22:04:41 | openstackgerrit | Michael Still proposed openstack/nova master: Use a pythonic delete, with a retry. https://review.openstack.org/554793 | |
| 22:05:30 | melwitt | mriedem, dansmith: finally tracked down what's going on in the bug where after upgrading the pike, if there are service records with no uuid (from the N-1 version), they get the "TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction" error | |
| 22:05:45 | melwitt | explained it here https://bugs.launchpad.net/nova/+bug/1746509/comments/9 | |
| 22:05:46 | openstack | Launchpad bug 1746509 in OpenStack Compute (nova) "TypeError: Can't upgrade a READER transaction to a WRITER mid-transaction" [Medium,Confirmed] | |
| 22:06:06 | mriedem | sean-k-mooney[m]: i seem to remember diana clarke trying to fix something wrt swap disks and resize, found https://github.com/dianaclarke/openstack-notes/wiki/resize-disks but not the thing she was trying to fix | |
| 22:06:09 | melwitt | the good news is it's no longer a bug in queens or rocky, but it is a bug in pike | |