Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-04
20:35:16 cdent that doesn't remove the possibility of fixing it better late
20:35:25 efried yeah, I get it.
20:35:37 cdent I personally think we should get rid of PlacementDirect, probably by exposing the intercept functionality in PlacementFixture
20:35:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Flavor extra spec and image properties validation from API https://review.openstack.org/620706
20:36:01 cdent PlacementDirect is for a _very_ specific thing, in which a placement conf (a real one) is expected to exist
20:36:22 cdent however I have seen zero evidence that PlacementDirect ever should have existed, so I'm not inclined to put a lot of energy into it
20:36:24 cdent (for now)
20:37:01 cdent s/get rid of/get rid of (on the nova side)/
20:37:38 sean-k-mooney by the way i assume the current lag in jobs starting in the gate is jsut the usual 3 days to feature freeze lag?
20:38:09 cfriesen no, zuul has a personal grudge
20:38:27 melwitt speaking of test coverage, mnaser: how goes it with the OSA job patch? https://review.openstack.org/635852
20:38:52 mriedem btw http://status.openstack.org/elastic-recheck/data/integrated_gate.html "Generated at: 2019-02-08" so we don't know the true status of the gate right now
20:39:00 mriedem clark is investigating
20:39:05 sean-k-mooney cfriesen: :) first jenkins and now zuul well at least the ci is consitnt
20:39:20 mnaser melwitt: that test should be working and pulling down nova
20:39:45 mnaser we could probably add it as non voting
20:39:45 mnaser i need to find time to integrate os_placement into the main tests/roles but
20:39:55 mnaser a lot of our cores were laid off so we're struggling in osa land </3
20:40:02 melwitt oh :(
20:40:07 melwitt I'm sorry to hear that
20:40:24 cfriesen This is a standalone disk performance optimization patch...has +1 from mdbooth. https://review.openstack.org/#/c/616692/
20:40:37 mnaser yeah, so just trying to keep things afloat, the placement role is ready but ill make sure we get a job that continues to deploy using in-repo placement, and another one that deploys using out-of-repo
20:41:09 melwitt mnaser: ok, I wasn't sure what the job in that patch is missing/why WIP
20:41:56 mnaser melwitt: i wanted to leave it that way in case someone was to make an OSA change that starts deploying with out-of-repo by default and then the job becomes redundant
20:42:37 mnaser i need to put sometime to add an extra tox target to deploy with in-repo placement, which will ensure we don't accidentally start testing from out-of-tree when we switch the default
20:43:04 melwitt oh, I see
20:44:36 mnaser i'll get around that eventually
20:44:55 mnaser (and i can help someone if they want to help do it as well)
20:45:19 melwitt ok, and that's different than the TODO here? https://review.openstack.org/#/c/635852/1/.zuul.yaml@211
20:46:15 mnaser melwitt: oh no that's exactly that todo :P
20:47:05 melwitt "without placement" confused me bc whether it's in-repo or out, I thought that's still "with placement" :P
20:47:21 mnaser pretty much involves creating a new tox.ini target in openstack/openstack-ansible-os_nova and creating an extra var like "nova_use_in_repo_placement=True"
20:47:36 mnaser yeah that should say "without os_placement" (which is our extra role)
20:47:41 melwitt ahhh
20:48:05 melwitt and the role is already merged? I can try the tox target thing but I have no clue what I'm doing
20:55:26 openstackgerrit Chris Dent proposed openstack/nova master: Use a placement conf when testing report client https://review.openstack.org/640853
20:56:42 cdent mriedem, efried: it will soon be past my bedtime, if you need/want more changes to that ^ and it's partner please feel free
20:56:56 efried ack
20:57:36 efried cdent: what's up with the merge failure?
20:58:01 cdent it's because I submitted the dependent change at the same time
20:59:43 openstackgerrit Kashyap Chamarthy proposed openstack/nova-specs master: cpu-model-selection: s/cpu_models/cpu_model_list/ https://review.openstack.org/640866
21:31:01 mriedem cfriesen: some questions in https://review.openstack.org/#/c/620706/
21:31:07 mriedem cfriesen: happy to discuss here
21:31:15 cfriesen checking
21:31:49 openstackgerrit guang-yee proposed openstack/nova master: pass endpoint interface to Ironic client https://review.openstack.org/640879
21:33:37 openstackgerrit guang-yee proposed openstack/nova master: pass endpoint interface to Ironic client https://review.openstack.org/640879
21:34:22 cfriesen mriedem: I've got a change queued up for get_pci_requests_from_flavor. can add to existing or do as followup. Happy to split out PciInvalidAlias.
21:35:13 cfriesen rest of comments look reasonable, will address later today.
21:35:19 mriedem cfriesen: i'd split as much as possible for review sanity
21:36:35 cfriesen the get_pci_requests_from_flavor change is pretty straightforward, just hadn't pushed it yet since I was expecting to respin this.
21:43:23 openstackgerrit Matt Riedemann proposed openstack/nova master: doc: cleanup pci.alias references https://review.openstack.org/640882
21:58:24 openstackgerrit Merged openstack/nova master: Add nits from Id2beaa7c4e5780199298f8e58fb6c7005e420a69 https://review.openstack.org/640729
22:14:44 sean-k-mooney from the description of https://review.openstack.org/#/c/603844/ im not sure if its needed due to https://review.openstack.org/#/c/634276/
22:15:08 sean-k-mooney it look like we are fixign the same or related bugs in different ways on both the nova and neutron side.
22:15:23 sean-k-mooney that said i need to review https://review.openstack.org/#/c/603844/ properly
22:31:40 openstackgerrit Eric Fried proposed openstack/nova master: Stop using PlacementDirect https://review.openstack.org/640887
22:31:55 efried mriedem, cdent: A different solution ^
22:38:59 sean-k-mooney efried: ignore whitespacce helps alot
22:39:18 efried sean-k-mooney: Indeed.
22:39:32 efried Are you telling me that having seen my comment already?
22:40:22 sean-k-mooney initally i tought you rewrote most of the file. and yes i was about to ask if they were fucntuionally the same after then i made that change looked and commented on irc
22:43:27 sean-k-mooney so basically you removed the _interceptor context manager which used placement direct and instead repace teh client with the versionCheckingReportClient after enabling the placemetnFixture
22:51:35 sean-k-mooney efried: im more or less trusting that you ran the fucntional test locally with master placement and it worked but ya droping placement direct form the fuctional test makes sense.
22:52:06 sean-k-mooney is there a similar followup to remove placement direct form placement itself or is there a usecase to keep it in placement
22:52:17 efried sean-k-mooney: I ran those two suites locally. I didn't run the whole world.
22:52:21 efried letting zuul do that.
22:52:36 efried We don't know whether there's a real use case, cdent and I were talking about that (again) earlier.
22:52:50 sean-k-mooney hehe ya thats resonable :) zull is rather slow currently
22:52:53 efried It was deemed a hard requirement for something-or-other (we can't remember exactly what) for extraction, so we did it.
22:53:06 efried but at this point it's not clear that it's needed at all.
22:53:17 efried I want to say maybe it was going to be a prerequisite for cinder using it in certain cases
22:53:31 efried but then in denver they decided they weren't going to use placement for those cases
22:53:41 efried so it's possible it will never be needed.
22:54:04 efried cdent did point out that it would probably be expected to work better from "real deployment" than from test
22:54:43 sean-k-mooney placement direct just allows you to bypass the placement api is thtat correct?
22:54:45 efried anyway, at this point I think the plan is to let it languish, see if anyone decides they want/need it, and if/when they do, find out if it's broken for whatever problem they're trying to solve.
22:55:09 efried sean-k-mooney: You're not bypassing it. It behaves as a real placement service. It just doesn't go "over the wire" and uses an in-memory database (I think).
22:55:21 efried sean-k-mooney: It really does effectively the same thing as PlacementFixture, just in a slightly different way.
22:55:43 sean-k-mooney oh i tougth you could use placemetn direct to talk to a real database too
22:55:44 efried I don't understand all the "wsgi intercept" magic.
22:55:51 efried oh, yeah, I think you can.
22:56:03 efried in this case we didn't want to do that, because we wanted to use a fixture.
22:56:51 sean-k-mooney so addmitly flawed understading was it was a way to run a placement api service in process to directly acess and maniupluate placement without needing to go over the wire
22:57:13 sean-k-mooney e.g. just skip the http transprot part
22:57:31 efried yes
22:57:51 efried sorry, I was focused in on the change I just made, where I believe we were faking the database as well.
22:58:35 sean-k-mooney oh ya in the fuctional test i woudl expect us to be using a fake deb or a per test sqlite db
22:58:43 efried sean-k-mooney: Yeah, https://review.openstack.org/#/c/640887/1/nova/tests/functional/test_report_client.py@a95
22:58:47 efried mocking the db
22:59:44 sean-k-mooney right makes sense.
23:45:20 mriedem efried: i'm +2 on cdent's gate fixes https://review.openstack.org/#/q/topic:bug/1818560+(status:open+OR+status:merged) - i see you have an alternative but i can't grok that right now
23:45:36 mriedem and we could do your alternative later yeah?
23:46:57 sean-k-mooney mriedem: looking at cdents change yes there woudl be no real conflcit with merging that quick and then efried change
23:48:56 sean-k-mooney efried: change looks resoable to me as it just drops teh use of placement direct an swaps to useing the palcement fixture instead but i also dont review the report clinet stuff often so im +1 but i also am not that familar those tests
23:49:02 sean-k-mooney mriedem: ^
23:51:24 sean-k-mooney mriedem: for what its work like at the live ci log it just passed all the functional test and should report back in about 3-4 mins
23:51:50 sean-k-mooney http://logs.openstack.org/87/640887/1/check/nova-tox-functional/1ba43e5/testr_results.html.gz
23:52:08 sean-k-mooney that ^ is the result of efried alternitve
23:52:19 mriedem the gate is broken and it's 6pm and i literally do not want to spend the time reviewing a new change
23:52:52 mriedem i only logged back on to +2 those changes

Earlier   Later