Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-04
20:17:26 cdent the fact that nova's conf was being used to run a placement wsgi app was revealed
20:17:35 cdent by the fact that we added a config on the placement side
20:18:14 mriedem link to the placement change?
20:18:15 cdent so it was going to happen as soon as that divergence happened and it just so happens it was today
20:18:37 cdent mriedem: it's linked from both commit messages at the topic above
20:18:40 mriedem https://review.openstack.org/#/c/619050/
20:18:45 mriedem ok i was looking at the bug
20:19:13 cdent the bug description wasn't quite right, because I was trying to be hasty
20:23:24 cdent I've udpated the title and description
20:23:59 efried cdent: I'm back now. How can I help?
20:24:26 cdent efried: I figured it out, but if you could review https://review.openstack.org/#/q/topic:bug/1818560 to confirm I figured it out the right way, that would be good
20:25:55 efried ...
20:28:39 mriedem cdent: comments inline but if it passes it passes
20:29:05 mriedem we likely need to talk about test coverage for the externally-used placement fixture
20:29:12 mriedem *at some point
20:30:00 cdent yes
20:30:10 cdent but I figured at this stage of the game it was better to just fix it
20:30:33 mriedem yeah i agree
20:30:44 openstackgerrit Chris Friesen proposed openstack/nova master: Improve existing flavor and image metadata validation https://review.openstack.org/640841
20:34:24 sean-k-mooney mriedem: cdent i dont know if we could somehow run the nova functional tests on change to the placement repo or perhaps just that fixture in the placmeent repo?
20:34:36 efried cdent: ugh, I had wanted to get rid of either PlacementFixture or PlacementDirect, and now we've made them inextricably interdependent.
20:35:06 cdent efried: we already had both and we're trying to make a quick fix
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.

Earlier   Later