| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-31 | |||
| 13:48:15 | fried_rice | poojajadhav: You definitely shouldn't be attempting to access the placement database directly from anywhere other than within the placement API code itself. If you're trying to get placement information from nova, you would need to do it by talking to the placement API service through the REST interface, via the SchedulerReportClient. | |
| 13:49:44 | poojajadhav | fried_rice, leakypipes: If the instance is BFV then also its shwoing the flavor,root_gb as per https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/simple_tenant_usage.py#L165-L168 | |
| 13:50:16 | fried_rice | poojajadhav: I thought we had some patches already proposed that were looking at fixing this... | |
| 13:50:52 | dansmith | leakypipes: despite the legacyness, placement doesn't provide the data it would need anyway | |
| 13:51:58 | mriedem | the is_bfv fix doesn't extend to the simple tenant usage API right?/ | |
| 13:52:02 | mriedem | since the API doesn't look at the request spec | |
| 13:52:47 | poojajadhav | fried_rice : But in the simple tenant usage api, we can get flavors details and falvor is exists in nova_api only. | |
| 13:53:32 | dansmith | mriedem: unrelated, but .. haven't we already had a discussion about allowing delete-on-termination to be provided in attach volume? | |
| 13:53:56 | dansmith | I found a very old blueprint and code from like 2013, but couldn't find anything more recent | |
| 13:54:08 | dansmith | and I thought we had (and you were opposed) | |
| 13:54:25 | mriedem | dansmith: someone was asking for adding delete_on_termination for ports during server create to match bdms, but not sure about adding that flag during attach | |
| 13:54:39 | mdbooth | dansmith: It does seem like a kinda weird thing to do, tbh. Also, how would that work with multi-attach? | |
| 13:54:53 | mriedem | i agree it would be weird | |
| 13:55:03 | dansmith | mdbooth: mriedem: yeah, I think it violates the "nova created it so nova deleted it" rule | |
| 13:55:05 | mriedem | delete_on_termination, to me, makes the most sense when nova creates the resoruce | |
| 13:55:10 | dansmith | and could be quite surprising | |
| 13:55:11 | poojajadhav | fried_rice : Can u please locate me to those patches | |
| 13:55:34 | fried_rice | poojajadhav: I'm probably thinking of something different. | |
| 13:55:45 | dansmith | mriedem: anyway i was sure you had already thrown down the hammer on this previously so I was just looking for a shortcut | |
| 13:55:55 | leakypipes | dansmith: ack | |
| 13:56:20 | mriedem | poojajadhav: https://github.com/openstack/nova/commit/03c596a9f4324e572bc04d4bbad09a6d3d47366c#diff-cbbdc4d7c140314a7e0b2d97ebcd1f9c ? | |
| 13:56:40 | mriedem | dansmith: b/c today is friday red hat bug squash day and you have a customer that wants this ability? | |
| 13:56:50 | dansmith | mriedem: I told you never to call me here | |
| 13:56:53 | mriedem | and you can say "ol man riderman doesn't like it" | |
| 13:57:15 | dansmith | mriedem: no, I was hoping for a link to an ML thread where "ol man riderman said no" | |
| 13:57:28 | mriedem | for that you'd have to dig :) | |
| 13:57:34 | dansmith | well, I did | |
| 13:57:36 | mriedem | i wouldn't be surprised if it came up | |
| 13:57:46 | leakypipes | I think we'd need hansmoleman for that. | |
| 13:58:40 | mriedem | https://review.openstack.org/#/c/89777/ | |
| 13:58:42 | poojajadhav | mriedem: I have checked on current master, in the usages for BFV instance it shows local_gb=1. | |
| 13:58:45 | mriedem | there are -1s in that spec from not me | |
| 13:58:54 | dansmith | mriedem: just procedural | |
| 13:59:01 | dansmith | I already found that and the conversation on the ML about it | |
| 13:59:03 | mriedem | poojajadhav: yeah, because the simple tenant usage API doesn't take into account whether or not an instance is volume-backed | |
| 13:59:07 | dansmith | nobody seemed to actually discuss it | |
| 13:59:07 | mriedem | as i said above | |
| 13:59:43 | mriedem | phil day was -1 onn it | |
| 13:59:46 | mriedem | for the reasons we just said | |
| 14:00:08 | mriedem | "I think the option for delete on termination when booting an instance is a hang over from the original boot from volume work where Nova would create a new (in effect ephemeral) volume from an image in Glance. I'm not sure that use case really exist anymore - but even if it does it doesn't seem like it should be extended to more general cases." | |
| 14:00:15 | mriedem | doesn't exist anymore....oh phil | |
| 14:00:41 | mriedem | heh jay was +1 on it | |
| 14:00:47 | mriedem | 2014 jay loved bfv! | |
| 14:01:12 | poojajadhav | mriedem: but should we consider instance is volume-backed or not for simple tenant usage api? | |
| 14:01:22 | mriedem | poojajadhav: yes we likely should | |
| 14:01:40 | melwitt | . | |
| 14:01:50 | openstack | bug 1469179 in OpenStack Compute (nova) "instance.root_gb should be 0 for volume-backed instances" [Medium,Fix released] https://launchpad.net/bugs/1469179 - Assigned to Dan Smith (danms) | |
| 14:01:50 | mriedem | poojajadhav: you could start by reporting a bug, related to bug 1469179 and mention the issue | |
| 14:01:52 | leakypipes | mriedem: I must have mistaken it to mean delete bfv. | |
| 14:01:59 | mriedem | leakypipes: heh | |
| 14:02:33 | openstack | Launchpad bug 1715570 in OpenStack Compute (nova) "simple tenant usage api calculating disk usages incorrectly" [Undecided,In progress] - Assigned to Bhagyashri Shewale (bhagyashri-shewale) | |
| 14:02:33 | poojajadhav | mriedem: bug is already reported https://bugs.launchpad.net/nova/+bug/1715570 | |
| 14:06:06 | mriedem | poojajadhav: ok | |
| 14:06:34 | mriedem | i'm not really sure how we could efficiently calculate volume-backed for all servers in the simple tenant usage API call - since it's not a flag on the instance itself | |
| 14:07:00 | mriedem | unless we started reporting that into the instance system_metadata or something | |
| 14:07:04 | mriedem | dansmith: any thoughts on that? | |
| 14:10:02 | dansmith | mriedem: yeah, we'd need a new element of data I think to be able to do it at all efficiently | |
| 14:10:06 | dansmith | sysmeta would not be that thing though | |
| 14:10:08 | poojajadhav | mriedem: I was thought like currently placement usage api giving correct usages so we can get them and use it for the simple tenant usage api | |
| 14:10:27 | dansmith | poojajadhav: that's not what simple tenant usage wants or needs | |
| 14:10:31 | dansmith | it needs historical data | |
| 14:10:47 | poojajadhav | dansmith: hmm | |
| 14:11:18 | mriedem | simple tenant usage is asking for an aggregate usage of resources for a given tenant over a specific time period | |
| 14:12:15 | mriedem | placement doesn't expose consumer type (instance or volume or container) nor created_at time | |
| 14:12:28 | dansmith | nor keep anything after deletion | |
| 14:12:34 | dansmith | i.e. no historical archive | |
| 14:12:41 | mriedem | right | |
| 14:12:49 | mriedem | when a record is deleted in placement, it's gone | |
| 14:12:57 | mriedem | it's not put in a box in the basement like in nova | |
| 14:14:19 | leakypipes | mriedem: gonna have to wait until the glacier unfreezes I'm afraid. /me goes back to reviewing nova-specs | |
| 14:16:35 | poojajadhav | mriedem: I was trying to fix it.. but which can be then efficient way now :( | |
| 14:19:44 | mriedem | well, system_metadata isn't great | |
| 14:20:47 | mriedem | dansmith: thinking out loud, how crazy would it be to have a joinedload field in the db api when we get an instance that adds a boolean attribute to the sqla instance object that queries the bdms table for that instance to see if it's volume-backed, i.e. (1) bdm exists for the instance (2) boot_index=0, (3) destination_type='volume' ? | |
| 14:21:29 | mriedem | essentially nova.compute.utils.is_volume_backed_instance but in the db api | |
| 14:22:47 | dansmith | are those all columns? | |
| 14:23:18 | dansmith | even if they are, I dunno if you can easily synthesize a field on an ORM object or not | |
| 14:24:59 | mriedem | we do that in other places, like what we had for instance_group.policies | |
| 14:25:23 | mriedem | definitely not something i'm going to spend a lot of time thinking about today | |
| 14:25:28 | mriedem | i'll throw it in the bug as an idea | |
| 14:27:08 | dansmith | we synthesize something with a subquery? | |
| 14:29:28 | mriedem | well i was thinking stuff like htis | |
| 14:29:29 | mriedem | *this | |
| 14:29:30 | mriedem | primaryjoin='InstanceGroup.id == InstanceGroupPolicy.group_id') | |
| 14:29:30 | mriedem | _policies = orm.relationship(InstanceGroupPolicy, | |
| 14:30:03 | mriedem | or this from the services table: | |
| 14:30:04 | mriedem | 'Service.binary == "nova-compute",' | |
| 14:30:04 | mriedem | primaryjoin='and_(Service.host == Instance.host,' | |
| 14:30:04 | mriedem | backref='services', | |
| 14:30:04 | mriedem | "Instance", | |
| 14:30:04 | mriedem | instance = orm.relationship( | |
| 14:30:05 | mriedem | foreign_keys=host, | |
| 14:30:05 | mriedem | 'Instance.deleted == 0)', | |
| 14:30:06 | mriedem | ) | |
| 14:30:29 | mriedem | that's how we get the instance.services column right? | |
| 14:30:47 | mriedem | calling it a column isn't really appropriate i suppose | |
| 14:30:50 | mriedem | model attribute | |
| 14:33:36 | dansmith | that's a join so we get the right thing, | |
| 14:33:53 | dansmith | that just gets us a list of matching records, not a boolean calculated, | |
| 14:34:04 | dansmith | but we could turn it into a boolean on the nova object if there's a bdm in that list I guess | |
| 14:34:44 | dansmith | do we keep the bdms after instance deletion? I think we use the flavor data for that usage info, so we'd need the bdms to still be there on deleted instances for that to work properly | |