| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-03-05 | |||
| 12:15:14 | lyarwood | https://review.opendev.org/#/q/topic:bug/1861071_followups is also ready for review again if you have time | |
| 13:35:37 | efried | Nova meeting in ~25 mins in #openstack-meeting | |
| 13:36:14 | shilpasd | efried: thanks, i will be IN | |
| 13:36:43 | openstackgerrit | Eric Fried proposed openstack/nova master: Stop using PlacementDirect https://review.opendev.org/640887 | |
| 13:37:31 | efried | stephenfin: Cleaning house, that ^ is an oldymoldy that I just rebased manually. mriedem was +2 before. Would you consider fast approving? | |
| 13:37:57 | efried | Highly recommend switching on "ignore whitespace = All" to review effectively. | |
| 13:52:05 | stephenfin | efried: Thanks for the tip with whitespace all. Yeah, it's test only so let's do it | |
| 13:52:21 | efried | thanks | |
| 14:01:40 | efried | nova meeting now | |
| 14:05:34 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Add retry_on_deadlock to migration_update DB API https://review.opendev.org/711461 | |
| 14:59:18 | brinzhang | root@ubuntu-OpenStack:~# nova flavor-show 1 | |
| 14:59:19 | brinzhang | | Property | Value | | |
| 14:59:19 | brinzhang | +----------------------------+----------------------------+ | |
| 14:59:19 | brinzhang | | OS-FLV-DISABLED:disabled | False | | |
| 14:59:19 | brinzhang | | OS-FLV-EXT-DATA:ephemeral | 0 | | |
| 14:59:46 | brinzhang | this is part of from flavor-show | |
| 15:00:17 | dansmith | yeah, it's a column field, not an extra spec | |
| 15:00:21 | efried | brinzhang: right, so how do you set it to True? | |
| 15:00:28 | dansmith | from looking at all the APIs, there's no way for it to become true | |
| 15:00:42 | efried | ah, neat. | |
| 15:00:44 | brinzhang | No, it cannot to be changed now | |
| 15:00:57 | dansmith | disabled = Column(Boolean, default=False) | |
| 15:01:18 | dansmith | tframbo: what version of nova? | |
| 15:01:47 | tframbo | Rocky | |
| 15:01:55 | dansmith | installed from where? | |
| 15:02:13 | brinzhang | in 2013 year, https://review.opendev.org/#/c/61291/ this patch want to change this set, but fast abandon | |
| 15:02:23 | tframbo | github.com | |
| 15:02:45 | dansmith | brinzhang: yeah, a good example of why we try to closely examine our features and merge them when ready :) | |
| 15:02:49 | tframbo | I have seen the master branch ,it also can't change | |
| 15:02:53 | dansmith | tframbo: what backend database? | |
| 15:03:06 | tframbo | maraidb | |
| 15:03:33 | dansmith | tframbo: really not sure how this could be happening then | |
| 15:04:03 | dansmith | tframbo: can you post the output of "use nova_api; SELECT * FROM flavors;" ? | |
| 15:04:11 | efried | dansmith: Perhaps I'm misunderstanding, but I think tframbo is saying they want to be able to change the value to 'true' | |
| 15:04:21 | efried | i.e. they want to be able to disable the flavor | |
| 15:04:27 | dansmith | efried: oh I thought he was saying it's always true when they create flavors | |
| 15:04:39 | dansmith | (based on context from yesterday) | |
| 15:04:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Add framework for extra spec validation https://review.opendev.org/704643 | |
| 15:04:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api: Add microversion 2.83, extra spec validation https://review.opendev.org/708436 | |
| 15:04:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Add documentation for flavor extra specs https://review.opendev.org/710037 | |
| 15:04:54 | tframbo | yeah ,I just want to set it to true | |
| 15:05:08 | brinzhang | dansmith: I think tframbo want to re-set this property | |
| 15:05:10 | dansmith | tframbo: ah, sorry I misunderstood.. as brinzhang said, it is half a feature that was never finished | |
| 15:05:17 | dansmith | yep, sorry, my bad | |
| 15:05:35 | efried | iow we have this property that's part of the API and the output but doesn't do anything and can't be changed. | |
| 15:05:56 | brinzhang | dansmith: yes, it's not completed, maybe we can consider whether to do this in the future | |
| 15:06:25 | efried | so, tframbo, what you're asking is essentially "let's finish this feature". To do that, you would need to produce a blueprint and a spec, as well as a compelling use case for being able to do it in the first place. | |
| 15:06:44 | efried | We're past spec freeze for the current release, but you could get started working on proposing it for the Victoria release. | |
| 15:06:45 | dansmith | and implement it, or find someone interested in doing so | |
| 15:07:08 | efried | Has anyone else asked for it since Havana to your knowledge dansmith? | |
| 15:07:22 | brinzhang | Now in our create flavor, the request body does not contain this proerty | |
| 15:07:37 | dansmith | efried: not that I've ever heard of | |
| 15:07:42 | dansmith | not really sure what the point of it would be | |
| 15:07:46 | efried | seems like, since we now copy the flavor body into the instance, there's an easy workaround for this: delete the flavor | |
| 15:07:48 | dansmith | it came from a time before we kept flavors with instances | |
| 15:07:54 | efried | jynx | |
| 15:08:13 | dansmith | right, the only case I can imagine is wanting to keep flavors for records of what things were booted from, but it seems confusing to me for the user, | |
| 15:08:22 | dansmith | unless disabled flavors are filtered most of the time | |
| 15:08:31 | dansmith | but it's so much easier to just see it in the instance detail | |
| 15:08:45 | brinzhang | If we want to do this feature, I think I can continue this, but we should consider, whether pay time to do ^^ | |
| 15:09:15 | dansmith | brinzhang: we should only do it if there is a compelling reason to, not just because "it's half finished already" | |
| 15:09:22 | dansmith | I'd rather see the field go away personally | |
| 15:09:37 | efried | yeah, if we do anything at all, that would be my vote | |
| 15:09:48 | brinzhang | dansmith: yeah, agree | |
| 15:09:49 | efried | tframbo: let me clarify: We now copy flavor information into the instance record, so that any changes to the original flavor (including deleting it) don't get confused with how the instance was originally created. | |
| 15:10:02 | efried | tframbo: which means there's really no need to "disable" a flavor -- just delete it instead. | |
| 15:10:18 | efried | dansmith: since what release do we do the flavor-into-instance copy? | |
| 15:10:38 | efried | certainly before rocky. | |
| 15:10:39 | dansmith | technially, not sure, a long time.. not sure when we *expose* it | |
| 15:10:46 | dansmith | but it was sdague era, IIRC, so it's been a while | |
| 15:11:32 | dansmith | 2.47 | |
| 15:11:32 | efried | microversion 2.47... | |
| 15:11:48 | efried | ...pike. | |
| 15:12:23 | dansmith | 2017 | |
| 15:12:29 | brinzhang | https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id43 | |
| 15:12:33 | tframbo | yeah ,I just want to know the "disable" value , if is meaningful.if is unuseful, we might should delete it. | |
| 15:16:25 | efried | tframbo: agree. Though that would require a microversion, so it may not be worth the effort. Perhaps the next time gmann does an "API cleanup" sweep... | |
| 15:16:54 | efried | tframbo: you could propose to filter it out of the CLI output I suppose. Though again, not sure that would be worth the effort. | |
| 15:17:45 | efried | Okay, I'm going to go take actual vacation for a couple days, and then I'm really gone. o/ | |
| 15:17:47 | brinzhang | yes, we should consider is it worth to do | |
| 15:18:25 | brinzhang | too later for me, good morning everyone ^^ | |
| 15:18:28 | brinzhang | bye | |
| 15:18:48 | tframbo | thank you ,all | |
| 15:18:54 | tframbo | bye | |
| 15:20:13 | brinzhang | later.. | |
| 15:20:33 | brinzhang | dansmith: Can you review these bug fix? https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bug/1663456 | |
| 15:21:14 | brinzhang | dansmith: stephenfin want to get your check ^^ | |
| 15:21:56 | dansmith | brinzhang: I'm not sure why stephenfin +2d that but also said it should be done in oslo.db | |
| 15:23:06 | dansmith | I dunno if TimestampMixin does the time the same way, but if it didn't or it changed, it would be confusing that we're setting it to one thing and then potentially have that roll backwards, forwards, or be different from the create time | |
| 15:23:19 | dansmith | why not fix this at the oslo.db layer and not merge this? | |
| 15:23:22 | brinzhang | dansmith: I tend to use the current implementation, and I think it's good enough. | |
| 15:24:34 | dansmith | I'm not sure what that means | |
| 15:26:11 | stephenfin | dansmith: I don't know what that means either but the oslo.db part wouldn't fix this entirely | |
| 15:26:43 | dansmith | I see now it's add/delete host that they want it updated at | |
| 15:26:54 | dansmith | the thing I don't like here, | |
| 15:26:55 | stephenfin | brinzhang is saying that when we add a new host to an aggregate, we should update the updated_at updated | |
| 15:26:56 | stephenfin | yeah | |
| 15:27:12 | dansmith | is that I don't know that oslo.db is using time.utcnow() (I expect it is, but..) | |
| 15:27:13 | stephenfin | the updated_at of the aggregate itself | |
| 15:27:41 | dansmith | so us doing that here just worries me a little that if that changed or there was some other detail, we could have situations where updated is before created, etc | |