Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-03-05
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 | OS-FLV-EXT-DATA:ephemeral | 0 |
14:59:19 brinzhang | OS-FLV-DISABLED:disabled | False |
14:59:19 brinzhang +----------------------------+----------------------------+
14:59:19 brinzhang | Property | Value |
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 tframbo yeah ,I just want to set it to true
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 openstackgerrit Stephen Finucane proposed openstack/nova master: api: Add microversion 2.83, extra spec validation https://review.opendev.org/708436
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 efried microversion 2.47...
15:11:32 dansmith 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
15:27:44 dansmith which is just messy
15:28:32 stephenfin It's using 'timeutils.utcnow()'
15:28:49 stephenfin I'm not sure how you'd approach this if you'd want it wholly contained in oslo.db though
15:29:30 stephenfin We want to say if a "child" is updated, set the 'updated_at' for the child and also the "parent" linked by the fk
15:30:21 dansmith stephenfin: make TimestampMixin have a .touch() method
15:30:59 dansmith brinzhang: I -1'd for a couple of things.. I'm really not sure why this is important, but we should be consistent if we're going to do this, IMHO
15:31:28 dansmith brinzhang: it seems very weird to me to get an aggregate updated when I add a host, but not when I set some property on that aggregate, which from the user's perspective is a direct property of the aggregate itself
15:31:43 stephenfin (or more specifically, when updating an instance of AggregateHost, also update the Aggregate referenced via the 'aggregate_id' field)

Earlier   Later