| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-10 | |||
| 12:57:12 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Remove the extensions framework from wsgi.py https://review.openstack.org/607092 | |
| 13:04:13 | pooja_jadhav | hi team, how I can create a BFV instance in the api-sample-tests for testing. please guide me for the same. | |
| 13:05:59 | pooja_jadhav | I am referring https://github.com/openstack/nova/blob/85b36cd2f82ccd740057c1bee08fc722209604ab/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl | |
| 13:07:11 | pooja_jadhav | for creating an instance. but compute_utils. is_volume_backed giving False for the instance. | |
| 13:11:27 | pooja_jadhav | gmann: ^^ | |
| 13:13:30 | gmann | pooja_jadhav: oh, that did not work ? | |
| 13:14:18 | pooja_jadhav | gmann: unfortunately, no :( | |
| 13:14:59 | mriedem | you'd have to use the cinder fixture for one thing | |
| 13:15:06 | pooja_jadhav | gmann: can u plz tell me.. which existing json.tpl should I use to create a BFV instance?? | |
| 13:15:22 | mriedem | https://github.com/openstack/nova/blob/85b36cd2f82ccd740057c1bee08fc722209604ab/nova/tests/functional/api_sample_tests/api_samples/servers/v2.42/server-create-req.json.tpl isn't going to work since the cinder fixture doesn't create volumes | |
| 13:15:59 | gmann | i suggest to write funtional test for that instead of sample tests | |
| 13:16:02 | mriedem | see how https://github.com/openstack/nova/blob/85b36cd2f82ccd740057c1bee08fc722209604ab/nova/tests/fixtures.py#L1583 is used in functional tests | |
| 13:16:17 | mriedem | e.g. https://review.openstack.org/#/c/608771/ | |
| 13:21:30 | Luzi | hey Nova, we've written the Spec for Image Encryption for Nova and would appreciate reviews :) https://review.openstack.org/#/c/608696/ | |
| 13:21:53 | pooja_jadhav | mriedem, gmann: thanks will look into this | |
| 13:22:13 | gmann | mriedem: yeah, pooja_jadhav tried to add the create volumes stub there | |
| 13:22:13 | jangutter | sean-k-mooney, jaypipes: I'm looking at https://github.com/openstack/os-vif/blob/master/os_vif/objects/host_info.py and I've got some questions. (will post them to the room in general) | |
| 13:22:32 | sean-k-mooney | jangutter: sure go for it | |
| 13:22:40 | jaypipes | I blame sean-k-mooney. | |
| 13:22:48 | stephenfin | sean-k-mooney: I wonder if we should be setting 'model' here for SR-IOV VIFs? https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L130-L133 | |
| 13:23:05 | gmann | pooja_jadhav: mriedem i feel normal functional tests make more sense to verify the compute_utils. is_volume_backed instead of api sample tests | |
| 13:23:15 | gmann | like example mriedem mentioned | |
| 13:23:18 | jangutter | for the host_info objects, there's a mechanism to filter vif types by name and version, but not a corresponding mechanism for port_profiles. | |
| 13:23:23 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Remove the caching the resource on Request object https://review.openstack.org/609403 | |
| 13:23:51 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Remove more code related to extensions and testing https://review.openstack.org/607088 | |
| 13:23:59 | sean-k-mooney | stephenfin sorry taught that message was from jangutter am in what context | |
| 13:24:02 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Remove the extensions framework from wsgi.py https://review.openstack.org/607092 | |
| 13:24:03 | stephenfin | sean-k-mooney: If we do and that hw_vif_model==virtio, we'll undo the hard work of https://github.com/openstack/nova/commit/622ebf2fab0a9bf75ee12437bef28f60e083f849 right? | |
| 13:24:21 | openstackgerrit | Yikun Jiang proposed openstack/nova-specs master: Support initial allocation ratios https://review.openstack.org/552105 | |
| 13:24:31 | sean-k-mooney | jangutter: yes currently nothing uses the filter. it was there for future use but its still in the future | |
| 13:24:42 | sean-k-mooney | jangutter: so we can extend it | |
| 13:24:57 | stephenfin | sean-k-mooney: Heh, yeah, two people talking about related stuff is confusing :) I'll test this myself in a bit. Just sanity checking | |
| 13:25:29 | gmann | alex_xu: mriedem melwitt this complete the API extensions merge work - https://review.openstack.org/#/q/topic:bp/api-extensions-merge-stein+status:open | |
| 13:25:40 | mriedem | pooja_jadhav: gmann: i'm not sure why we need a new functional test for bfv? | |
| 13:25:45 | sean-k-mooney | well if you set hw_vif_model=Anything we shoudl respect that | |
| 13:25:50 | mriedem | we have existing functional tests that cover that flow | |
| 13:25:54 | mriedem | including the patch i just linked to you | |
| 13:26:00 | gmann | i will check the gate if any tests i need to fix otherwise it is good to go | |
| 13:26:09 | sean-k-mooney | stephenfin: but i need to check both patches to see how they interact | |
| 13:26:27 | mriedem | gmann: ack on the api extension merge series - if you haven't, you should queue that up in a runway | |
| 13:26:29 | jangutter | sean-k-mooney, jaypipes: (host_info context) right, so there's two ways of doing a check like this. the 'naive bayes' way, or the really explode the dependencies way. | |
| 13:26:41 | gmann | mriedem: right, those should work untill pooja_jadhav trying anything special or new feature (that is not up for review so not sure) | |
| 13:26:55 | pooja_jadhav | mriedem: I am writting in api-sample-test for (if instance is BFV, then local_gb_used should be 0 in in the simple tenant usage api) | |
| 13:27:20 | sean-k-mooney | stephenfin: if your asking should we remove https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L130-L133 the answer is no but we may need to adapt the queue size patch | |
| 13:27:34 | pooja_jadhav | mriedem, gmann : https://bugs.launchpad.net/nova/+bug/1715570 | |
| 13:27:34 | openstack | Launchpad bug 1715570 in OpenStack Compute (nova) "simple tenant usage api calculating disk usages incorrectly" [Medium,In progress] - Assigned to Bhagyashri Shewale (bhagyashri-shewale) | |
| 13:27:40 | jangutter | sean-k-mooney, jaypipes: (host_info context) the difference comes in, are you going to ever worry about a case where VIFobject 1.0 supports PortProfile 1.0 and not 2.0? | |
| 13:27:42 | jaypipes | jangutter: the host_info should be able to communicate to Neutron's agents exactly the kinds of VIFs that it can plug. If that means we need to pass some additional information about port profiles supported by the host, so be it. | |
| 13:28:10 | sean-k-mooney | jaypipes: we should but we dont currently | |
| 13:28:15 | gmann | mriedem: added | |
| 13:28:19 | jaypipes | jangutter: if that makes a difference to what Neutron negotiates with the host, then yes... | |
| 13:29:17 | jangutter | jaypipes: (host_info context) it's similar to the question: does the base object need to bump its version if any of the members bump up a version? | |
| 13:29:46 | mriedem | pooja_jadhav: we don't need to use an api sample test for that bug | |
| 13:29:47 | sean-k-mooney | jangutter: unfortunetly no. i think it should be we do not bump for compostion | |
| 13:29:55 | jaypipes | jangutter: no | |
| 13:30:20 | mriedem | pooja_jadhav: a simple functional test which creates a volume-backed server and then queries the simple tenant usage API to assert it's showing disk usage when it shouldn't should suffice | |
| 13:30:25 | jaypipes | jangutter: but see my comment on the datapath offload port profile patch that you shouldn't be modifying the VIFPortProfileBase object like that. | |
| 13:30:26 | pooja_jadhav | mriedem: but existing tests for simple tenant usage api are in api sample tests. | |
| 13:30:38 | mriedem | pooja_jadhav: that doesn't really matter | |
| 13:30:39 | sean-k-mooney | jangutter: if you add a filed to base then you bump the version of all the derived types too | |
| 13:30:47 | pooja_jadhav | mriedem: okies | |
| 13:30:52 | jangutter | jaypipes: yep, that's revving because of inheritance, another story. | |
| 13:30:53 | sean-k-mooney | jangutter: so bump for inheritance changes but not for composition | |
| 13:30:54 | jaypipes | sean-k-mooney: that's not true any more AFAIK. | |
| 13:31:01 | mriedem | pooja_jadhav: something like the setup in https://review.openstack.org/#/c/608771/ should be most of the work, | |
| 13:31:08 | mriedem | then it's just querying the simple tenant usage API and asserting the results | |
| 13:31:17 | sean-k-mooney | jaypipes: it better be or we are screwed | |
| 13:31:38 | pooja_jadhav | mriedem: yeah.. thanks | |
| 13:31:48 | mriedem | pooja_jadhav: we could just put the test patch on top of ^ to re-use the same setup | |
| 13:31:56 | mriedem | i can try that quick | |
| 13:32:06 | jaypipes | sean-k-mooney: the version manifest tracks versions for sub-classes separately from the base classes, meaning you don't need to bump the sub-class versions when a base version increases. | |
| 13:32:42 | sean-k-mooney | jaypipes: correct but if we dont then adding a filed to the base mean the derived has a new filed also and no version bump | |
| 13:33:12 | sean-k-mooney | similary if we remove a filed form the base that filed goes away in the derived without a version bump and we are similarly screwed | |
| 13:33:13 | jaypipes | sean-k-mooney: the derived version is only indicating the version of the derived-specific fields. | |
| 13:33:54 | jaypipes | dansmith: you up yet? :) | |
| 13:34:00 | sean-k-mooney | jaypipes: no its indicating the version fo the whole object if not then its not safe to inherit OVOs | |
| 13:34:12 | dansmith | jaypipes: yes | |
| 13:34:42 | jaypipes | dansmith: we are discussing whether it is required to bump derived class object versions if a base versions is incremented. | |
| 13:34:44 | jangutter | jaypipes, sean-k-mooney: If you don't bump because of composition, then you can flatten out the list of objects, regardless of how which members they belong to. | |
| 13:35:08 | dansmith | jaypipes: if you change something in the base, you've changed it in the sub and the hash should change (thus need a version bump) IIRC | |
| 13:35:29 | sean-k-mooney | dansmith: that is my understanding too. | |
| 13:35:30 | dansmith | jaypipes: _simply_ changing the master version shouldn't require a child bump, but there would be no reason | |
| 13:35:30 | jaypipes | dansmith: oh, am I confusing the composition rules with inheritance rules? | |
| 13:35:35 | stephenfin | sean-k-mooney: Nope. Rather, we just should make that if conditional on whether we're requesting SR-IOV VIFs or not | |
| 13:35:49 | jangutter | jaypipes, sean-k-mooney: I mean then you don't need to specifically associate _which_ VIF and which port-profile object/version combination go together, you just need to check if you understand each object/version combination separately. | |
| 13:35:55 | dansmith | jaypipes: I think you're confusing how including an object in another object used to require lockstep versioning, but that isn't the case any more | |
| 13:36:13 | sean-k-mooney | stephenfin: nope for macvtap i think you can set the model | |
| 13:36:22 | jaypipes | dansmith: right. I was confusing composition rules with inheritance rules. sorry (again) :( | |
| 13:36:25 | stephenfin | *SR-IOV direct | |
| 13:36:52 | sean-k-mooney | stephenfin: im also not sure if that assumtion is safe to make but it might be | |
| 13:37:10 | pooja_jadhav | mriedem: Let me try if u dont mind? | |
| 13:37:13 | stephenfin | moshele: Any thoughts on the above? | |
| 13:37:32 | stephenfin | moshele: tl;dr: I wonder if we should be setting 'model' here for SR-IOV VIFs? https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L130-L133 | |
| 13:37:42 | sean-k-mooney | dansmith: right but if the composed object version change how we make object compatible today is actully inccorect | |
| 13:37:46 | stephenfin | moshele: If we do and that hw_vif_model==virtio, we'll undo the hard work of https://github.com/openstack/nova/commit/622ebf2fab0a9bf75ee12437bef28f60e083f849 I think | |
| 13:38:02 | stephenfin | moshele: SR-IOV direct VIFs, that is | |
| 13:38:27 | jangutter | dansmith: if you use composition, would it make sense to check a list of classes and versions for compatibility, or would you need to check the combinations? | |
| 13:38:53 | sean-k-mooney | stephenfin: you can have VF that are virtio just an FYI | |
| 13:38:58 | dansmith | sean-k-mooney: I don't understand what you're saying | |
| 13:39:31 | dansmith | jangutter: heh, I'm also not sure what you mean | |