Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-11
09:56:38 yikun we do conversion thing in group obj, see https://review.openstack.org/#/c/563375/39/nova/objects/instance_group.py@140
10:00:28 yikun I'm not sure should we keep same with it.
10:00:44 yikun and here is some message history:
10:00:45 yikun http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2018-07-11.log.html#t2018-07-11T02:18:53
10:00:59 yikun ^ gibi
10:19:27 openstackgerrit Takashi NATSUME proposed openstack/nova master: Transform metrics.update notification https://review.openstack.org/580567
10:28:05 openstackgerrit Lee Yarwood proposed openstack/nova stable/ocata: libvirt: slow live-migration to ensure network is ready https://review.openstack.org/559034
10:28:55 openstackgerrit huanhongda proposed openstack/nova master: Microversion 2.64 - Add "deleted" time in GET server response https://review.openstack.org/574159
10:44:03 openstackgerrit Van Hung Pham proposed openstack/nova master: convert py35 jobs to py3 https://review.openstack.org/581663
10:46:15 openstackgerrit Surya Seetharaman proposed openstack/nova master: Update queued-for-delete from the ComputeAPI during deletion. https://review.openstack.org/566813
10:48:01 openstackgerrit Surya Seetharaman proposed openstack/nova master: Update queued-for-delete from the ComputeAPI during deletion/restoration https://review.openstack.org/566813
10:55:53 gibi yikun: it is on my list
10:57:31 cnf hi, anyone know what would cause the creation of a new instance to give me "Block Device Mapping is Invalid" ?
10:57:42 cnf on -cinder, someone said it looks like a nova issue
11:11:52 yikun gibi: ok, thanks :)
11:28:41 openstackgerrit huanhongda proposed openstack/nova master: Microversion 2.64 - Add "deleted" time in GET server response https://review.openstack.org/574159
12:26:12 gibi yikun: did I expressed my opininon in every open issue in https://review.openstack.org/#/c/563401/28/nova/notifications/objects/server_group.py@41 or did I missed something?
12:27:08 gibi yikun: ahh the conversion thing
12:27:12 gibi yikun: i missed that
12:27:22 yikun yep, : )
12:27:33 yikun conversion
12:27:46 gibi yikun: would it be hard to make the int conversion in the notification body as well?
12:28:30 yikun No, just need put some duplicate code into notification obj.
12:29:10 yikun like https://review.openstack.org/#/c/563375/39/nova/objects/instance_group.py@140
12:30:36 gibi yikun: I see. We can only store a DictOfString even if your values will be in different types in the future
12:31:19 gibi yikun: can we call the rules property from the notification payload generation? let me look a bit deeper in the code...
12:32:11 gibi yikun: I guess we call the rules property but then we store the result in a DictOfString in the payload as well
12:32:23 gibi yikun: which converts it back to string
12:32:57 yikun yes, so if we don't do convert, we will get back a "3"
12:32:58 gibi yikun: as we don't have a Dict type with variable value type
12:33:11 gibi yikun: we cannot define a proper field in the payload class
12:33:44 gibi yikun: and the payload class is something we think about as a contract for the consumers
12:33:49 yikun yes, at least I can't see a DictField type in ovo. :)
12:34:14 gibi yikun: let's keep it as "3"
12:34:35 gibi yikun: I would not like to have a DictOfString defined in the class but we would emit an int as a value
12:34:38 gibi in the notification
12:35:48 gibi yikun: commenting it in the review...
12:37:47 gibi yikun: done
12:38:00 gibi yikun: thanks for your patientes
12:38:42 yikun OK, I see, much thanks for your time and help. :)
12:38:45 gibi s/patientes/patience
12:39:07 yikun :), ha, I know
12:39:22 gibi :)
12:40:30 yikun and it time to leave and back home, have a good day.
12:40:46 gibi good day to you too
12:41:17 mriedem gibi: so we're leaving it as a string in the notification payload?
12:41:25 gibi mriedem: yes
12:42:32 gibi mriedem: because the field type is DictOfString and when we finally generate json schemas for notification classes then that will also created from the field typw
12:42:35 gibi type
12:44:23 mriedem i'm assuming yikun pointed out that for InstanceGroup.rules we coerce the known max_server_per_host field value to an int
12:47:31 gibi mriedem: yes, I saw that. gmann also pointed out that can be lead to some json validation inconvenience
12:47:41 gibi mriedem: if we flatten the API
12:49:03 openstackgerrit Matt Riedemann proposed openstack/nova master: Skip ServerShowV257Test.test_rebuild_server for cells v1 job https://review.openstack.org/581717
12:51:04 mriedem for the rest api, we accept strings or ints apparently
12:51:07 mriedem which was news to me
12:51:13 mriedem for the positive_integer type
12:51:50 mriedem https://github.com/openstack/nova/blob/master/nova/api/validation/parameter_types.py#L238
12:51:51 gibi mriedem: and we patternmatch, I see
12:52:16 mriedem so i'm not sure why {'max_server_per_host': 3} vs {'max_server_per_host': '3'}
12:52:20 mriedem will make a difference
12:52:39 mriedem tbc, the InstanceGroup.rules doing the cast to int for that value is for internal convenience more than anything,
12:52:54 mriedem so we don't have to remember to cast the value in all of the code that uses it, like the view builder in the api, the scheduler filter and the late affinity check in the compute
12:54:45 gibi mriedem: I think the api accepts strings for historical reasons
12:55:33 gibi mriedem: I sure I'm OK to cast it to int internally for counting and comparison
12:57:08 mriedem ok. i just don't understand the json schema validation concern
12:57:19 mriedem is that a concern for the consumer of the notification?
12:59:01 gibi mriedem: no it is not. I refer to gmann's comment in https://review.openstack.org/#/c/563401/28/nova/notifications/objects/server_group.py@41 about the need to move the validation from json to code
13:01:19 mriedem oy
13:01:36 mriedem there is only 1 policy per group, and the rules are applied to that policy on that group,
13:01:46 mriedem we've already named the InstanceGroup.rules field
13:03:58 gibi yeah that is gmann's point
13:03:59 mriedem which i think is ok, it's like a 2 line validator
13:04:12 gibi I agree
13:04:15 mriedem if group.policy == 'anti-affinity' and group.rules: raise HTTPBadRequest
13:04:23 mriedem oops
13:04:23 openstackgerrit Stephen Finucane proposed openstack/nova master: Revert "docs: Disable smartquotes" https://review.openstack.org/578841
13:04:29 mriedem != 'anti-affinity'
13:05:04 gibi sure
13:05:30 gibi policy_rule feels a better name for me but I accept rule with a good api doc
13:06:51 openstackgerrit sahid proposed openstack/nova stable/queens: hardware: fix hugepages memory usage per intances https://review.openstack.org/581736
13:08:14 openstack mriedem: Error: "=" is not a valid command.
13:17:08 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: Fix TypeError in prep_resize allocation cleanup https://review.openstack.org/581741
13:17:20 mriedem gibi: i think you might have officially made yourself the other core to review https://review.openstack.org/#/c/563375/
13:17:32 mriedem dansmith would +2 it but he made some non-trivial changes to it
13:19:56 openstackgerrit Chris Dent proposed openstack/nova master: [placement] add error.code on a ConcurrentUpdateDetected https://review.openstack.org/581742
13:21:25 mriedem gibi: btw, you've been quite about reviews on the bw resource provider series - what's the status on that? are you looking for reviews on incremental things? is it done end to end or still working on changes for the full stack in nova (and maybe deps in neutron)?
13:21:55 mriedem my giant port binding live migration series is basically blocked on neutron dependencies
13:24:29 openstackgerrit Chris Dent proposed openstack/nova master: Test for unsanitized consumer UUID https://review.openstack.org/581137
13:28:53 gibi mriedem: I will go through the remainings of https://review.openstack.org/#/c/563375/ soon
13:29:23 gibi mriedem: the status of bandwidth is that the code up on review stopped working after the official support for nested allocation_candidate support is merged
13:29:38 gibi mriedem: the solution is to support the new placement API versions from the scheduler report client
13:30:28 gibi mriedem: for that I started with a failing functional test here https://review.openstack.org/#/c/527728/
13:31:05 mriedem yikun: gmann: alex_xu: so i see there must have been some discussion on https://review.openstack.org/#/c/567534/ and the request format wasn't changed but the response fields were changed...but no comments in the review about that decision?
13:37:32 alex_xu mriedem: no decision yet, I will try to go through the review
13:38:15 alex_xu mriedem: do you remember whether we discuss what should we do if the image traits check failed for rebuild https://review.openstack.org/#/c/569498/ ?
13:43:09 mriedem alex_xu: glancing at the comments, the question is if the instance should be in ACTIVE or ERROR state?
13:43:23 alex_xu mriedem: yea, that is the question
13:43:56 mriedem if we hit the scheduler during rebuild today because the image changes, and we get NoValidHost, we put the instance into ERROR state
13:44:22 mriedem so i'd expect we do the same if the new image traits are invalid for the instance.host - which is essentially the same thing as a NoValidHost we'd get from the scheduler
13:45:31 mriedem you can rebuild an instance in ERROR state btw

Earlier   Later