| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-13 | |||
| 20:23:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update instance.availability_zone on revertResize https://review.openstack.org/643155 | |
| 20:23:20 | mriedem | dansmith: exhibit B ^ | |
| 20:23:43 | dansmith | that's an upcall no? | |
| 20:24:04 | dansmith | oh sorry | |
| 20:24:10 | dansmith | god I'm stupid, sorry | |
| 20:25:30 | mriedem | i can optimize this | |
| 20:25:36 | mriedem | left a comment | |
| 20:25:47 | dansmith | was about to call your typo | |
| 20:25:56 | mriedem | i know :) | |
| 20:26:01 | mriedem | i could feel it | |
| 20:26:37 | dansmith | so | |
| 20:26:47 | dansmith | talk me through this on your reqspec optimization | |
| 20:26:53 | dansmith | let's say... | |
| 20:27:00 | dansmith | I have an instance with no specific az, | |
| 20:27:09 | dansmith | I move it to another host, and it's mid-resize | |
| 20:27:18 | dansmith | or, waiting for confirm/revert I mean | |
| 20:27:33 | dansmith | the operator could change the az affiliation of that host in the middle right? | |
| 20:27:46 | mriedem | of the source? | |
| 20:27:50 | dansmith | of either | |
| 20:28:01 | mriedem | not the target, | |
| 20:28:02 | dansmith | it wouldn't be smart, but.. | |
| 20:28:08 | mriedem | because of avolkov's recent fix | |
| 20:28:15 | mriedem | but they could rename the az of the source yeah | |
| 20:28:45 | dansmith | barring the source change would be hard, looking up instances that still have residue on hosts | |
| 20:28:48 | dansmith | anyway, | |
| 20:28:54 | dansmith | seems like maybe not worth the optimization, | |
| 20:29:09 | dansmith | if it's still possible to be wrong but creates a much more obscure path to get there | |
| 20:30:07 | mriedem | right his change only deals with the az that contains the instance based on instance.host https://review.openstack.org/#/c/641351/1/nova/objects/instance.py | |
| 20:30:13 | mriedem | which would be the target at the time of the VERIFY_RESIZE status | |
| 20:30:23 | mriedem | i'm fine with not trying to optimize it | |
| 20:30:47 | mriedem | i could leave a comment to that effect as well if it's something we think we'll forget and care about in a year :) | |
| 20:31:31 | dansmith | I say just don't optimize it now, regardless of how you spin the comments | |
| 20:31:35 | mriedem | melwitt: you were +2 on this before https://review.openstack.org/#/c/567701/ if you can check it again, i think it was just rebased | |
| 20:32:00 | melwitt | mriedem: sure | |
| 20:32:16 | mriedem | dansmith: ok if you're ok with it as is i can just leave it alone, or fix the typo in the commit message | |
| 20:32:27 | dansmith | oh I'm not fine with the typo | |
| 20:32:29 | dansmith | you must fix that | |
| 20:32:33 | mriedem | ok then | |
| 20:32:49 | dansmith | there, made explicit | |
| 20:33:10 | dansmith | I'll also explain why no opt I gess | |
| 20:34:18 | mriedem | actually, | |
| 20:34:26 | mriedem | no, | |
| 20:34:33 | dansmith | mriedem: if I'm using default_availability_zone, I can change the target's affiliation with a restart right? | |
| 20:34:36 | mriedem | if RequestSpec.availability_zone is not None, the server *has* to migrate within the AZ, | |
| 20:34:56 | mriedem | meaning while the server is in VERIFY_RESIZE, it's in the same az on the dest host as the source host, so the admin can't rename the az | |
| 20:35:52 | mriedem | and if RequestSpec.availability_zone is None, then we'd end up updating the instance.availability_zone anyway | |
| 20:35:59 | mriedem | i mean with my optimization | |
| 20:36:10 | dansmith | yeah, but what about my CONF-based attack? | |
| 20:36:37 | dansmith | two hosts, both in default_availability_zone=foo, then I can change that conf to =bar on one and restart it and you'll do the wrong thing | |
| 20:36:49 | dansmith | (and instance.az will be wrong) | |
| 20:38:27 | mriedem | yeah i suppose | |
| 20:38:56 | dansmith | it just doesn't seem like enough optimization to worry about | |
| 20:39:01 | dansmith | to me anyway | |
| 20:39:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update instance.availability_zone on revertResize https://review.openstack.org/643155 | |
| 20:39:59 | mriedem | where is jaypipes when i want to hear someone say we should just delete availability zones as a concept from nova | |
| 20:40:20 | dansmith | pfft, he'll say that about anything | |
| 20:40:45 | edleafe | (uses my jaypipes imitation) Availability zones suck! Let's just get rid of them! | |
| 20:41:10 | mriedem | * suck! Let's just get rid of them! | |
| 20:41:11 | mriedem | volumes | |
| 20:41:12 | mriedem | quotas | |
| 20:41:14 | mriedem | everything | |
| 20:41:23 | mriedem | *root volumes | |
| 20:41:34 | dansmith | instances kinda suck too | |
| 20:41:37 | dansmith | we should nix those | |
| 20:48:08 | openstackgerrit | Merged openstack/nova master: Trivialfix for help description of images_type https://review.openstack.org/642942 | |
| 20:48:18 | openstackgerrit | Merged openstack/nova stable/rocky: Fix an error when generating a host ID https://review.openstack.org/641173 | |
| 20:48:27 | openstackgerrit | Merged openstack/nova stable/rocky: Move legacy-grenade-dsvm-neutron-multinode-live-migration in-tree https://review.openstack.org/640191 | |
| 20:48:35 | openstackgerrit | Merged openstack/nova stable/rocky: Fix resource tracker updates during instance evacuation https://review.openstack.org/640982 | |
| 21:00:08 | openstackgerrit | Merged openstack/nova master: Add missing libvirt exception during device detach https://review.openstack.org/641480 | |
| 21:03:18 | openstackgerrit | Merged openstack/nova stable/queens: Make service all-cells min version helper use scatter-gather https://review.openstack.org/637594 | |
| 21:03:24 | openstackgerrit | Merged openstack/nova master: api-ref: Add description for BDM volume_size https://review.openstack.org/641255 | |
| 21:03:34 | openstackgerrit | Merged openstack/nova master: docs: Fix nits in remote console guide https://review.openstack.org/641098 | |
| 22:11:00 | gmann | legacy base jobs to Bionic are merged and nova-next job failing, hold the recheck until this workaround is merged (in gate) - https://review.openstack.org/#/c/643129/3 | |
| 22:11:08 | gmann | sent on ML too. | |
| 22:17:29 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Update instance.availability_zone during live migration https://review.openstack.org/643173 | |
| 22:45:31 | fungi | melwitt: oops, i missed that https://review.openstack.org/642940 had a depends-on to the demonstration patch. that commit message needs to be amended to remove the depends-on line | |
| 22:45:51 | fungi | mriedem_afk: dansmith: ^ | |
| 22:48:51 | efried | fungi, melwitt, mriedem_afk, dansmith: I'll take care of it, stand by... | |
| 22:49:07 | fungi | thanks!!! | |
| 22:49:07 | openstackgerrit | Eric Fried proposed openstack/nova master: Update nova-tox-functional-py35 parent job https://review.openstack.org/642940 | |
| 22:49:14 | fungi | and sorry for missing that in review | |
| 22:49:35 | efried | fungi: done | |
| 22:49:43 | efried | I noticed that, but I thought it was intentional | |
| 22:49:54 | efried | like, I thought the other patch was going to be non-DNM'd soon | |
| 22:50:02 | efried | or something | |
| 22:50:10 | efried | I didn't read the backstory closely at all | |
| 22:50:19 | fungi | nope, it was entirely a demonstration. apologies for the confusion | |
| 22:50:51 | fungi | we couldn't actually show the effect of the change without a depends-on chain to an alternate universe base job | |
| 22:51:06 | fungi | at least not until the actual base job change merged | |
| 22:52:07 | efried | and with that, I'm outta here o/ | |
| 22:52:07 | efried | ack | |
| 23:25:13 | openstackgerrit | Merged openstack/nova master: Update instance.availability_zone during live migration https://review.openstack.org/567701 | |
| 23:25:24 | openstackgerrit | Merged openstack/nova master: Skip the ceph based live migration testing https://review.openstack.org/643122 | |
| 23:40:27 | gmann | fungi: efried we have cross-dependency to unblock the gate. 643129 cannot merge which need 642940 and vice versa. | |
| 23:41:17 | fungi | possible we'll need to squash those | |
| 23:41:26 | gmann | fungi: can we get this forcefully in - 642940? or i can make temp patch to unblock either one | |
| 23:43:13 | fungi | they'll be properly tested if we squash the two commits together, but would need a nova-core on hand to approve the resulting revision of whichever change we squash into | |
| 23:43:54 | gmann | ok, let me squash first then i can catch someone alex_xu might be coming online soon | |
| 23:47:18 | fungi | i'm just veeery hesitant to bypass ci for a change where it's not strictly necessary | |
| 23:47:36 | fungi | pretty much every time i do, i break something a job would have caught | |