Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-02-18
14:16:22 sean-k-mooney so between the time os-vif plugs the vif and the managner updates the db to set the stat to active if we recive a delete request we dont clean up properly
14:33:32 dansmith mriedem: tssurya: we should probably have some stuff in the docs around best practices for effectively tolerating down cells.. like db and mq timeout settings
14:37:32 mriedem dansmith: e.g. https://bugs.launchpad.net/nova/+bug/1815697 ?
14:37:33 openstack Launchpad bug 1815697 in OpenStack Compute (nova) "[upgrade_levels]compute=auto grinds the API response times when a cell is down" [Medium,Confirmed]
14:37:45 dansmith mriedem: yeah and that
14:37:56 dansmith very op-focused docs
14:38:40 mriedem sure, that would go in https://docs.openstack.org/nova/latest/admin/
14:39:04 mriedem https://review.openstack.org/#/c/635147/ is already pretty big, so i'd suggest we split that off
14:39:13 dansmith yep
14:39:46 NewBruce sean-k-mooney wrt your open question above, i think there are a few paths that don’t clean up when an error occurs
14:40:12 NewBruce for instance, the issue i’m debugging which we were discussing last week is one as well -
14:40:30 sean-k-mooney NewBruce: yes in this case i dont think we ever call the drivers destroy or cleanup fucntions
14:41:14 NewBruce yep, and in some cases it tends to leave allocations - which i suspect will eventually play havoc with the scheduler/placement
14:41:37 NewBruce def _update_port_with_migration_profile(
14:41:37 NewBruce self, instance, port_id, port_profile, admin_client):
14:41:37 NewBruce try:
14:41:37 NewBruce updated_port = admin_client.update_port(
14:41:37 NewBruce port_id, {'port': {BINDING_PROFILE: port_profile}})
14:41:37 NewBruce return updated_port
14:41:37 NewBruce except Exception as ex:
14:41:38 NewBruce with excutils.save_and_reraise_exception():
14:41:38 NewBruce LOG.error("Unable to update binding profile "
14:41:39 NewBruce "for port: %(port)s due to failure: %(error)s",
14:41:39 NewBruce {'port': port_id, 'error': ex},
14:41:40 NewBruce instance=instance)
14:41:58 sean-k-mooney in this case the allcoation are being cleaned up but its not cleaning upu the network ports
14:42:04 sean-k-mooney e.g. unplginning the vifs
14:42:18 NewBruce Aha - ok
14:42:44 sean-k-mooney im not sure if we are cleaning up disk image ectra either so i think just calling self.driver.cleanup would be the simplest solution
14:43:31 NewBruce sean-k-mooney will you in the channel for a while? ill be back in a few hours and if possible would love to continue our chat from last week - ive got some extra debug info which i can’t make sense of
14:43:57 sean-k-mooney yes ill be around for anohter few hours
14:49:28 stephenfin kashyap: Is there an open bug for s390x before this patch goes in so? https://review.openstack.org/#/c/632507/12
14:50:39 stephenfin kashyap: i.e. if you have a new enough version of libvirt, you'll end up with the duplicate pty devices?
14:58:09 mriedem dansmith: in addition to config i'd think you'd want to disable any down cells via the cli and sighup the scheduler service so we don't waste time trying to pull compute nodes from them for scheduling
14:58:26 mriedem with that bug above i was wondering if filtering disabled cells could be more generally applied
14:58:30 dansmith mriedem: yep, probs
14:58:44 mriedem the min service version check specifically
15:03:56 kashyap stephenfin: No open bug is required.
15:04:08 kashyap Because:
15:06:58 kashyap stephenfin: Scratch that; I need to determine if we really one. But having one is always useful, though.
15:07:31 stephenfin kashyap: It's mostly because if there is a bug, it would be better fix that in a separate precursor patch so that we can backport that wholesale
15:08:04 kashyap stephenfin: No, there is no pre-existing bug. I discovered this the hard way (from zKVM CI), after the version bump analysis
15:08:53 kashyap stephenfin: I see what you mean, though -- I'd have _really_ preferred this to be a separate change ("one logical change per commit"). But I don't think we can do it in this instance - as it's tied to version bump :-(
15:09:20 kashyap (Aside: Do we have any active s390x contributors?)
15:13:58 mriedem no
15:16:32 kashyap Nod. In this case, we're reasonably safe: I checked with the s390x maintainer (for 10-ish years) in QEMU, and she said what I'm doing is sane, FWIW.
15:17:25 kashyap s/reasonably// :D ("Avoid tame, colorless, hesitating, noncommittal language.")
15:19:15 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: Ignore VolumeAttachmentNotFound exception in compute.manager https://review.openstack.org/637391
15:19:50 mriedem need another core on this api-ref change https://review.openstack.org/#/c/635315/ to unblock this code fix https://review.openstack.org/#/c/509206/ for the longstanding "don't allow az renames when the az has instances in it"
15:26:13 gibi mriedem: I'm looking at the heal_allocation code and I'm wondering how to handle when more than on thing is missing
15:26:16 gibi (e.g. project_id and some port allocation) from the instance allocation:
15:26:22 gibi a) do a separate PUT /allocations/{instance.uuid} for each step
15:26:24 kashyap mriedem: When you get a moment, on this: https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/cpu-model-selection.html
15:26:29 gibi b) generate all the missing allocation of the instance and
15:26:44 gibi do a single PUT /allocations/{instance.uuid} at the end
15:26:45 kashyap mriedem: Do you know if Jack Ding / cfriesen et al are working on it?
15:27:00 gibi #a) has the benefit to allow only healing project_id when there is no bandwidth inventory to heal the port allocations
15:27:08 gibi #b) has the benefit of only calling PUT /allocations once
15:28:18 kashyap gibi: Sorry for breaking your chat, didn't realize it for a moment.
15:28:41 gibi kashyap: no worries I think we break each other chat :D
15:28:55 kashyap (I know there is no 'mutex' on IRC, but still :-))
15:28:59 kashyap Matt: I ask because, there is a related blueprint I filed the other day: https://blueprints.launchpad.net/nova/+spec/cpu-selection-with-hypervisor-consideration
15:32:02 kashyap Anyway, it can be worked out; as no one posted any code for it.
15:32:52 mriedem kashyap: no https://blueprints.launchpad.net/nova/+spec/cpu-model-selection
15:32:58 mriedem no patches proposed
15:33:07 kashyap mriedem: Right, I saw that.
15:33:34 kashyap But most important thing is to rip out existing two CPU-related APIs and use the more sensible APIs that take into account the hypervisor capabilities.
15:33:57 mriedem gibi: i'm assuming (a) is less change in the heal_allocations code
15:34:26 mriedem gibi: for healing missing bw allocations i would hope most of that can be a separate private method that does the check and calculates the allocations to PUT
15:34:52 gibi mriedem: (a) seems a bit less intrusive to the existing code yes
15:35:10 gibi mriedem: yes, I will definitly separate out the bw healing to methos
15:35:13 gibi methods
15:35:37 mriedem gibi: so we already do at most 2 PUTs per instance right?
15:35:40 mriedem first here https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L1892
15:35:49 mriedem second here https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L1912
15:35:57 mriedem the second is getting allocations from the flavor
15:36:25 mriedem gibi: so i think for bw allocations, you add another private method and merge the results of scheduler_utils.resources_from_flavor before calling the 2nd PUT
15:37:08 gibi mriedem: if the code needs to create allocation then it is a single PUT
15:37:29 gibi mriedem: also if there is allocation but no project_id then it is also a single PUT
15:37:48 gibi mriedem: there is a return here https://github.com/openstack/nova/blob/e3c24da89aa3e6462f1b07e00659c87f252ba4ba/nova/cmd/manage.py#L1898
15:38:34 mriedem yeah that complicates things...
15:39:09 gibi mriedem: so adding bw to both places means calling my new bw calculation function in both places
15:39:42 mriedem or, get bw allocations first, then if allocations exist but don't have bw resources, skip that return statement and merge for the 2nd PUT
15:40:15 mriedem well,
15:40:32 mriedem i guess you don't want to have to do the work of calculating the bw allocations if you can determine the existing allocations have the bw resource classes
15:41:14 gibi mriedem: I have to check each neutron port attached to the instance to see if there is resoruce_request in the port but no rp uuid in the binding
15:41:37 mriedem yeah i guess the lack of bw in the existing allocatoins does'nt tell us if you need to heal them
15:41:41 mriedem because not all instances will have bw allocations
15:41:51 gibi mriedem: yes
15:42:05 mriedem so it sounds like you need to do that calculation up front anyway
15:42:40 gibi mriedem: yeah, I can do the check and the calculation upfront and merge it to whatwhever PUT it is needed
15:43:05 mriedem we may want an option on the command itself to skip that check
15:43:06 gibi and introduce a new PUT branch to update the bw only, if other data doesn't triggered the PUT
15:43:12 mriedem --skip-bw-allocations
15:43:18 gibi mriedem: ack
15:43:39 mriedem because if my cloud doesn't enable any of that via policy, i don't want this command to waste time spamming the neutron API to figure out that information
15:44:01 mriedem note that this is another argument for stashing the port resource request information in the info_cache :)
15:44:27 mriedem if it was in the info cache we wouldn't need to hit the neutron api for each instance here
15:44:39 gibi mriedem: that cache could be stale when you turn on the policy :)
15:44:57 mriedem that's what we have _heal_instance_info_cache in the compute for

Earlier   Later