Skip to content
This repository has been archived by the owner on Jul 16, 2020. It is now read-only.

Weekly Meeting 2016 09 22

Jump to bottom
Tim Pepper edited this page Sep 22, 2016 · 2 revisions

Agenda

##Minutes

#ciao-project: weekly_meeting

Meeting started by kristenc at 16:05:00 UTC. The full logs are available at ciao-project/2016/ciao-project.2016-09-22-16.05.log.html .

Meeting summary

Meeting ended at 17:01:00 UTC.

Action Items

  • tcpepper disposition issues 572, 573, 575, 580, 581, 587
  • tcpepper update agenda to include ceph in test discussion Sep 29

Action Items, by person

  • tcpepper
    • tcpepper disposition issues 572, 573, 575, 580, 581, 587
    • tcpepper update agenda to include ceph in test discussion Sep 29
  • UNASSIGNED
    • (none)

People Present (lines said)

  • tcpepper (119)
  • kristenc (67)
  • markusry (66)
  • mrkz (5)
  • ciaomtgbot (4)
  • mcastelino (4)
  • sameo (3)
  • wdouglas (2)
  • jvillalo (1)
  • david-lyle (1)

Generated by MeetBot_ 0.1.4

.. _MeetBot: http://wiki.debian.org/MeetBot

###Full IRC Log

16:05:00 <kristenc> #startmeeting weekly_meeting
16:05:00 <ciaomtgbot> Meeting started Thu Sep 22 16:05:00 2016 UTC.  The chair is kristenc. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:05:00 <ciaomtgbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
16:05:00 <ciaomtgbot> The meeting name has been set to 'weekly_meeting'
16:05:09 <mrkz> (#topic #agreed #info etc)
16:05:23 <kristenc> i get to be here after all.
16:05:27 <kristenc> o/
16:05:28 <tcpepper> ah ok
16:05:34 <tcpepper> #topic rollcall
16:05:36 <tcpepper> :)
16:05:39 <tcpepper> o/
16:05:41 <markusry> o/
16:05:42 <mcastelino> o/
16:05:47 <jvillalo> o/
16:06:41 <mrkz> o/
16:06:53 * kristenc is assuming tcpepper will drive the meeting this week.
16:07:05 <david-lyle> o
16:07:16 * tcpepper can drive the meeting this week
16:07:34 <tcpepper> dunno if the bot will record my #commands since I didn't start it?
16:07:45 <tcpepper> #topic opens
16:08:14 <tcpepper> hopefully there are some or it'll be a quick meeting...not much on the agenda
16:08:35 * kristenc hopes for quick meeting instead :)
16:08:47 <mrkz> tcpepper: kristenc could give you chair power by doing #chair tcpepper
16:08:57 <kristenc> #chair tcpepper
16:08:57 <ciaomtgbot> Current chairs: kristenc tcpepper
16:09:02 <mrkz> but few commands are reserved only for chairs :)
16:09:11 <kristenc> thanks
16:10:13 <tcpepper> well then....
16:10:18 <tcpepper> #topic bugs
16:10:27 <tcpepper> #link https://github.com/01org/ciao/issues?q=is0X0P+0open+is0X0P+0issue+label0X0P+0bug
16:10:36 <tcpepper> we've got four that are not prioritized
16:10:41 <tcpepper> let's go in numerical order
16:10:47 <tcpepper> #584
16:10:53 <tcpepper> #link https://github.com/01org/ciao/issues/584
16:11:09 <markusry> Right I added that
16:11:16 <markusry> The singlevm seemed to work fine
16:11:22 <tcpepper> annoyance level then..P3?
16:11:23 <markusry> But I saw the error log
16:11:27 <markusry> ged
16:11:33 <markusry> Yep, sounds good to me
16:11:48 <tcpepper> #589
16:11:52 <tcpepper> #link https://github.com/01org/ciao/issues/589
16:12:04 <tcpepper> this is one we've had since...April?
16:12:14 <tcpepper> so almost by definition I'd call it a P3 or lower
16:12:29 <tcpepper> even janitorial
16:12:40 <tcpepper> I thought it was captured in another issue, but I didn't see it in an open issue
16:12:54 <markusry> I did enter some unit test failures this week
16:13:05 <tcpepper> #591
16:13:13 <tcpepper> #link https://github.com/01org/ciao/issues/591
16:13:23 <markusry> Is it the same as https://github.com/01org/ciao/issues/585?
16:13:30 <mrkz> just a comment here, I think janitorial issues would be even friendlier if it has a comment/description of how to fix, so newcomers gets the `click` really fast
16:13:35 <markusry> The unit test that fails is different
16:13:44 <markusry> But the panic seems to have the same underlying cause
16:13:49 <tcpepper> mrkz: yes!  good sugestion
16:14:11 <tcpepper> hmm 585 and 589 are similar yes
16:14:12 <markusry> panic: assignment to entry in nil map [recovered]
16:14:42 * tcpepper labels 585 a bug
16:15:46 <tcpepper> on 591...
16:16:03 <tcpepper> markusry: P2?
16:16:16 <markusry> Yep.  This is bad and it needs fixing.
16:16:27 <markusry> It could prevent a CN from connecting to the cluster
16:16:33 <tcpepper> p1 then?
16:16:35 <markusry> or a NN for that matter
16:16:40 <markusry> And then you're in real trouble
16:16:47 <tcpepper> I'm guessing it is short lived anyway..you clearly know what's up on it
16:16:55 <markusry> I think it's been there for ages
16:17:12 <tcpepper> ok i'll p2 it then if it's been there a while and is rare
16:17:13 <kristenc> I would vote for P2 since it's not a regression.
16:17:14 <markusry> But its a timing thing, more easily reproduced inside a VM
16:17:28 <markusry> which is how I spotted.
16:17:39 <markusry> it
16:18:30 <tcpepper> kristenc: are you ok with 585 and 589 are P3's?
16:19:05 <tcpepper> I'd go higher, but, they're not getting focus and locking is maybe a hard place to engage as a new contributor
16:19:10 <markusry> How should be label issues that periodically break the builds?
16:19:15 <markusry> Or cause them to fail?
16:19:25 <markusry> We have a few of these bugs this week.
16:19:26 <kristenc> tcpepper, yes, P3.
16:19:28 <tcpepper> by mrkz's suggestion a locking bug's probably hard to suggest as janitorial
16:19:33 <markusry> Last week we gave the swagger bug a P1
16:19:47 <tcpepper> markusry: yeah I'm worried...a lot more failures this week in semi-known race situations
16:19:51 <markusry> because it caused periodic build failures.
16:20:01 <kristenc> most of these problems need a fix which involves refactoring parts of controller.
16:20:06 <kristenc> to make it easier to test.
16:20:14 <tcpepper> something changed in travis or AWS which makes these trigger easier this past week
16:21:25 <markusry> maybe once storage is done we should allow ourselves a cleanup week
16:21:44 <kristenc> i think i'd need 2.
16:21:47 <kristenc> at least.
16:21:51 <kristenc> to do what needs to be done.
16:22:11 <kristenc> wrt to fixing unit test races
16:22:12 <tcpepper> kristenc: if you have a sense of the what needs to be done...a vision of the refactored architecture...we might be able to help?
16:22:43 <kristenc> tcpepper, I want to replace the use of a real datastore with a fake one and enable channels to signal when info makes it there.
16:23:00 <kristenc> it would be a very large rewrite of much of the test code.
16:23:12 <tcpepper> ok...mocking/interface-ing the datastore
16:23:17 <kristenc> yes.
16:23:36 <kristenc> part of the problem I have is that I need to wait for info to update in the datastore before doing a get to retrieve it.
16:23:47 <kristenc> It'd be better to just fake it and use channels.
16:24:02 <tcpepper> yeah it's a little odd
16:24:19 <kristenc> to much asynchronous updates are what make me rely on these time.sleep()
16:24:20 <tcpepper> in the normal runtime case we allow raciness right?  stat's are flowing independently from command actions
16:24:24 <kristenc> things that screw everything up.
16:24:35 <tcpepper> but in test cases we want to explicitly see the result of a specific command
16:24:59 <kristenc> yes - the difference between a unit test and a functional test.
16:25:18 <tcpepper> wdouglas is going to solve functional and integration tests for us ;)
16:25:27 <markusry> Hooray.
16:25:39 <kristenc> well, if wdouglas would like to refactor the controller tests I'd buy him a beer or 2 :).
16:25:39 <markusry> kristenc: This was the conclusion I came to in launcher as well.
16:25:57 <tcpepper> ok so we've got one remaining uncategorized issue I see...#595
16:25:58 <tcpepper> #link https://github.com/01org/ciao/issues/595
16:25:59 <markusry> You basically, want to use fake all your unit's dependencies
16:26:15 <markusry> But it takes longer to do
16:26:35 <kristenc> markusry, yes - the downside of having done a lot of coding before I became more advanced in my go skills to really understand why I needed interfaces.
16:26:46 <tcpepper> kristenc: ditto that!!  ^^
16:27:18 <kristenc> well - on the bright side, we are smart enough now to clean up after our former selves.
16:27:25 <tcpepper> yep
16:27:37 <tcpepper> so is #595 a clean up bug or a clean up feature?
16:27:40 <markusry> Yep.  I need that cleanup week as well.
16:27:57 <tcpepper> our logging is a mishmash that's still evolving towards a coherent entity that helps point at the code in question
16:28:12 <kristenc> tcpepper, I marked it as a bug because it was annoying me to death as I try to debug.
16:28:20 <tcpepper> yeah I've hit that too
16:28:31 <kristenc> I considered it a bug that I was having such a hard time due to this.
16:28:49 <markusry> It logger should be using glog.InfoDepth(2, ...)
16:28:52 <tcpepper> developer usability bug for sure
16:28:54 <markusry> The logger I mean
16:28:57 <markusry> ssntp has the same problem
16:29:24 <kristenc> well - let's point our whining at sameo then :)
16:29:40 <tcpepper> or fix it for him
16:29:45 <tcpepper> given markusry's comment...
16:29:51 <kristenc> tcpepper, well that is the reality of the matter, isn't it :).
16:29:52 <tcpepper> we could not that and janitorial it
16:29:53 * sameo reads backlog
16:30:47 * tcpepper copy paste's markusry's comments and marks as p2 janitorial
16:30:53 <kristenc> thanks.
16:31:49 <sameo> markusry: you mean ssntp should be using InfoDepth() ?
16:31:50 <tcpepper> so that's what I see for issues, did I miss any?
16:32:04 <markusry> sameo: Yes
16:32:12 <markusry> e.g., https://github.com/01org/ciao/blob/master/ciao-launcher/qemu.go#L53
16:32:13 <sameo> markusry: Makes sense.
16:32:46 <kristenc> tcpepper, did you check for new ones that did not get a bug label assigned to them?
16:32:52 <kristenc> not everyone remembers to do that.
16:33:10 <tcpepper> I thought I had, but...I'm seeing github's queries are screwy
16:33:13 <markusry> tcpepper: I think I see some
16:33:15 <markusry> 581
16:33:16 <markusry> 580
16:33:24 <markusry> 575
16:33:25 <tcpepper> there's a tonne of issues not marked as bug
16:33:25 <kristenc> tcpepper, can you use the one from last week?
16:33:27 <markusry> 573
16:33:31 <markusry> 572
16:34:16 <kristenc> https://github.com/01org/ciao/issues?utf8=0.000000E+002    16:34:24 <tcpepper> kristenc: that query assumes they're labelled as bug.  and it needs a "not lable:P*"
16:34:44 <kristenc> tcpepper, no this query is just issue: is open
16:34:54 <kristenc> and created after
16:35:03 <tcpepper> oh I thought I'd grabbed that one, but it's different
16:35:06 * tcpepper fixes
16:35:42 <tcpepper> so yeah: 571, 572, 573, 575, 580, 581, 587
16:35:50 <tcpepper> let's go through those in order
16:35:54 <tcpepper> #571
16:36:03 <tcpepper> #link https://github.com/01org/ciao/issues/571
16:36:58 <tcpepper> wdouglas noticed ciao-cert wasn't being tested.  and...
16:37:15 <kristenc> I effectively have blocked the PR for ciao-image until unit tests are enabled.
16:37:17 <tcpepper> since it doesn't have a _test.go, go test doesn't mark it as 0    16:37:27 <tcpepper> which feels like a test-cases bug in the end?
16:37:49 <kristenc> tcpepper, which but are we talking about?
16:37:54 <kristenc> which bug?
16:38:01 <kristenc> sounds different than 571
16:38:17 <tcpepper> 571 is about ciao-image not being testing
16:38:25 <tcpepper> we actually have multiple components not being tested
16:38:35 <markusry> Why is that a test-cases bug?
16:38:44 <kristenc> 571 isn't a test cases bug
16:38:45 <markusry> test-cases just tests what you tell it to test
16:38:57 <tcpepper> so here's the chain of issues I see
16:39:04 <kristenc> and doesn't have a problem with _test.go missing.
16:39:08 <markusry> We can't use a global wildcard as we need to run some tests as root and others as not
16:39:11 <tcpepper> ciao-images, ciao-cert aren't in the test-cases line(s) in .travis.yml
16:39:22 <markusry> If we ran everything as root we could just do
16:39:25 <markusry> test-cases ./...
16:39:30 <tcpepper> independent from 571, wdouglas and I were looking at ciao-cert
16:39:35 <tcpepper> we added it to the travis.yml
16:39:56 <kristenc> markusry, unfortunately, not everything can be run yet.
16:40:04 <tcpepper> go test via test-cases returns something (didn't dig deeply) that did not result in ciao-cert being marked as having 0�overage
16:40:07 <kristenc> for example - ciao-storage.
16:40:22 <tcpepper> it appeared to be because  there was no _test.go files in ciao-cert
16:41:02 <tcpepper> ciao-image]$ go test .
16:41:02 <tcpepper> ?   	github.com/01org/ciao/ciao-image	[no test files]
16:41:26 <tcpepper> ciao-image]$ cd ../ciao-cert/
16:41:26 <tcpepper> [tpepper@tcpepper-desk ciao-cert]$ go test .
16:41:26 <tcpepper> ?   	github.com/01org/ciao/ciao-cert	[no test files]
16:42:01 <tcpepper> that should result in a 0�overage and something like 300 lines of uncovered code being accounted
16:42:14 <kristenc> tcpepper, you have to do ./...
16:42:24 <kristenc> at least for ciao-image.
16:42:33 <kristenc> some of it does have _test.go
16:42:47 <markusry> tcpepper: I see.  I can take a look.  It could just be a go test issue
16:42:52 <kristenc> the issue is that it was never enabled in travis (my fault), and then code was added without adding new unit tests.
16:42:55 <tcpepper> sure.  what I want to insure is that we don't decide we have coverage in just github.com/01org/ciao/ciao-image/datastore
16:43:10 <tcpepper> but also note that we have 0�overage in github.com/01org/ciao/ciao-image github.com/01org/ciao/ciao-image/client github.com/01org/ciao/ciao-image/service
16:43:12 <markusry> If it doesn't generate the coverage data for go packages with no tests files there's not much we can do.
16:43:50 <tcpepper> markusry: test-cases might be able to note the "?" return and dump a line into the coverage file
16:44:16 <tcpepper> worst case it's inflated.  eg: main.go has 339 lines.  could call them all uncovered.  even though normally whitespace and comments and stuff aren't counted against you.
16:44:26 <markusry> It would need to understand the format of the coverage file.
16:44:27 <tcpepper> but then that inflation is a motivator to add a _test.go
16:44:39 <tcpepper> the coverage files are straight forward I think...
16:45:22 <markusry> Right so go test doesn't generate a coverprofile if there are no test cases.
16:46:18 <markusry> tcpepper: It could be done.
16:46:37 <tcpepper> wdouglas posted this yesterday https://www.elastic.co/blog/code-coverage-for-your-golang-system-tests
16:46:39 <markusry> Need to consider whether its really an issue with go test
16:46:59 <mcastelino> btw the CNCI agent itself does not have a functional test harness even though its library set has unit tests
16:46:59 <tcpepper> an example of somebody doing their own upper level functional coverage accounting
16:47:57 <tcpepper> so on the _specifics_ of just 571:
16:48:03 <tcpepper> P3?
16:48:08 <markusry> tcpepper: This is basically what test-cases does
16:48:22 <tcpepper> and open a separate issue to track figuring out if we can track files that have no testing
16:49:02 <markusry> Okay so there was a bug
16:49:03 <markusry> https://github.com/golang/go/issues/6725
16:49:07 <markusry> and rob pike closed it
16:49:27 <tcpepper> Status changed to WorkingAsIntended.
16:49:29 <tcpepper> boooo
16:49:35 <markusry> So if we want it fixed we have to fix it ourselves
16:49:54 <markusry> It's definitely doable.
16:50:18 <tcpepper> even outside of test-cases...
16:50:25 * wdouglas shakes fist at Mr. Pike
16:50:36 <wdouglas> We just can't get along
16:50:37 <markusry> I was thinking inside test-cases
16:50:39 <kristenc> I have a hard stop in 5 minutes btw to move to next mtg.
16:50:53 <tcpepper> we could have a shell line in the travis.yml that finds dir's with .go's and no _test.go's and appends via wc | sed to the coverage file a 100    16:50:58 <markusry> I'll enter an issue for this.
16:51:04 <tcpepper> markusry: thx!
16:51:21 <kristenc> just fyi: releases are still broken. having problems with the keystone certs for some reason.
16:51:33 <markusry> We just need to do one call to go list and we have all the info we need to generate the coverage profile.
16:51:38 <kristenc> (keystone certs for ciao-cli now that tls is enabled)
16:52:16 <tcpepper> since we're up against the top of the hour...
16:52:23 * tcpepper will go through the other issues and disposition them
16:52:28 <tcpepper> with something initially anyway
16:52:36 <tcpepper> kristenc: is there anything we can do to help on the release front?
16:53:10 <kristenc> tcpepper, I'm just having a hard time figuring out where my keystone cert is and how to make cli use it, that's all.
16:53:46 <kristenc> it works fine for the controller - so the cert is "somewhere" on the release cluster installed properly.
16:53:57 <tcpepper> ah yes
16:54:01 <kristenc> but, it's been so long since I set it up, I've forgotten where I put it, what it was called, etc.
16:54:12 <kristenc> I got away without it before the cli change.
16:54:19 <kristenc> now I can't.
16:54:20 <tcpepper> the new ansible deploy docu is being tweeked to encourage via examples setting things up with more obvious names on certificate files
16:54:44 <kristenc> one other item that I did want to draw attention to speaking of enabling unit tests.
16:54:53 <kristenc> do we have a plan for including storage testing in travis?
16:55:08 <kristenc> I'd like to enable unit tests for the ceph driver in ciao-storage
16:55:10 <kristenc> but I can't.
16:55:12 <tcpepper> I think next week obedmr- is going to steal some of our test clusters and re-provision them, via the newer scripting so their installed state is more self documenting
16:55:43 <tcpepper> kristenc: issue 569 covers that or?
16:55:48 <tcpepper> #link https://github.com/01org/ciao/issues/569
16:55:58 <tcpepper> we should glom on there and make a plan
16:56:25 <kristenc> tcpepper, that's the issue I filed - my description was a bit terse though I suppose.
16:56:50 <tcpepper> so back at you...:)
16:56:52 <tcpepper> what's your plan?
16:56:54 <tcpepper> :)
16:57:15 <tcpepper> and fuentess...any input on this?
16:57:19 <markusry> Does the ceph/demo container not work in travis?
16:57:22 <kristenc> I wish we could use the ceph demo container in travis. can you use containers inside travis?
16:57:34 <markusry> I think so.
16:57:45 <markusry> I bet mcastelino is already doing this
16:57:48 <kristenc> so that would be the most obvious thing to do then.
16:58:07 <markusry> I now we install docker right at the start of our travis script
16:58:11 <kristenc> is modify travis to pull in the ceph demo container, configure the travis container with ceph authentication.
16:58:14 <mcastelino> kristenc: yes you can use docker containers in travis
16:58:15 <markusry> SO I guess something must be using it
16:58:24 <kristenc> then we can use it for our ceph unit test.
16:58:34 <mcastelino> also the network unit tests launch docker containers
16:59:00 * kristenc has to run.
16:59:17 <tcpepper> we're up against the top of the hour anyway
16:59:27 <tcpepper> I'll put the ceph in test topic on the agenda for next week
16:59:53 <tcpepper> #action tcpepper disposition issues 572, 573, 575, 580, 581, 587
17:00:13 <tcpepper> #action tcpepper update agenda to include ceph in test discussion Sep 29
17:00:19 <tcpepper> anything else for today?
17:00:58 <tcpepper> ok then...

Weekly Meetings

Development

Architecture

Clone this wiki locally