| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 20:13:08 | dansmith | what if we just defined a pattern like volume-bdm-$devname and auto-tag BDMs from the image, and then you can use the tag on the bdm command line to override those? | |
| 20:13:18 | dansmith | I mean, I know it's kinda secret sauce, but so is what we have today | |
| 20:13:20 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM Driver: Localdisk https://review.openstack.org/549300 | |
| 20:13:31 | dansmith | we could actually document it though going forward | |
| 20:13:33 | mriedem | this is the merge-a-roo https://github.com/openstack/nova/blob/master/nova/compute/api.py#L670 | |
| 20:13:40 | dansmith | and avoid the surprise later when we all forget gain | |
| 20:14:07 | dansmith | that that suck-a-roo-s | |
| 20:14:10 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: Resize https://review.openstack.org/553583 | |
| 20:15:03 | dansmith | the ML thread made it sound like he was amenable to something else if you provided it | |
| 20:15:22 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: Resize https://review.openstack.org/553583 | |
| 20:15:46 | dansmith | presumably the client (ec2api in this case) could also just get the properties from the image, and generate a fresh new set of BDMs that are the union of the two, if we convert over to not merging in some microversion | |
| 20:17:20 | mriedem | yeah they'd have to do the full override client side | |
| 20:17:32 | dansmith | yeah, and what's wrong with that? | |
| 20:17:49 | dansmith | it'd suck for someone using novaclient, but not so much for ec2api using REST | |
| 20:18:30 | dansmith | what does horizon do if you have an image with BDMs in it? does it properly show what your instance is going to look like pre-boot? | |
| 20:19:54 | mriedem | not sure, don't have a devstack handy | |
| 20:20:04 | mriedem | oh, but, i have a vexxhost handy :) | |
| 20:20:38 | mriedem | also, if we stopped doing the merging in a new microversion that drops device name, the client has two options: use an older microversion for the server create request and hope they didn't need a newer microversion, or merge client side | |
| 20:21:18 | mriedem | nvm i don't know if vexxhost has any images with BDMs in them | |
| 20:21:34 | dansmith | mriedem: did you look or find any documentation that advertises this as a thing? | |
| 20:22:13 | mriedem | https://docs.openstack.org/nova/pike/user/block-device-mapping.html#intermezzo-problem-with-device-names | |
| 20:22:20 | mriedem | "Currently (mid Liberty) users are discouraged from specifying device names for all calls requiring or allowing block device mapping, except when trying to override the image block device mapping on instance boot, and it will likely remain like that in the future. " | |
| 20:24:07 | dansmith | hrm | |
| 20:24:33 | mriedem | that's about all i can find | |
| 20:24:50 | mriedem | from that ML thread: http://lists.openstack.org/pipermail/openstack-dev/2017-April/114866.html | |
| 20:24:59 | mriedem | ess important for pure OpenStack users." | |
| 20:24:59 | mriedem | this is also timely: "Well, i cannot estimate the importance in absolute measurement, but in comparison with OpenStack this use case is more important in AWS. Volume backed images (EBS images) are used in AWS much more widely than in OpenStack. There are some difficulties in Nova and Cinder because that users try to avoid using volume backed images in favor of disk based (instance-store) ones. This explain why this use case | |
| 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 | |