Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-27
13:36:21 gibi mriedem: you mean if we see allocaton key in the cache then we assume that such port doesn't need to be healed?
13:36:22 mriedem i do like that you added an option to skip this though
13:37:04 mriedem gibi: something like that
13:37:41 mriedem if we would have stored some flag in the vif cache originally that indicated the port had resource requests (not necessarily the resource request itself), heal_allocations could have just checked that flag in the cache and then call neutron for that port
13:38:28 mriedem and if ifs and buts were candy and nuts we'd all have a very fine christmas
13:38:55 gibi mriedem: these vifs are created _before_ there was any logic in nova about bandwidth resource. So I don't see how to have a flag in nova about it
13:39:20 mriedem oh right this is also for healing the case of ports attached with qos before stein...
13:39:22 mriedem i forgot about that
13:39:41 gibi this is the case exaclty that the port was attached without allocation
13:39:54 mriedem anyway, i'm trying to optimize something that we just want to work on the first go, so i can drop it
13:39:57 gibi because neutron had support way earlier for bandiwdth
13:40:50 mriedem the other good thing is that heal_allocations now has the option to heal a single instance so if an operator is trying to fix one specific instance (b/c of some user ticket or something) then they can just target that one rather than all instances to heal 1
13:41:40 gibi yeah, I remember I had to adapt to that improvement at some point
13:42:52 gibi one extra complexity to note. as we need to update both placement and neutron the update cannot be made atomic
13:43:04 mriedem and i'm assuming the earlier patches to refactor the code didn't really require changes to the functional tests the validate the interface (unlike you'd need with unit tests)
13:43:31 mriedem i wrote all of the heal_allocations stuff with functional tests b/c i didn't trust unit tests for testing the interface
13:43:46 gibi mriedem: yes, no functional test was harmed in the refactoring process
13:43:52 mriedem if we fail to update one (placement or neutron) we could rollback the other...
13:44:03 mriedem heh, "no functional test was harmed in the making of this patch"
13:44:37 mriedem ok this has been a useful discussion before i dive in
13:44:39 mriedem thanks
13:45:06 gibi mriedem: yes, rollback is one option. I went for just printing what failed and how to update neutron manually
13:46:05 gibi mriedem: if you think rollback is better then I have to temprarly store the original allocation of the instance before the heal, and put that back if neutron update fails
13:50:05 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Multiple API cleanup changes https://review.opendev.org/666889
13:51:14 mriedem we could also retry the port updates with a backoff loop, like 3 retries or something if it's a temporary network issue
13:51:19 mriedem could be follow ups though
13:51:35 mriedem i'll keep it in mind when i review and leave a comment
13:54:42 gibi mriedem: ack
14:02:36 mriedem nova meeting happening
14:10:21 mriedem gibi: also in the back of my mind i've been thinking of adding something to the nova-next job post_test_hook, like creating a server, deleting it's allocatoins in placement and then running heal_allocations just to make sure we have integration test coverage as well, but it's lower priority
14:11:22 gibi mriedem: I did similar manual testing for my patch in devstack and discovered bugs. So I agree that it would be useful
14:20:27 jrosser guilhermesp: this lot https://review.opendev.org/#/q/topic:fix-octavia+(status:open+OR+status:merged)
14:20:54 jrosser guilhermesp: there are some jobs running with some depends-on, but it's difficult to see whats going on
14:21:34 jrosser oops -ECHAN
14:50:10 openstackgerrit Surya Seetharaman proposed openstack/nova stable/stein: Grab fresh power state info from the driver https://review.opendev.org/667948
14:54:35 mriedem http://status.openstack.org/reviews/#nova sure is fun for abandon fodder
14:55:05 openstackgerrit Vrushali Kamde proposed openstack/nova master: Support filtering of hosts by forbidden aggregates https://review.opendev.org/667952
14:58:33 efried abandon away
15:09:07 openstackgerrit Surya Seetharaman proposed openstack/nova stable/rocky: Grab fresh power state info from the driver https://review.opendev.org/667955
15:24:55 openstackgerrit Matt Riedemann proposed openstack/nova master: RT: replace _instance_in_resize_state with _is_trackable_migration https://review.opendev.org/560467
15:25:18 mriedem efried: another rebase on that one ^
15:25:39 mriedem dansmith: can you come back on this instance.hidden patch https://review.opendev.org/#/c/631123/ ?
15:26:06 dansmith <insert hidden pun here>
15:26:13 mriedem dansmith.hidden = True
15:27:40 efried mriedem: I can't even see where the conflict was on that one.
15:28:46 efried oh, never mind
15:30:52 dansmith mriedem: is the hidden check in the api really required? aren't you filtering out hidden instances from the list query?
15:31:35 mriedem dansmith: i'd have to look again to confirm but i believe there is a window where both are not hidden
15:31:38 mriedem while swapping over
15:31:50 mriedem which,
15:31:52 dansmith mriedem: in that case, the check for hidden-ness doesn't help right?
15:32:00 mriedem arguably the api will still filter - it might pick the "wrong" one but...
15:32:13 mriedem the comment in there mentions something about that right? updated_at and such
15:32:14 dansmith right, but the check of the hidden field would be pointless in that case
15:32:56 dansmith the comment is talking about the case where they're both not hidden
15:33:10 dansmith I'm talking about the case where one is.. what's the point of checking it if instance_list doesn't return them?
15:34:14 mriedem ok so this is the point where we could have 2 copies where hidden=False https://review.opendev.org/#/c/635646/32/nova/conductor/tasks/cross_cell_migrate.py@593 so the DB API would return both from each cell while listing
15:34:37 mriedem now that DB API isn't returning hidden=True by default...
15:34:42 dansmith ...and so checking for instance.hidden in compute/api does what?
15:35:58 mriedem yeah in this case now b/c db api won't return the hidden one, "or instance.hidden" will always be false
15:36:06 dansmith right
15:36:22 mriedem the note above still applies, but the logical or doesn't
15:36:42 mriedem so are you ok with removing the or condition and leaving the comment?
15:37:02 dansmith yep, just said that in the review
15:37:33 mriedem yup, thanks
15:48:24 openstackgerrit melanie witt proposed openstack/nova master: Require at least cryptography>=2.7 https://review.opendev.org/667765
16:09:17 mriedem dansmith: i'm going to drop https://review.opendev.org/#/c/631123/34/nova/tests/unit/compute/test_compute_api.py then since it's not really valid since the db api wouldn't return a hidden=True instance
16:09:27 dansmith yeah
16:13:42 openstackgerrit Miguel Ángel Herranz Trillo proposed openstack/nova master: Add support for 'initenv' elements https://review.opendev.org/667975
16:13:42 openstackgerrit Miguel Ángel Herranz Trillo proposed openstack/nova master: Add support for cloud-init on LXC instances https://review.opendev.org/667976
16:15:40 mriedem looks like someone is trying to make libvirt+lxc work again
16:16:33 openstackgerrit Miguel Ángel Herranz Trillo proposed openstack/nova master: Add support for cloud-init on LXC instances https://review.opendev.org/667976
16:21:24 openstackgerrit Merged openstack/nova-specs master: support virtual persistent memory https://review.opendev.org/601596
16:30:48 efried mriedem: At some point recently you wrote a functional test that used a weigher to prefer host1 so that the assertion that we landed on host2 was provably valid every time (instead of just by chance). Can you put your finger on that easily?
16:31:30 mriedem efried: look for HostNameWeigher in the nova/tests/functional
16:31:32 mriedem there are several examples
16:31:39 efried thanks
16:31:54 mriedem ima push this big ass series b/c i've been rebasing it locally for weeks and want to flush it
16:33:20 openstackgerrit Matt Riedemann proposed openstack/nova master: Add InstanceAction/Event create() method https://review.opendev.org/614036
16:33:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add Instance.hidden field https://review.opendev.org/631123
16:33:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add TargetDBSetupTask https://review.opendev.org/627892
16:33:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Add CrossCellMigrationTask https://review.opendev.org/631581
16:33:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Execute TargetDBSetupTask https://review.opendev.org/633853
16:33:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Add prep_snapshot_based_resize_at_dest compute method https://review.opendev.org/633293
16:33:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Add PrepResizeAtDestTask https://review.opendev.org/627890
16:33:24 openstackgerrit Matt Riedemann proposed openstack/nova master: Add prep_snapshot_based_resize_at_source compute method https://review.opendev.org/634832
16:33:24 openstackgerrit Matt Riedemann proposed openstack/nova master: Add nova.compute.utils.delete_image https://review.opendev.org/637605
16:33:25 openstackgerrit Matt Riedemann proposed openstack/nova master: Add PrepResizeAtSourceTask https://review.opendev.org/627891
16:33:25 openstackgerrit Matt Riedemann proposed openstack/nova master: Refactor ComputeManager.remove_volume_connection https://review.opendev.org/642183
16:33:26 openstackgerrit Matt Riedemann proposed openstack/nova master: Add power_on kwarg to ComputeDriver.spawn() method https://review.opendev.org/642590
16:33:26 openstackgerrit Matt Riedemann proposed openstack/nova master: Add finish_snapshot_based_resize_at_dest compute method https://review.opendev.org/635080
16:33:27 openstackgerrit Matt Riedemann proposed openstack/nova master: Add FinishResizeAtDestTask https://review.opendev.org/635646
16:33:27 openstackgerrit Matt Riedemann proposed openstack/nova master: Add Destination.allow_cross_cell_move field https://review.opendev.org/614035
16:33:28 openstackgerrit Matt Riedemann proposed openstack/nova master: Execute CrossCellMigrationTask from MigrationTask https://review.opendev.org/635668
16:33:28 openstackgerrit Matt Riedemann proposed openstack/nova master: Plumb allow_cross_cell_resize into compute API resize() https://review.opendev.org/635684
16:33:29 openstackgerrit Matt Riedemann proposed openstack/nova master: Filter duplicates from compute API get_migrations_sorted() https://review.opendev.org/636224
16:33:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Add cross-cell resize policy rule and enable in API https://review.opendev.org/638269
16:33:40 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Enable cross-cell resize in the nova-multi-cell job https://review.opendev.org/656656
17:00:02 efried Anyone know artom's status?
17:00:15 efried seemed like he was traveling recently

Earlier   Later