| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-13 | |||
| 07:41:10 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Compute: add support to abort queued live migration https://review.openstack.org/568542 | |
| 07:53:41 | openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Fix ServerMigrationSampleJsonTestsV2_24 to use its own sample file https://review.openstack.org/581562 | |
| 08:26:51 | Miouge | As an admin, can I add a VM to a server group after it’s creation? | |
| 08:27:16 | Miouge | I don’t find it in the API, so is the only option to go an edit the DB and re-schedule the instances? | |
| 08:35:24 | giblet_off | I'm taking the day off | |
| 08:50:47 | gmann | giblet_off: take rest do not be always too hard working :) | |
| 09:03:36 | openstackgerrit | tianhui proposed openstack/nova master: Change default video model from cirrus to vga https://review.openstack.org/582471 | |
| 09:14:46 | eumel8 | Hello Nova Cores! Maybe it's possible to import Zanata translations: https://review.openstack.org/#/c/578019/ | |
| 09:43:53 | openstackgerrit | Chen proposed openstack/nova master: Clean up group members in db when delete https://review.openstack.org/581403 | |
| 09:57:06 | openstackgerrit | huanhongda proposed openstack/nova-specs master: Return "deleted" and "estimated_reclaim" time of instance for admin https://review.openstack.org/581638 | |
| 10:07:19 | openstackgerrit | Chen proposed openstack/nova master: fix cellv2 delete_host https://review.openstack.org/582171 | |
| 10:47:31 | stephenfin | eumel8: Sure, done | |
| 11:16:36 | openstackgerrit | Merged openstack/nova master: Imported Translations from Zanata https://review.openstack.org/578019 | |
| 12:10:09 | eumel8 | thx stephenfin | |
| 12:54:28 | mriedem | sahid: i replied in https://review.openstack.org/#/c/515423/ - are you aware of certain vif types that will return None for port binding profile and/or vif_details? | |
| 12:54:37 | mriedem | b/c otherwise those should be coming back as dicts from the neutron api | |
| 13:12:40 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Online migration tool for populating queued-for-delete https://review.openstack.org/582536 | |
| 13:30:59 | openstackgerrit | Merged openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | |
| 13:36:51 | mriedem | oh most illustrious, handsome and just dansmith, i draw your attention to https://review.openstack.org/#/c/568542/19/nova/compute/rpcapi.py@693 for discussion once i'm back from delivering my child to the quarry hill day camp | |
| 13:37:22 | mriedem | tl;dr i think we should drop the 5.1 rpc api bump since the interface doesn't change, and just check the compute service version in the api iff new microversion and status is queued/preparing | |
| 13:37:31 | dansmith | oye that's a lot of reading for a friday | |
| 13:37:52 | dansmith | I said that was one option yeah | |
| 13:37:58 | dansmith | I'll read for your reasoning | |
| 13:59:34 | mriedem | yup 3am rambling | |
| 13:59:36 | melwitt | efried, leakypipes, stephenfin: reminder that non-client library release freeze (os-traits and os-vif) is next thurs July 19 so if there's anything you want to make the release, get them in by the end of next thurs | |
| 14:00:06 | mriedem | yikun: ^ you should start working on the advanced policy changes to novaclient | |
| 14:03:11 | melwitt | novaclient release freeze is same as r-3 July 26 | |
| 14:29:52 | mriedem | dansmith: thanks; i think i'll clean this change up and then rebase the api change on top with the service version check i'm talking about, then Kevin_Zheng can flesh out the rest | |
| 14:30:04 | dansmith | ack | |
| 14:30:04 | melwitt | dansmith: I wanted to get your thoughts on this approach for a bug fix before you are out next week https://review.openstack.org/582408 | |
| 14:32:07 | mriedem | melwitt: btw, i've had a functional test up for one of those forever https://review.openstack.org/#/c/562041/ | |
| 14:32:39 | melwitt | mriedem: oh, sorry, I didn't know | |
| 14:32:40 | dansmith | melwitt: we probably don't want to filter on deleted for the join, but I would think we should sort by deleted and pick the first service in the list, which would be the non-deleted one, if there is one | |
| 14:33:07 | dansmith | would that solve the bug without this patch? | |
| 14:33:40 | melwitt | dansmith: I think it would because it would also avoid generating a UUID for a deleted service | |
| 14:34:02 | dansmith | so that might be a better approach | |
| 14:34:09 | melwitt | Service.save()ing a deleted service will always raise ServiceNotFound, for example | |
| 14:35:09 | dansmith | because of _check_minimum_version() yeah? | |
| 14:35:27 | melwitt | no, ServiceNotFound will raise if version checks out | |
| 14:35:40 | melwitt | if version is too old, we raise ServiceTooOld first before we get to the db.service_update call | |
| 14:36:15 | dansmith | aight, well, anyway | |
| 14:36:24 | melwitt | to be clear, _check_minimum_version raises ServiceTooOld if the deleted service has too old a version, db.service_update raises ServiceNotFound for the deleted service | |
| 14:36:30 | dansmith | yeah | |
| 14:36:46 | melwitt | dansmith: thanks, I can update to the approach you suggested | |
| 14:36:51 | dansmith | I'm no opposed to what you have here, but it sounds like it's more the symptom than the cause | |
| 14:36:57 | dansmith | *not | |
| 14:39:02 | melwitt | I suppose the potential problem with the idea of sorting by deleted is if somehow an instance does not have a non-deleted service... but I'm not sure if/how that could happen. it could definitely happen before we started blocking service delete for compute services that are still hosting instances | |
| 14:39:47 | dansmith | well, that's why I'm saying the thing you're doing here is also not bad, | |
| 14:40:04 | dansmith | although we might should put the check in save() itself, | |
| 14:40:17 | dansmith | since any save() on a deleted service will fail, this is just one scenario | |
| 14:40:57 | dansmith | point being, if the join can return multiple services for the instance, potentially in insert-order, then getting the first one from the list if we _don't_ sort will likely be wrong | |
| 14:41:06 | dansmith | I almost feel like we've had a bug about that at some point actually | |
| 14:41:09 | melwitt | I considered that, but what should we do if trying to save a deleted service other than raise? | |
| 14:41:47 | dansmith | say what? :) | |
| 14:41:53 | dansmith | but you said save() will never work on deleted right? | |
| 14:42:04 | melwitt | oh, we don't get only the first one, we process all of them (there's a Instance.services field) and something else takes the first one (something in the API I guess?) | |
| 14:42:06 | dansmith | or I guess it will if you have a proper context | |
| 14:42:35 | melwitt | when you said "put the check in save() itself" I was asking what we would do after we checked self.deleted there | |
| 14:42:49 | melwitt | or did you mean the min version check, just skip it if deleted | |
| 14:43:33 | dansmith | I was going to say, if deleted, don't freak out about the version check failure in save9), | |
| 14:43:38 | dansmith | but that won't fix your problem during load | |
| 14:43:53 | melwitt | ah, ok | |
| 14:44:31 | melwitt | I'm realizing I was wrong when I said sorting would help because it processes all services that came out of the join. my bad | |
| 14:45:07 | dansmith | so why are we getting the deleted one | |
| 14:45:08 | dansmith | ? | |
| 14:45:28 | melwitt | because it has the same 'host' as a non-deleted one | |
| 14:45:53 | dansmith | right, which means we chose the wrong one yeah? | |
| 14:45:59 | melwitt | if the operator has ever deleted a service and replaced it with another with the same host name | |
| 14:46:24 | melwitt | the field on Instance is 'services' so we get all of them and something in the API must be picking one of them | |
| 14:46:43 | melwitt | so we get a _from_db_object call per service returned from the join | |
| 14:47:00 | mriedem | the lazy load on instance.services is generic right? | |
| 14:47:50 | dansmith | melwitt: okay so we process all of them and try to generate uuids for all the previously-deleted ones? | |
| 14:47:53 | melwitt | what do you mean by generic? | |
| 14:48:15 | mriedem | in that we don't do a targeted query to get services by instance uuid and filter out deleted ones | |
| 14:48:17 | melwitt | dansmith: yeah process all of them and try to generate uuids for all that do not have a uuid (including the previously deleted ones) | |
| 14:48:23 | mriedem | we call instance_get_by_uuid and join on the services table | |
| 14:48:39 | dansmith | I see | |
| 14:49:20 | melwitt | mriedem: oh, for the lazy load I'm not sure. the scenario of the bug is the API is adding the 'services' expected attr before getting instances for the list | |
| 14:49:27 | mriedem | 'Service.binary == "nova-compute",' | |
| 14:49:27 | mriedem | primaryjoin='and_(Service.host == Instance.host,' | |
| 14:49:27 | mriedem | backref='services', | |
| 14:49:27 | mriedem | "Instance", | |
| 14:49:27 | mriedem | instance = orm.relationship( | |
| 14:49:27 | mriedem | and this is the backref | |
| 14:49:28 | mriedem | foreign_keys=host, | |
| 14:49:28 | mriedem | 'Instance.deleted == 0)', | |
| 14:49:29 | mriedem | ) | |
| 14:49:40 | mriedem | umm, so why would we load deleted services? | |
| 14:49:53 | mriedem | oh so not a lazy-load ok | |
| 14:50:04 | melwitt | because that's not filtering on Service.deleted? | |
| 14:51:30 | mriedem | query_prefix = query_prefix.options(joinedload(column)) | |
| 14:51:30 | mriedem | righ | |
| 14:52:26 | melwitt | why would the lazy load filter out deleted services? | |
| 14:53:14 | mriedem | maybe it doesn't, i thought it would use the backref | |
| 14:54:58 | melwitt | the backref says Instance.deleted == 0, not Service.deleted, so I thought it wouldn't filter out deleted service? maybe I'm missing something else | |
| 14:55:16 | mriedem | oh right, | |
| 14:55:20 | mriedem | glossed right over that | |
| 14:55:25 | mriedem | anyway, ignore me now | |
| 15:18:03 | openstackgerrit | Chen proposed openstack/nova master: fix cellv2 delete_host https://review.openstack.org/582171 | |
| 15:24:39 | mriedem | tssurya: i'm reminded that we can get a HostMappingNotFound when deleting a compute service https://review.openstack.org/#/c/554920/12/nova/api/openstack/compute/services.py@243 - we were just talking about that this week (or late last week), but i can't remember if a bug was reported for it? | |