| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-15 | |||
| 13:45:52 | sean-k-mooney | but i also found we do not resize the image and as a result cloud init does not run | |
| 13:46:33 | sean-k-mooney | * we might resize the image but we dont resize the filesystem in the image | |
| 13:47:17 | gmann | alex_xu: cc for ^^ policy default refresh changes review and early feedback- http://lists.openstack.org/pipermail/openstack-discuss/2019-August/008504.html | |
| 13:47:50 | gmann | mriedem: can you check this now, added functional tests - https://review.opendev.org/#/c/676275/ | |
| 13:48:01 | sean-k-mooney | mriedem: so tl;dr if you have a image preprepared with enough space for cloud-init to run we might be oke but i dont know if cloud inti can resize the file system when dealing with lxc | |
| 13:55:28 | openstackgerrit | Chris Dent proposed openstack/nova master: DNM: add profiling to scheduler https://review.opendev.org/674810 | |
| 14:02:10 | yonglihe | mriedem: I got Alex's +2, you recently reviewed this patch, Hope you had bandwidth to review it again: https://review.opendev.org/#/c/621476/ | |
| 14:02:30 | efried | melwitt: Thank you for sending that. The u-c bump merged as well. That opens up several improvements in nova. | |
| 14:12:34 | mordred | efried: did your mock.patch thing wind up working? | |
| 14:12:42 | efried | yes! | |
| 14:12:43 | gibi | mriedem: ack, I try to look at it today | |
| 14:13:12 | efried | mordred: which excites me not because it's a lovely solution, but because it lets me use all those shiny new features in nova now. | |
| 14:13:44 | sean-k-mooney | efried: you are not useing mock.patch outside of tests are you? | |
| 14:13:45 | efried | which I will start doing just as soooon as I wake up. | |
| 14:14:00 | efried | sean-k-mooney: Why, would that be a bad thing? | |
| 14:14:07 | efried | it's exactly the same as temporary_mutation | |
| 14:14:24 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Remove nova.image.download.modules extension point https://review.opendev.org/676540 | |
| 14:14:24 | openstackgerrit | Mohammed Naser proposed openstack/nova master: config: remove deprecated checksum options https://review.opendev.org/676539 | |
| 14:14:25 | openstackgerrit | Mohammed Naser proposed openstack/nova master: requirements: move to using psycopg2-binary https://review.opendev.org/676684 | |
| 14:14:35 | efried | sean-k-mooney: I'm yanking your chain. We're talking about this https://review.opendev.org/676495 | |
| 14:14:52 | sean-k-mooney | because you are monkeypatchin other code at runtime which can fail in really hard to debug ways | |
| 14:15:03 | sean-k-mooney | :) | |
| 14:15:47 | sean-k-mooney | ok that is less scary | |
| 14:16:46 | cdent | sigh: the nova functional tests are _still_ reading /etc/nova/nova.conf (if it exists). I thought we fixed that? | |
| 14:17:07 | sean-k-mooney | what test? | |
| 14:17:19 | sean-k-mooney | its proably missing a config fixture | |
| 14:18:03 | tssurya | mriedem, dansmith: hmm I was thinking for the UnexpectedTaskStateError exception for the power-update event, should we also handle the faults/error notifications in case its actually another action that won the race ? | |
| 14:18:03 | aspiers | AFAICS it just doesn't do what it claims to | |
| 14:18:26 | tssurya | as of now if we simply return the event is considered as a successful attempt | |
| 14:19:25 | dansmith | tssurya: I'm not sure what you're asking | |
| 14:20:30 | openstackgerrit | Chris Dent proposed openstack/nova master: single pass instance info fetch in host manager https://review.opendev.org/623558 | |
| 14:20:40 | mriedem | ose details. | |
| 14:20:40 | mriedem | yonglihe: i can review again based on my previous comments but i likely won't be approving that change, because (1) we're trying to land tssurya's change for 2.76 since there is an approved ironic change that depends on the nova change (see the ML) and (2) i can't vote on the low-level contents of what's going into the numa topology response, i think dansmith or others that were more involved in the spec approval should revie | |
| 14:21:08 | mriedem | sean-k-mooney: ack on the lxc thing; we can define an image to use in the job, but if that hard-coding in the libvirt driver is a problem we'll need to do something about that. | |
| 14:21:42 | mriedem | sean-k-mooney: i'm not sure how much we care about cloud-init in the lxc job, but someone does have some patches up for making cloud-init work with lxc (so saying that, it might not work today anyway) | |
| 14:21:56 | tssurya | dansmith: ah nvm, I got the answer | |
| 14:21:58 | mriedem | sean-k-mooney: yeah this https://review.opendev.org/#/c/667976/ | |
| 14:22:05 | tssurya | :D sorry for the noise | |
| 14:22:13 | dansmith | tssurya: heh, okay, sorry | |
| 14:22:13 | openstackgerrit | Eric Fried proposed openstack/nova master: Bump minimum ksa (3.16.0) and sdk (0.34.0) https://review.opendev.org/676707 | |
| 14:22:16 | cdent | mriedem: I reckon that single pass instance info thing ^ is a real winner. I've removed some dead code (I think jay had future plans) but I'm a bit lost on what to do test wise, so if you have ideas, I've got ears. | |
| 14:22:21 | sean-k-mooney | "/sbin/init" has been the location of the init script for decades so i think cirros just need to add a symlink | |
| 14:22:29 | dansmith | sean-k-mooney: I have that open in a tab, working on trying to focus on it for a bit... is there a change coming? | |
| 14:22:34 | sean-k-mooney | im more worried about the symlink | |
| 14:22:40 | sean-k-mooney | *filesystem | |
| 14:22:59 | sean-k-mooney | e.g not resizing it before starting the container | |
| 14:23:20 | sean-k-mooney | i dont know if you can resize it inside the container and a spent a while trying to do it last night and failed | |
| 14:23:34 | efried | aspiers: what are you trying to do with PropertyMock? | |
| 14:23:50 | aspiers | trying to patch objects.Instance.image_meta to return my image_meta fixture | |
| 14:24:24 | mriedem | cdent: fwiw track_instance_changes might be true in grenade jobs, would need to check, but grenade jobs don't run in superconductor mode | |
| 14:24:25 | sean-k-mooney | dansmith: sorry context are you refering to stephens chagnes? | |
| 14:24:25 | aspiers | but it just returns a mock object instead | |
| 14:24:30 | sean-k-mooney | or something else | |
| 14:24:44 | dansmith | sean-k-mooney: gah, sorry that was supposed to be aimed at tssurya | |
| 14:24:50 | sean-k-mooney | ah | |
| 14:24:51 | aspiers | calling instance.image_meta() returns the fixture I want, but not without the () | |
| 14:24:59 | tssurya | dansmith: ok no wait I didn't get the answer :( . So in the API we do the action_start thing which creates the instance-action event, however when we specially handle the UnexpectedTaskStateError (so that we can continue to process other events) in the manager, the "compute_power_update' event is recorded as a success | |
| 14:25:00 | cdent | mriedem: there's something in devstack comments about that, lemme refresh memory | |
| 14:25:10 | aspiers | it's infuriating, been staring for hours and can't see what I'm doing wrong | |
| 14:25:13 | tssurya | which is wrong right ? | |
| 14:25:31 | dansmith | tssurya: ah, yeah, can we record that as fail instead in the exception handler? | |
| 14:25:38 | mriedem | cdent: having said that, i'm crunched for time today, leaving this afternoon for a long weekend | |
| 14:25:59 | sean-k-mooney | mriedem: ill look at the cloud-init patch but without it we are not doing the ssh key injection or creating a user so you would have to build all that into the image | |
| 14:26:00 | cdent | i don't think there's any rush on this, just think it is definitely worth bring back from the dead | |
| 14:26:02 | tssurya | dansmith: yea probably, let me check what does that | |
| 14:26:29 | dansmith | tssurya: it would be super good if we can get mriedem's ack on that patch before he leaves and then you leave | |
| 14:26:50 | tssurya | wait mriedem is also going on vacation soon ? | |
| 14:27:06 | dansmith | for a day yeah | |
| 14:27:40 | tssurya | hmm, ok I am just a bit confused with instance faults/actions/error notification handling for the UnexpectedTaskStateError part | |
| 14:28:21 | mriedem | if you're trapping that error, there will be no fault | |
| 14:28:24 | mriedem | or error notification | |
| 14:28:28 | dansmith | oh | |
| 14:28:32 | dansmith | yeah, mriedem just answered | |
| 14:28:36 | mriedem | you'll only record a fault or send an error notification if something raises, like NotImplementedError | |
| 14:28:58 | mriedem | @wrap_instance_event will record a start and finish event on entry and exit of the method regardless of how it exits | |
| 14:29:10 | mriedem | the event gets record on the instance action (Created in the api) keyed by instance uuid and context.request_id | |
| 14:29:10 | dansmith | you can log and re-raise maybe? | |
| 14:29:19 | mriedem | *recorded | |
| 14:29:52 | tssurya | mriedem: yea but since I do a return from the preemption if any it records a success I guess ? | |
| 14:30:32 | mriedem | yes, it will say it was successful | |
| 14:30:51 | mriedem | https://github.com/openstack/nova/blob/872a823d9a02f31e266882bfb499673c51fb4075/nova/compute/utils.py#L1302 | |
| 14:31:13 | mriedem | https://github.com/openstack/nova/blob/872a823d9a02f31e266882bfb499673c51fb4075/nova/objects/instance_action.py#L184 | |
| 14:31:36 | aspiers | efried: *finally* figured it. patch(..., new_callable=mock.PropertyMock), *not* patch(..., new_callable=mock.PropertyMock(name='fake_prop')) | |
| 14:31:41 | mriedem | i think i mentioned something in my earlier big round of comments that the functional test should assert the events are recorded as expected for the compute_power_update event | |
| 14:32:00 | tssurya | mriedem: yea I added that | |
| 14:32:20 | tssurya | but this should not be a success for the task_state race error right ? | |
| 14:32:38 | mriedem | idk | |
| 14:32:52 | mriedem | yesterday i thought we said that we'd handle UnexpectedTaskStateError, log and return | |
| 14:33:02 | mriedem | if you want to record the action as failed, then you'd log and re-raise | |
| 14:33:08 | yonglihe | mriedem: ok if i need to bump up minor version, i gonna do that, but thanks. | |
| 14:33:13 | tssurya | ok yea | |
| 14:33:18 | tssurya | I'll do that | |
| 14:33:23 | dansmith | mriedem: I didn't grasp that detail about letting it go | |
| 14:33:24 | mriedem | well, what does dansmith think | |
| 14:33:27 | mriedem | oh | |
| 14:33:39 | mriedem | note that ironic at this point has got a response and moved on | |
| 14:33:52 | dansmith | ironic doesn't care, | |
| 14:33:54 | mriedem | so this is only useful for an admin debugging that power-update exchange and if it worked | |
| 14:33:56 | dansmith | this is more like what a user would see | |
| 14:34:03 | dansmith | or the user itself right? | |