| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-31 | |||
| 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 | |
| 14:35:02 | mriedem | the bdms are soft deleted just like the instance | |
| 14:35:07 | dansmith | okay | |
| 14:38:31 | openstackgerrit | Merged openstack/nova master: api-ref: add a warning about calling swap volume directly https://review.openstack.org/596492 | |
| 14:48:09 | mriedem | bauzas: see my todo/question in https://review.openstack.org/#/c/598365/1/nova/compute/resource_tracker.py - i'm assuming this is something operators may do in case they need to 'reset' allocation ratios on computes in the db back to the default | |
| 14:48:38 | mriedem | i.e. i set a config value for the allocation ratio, then want to get back to defaults, so remove my custom config and restart nova-compute and expect the compute to report the default allocation ratio | |
| 14:50:46 | bauzas | mriedem: ack, I need to go out, but I'll look at it | |
| 14:51:31 | bauzas | mriedem: mmm, about your question | |
| 14:52:12 | bauzas | mriedem: I think we can say something in a reno note that in case they want to use the default values, they should pass 16.0 for example | |
| 14:52:21 | bauzas | some upgrade section | |
| 14:52:38 | bauzas | setting it back to 0.0 looks insane to me | |
| 14:53:04 | bauzas | or another way to fix the problem would be to use the object method there | |
| 14:53:10 | mriedem | if we leave it and doc it anywhere, it should be in the option help, | |
| 14:53:22 | mriedem | release notes aren't read and are lost in the sands of time once you FFU | |