| mup | Bug #1447390 was opened: mongo tools missing on centos <juju-core:New> <https://launchpad.net/bugs/1447390> | 00:12 |
|---|---|---|
| mup | Bug #1447392 was opened: ssh args list too long when bootstrapping <juju-core:New for bteleaga> <https://launchpad.net/bugs/1447392> | 00:12 |
| wallyworld_ | thumper: you around? | 01:05 |
| perrito666 | wallyworld: http://reviews.vapour.ws/r/1472/ | 03:16 |
| wallyworld | ty, will look real soon | 03:17 |
| perrito666 | wallyworld: pushing a cleaner branch now that one got some odd merges from master | 03:24 |
| * perrito666 wonders if rb will cope with it | 03:24 | |
| perrito666 | it did and it fits one page | 03:25 |
| perrito666 | k its midnight I am out cheers | 03:25 |
| jam | wwitzel3: ping | 04:27 |
| mup | Bug #1447446 was opened: 1.23.1: bootstrap failure, vivid, local provider <landscape> <juju-core:New> <https://launchpad.net/bugs/1447446> | 05:07 |
| wallyworld | thumper: you around? | 05:12 |
| === urulama|afk is now known as urulama | ||
| wallyworld | jam: you have a few minutes? | 06:15 |
| jam | wallyworld: sure | 06:16 |
| jam | what's up ? | 06:16 |
| wallyworld | jam: need someone to tell me i'm an idiot, can you join say the TL hangout | 06:16 |
| jam | core-leads-call? | 06:17 |
| wallyworld | yeah | 06:17 |
| === mgz is now known as mgz_ | ||
| === liam_ is now known as Guest58476 | ||
| mgz | wallyworld: still up? | 09:05 |
| mgz | jam: poké | 09:30 |
| mgz | jam: <http://reviews.vapour.ws/r/1470/> 'm not being unreasonable right? | 09:31 |
| === urulama is now known as urulama|lunch | ||
| wallyworld | mgz: sorry, was at soccer | 10:57 |
| mgz | wallyworld: no problem, sending email with update | 10:58 |
| perrito666 | wallyworld: we really need a video of you playing socker | 11:02 |
| perrito666 | soccer* | 11:02 |
| wallyworld | perrito666: no we don't :-) | 11:02 |
| perrito666 | for... negotiation purposes | 11:02 |
| mgz | okay, I'm having lunch now, see email/review for init woes | 11:03 |
| wwitzel3 | jam: pong | 12:02 |
| === urulama|lunch is now known as urulama | ||
| tasdomas | hi, could somebody take a look at http://reviews.vapour.ws/r/1116/ ? | 12:36 |
| === mwenning is now known as mwenning-wfh | ||
| mup | Bug #1447595 was opened: TestLeadership fails on windows test slave <ci> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1447595> | 13:21 |
| === liam_ is now known as Guest34021 | ||
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === kadams54 is now known as kadams54-away | ||
| mgz | bogdanteleaga: when proposing crazy mps some more explaination on *why* changing the whitespace fixes the issue would be helpful :) | 13:41 |
| mgz | and what the issue is, for that matter | 13:41 |
| bogdanteleaga | mgz: I accidentally discovered bash doesn't like the script with the old whitespace formatting | 13:55 |
| bogdanteleaga | mgz: I'll edit the message, sure | 13:56 |
| katco | natefinch: ericsnow: stand up | 14:02 |
| === urulama is now known as urulama__ | ||
| mgz | bogdanteleaga: see, that does sound fun, I want the whole story in the mp :) | 14:04 |
| mgz | (also, I think there's an associated lp bug?) | 14:04 |
| bogdanteleaga | mgz: :) | 14:08 |
| bogdanteleaga | mgz: there's no bug for this one | 14:08 |
| bogdanteleaga | mgz: but it would be nice to get it in along with the bugfix | 14:08 |
| natefinch | ericsnow: are you looking at https://bugs.launchpad.net/juju-core/+bug/1447446 ? Or are you working on something else? | 15:31 |
| mup | Bug #1447446: 1.23.1: bootstrap failure, vivid, local provider <landscape> <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1447446> | 15:31 |
| ericsnow | natefinch: yep (forgot to assign it to me) | 15:31 |
| natefinch | ericsnow: cool | 15:31 |
| katco | ericsnow: please add that to the kanban as well | 15:51 |
| ericsnow | katco: will do | 15:51 |
| katco | ericsnow: ty sir | 15:51 |
| katco | ericsnow: pro tip: you can just paste the bug ID in the "card id" field of the card, and it will auto link everything | 15:54 |
| ericsnow | katco: yeah, just noticed :) | 15:55 |
| lazyPower | is this something awesome you have setup or is this auto-behavior? | 15:55 |
| katco | lazyPower: it's board specific | 15:55 |
| katco | lazyPower: the board owner must set it up | 15:55 |
| lazyPower | ah, we have an intermediary service we're routing through to get the functionality | 15:55 |
| lazyPower | i can see wher this works for your team as juju-core is one project | 15:56 |
| lazyPower | we're tracking across ~ 8 billion (give or take a few billion) | 15:56 |
| ericsnow | lazyPower: we scored a good lead with katco :) | 15:56 |
| lazyPower | ericsnow: she's prettttyyy cool, i'll admit. | 15:56 |
| * lazyPower hands katco a gold star | 15:56 | |
| ericsnow | natefinch was good too :) | 15:57 |
| * katco spends the next five minutes trying to press the star onto her nose | 15:57 | |
| ericsnow | katco: doesn't it go on your belly (or was that 2 stars) | 15:57 |
| * katco is a bit lost there | 15:58 | |
| ericsnow | katco: Dr. Suess (star belly sneeches) | 15:58 |
| katco | my daughter hasn't reached the age where i'm back up on Dr. S yet :p | 15:59 |
| natefinch | ericsnow: katco is 1000x as organized as I was. I think we | 16:08 |
| natefinch | we'll be a lot more successful with her leading :) | 16:08 |
| ericsnow | natefinch: :) | 16:08 |
| * perrito666 imagines moonstone conquering countries and taking power | 16:10 | |
| katco | emacs will be mandatory. | 16:11 |
| katco | which will lead to us being overthrown eventually | 16:11 |
| * perrito666 starts a preemtive revolution | 16:11 | |
| katco | but it will be beautiful and terrifying while it lasts | 16:11 |
| katco | ;p | 16:12 |
| perrito666 | <esc>:overthrow | 16:12 |
| wwitzel3 | perrito666: are you working on that issue with maas? | 16:15 |
| perrito666 | wwitzel3: bogdanteleaga is | 16:15 |
| bogdanteleaga | there's a fix waiting for review, feel free to try it out in the meantime | 16:16 |
| sinzui | natefinch, I will now bug you less. | 16:26 |
| natefinch | huzzah! | 16:27 |
| * natefinch notes that katco will probably now bug him a lot more, though ;) | 16:28 | |
| katco | haha | 16:29 |
| === redelmann is now known as rudi|bullingcomi | ||
| === rudi|bullingcomi is now known as rudi|bullingfood | ||
| katco | ericsnow: natefinch: it's certainly not hurting anything, but for now, don't feel the need to put points to bugs. if it makes you happy, please continue :) | 16:36 |
| natefinch | katco: yeah, I meant to ask about that. | 16:38 |
| katco | natefinch: i think we'll only need to point planned feature work | 16:39 |
| natefinch | katco: fair enough | 16:40 |
| natefinch | katco: maybe we should actively not put points on bugs, so we don't screw with our velocity? | 16:40 |
| === kadams54 is now known as kadams54-away | ||
| katco | natefinch: i'm really flexible... i think the reporting can sift that out | 16:41 |
| === kadams54-away is now known as kadams54 | ||
| wwitzel3 | alexisb: ping | 17:31 |
| alexisb | wwitzel3, omy | 17:32 |
| alexisb | omw | 17:32 |
| === liam_ is now known as Guest7885 | ||
| ericsnow | what's our restriction on Go version? | 18:02 |
| natefinch | 1.2.2 | 18:02 |
| ericsnow | looks like the patch for the vmware provider relies on Go 1.3 features | 18:02 |
| natefinch | REJECTION | 18:03 |
| ericsnow | (or rather struct fields that don't exist in 1.3) | 18:03 |
| katco | wwitzel3: ericsnow: core meeting | 18:03 |
| === rudi|bullingfood is now known as redelmann | ||
| natefinch | gsamfira: you around? | 19:05 |
| natefinch | ahh crap, I forgot the uniter tests are an annoyingly monolithic table driven test | 19:11 |
| perrito666 | :D yes they are | 19:12 |
| natefinch | fwereade: what's wrong with this line? runCommands{`if [ $(is-leader) != "False" ]; then exit -1; fi`}, | 19:27 |
| natefinch | fwereade: to be more specific, what happens when that test runs on windows? :P | 19:28 |
| perrito666 | the fact that is a bash ugly oneliner | 19:29 |
| * fwereade did something bashy again? sorry :( | 19:29 | |
| natefinch | fwereade: or you committed someone else's work so you get blamed for it ;) | 19:29 |
| fwereade | natefinch, nah, that was me | 19:29 |
| natefinch | fwereade: I'm looking to fix https://bugs.launchpad.net/juju-core/+bug/1446871 | 19:30 |
| mup | Bug #1446871: Unit hooks fail on windows if PATH is uppercase <ci> <hooks> <windows> <juju-core:Triaged by natefinch> <https://launchpad.net/bugs/1446871> | 19:30 |
| natefinch | fwereade: but when I went to run the tests on windows, saw some other failures too | 19:30 |
| fwereade | natefinch, I guess it should have different constants in util_*_test.go | 19:31 |
| natefinch | fwereade: it would be a lot easier to understand how to fix it if I didn't need to parse a 1600 line DSL first :/ Last time I had to fix a uniter test it took me like 3 days for a relatively simple fix :/ | 19:34 |
| fwereade | natefinch, which is why I've pulled so much of it out into separate packages with things resembling actual unit tests | 19:34 |
| natefinch | fwereade: didn't realize you were doing that. I hugely appreciate that. | 19:35 |
| katco | natefinch: the most succinct way to state why table tests should be used sparingly is: you are defining your own test runner which has a very small sub-set of its larger test runner. | 19:35 |
| katco | why that is bad should be pretty obvious | 19:35 |
| katco | sorry, very small sub-set of features of its larger test runner | 19:36 |
| fwereade | natefinch, that bug looks like it should be reasonably possible to repro it in unit tests somewhere in uniter/runner | 19:36 |
| natefinch | katco: ironically, we're already one more level deep using gocheck | 19:36 |
| katco | yep | 19:36 |
| katco | but at least gocheck is a proper test runner | 19:36 |
| perrito666 | katco: anyone against your argument should be forced to change the current accepted values for statuses :p it is much like a domino castle | 19:37 |
| fwereade | natefinch, ...which remains less well tested than I would like, but *should* have some explicit tests for the env-var population bits | 19:37 |
| katco | perrito666: i had to touch that code once and wowwwww | 19:38 |
| perrito666 | katco: it took me five minutes to change the values and validators and setters.. and 4 days to fix that test | 19:39 |
| katco | perrito666: yeah exactly my experience | 19:40 |
| natefinch | fwereade: so I'm just trying to get the tests to actually run on windows and then I'll tackle the env casing problem... is there not some more simple way to verify that the hook tool isn't there, rather than sending a command-line for the uniter to run? Yes, we could make one that runs on windows and one on linux... but that still seems like the most complicated way to test if a file exists on disk. | 19:48 |
| fwereade | natefinch, well, those tests are the effectively the functional tests for the uniter | 19:49 |
| fwereade | natefinch, you should be able to write a test case in uniter/runner that exposes the problem, though | 19:49 |
| fwereade | natefinch, env.go | 19:50 |
| fwereade | natefinch, ha: | 19:50 |
| fwereade | if runtime.GOOS == "windows" { | 19:50 |
| fwereade | c.Skip("bug 1403084: There are some problems regarding os.Environ() on windows") | 19:50 |
| fwereade | } | 19:50 |
| mup | Bug #1403084: Tests that need to be fixed on windows <ci> <tech-debt> <testing> <windows> <juju-core:Fix Released> <https://launchpad.net/bugs/1403084> | 19:50 |
| fwereade | natefinch, so I'd start there, I think | 19:50 |
| === kadams54 is now known as kadams54-away | ||
| natefinch | fwereade: ok | 19:51 |
| fwereade | natefinch, I know there's a bit of jiggery-pokery regarding env vars for windows -- we were doing <something> twice in some circumstances, and I distorted the code a bit to make sure we only did it once | 19:51 |
| fwereade | natefinch, so there is another path you need to check somewhere to make sure it works in some weird context (juju-run, or debug-hooks, or something) | 19:53 |
| bogdanteleaga | iirc that test was skipped because os.Environ() introduced some flaky variables, but it should not affect anything else | 19:54 |
| fwereade | natefinch, at a guess the problem is env.go:54? | 19:54 |
| fwereade | "Path=" + paths.GetToolsDir() + ";" + os.Getenv("Path"), | 19:54 |
| fwereade | natefinch, a few extra tests similar to TestEnvWindows should pick it up, I think? | 19:55 |
| fwereade | natefinch, although you should figure out what happens when you've got path and PATH and Path all defined -- presumably the `Path` we write is lower priority than whichever one's already set in problematic situations? | 19:56 |
| fwereade | natefinch, regardless, you shouldn't need to touch the giant uniter functional tests just for that fix, so long as you can demonstrate it in the unit tests | 19:57 |
| bogdanteleaga | actually, since I don't get the juju-log.exe not found on my machine, the env_test.go might reveal the issues on the problematic machines | 19:57 |
| natefinch | fwereade: The uniter tests don't pass on windows right now. That's part of that bug. | 19:58 |
| natefinch | fwereade: unrelated to the environment variables | 19:58 |
| fwereade | natefinch, ok, and they should; but the bug has "Any windows machine with casing of the path variable other than 'Path' will fail to find hook tools." | 20:00 |
| mgz_ | bogdanteleaga: thanks! | 20:00 |
| === mgz_ is now known as mgz | ||
| natefinch | fwereade: yes, I know. I'm just trying to get the tests to a point where they only fail for that reason, then I can fix that reason | 20:01 |
| bogdanteleaga | natefinch: I think it's only env issues at this point | 20:01 |
| bogdanteleaga | natefinch: it complains about not finding juju-log.exe at some point | 20:02 |
| bogdanteleaga | natefinch: after executing it moments before | 20:02 |
| mgz | bogdanteleaga: assign bug 1447595 to yourself :) | 20:03 |
| mup | Bug #1447595: TestLeadership fails on windows test slave <ci> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1447595> | 20:03 |
| fwereade | natefinch, I submit that you will have an easier time of it if you repro and address the path issue in isolation | 20:03 |
| fwereade | natefinch, lest you get caught in a morass of other unexpected problems with the uniter test on windows | 20:03 |
| bogdanteleaga | mgz: done | 20:04 |
| bogdanteleaga | http://reviews.vapour.ws/r/1477/ anybody in for a fast review? | 20:04 |
| mgz | bogdanteleaga: lgtmed | 20:04 |
| fwereade | natefinch, I don't know what they might be but I'm not so optimistic as to assume the path issue is the only obstacle | 20:04 |
| fwereade | bogdanteleaga, I don't see where it's successfully executing juju-log.exe? | 20:06 |
| fwereade | brb | 20:06 |
| bogdanteleaga | fwereade: on install it executes, on config-changed it says not found | 20:06 |
| bogdanteleaga | fwereade: http://data.vapour.ws/juju-ci/products/version-2549/run-unit-tests-win2012-amd64/build-276/consoleText | 20:07 |
| bogdanteleaga | fwereade: second test | 20:07 |
| mgz | be a bit wary of that log, it would be one with both PATH and Path set in the environment block, which could well do weird things | 20:10 |
| fwereade | bogdanteleaga, ...I see | 20:11 |
| natefinch | huh, interesting, for some reason go test's -test.foo style flags don't work in powershell | 20:11 |
| fwereade | bogdanteleaga, well, that's exciting | 20:11 |
| mgz | basically we just need a helper in utils that handles this correctly that both utils/exec and the uniter can use | 20:11 |
| bogdanteleaga | mgz: afaik, I can't merge it so you can do it if you want to | 20:11 |
| fwereade | mgz, yeah, PrependToPath or something | 20:11 |
| bogdanteleaga | they pass for me on both win8 and win server so I can't really reproduce this one | 20:12 |
| fwereade | mgz, I am somewhat convinced that we could come up with a happier arrangement of utils/exec so we didn't have that func cloned in both places | 20:12 |
| mgz | fwereade: indeed | 20:13 |
| fwereade | bogdanteleaga, can you try changing the env we set up in the tests? just tweak the value in env_test.go:125 and see what you get | 20:13 |
| bogdanteleaga | but the juju-log.exe thing has been there for a while, did you have both set up for a long time? | 20:13 |
| mgz | bogdanteleaga: $$merge$$ requested - I'll look at your other branches again later if I get a mo | 20:14 |
| bogdanteleaga | cool, they're just as small :) | 20:14 |
| mgz | the other complication I didn't call out in the bug - the go behaviour of os.Getenv on windows changes | 20:14 |
| mgz | it's exact case in 1.2 but uses windows api in I think 1.3 and that will retrieve Path if you ask for PATH and visa versa | 20:15 |
| bogdanteleaga | now that's fun | 20:15 |
| mgz | I had the intention of getting to fix this some point this week... but many things have intervened. I'll happily throw peanuts at nate though :) | 20:17 |
| natefinch | I like peanuts :) | 20:18 |
| bogdanteleaga | fwereade: http://paste.ubuntu.com/10873918/ not sure what you're looking for | 20:19 |
| fwereade | bogdanteleaga, I was thinking more about s/Path/PATH/ than s/bar/obar/? | 20:20 |
| fwereade | and, well, it'll fail for sure, but it might fail in an interesting and instructive way... for someone running the right version of go... possibly on a suitably idiosyncratic windows box | 20:21 |
| bogdanteleaga | fwereade: sounds like quite the setup | 20:22 |
| fwereade | bogdanteleaga, yeah, I guess it takes some proper investigation | 20:23 |
| bogdanteleaga | mgz: looks like it crashed, not sure why though | 20:25 |
| mgz | hm, godeps failed, 'unrecognized import path "gopkg.in/natefinch/lumberjack.v2"' | 20:27 |
| mgz | did someone delete nate? | 20:27 |
| natefinch | mgz: better not have | 20:31 |
| natefinch | mgz: works for me | 20:32 |
| natefinch | mgz: godeps and go get | 20:32 |
| mgz | natefinch: likely something intermittent and networky | 20:33 |
| natefinch | mgz: could be a bad HTTP response from gopkg.in confused it | 20:33 |
| mgz | I'll remerge and see | 20:33 |
| bogdanteleaga | fwereade: changing it to PATH doesn't make it fail :) | 20:34 |
| bogdanteleaga | mgz: that's what I get for pushing from windows with no prepush hook. can you try again? | 20:43 |
| fwereade | anyone for a *really* trivial review? http://reviews.vapour.ws/r/1478/diff/ | 20:43 |
| natefinch | fwereade: ship it! | 20:45 |
| fwereade | natefinch, cheers | 20:45 |
| mgz | bogdanteleaga: sad gofmt :) regoing | 20:47 |
| natefinch | fwereade, mgz, bogdanteleaga: for the record, doing sets and gets in a simple program, I can't get setenv and getenv to act incorrectly on 1.2.2 .... get always seems case insensitive | 20:57 |
| natefinch | (on windows) | 20:57 |
| mgz | okay, maybe that bug report was wrong, good good | 20:58 |
| bogdanteleaga | for me os.Environ() returns =ExitCode= in the environment on win2012r2 | 20:58 |
| natefinch | (sorry, get and set are both case insensitive) | 20:58 |
| mgz | so, it's just the use of os.Environ() that's particularly suspect | 20:58 |
| natefinch | mgz: probably just our use of os.Environ.... we probably expect it to be case sensitive, and it's not | 20:59 |
| mgz | well, it's case-preserving | 21:00 |
| mgz | you just can't do straight string matches vs things you've pulled out previously with os.Getenviron | 21:01 |
| natefinch | yes | 21:01 |
| mgz | -iron | 21:01 |
| natefinch | gotta run, talk later. | 21:01 |
| mgz | later nater | 21:01 |
| fwereade | I have an even simpler review: http://reviews.vapour.ws/r/1479/ | 21:08 |
| fwereade | the description is noticeably larger than the change | 21:08 |
| fwereade | menn0, good morning, can I hit you up for a review or 2? | 21:26 |
| fwereade | menn0, http://reviews.vapour.ws/r/1479/ is genuinely trivial | 21:26 |
| menn0 | fwereade: sure | 21:26 |
| fwereade | menn0, and http://reviews.vapour.ws/r/1224/ is less so, but I think and hope it's well-commented and documented, and has been used in anger in enough pending CLs, that I think it should be good | 21:27 |
| fwereade | tasdomas, ...unless you're around and want to celebrate your graduation by upgrading your I-think-it-makes-sense to and LGTM? ;p | 21:28 |
| menn0 | fwereade: I'll take a look | 21:30 |
| === kadams54-away is now known as kadams54 | ||
| menn0 | fwereade: sorry, just dropped out (keyboard problems, stuck capslock) | 21:39 |
| menn0 | fwereade: looking now | 21:39 |
| fwereade | menn0, no worries :) | 21:40 |
| alexisb | so menn0 fair warning, critical interrupt headed your way | 21:42 |
| fwereade | menn0, then definitely don't worry about 1224 | 21:43 |
| menn0 | alexisb: ok... a critical bug fix? | 21:44 |
| alexisb | menn0, yes, maybe you and fwereade can have a critical bug party :) | 21:44 |
| alexisb | wallyworld, has the details | 21:44 |
| menn0 | fwereade: ship it for 1479 | 21:48 |
| menn0 | fwereade: it wasn't immediately obvious why it fixes the problem but after a bit of digging I think I get it | 21:49 |
| menn0 | fwereade: at any rate, it's a more straightforward way of getting the job done | 21:49 |
| fwereade | menn0, indeed | 21:49 |
| === kadams54 is now known as kadams54-away | ||
| fwereade | menn0, glad it passes a sniff test though, I lost much of the detailed context by not extracting it immediately | 21:50 |
| menn0 | fwereade: just looking at 1224 now. i'm glad that you've been looking at this. the way works are started and managed has become a mess. | 21:51 |
| menn0 | workers even | 21:52 |
| fwereade | menn0, yeah | 21:52 |
| fwereade | menn0, I am currently rather quailing at the prospect of fixing the machine agent | 21:52 |
| fwereade | menn0, the unit agent was hard enough | 21:52 |
| fwereade | menn0, but I've had some practice now :) | 21:52 |
| wallyworld | menn0: did you have a moment to chat? maybe in the onyx standup hangout? | 22:01 |
| menn0 | wallyworld: sure. give me a minute. | 22:02 |
| wallyworld | np | 22:02 |
| mgz | ...do I send my daily dose of good news to the mailing list? | 22:21 |
| mgz | "hey everyone, have these orrible breaking bugs to play with..." | 22:21 |
| menn0 | fwereade: are you still around? one of the bugs i'm looking at now might be related to recent uniter changes | 22:22 |
| menn0 | jw4: ping/ | 22:24 |
| jw4 | menn0: ola | 22:24 |
| menn0 | jw4: you've been looking at bug 1438489 | 22:24 |
| mup | Bug #1438489: juju stop responding after juju-upgrade <upgrade-juju> <juju-core:In Progress by johnweldon4> <juju-core 1.23:Triaged by johnweldon4> <https://launchpad.net/bugs/1438489> | 22:24 |
| menn0 | jw4: where are things at right now? | 22:25 |
| jw4 | menn0: yes, I'm afraid I'm responsible for that one | 22:25 |
| jw4 | menn0: I think the cleanest might be to back out my hook changes from a few weeks ago | 22:25 |
| mgz | menn0: see also https://chinstrap.canonical.com/~gz/ for that, some logs and things that may be of interest | 22:25 |
| menn0 | jw4: what's the PR with your hook changes? | 22:26 |
| jw4 | menn0: let me find it | 22:26 |
| menn0 | mgz: thanks | 22:27 |
| jw4 | menn0: https://github.com/juju/juju/pull/1897 | 22:28 |
| jw4 | menn0: I'd prefer to fix it though rather than backing it out | 22:29 |
| jw4 | menn0: I haven't figured out why the hooks quit firing after the upgrade - I just assumed it was because of those changes of mine | 22:30 |
| menn0 | jw4: shall I take a look? | 22:31 |
| menn0 | agreed that fixing is preferrable to backing out if possible | 22:31 |
| jw4 | menn0: feel free - the repo is easy | 22:31 |
| jw4 | s/repo/repro/ | 22:31 |
| jw4 | all I did to reproduce was to install 1.22, install charm, upgrade to 1.23, observe that hooks quit firing | 22:32 |
| menn0 | jw4: cool | 22:33 |
| jw4 | menn0: mgz just added a simple repro step to the bug | 22:33 |
| menn0 | jw4: what about the recent attempts to fix this? should they stay or be pulled? | 22:33 |
| menn0 | jw4: PRs 2067 and 2058 | 22:34 |
| mgz | menn0: if I understand correctly, the earlier landing was just making trying to make the error not as fatal, didn't change the upgrade step itself | 22:34 |
| jw4 | menn0: the other fixes are actually valid... | 22:34 |
| menn0 | jw4: ok. just confirming. | 22:34 |
| jw4 | mgz: menn0 yeah.. actually this upgrade issue shouldn't be related to the upgrade steps | 22:34 |
| jw4 | more likely that it's related to the uniter logic itself that changed, if indeed it was caused by my original PR | 22:35 |
| menn0 | jw4: what about the errors related to not being able to read the uniter state? | 22:36 |
| jw4 | (the upgrade steps are effectively a no-op, because of a misunderstanding in how upgrades were handled) | 22:36 |
| mgz | this is one of those kinda-shoulda just backed out in response to the bug cases, but the fact we weren't yelling because our upgrade job passed let it slip | 22:36 |
| jw4 | menn0: that was because of tightened validation logic. Since the upgrade steps actually didn't work it was reverted to what was originally there | 22:37 |
| jw4 | mgz: yeah... however, I haven't established yet that the hooks not firing is a result of the original PR, but it sure looks suspicious | 22:38 |
| menn0 | jw4, mgz: ok well let me run with it for a bit and see what I find | 22:39 |
| jw4 | menn0, mgz originally this bug was about the uniter getting into a validation error loop, but now we're past that because of the other fixes, and now we're encountering this issue with hooks not firing | 22:39 |
| menn0 | jw4: understood, thanks | 22:40 |
| * jw4 needs to drop off to take son to drum lessons... bbl | 22:40 | |
| menn0 | jw4: np, talk to you later | 22:41 |
| menn0 | mgz: do we still have a problem with upgrades to 1.23 as well? | 22:41 |
| menn0 | mgz: or is it just 1.24 at this stage? | 22:42 |
| mgz | menn0: the critical part is the upgrade path from 1.22 to 1.23, which exhibits this bug | 22:42 |
| mgz | I've not seperately tested 1.23 to 1.24 but the next CI run of a trunk branch will do that for us. | 22:42 |
| menn0 | mgz: ok, i'll focus on the upgrade to 1.23 to start | 22:43 |
| mup | Bug #1447841 was opened: eu-central-1 AWS region V4 signing required and not supported <juju-core:New> <https://launchpad.net/bugs/1447841> | 22:52 |
| mup | Bug #1447841 changed: eu-central-1 AWS region V4 signing required and not supported <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1447841> | 23:04 |
| menn0 | mgz: I think I see the problem. just trying to confirm now. | 23:06 |
| mgz | menn0: ace | 23:07 |
| menn0 | mgz: but wallyworld needs something reviewed first | 23:07 |
| mgz | circular favours! I have a branch I'd like wallyworld to review (it's not urgent though :) | 23:08 |
| wallyworld | i'll get to it soon | 23:09 |
| mup | Bug #1447841 was opened: eu-central-1 AWS region V4 signing required and not supported <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1447841> | 23:19 |
| wallyworld | mgz: why is Shutoff considered to be alive? | 23:26 |
| mgz | wallyworld: I presume because it's fine if you then manually nova start it, want be to dig up the change it was added in? | 23:29 |
| mgz | wallyworld: I also wanted to allow BUILD(anything) but was scared we actually depend on the machine to be in a somewhat usable state elsewhere | 23:30 |
| wallyworld | mgz: i ask because the behaviour is being changed from active | build and i'm not sure we want to do that | 23:30 |
| mgz | wallyworld: bug 1382709 | 23:31 |
| mup | Bug #1382709: Openstack provider, Instance-state doesn't change on instance shutdown <cts> <cts-cloud-review> <status> <ubuntu-openstack> <juju-core:Fix Released by dimitern> <juju-core 1.21:Fix Released by wallyworld> <https://launchpad.net/bugs/1382709> | 23:31 |
| mgz | wallyworld: ah, in AllInstances? that is actually dead code, I probably should have left alone | 23:31 |
| wallyworld | yeah, in AllInstances | 23:32 |
| wallyworld | you saying we don't call that anymore? | 23:32 |
| mgz | no usage of a provider in the juju codebase ever calls AllInstances | 23:32 |
| mgz | it's just part of the provider api :) | 23:32 |
| wallyworld | mgz: nah, it's called in bootstrap | 23:32 |
| wallyworld | and AllInstances should be a superset of Instances | 23:33 |
| wallyworld | so i'm worried about changing that | 23:33 |
| wallyworld | oh, right, i read the bug. doing stuff out of band we seem to be | 23:35 |
| mgz | hm, so it is, somewhat oodly | 23:36 |
| mgz | wallyworld: anyway, change just makes it more like the Instances() call so finds more stuff | 23:37 |
| wallyworld | yeah, fair enough | 23:37 |
| mgz | wallyworld: I'm not sure on the logging I added | 23:37 |
| mgz | I want some ammount more, but not completely sure on the balance between useful debugging and spam | 23:38 |
| wallyworld | make it trace perhaps | 23:38 |
| mgz | wallyworld: that AllInstances call in bootstrap is new, and puzzling... | 23:40 |
| mgz | well, new, 2014... new since I looked at that code | 23:41 |
| wallyworld | i can't recall the specifics | 23:41 |
| wallyworld | mgz: i'm not comfortable about landing this without tests - the goose test service should be updated to match | 23:42 |
| wallyworld | awesome | 23:43 |
| wallyworld | provider/cloudsigma/config.go:97: no formatting directive in Errorf call | 23:43 |
| wallyworld | why was stuff landed when there was a govet error | 23:43 |
| wallyworld | i thought we rejected such things now | 23:43 |
| wallyworld | mgz: ? | 23:43 |
| mgz | wallyworld: hm, is that still not fataled in the check script? I thought that had been flipped | 23:44 |
| wallyworld | mgz: me too, but i just pulled master | 23:44 |
| wallyworld | and now get that error | 23:44 |
| mgz | ./scripts/verify.bash; echo $? | 23:45 |
| ericsnow | could someone spare we a review on a small patch: https://github.com/juju/govmomi/pull/1 | 23:46 |
| jw4 | menn0: you figured it out? | 23:47 |
| menn0 | jw4: still reviewing wallyworld's change. almost done. | 23:48 |
| jw4 | ah. kk | 23:48 |
| menn0 | jw4: but what i think is happening is that the upgrade step runs while the machine agent is upgrading | 23:48 |
| menn0 | jw4: but at that point the unit agent is still running the previous juju version | 23:49 |
| menn0 | jw4: so when the unit agent shuts down it overwrites the changes made by the upgrade step | 23:49 |
| mup | Bug #1447846 was opened: Hooks don't fire after upgrade 1.23.0 <hooks> <regression> <upgrade-juju> <juju-core:Triaged> <juju-core 1.23:Triaged> <https://launchpad.net/bugs/1447846> | 23:49 |
| menn0 | jw4: sound plausible/ | 23:49 |
| menn0 | ? | 23:49 |
| jw4 | menn0: oooh interesting. | 23:49 |
| wallyworld | ericsnow: i'm not sure the build directives are correct. don't we just want go1.2 and !go1.2 | 23:50 |
| jw4 | menn0: I don't think the hook.Info upgrade step *ever* runs, because the upgrade step is supposed to find all the units for a given machine and run the upgrade on each one manually | 23:51 |
| wallyworld | otherwise how would it work on go1.4 etc | 23:51 |
| jw4 | i.e. I don't think the upgrade process automatically runs upgrade steps on units, only on machines | 23:51 |
| jw4 | maybe I'm wrong though... | 23:52 |
| menn0 | jw4: yeah you're right... so there's 2 problems | 23:52 |
| menn0 | jw4: the upgrade mechanics only run on the machine agents so the check for a unit tag in the upgrade step is never going to be true | 23:53 |
| jw4 | menn0: I backburnered my upgrade steps problem by reverting the validation logic... but how does your proposed scenario cause the hooks to stop firing after upgrade? | 23:54 |
| jw4 | (I think it's plausible, but I'm missing the connecting steps) | 23:55 |
| menn0 | jw4: i haven't quite figured it out either | 23:55 |
| jw4 | menn0: kk - well I'm glad to have your eyes on it... I'm embarassed by this whole issue | 23:56 |
| jw4 | :) | 23:56 |
| menn0 | jw4: but i'm wondering if the uniter state file hasn't been upgraded then we can end up with the "unexpected hook info with Kind Continue" error | 23:56 |
| jw4 | menn0: yes - that error will always be logged - it's almost certainly a red herring to the real problem | 23:57 |
| jw4 | when I 'fixed' the validation I just changed it so that instead of crashing the uniter, it just logs the problem once and then continues | 23:57 |
| ericsnow | wallyworld: go1.2 means Go 1.2 and later | 23:58 |
| jw4 | so the error will always show once when the uniter starts up (until I/we fix the upgrade steps), but it shouldn't prevent the uniter from continuing normally after that | 23:58 |
| wallyworld | ericsnow: ah, i see, thanks | 23:59 |
| menn0 | wallyworld: review done. for some reason RB has duplicated one of my comments for a particular section of code and I can't figure out how to fix it. please ignore the dup. | 23:59 |
| wallyworld | menn0: tyvm | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!