17:00:18 <hogepodge> #startmeeting refstack
17:00:19 <openstack> Meeting started Tue Oct 24 17:00:18 2017 UTC and is due to finish in 60 minutes.  The chair is hogepodge. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:20 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:23 <openstack> The meeting name has been set to 'refstack'
17:00:36 <catherineD> o/
17:00:39 <chandankumar> \o/
17:00:47 <hogepodge> #link https://etherpad.openstack.org/p/refstack-meeting-17-10-24 agenda
17:01:14 <mguiney> o/
17:01:19 <tosky> o/
17:01:57 <hogepodge> Hi everybody. I'll get us stated in about a minute.
17:02:32 <hogepodge> #topic refstack-client Tempest Autoconfig
17:02:53 <hogepodge> #link https://review.openstack.org/#/c/489421/ spec
17:03:01 <hogepodge> the spec was updated this morning.
17:03:10 <hogepodge> chandankumar: want to take the lead on this topic?
17:03:28 <chandankumar> hogepodge: i have cleaned up initial spec
17:03:48 <chandankumar> hogepodge: i need to add workitems there
17:04:05 <chandankumar> reviews and comments are welcome on the spec
17:04:40 <hogepodge> any comments on the work so far?
17:04:49 <hogepodge> everyone please read and make comments
17:05:14 <chandankumar> apart from the spec i have some more updates
17:05:18 <luzC> o/
17:05:54 <chandankumar> martinkopec is working on adding zuulv3 devstack gate against python-tempestconf repo
17:05:56 <chandankumar> https://review.openstack.org/#/c/489421/
17:06:12 <hogepodge> that's the same patch as the spec
17:06:38 <chandankumar> https://review.openstack.org/513330
17:07:21 <chandankumar> and also we have fixed python-tempestconf to work with keystone v3 api
17:07:35 <luzC> I added both to my list... I'll take a look
17:07:38 <hogepodge> great
17:07:44 * mguiney nods
17:07:47 <chandankumar> that's it from my side.
17:07:52 <mguiney> same, excited to see the spec updated
17:08:00 <hogepodge> the zuulv3 migration is supposed to be in a good place this week
17:08:38 <chandankumar> yup
17:08:45 <mguiney> been seeing exactly zero zuul based test failures recently
17:09:12 <hogepodge> glad to hear it's working with keystone v3. They're removing v2 soon.
17:09:24 <tosky> already removed
17:09:28 <hogepodge> We've already adjusted support away from v2 in the client
17:09:34 <hogepodge> haha soon as in yesterday :-)
17:10:00 <hogepodge> anything else on the autoconfig work?
17:10:02 <tosky> zuulv3 itself is working, including most of the legacy automatically migrated job; the conversion to native jobs will take a bit more
17:10:41 <hogepodge> since we're talking about that, there's also this patch
17:10:43 <hogepodge> #link https://review.openstack.org/#/c/512936/ (zuul v3 migration)
17:10:45 <chandankumar> hogepodge: nope
17:12:13 <hogepodge> tosky: chandankumar: are you zuul v3 experts? Does that work look correct?
17:12:26 <hogepodge> Gate jobs are passing, so that's a good sign.
17:12:34 <chandankumar> hogepodge: tosky is the best person
17:12:35 <tosky> but those are the old jobs
17:12:46 <chandankumar> hogepodge: i have started playing 2 days back
17:12:51 <hogepodge> yeah, it just looks like the first step in migration
17:12:55 <chandankumar> on tempest side
17:13:24 <hogepodge> I'll have to read the guide, but things at a high level look ok
17:13:27 <tosky> the -infra team converted (one-shot) the existing v2 jobs in a set of v3 jobs which are made of ansible which wraps shell scripts, with a lot of legacy assumptions
17:13:35 <tosky> that's why they are called legacy-
17:13:58 <hogepodge> yeah
17:14:09 <luzC> hogepodge: I reviewed it but I think that when approved jobs will run twice (legacy and locally copied)... was wondering if we should start working on the migration to have like a clean up...
17:14:16 <tosky> the same guide suggests to first copy those legacy jobs (if they are working, otherwise fix them first) into the repository which is relevant for them
17:14:25 <tosky> and then remove them from project-config
17:14:26 <hogepodge> luzC: that's a good idea
17:14:39 <tosky> and then finally work on converting them into native zuulv3 jobs
17:15:01 <chandankumar> hogepodge: i think for tox based jobs for refclient we can inherit from tox parent job something like this https://review.openstack.org/#/c/514269/
17:15:02 <tosky> now, it's up to each project whether to follow the full procedure above ^^
17:15:14 <hogepodge> I
17:15:17 <tosky> or write directly native jobs (which may take more time)
17:15:22 <hogepodge> I'm only seeing two legacy jobs running there
17:15:39 <hogepodge> Is there a volunteer to do the job migration?
17:15:51 <hogepodge> take the lead on that
17:16:06 <chandankumar> hogepodge: https://review.openstack.org/#/q/topic:tempest_plugin_sanity+(status:open) it will help as an example
17:16:54 <luzC> hogepodge I'll take a look to start with the steps
17:17:04 <hogepodge> thanks luzC
17:18:18 <hogepodge> #topic Subunit Upload
17:19:17 <hogepodge> #link https://review.openstack.org/#/c/506826/ subunit upload
17:19:24 <hogepodge> spec for the subunit upload
17:20:02 <mguiney> I did a major update on this one
17:20:30 <mguiney> several of the proposed endpoints were either unnecessary, or would be easily derivable from the data returned via the otehr endpoints
17:21:08 <hogepodge> catherineD: can you further explain your comment so we can get the PUT/POST semantics right?
17:21:09 <mguiney> so, for the sake of simplicity and sanity, i trimmed it down a great deal
17:21:19 <mguiney> ah sorry, hadnt seen the new reivew
17:21:52 <catherineD> ok both POST and PUT send subunit data up to the RefStack server
17:22:17 <catherineD> the addition is PUT link back to RefStack and POST does not
17:23:01 * mguiney nods
17:23:45 <hogepodge> catherineD: mguiney: on the get link, with the {id}, in restful apis I usually have the bare endpoint do a list, and the endpoint with the id just return the object
17:23:46 <catherineD> by allowing that the scenario is .. user may have uploaded the data ..
17:24:07 <catherineD> next he want to link the data to RefStack but ended up upload the same data again
17:24:31 <catherineD> hogepodge: correct
17:24:35 <mguiney> That shouldnt happen, i think
17:24:42 <hogepodge> catherineD: you want post to create and put to upload?
17:25:19 <catherineD> create and upload mean the samething right? both  just upload the data?
17:25:21 <mguiney> because the linkage function will check to see if the subunit data corresponds with the given result ID and, from there, should upload it or not based on whether the data actually is consistent with the existing result
17:26:19 <catherineD> mguiney: in that case the link command does not need the data ... just the uuid of the uploaded subunit data
17:26:24 <hogepodge> catherineD: mguiney: my understanding of put vs post is that post should create, and put is used to replace on an existing resource
17:26:32 <mguiney> upload generates test result data from subunit, link allows you to link and existing result to the corresponding (non uploaded) subunit data, and if it matches, will add the subunit data into the database
17:26:42 <hogepodge> but lots of apps treat them the same because it's confusing
17:26:43 <catherineD> hogepodge: correct
17:27:21 <mguiney> basically a way for vendors to upload an expanded dataset for results that have already been uploaded, so that they can securely get them to us for debugging assistance reasons
17:27:24 <hogepodge> so I just want to understand how we want the methods to behave
17:27:37 <catherineD> previous RefStack API was created that way .. PUT is for update
17:27:40 * mguiney nods
17:28:01 <catherineD> API --> APIs
17:28:03 <mguiney> yeah, I've had trouble, in the past, getting a consistent answer on the put vs post question, it's good to have a proper answer for that, finally
17:28:14 <hogepodge> ok, so let's be sure we capture that. It's the preferred behavior. Although, I don't think put should replace, once the data is verified it should be immutable
17:28:20 <mguiney> ok. apologies for the confusion, i will update that
17:28:37 <hogepodge> so put on existing result will verify first, and if it matches is accepted and can't be updated
17:28:44 <hogepodge> except for the entire result being deleted
17:29:03 <mguiney> oh! additional question
17:29:27 <hogepodge> let's capture these comments in the code review
17:29:43 <catherineD> so the endpoint here is v1/subunit ... unless you update anything related to subunit ... the PUT with that endpoint should not be used
17:29:54 <mguiney> if we arent uploading subunit data that isnt either used to generate a new result, or that will not upload unless consistent with an existing result
17:30:23 <mguiney> do we need a separate subunit delete functionality, or should we build that into the existing result deletion functionality
17:30:51 <hogepodge> catherineD: I'm thinking that the subunit data should share the ID with the test result? They shouldn't be separable from one another
17:31:39 <hogepodge> Or to put it another way, the relationship is test result can "have a" subunit field, and the subunit field is indexed through the test result
17:31:48 <catherineD> hogepodge: that means that you with have a subunit uuid in the refstack test table  ... subunit data should be standalone
17:31:51 <hogepodge> so the end user should never see the subunit database uuid
17:32:40 <mguiney> ok, that works
17:32:42 <hogepodge> subunit should always have an associated test result. It can have it's own database id, but imo the user should never see it
17:33:14 <mguiney> i think you are absolutely right on that one
17:33:37 <catherineD> let me clarify the table names in refstack ... test table save the test record info  ... results table store the individual test results (test name etc ..)
17:33:41 <mguiney> ok. This is really good feedback, excited to get this updated
17:34:21 <catherineD> so what you need is to link the subunit uuid to the test table by adding a subunit uuid in the test table
17:34:44 <hogepodge> catherineD: yes, I agree. It should not be visible to end user, though.
17:35:00 <mguiney> Wasn't the plan to link the two via a keypair in refstack's meta table? why not use that, instead
17:35:02 <hogepodge> catherineD: If you upload a bare subunit file, it will create a new parent test result
17:35:13 <catherineD> enduser (foundation) member want to get the subunit data right?
17:35:22 <catherineD> if so you need an API for that
17:35:31 <hogepodge> catherineD: they should get it through the test result id
17:36:18 <catherineD> hogepodge: but that is not how the POST API define ... there is no linkage in the POST command
17:36:33 <hogepodge> catherineD: POST should create a new test result
17:36:40 <hogepodge> catherineD: PUT links to existing result
17:37:20 <catherineD> a new test result should never be created by a subunit upload command
17:37:40 <hogepodge> catherineD: why?
17:38:17 <catherineD> a new test result is created by the current test result upload API
17:38:38 <catherineD> uploaded of a subunit result should link to a test result not creating a test result
17:38:56 <mguiney> but if we are uploading the subunit data directly, why not just generate the result from that?
17:39:09 <mguiney> given that it's the same data, just in expanded form
17:39:47 <mguiney> (sorry, curious as to why that wouldnt be considered correct practice)
17:40:10 <hogepodge> catherineD: we can eliminate the set of client side test result creation. If the user says to upload subunit data by default (which is the direction I want to go in... all users must upload subunit at some point in the future) it saves that round trip
17:40:40 <hogepodge> catherineD: I want us to be able to do both. Linking is critical, but to me creation is the future
17:40:55 <luzC> I agree with mguiney, I think we should encourage end users to upload the subunit file, endpoint can handle the creation of the test result out of it
17:40:58 <catherineD> mguiney: ah, if that is the case ...  you should specify in the spec to create the result from the subunit result and then update both the refstack "test" and "result" table
17:41:10 <mguiney> 07
17:41:14 <mguiney> * o7
17:41:16 <mguiney> can do
17:41:23 <hogepodge> yes, we can definitely clarify the spec :-)
17:41:28 <hogepodge> This is a good discussion.
17:41:44 <mguiney> apologies, this spec is one of those things that I've spent so much time with that I clearly have some unstated assumptions that i need to state
17:41:49 <catherineD> alright I did not see that is the case ....
17:42:06 <hogepodge> the spec becomes documentation in the case of refstack, so the time spent to nail it down is good
17:43:07 <hogepodge> Any more comments on this? Can everyone leave a review on the spec so we can iterate on it?
17:43:31 <hogepodge> With 15 minutes left, going to move on
17:43:36 <hogepodge> #topic Result Verification Field and Update
17:43:51 <hogepodge> #link https://review.openstack.org/#/c/507695/ (disable anonymous upload)
17:44:01 <hogepodge> still in progress, mguiney is taking that patch over
17:44:15 * mguiney nods
17:44:26 <hogepodge> added a configuration field to not break current users, but testing needs to be refined
17:44:51 <hogepodge> There's also a patch that adds token checking to the result marking script
17:44:51 <luzC> hogepodge I took a quick look, I thought you were removing some functionality not adding a new option
17:44:53 <hogepodge> #link https://review.openstack.org/#/c/503495/ (token checking)
17:45:20 <mguiney> yup! it's currently sticking on some unit testing issues, is making a call that may (based on config) required key auth, in some cases without a key
17:45:24 <hogepodge> luzC: the new option removes functionality, but there are people using RefStack, and I don't want this update to break their environment
17:45:31 <mguiney> have a solution ready for testing, hoping to get that pushed later today
17:45:43 <hogepodge> luzC: so by adding a configuration option, we maintain backwards compatibility
17:45:47 <mguiney> ah shoot we moved on
17:46:07 <hogepodge> mguiney: they're both under the same topic :-D
17:46:15 <mguiney> ah right lol
17:46:30 <mguiney> re the token patch, still doing a little formatting cleanup
17:46:35 <hogepodge> I don't want us to make all of the verified results until anonymous upload is disabled
17:46:39 <luzC> hogepodge: I'll take a look and leave comments on the patch
17:46:46 <mguiney> the suggested change broke some things, and i'm going through and amending them still
17:46:49 <hogepodge> I don't want to have to keep running the script.
17:46:55 <mguiney> (^re token auth patch)
17:47:11 <mguiney> (^re *refstack database update token auth patch)
17:47:23 <hogepodge> So the planned steps are to disable anonymous upload, then run the script.
17:47:50 <hogepodge> thanks luzC
17:47:55 <luzC> but it is an option, hence people can still upload anonym hence script will need to be run again
17:48:09 <hogepodge> luzC: when we deploy to production that parameter will be set to true
17:48:19 <luzC> ok
17:48:55 <hogepodge> opnvf is using a fork of refstack, and I don't want to break them
17:50:16 <hogepodge> #topic maintenance and questions
17:50:29 <hogepodge> luzC: has a couple of patches
17:50:31 <hogepodge> #link https://review.openstack.org/#/c/513582/ (debug environment)
17:50:42 <hogepodge> #link https://review.openstack.org/#/c/513580/ (py35)
17:50:47 <hogepodge> luzC: want to comment?
17:51:41 <luzC> I setup an environment with py35 and got an error due to string handling (common for py35)... those are the changes I proposed on the second patch
17:52:00 <luzC> the first one is to add tox environment to debug with tox
17:52:44 <luzC> if you can review and test it, it would be good
17:52:54 <mguiney> oh dang, so sorry
17:53:16 * mguiney sees that the files covered by the patch are files i've recently been working on
17:54:13 <luzC> mguiney: NP, just wondering why did we reduce the coverage threshold instead of adding more unit testing...
17:54:33 <hogepodge> luzC: to get that non-voting job to go green
17:54:40 <hogepodge> luzC: it was probably a bad idea
17:55:06 <hogepodge> on the alembic directory we weren't checking coverage on anything, so we just captured the whole directory rather than individual files.
17:55:30 <hogepodge> luzC: we should roll back the line change number, although the choice of four lines is arbitrary and a bit silly imo
17:56:20 <mguiney> it's not a bit of code that gets run very often, and though we had omitted it in tox.ini, it was still throwing errors in testing while pushed
17:56:30 <luzC> I don't have opinion on it, but prefer to have more tests added
17:56:38 <luzC> **strong opinion
17:56:42 <mguiney> although the job was, to be fair, nonvoting, but yeah
17:57:14 <mguiney> Honestly, that is kind of the plan, but there are some things a bit higher on the priority queue, just due to timeframe goals
17:57:30 <mguiney> I can bump it up the queue though, if needed
17:57:39 <luzC> ok, I think we can leave it as it is now
17:58:00 <luzC> or for good...
17:58:02 <luzC> :)
17:59:01 <hogepodge> I'm on the fence for a meeting next week.
17:59:17 <hogepodge> I fly that day, but not until the afternoon, but have errands to run beforehand.
17:59:24 <hogepodge> anyone want to lead next week?
17:59:37 <hogepodge> We can discuss in the #refstack channel
17:59:54 <hogepodge> thanks everyone
18:00:02 <hogepodge> #endmeeting