[02:04] another review request for OCR, or whenever some graduated reviewer gets a chance and is willing: http://reviews.vapour.ws/r/1035/ [15:47] wallyworld: ping? [16:14] katco, o/ [16:14] katco, I've decided it's easier to chat on irc than send tons of mails like that :) [16:14] dimitern: lol yeah! [16:14] dimitern: so, i don't have access to an ec2 console with these credentials [16:15] dimitern: wallyworld gave them to me [16:15] katco, are these the shared ones? [16:15] dimitern: i think so [16:15] * dimitern scans through emails to find those [16:16] dimitern: lmk if you want me to pm you [16:16] dimitern: so i ran the s3 tests again using the same methodology you do, and they all pass [16:16] katco, sure [16:18] dimitern: i was going to ask you if maybe using the symlink methodology, the s3 code was possible using v1/v2 signing on accident? [16:23] katco, as I've replied to one of the mails - I'm not using symlinks in my gopath for goamz [16:23] dimitern: ah sorry, i must have misunderstood [16:24] dimitern: well the tests have consistently passed for me. we'd have to start picking apart the signing pieces to figure out what's going wrong on your end. is it possible to run TravisCI again? [16:29] katco, it's configured to run on every commit or PR, but I guess we could try to re-run it [16:30] dimitern: i think we need a 3rd data point [16:30] dimitern: because me saying, "works for me!" isn't very helpful :) [16:31] katco, I've just re-run the travis tests and they passed again [16:32] katco, I'll retry on my machine with maximum debugging [16:32] dimitern: i guess it can't run the live tests [16:32] * katco getting more tea [16:33] katco, it *could*, but that'll mean adding some ec2 creds somewhere [16:34] katco, so running s3 tests on v3-unstable worked, on your branch failed the same way [16:40] dimitern: we will have to pick apart the signing pieces to troubleshoot this [16:42] dimitern: amazon returns what it expects for all the signing pieces, we can compare those to the debug output from aws/sign.go [16:42] dimitern: can you pick one test that's failing and pick out the equivalent pieces from debug and response? [16:46] katco, https://github.com/go-amz/amz/pull/27/files#diff-db02e2f1c976d269194375e02b0d62a4R73 << this is a bit worrying [16:46] katco, why append there? [16:46] dimitern: virtual domain buckets don't work with SSL since the cert is registered for the domain [16:47] dimitern: http://shlomoswidler.com/2009/08/amazon-s3-gotcha-using-virtual-host.html [16:47] katco, I believe the lowercasing might be the issue for us-east-1 [16:49] dimitern: mm? how so? [16:51] katco, because USEast is defined as S3LowercaseBucker: false [16:51] katco, and ResolveS3BucketEndpoint unconditionally lowercases [16:56] katco, does this look ok to you: !!!! url="/goamz-faux-region-1--20150301t185544.220574391+0200/index.html"; b=s3test.bucketResource{name:"goamz-faux-region-1--20150301t185544.220574391+0200", bucket:(*s3test.bucket)(nil)} [16:57] dimitern: yes [16:58] dimitern: the test suite had races, so i had to make the test bucket names more unique [16:58] dimitern: i corrected the casing locally and i'm re-running tests [16:58] katco, that's the problem I think [16:58] dimitern: the casing? [16:58] katco, because the timestamp is not escaped [16:59] dimitern: why would it work for me? [16:59] katco, can you please add this: defer func() { fmt.Printf("!!! url=%q\n", u) }() [17:00] katco, in the beginning of s3test/server.go - resourceForURL() and run the tests, paste the output [17:01] dimitern: with the casing, 3 tests now fail [17:02] dimitern: all because the returned bucket name is cased differently than the one we sent (returned is lowercase) [17:02] katco, ok, that's an improvement [17:02] dimitern: lol it is? [17:02] katco, I still think adding the timestamp to the bucket name will cause more trouble than solve [17:03] katco, if you have seen failures I mean :) [17:03] katco, which ones? [17:03] dimitern: i could not get anything to pass without. it just ensures a unique bucket for each test [17:03] dimitern: i can inject failures very easily. whether that's progress is a different question :) [17:04] FAIL: :8: AmazonClientSuite.TestBucketList [17:04] test 0 [17:04] s3i_test.go:367: [17:04] c.Check(resp.Name, Equals, b.Name) [17:04] ... obtained string = "goamz-us-east-1-akiajilh-20150301t105734.322973375-0600" [17:04] ... expected string = "goamz-us-east-1-AKIAJILH-20150301T105734.322973375-0600" [17:04] all the failures are this [17:05] katco, :) [17:05] katco, I think the problem is you TZ [17:06] katco, + in the url in my case translates to %20 [17:06] dimitern: i modified the test to check the region's casing and lowercase it if applicable [17:06] dimitern: ah ok, so you're getting a + for +to UTC? [17:06] katco, yes [17:07] there's our environmental difference :D [17:07] i'll remove that portion of the timestamp [17:12] katco, have you tried that thing with the defer? [17:12] dimitern: yes it's running now [17:12] katco, looking at the generated /goamz-faux-region-1--20150301t191139.281063565+0200/ name [17:12] !!! url="/goamz-faux-region-1--20150301t111233.411561549/" [17:12] !!! url="/goamz-faux-region-1--20150301t111233.411561549/" [17:12] !!! url="/goamz-faux-region-1--20150301t111233.412200038/" [17:12] !!! url="/goamz-/non-existent" [17:12] !!! url="/goamz-faux-region-1--20150301t111233.412956978/" [17:13] katco, ok [17:13] even with this i'm getting casing failures now: [17:13] expectedBucketName := b.Name [17:13] if b.Region.S3LowercaseBucket { [17:13] expectedBucketName = strings.ToLower(expectedBucketName) [17:13] } [17:13] c.Check(resp.Name, Equals, expectedBucketName) [17:14] katco, so just removing the timestamp from the urls made ALL the tests pass on your branch [17:14] dimitern: i found the tests to pass/fail randomly without that [17:14] katco, the local ones I mean [17:14] katco, I'll run them a few times just to be sure [17:14] dimitern: the local ones always worked for me [17:14] dimitern: the live ones were much more volatile [17:15] dimitern: the failures would move around randomly [17:16] katco, so the problem is finding out what's breaking in live tests [17:16] dimitern: race conditions [17:16] dimitern: it's non-deterministic [17:17] katco, well :) I found while fixing ec2 live tests a lot of these random issues could be fixed with retrying on cleanup [17:17] dimitern: i did that :) [17:18] dimitern: i fought with this issue for a long time, and making the buckets unique per test was the thing which finally got everything to pass. [17:19] katco, that's understandable, but they should be unique per suite run I think not changing each time you call testBucket [17:19] katco, like it is I bet there are tons of leftover undeleted buckets from the account [17:19] dimitern: well, as the failures were jumping tests, and not suites, i think that's the resolution we would need to be unique [17:19] dimitern: yes there are, i have to delete them after each run [17:20] katco, that's not a solution then [17:20] dimitern: i think we can probably accept that drawback for now given the deadline [17:21] dimitern: it's only an issue with the tests after all [17:22] katco, ah, right - but even then it should be fixed eventually [17:22] katco, so I think the issue is the region name is inconsistent for all tests [17:22] dimitern: definitely agreed [17:24] katco, so far with only this change to your branch http://paste.ubuntu.com/10489714/ [17:24] katco, all (local and live) tests except 5 in multi_test pass [17:32] dimitern: sorry irc client crashed [17:32] katco, np [17:33] dimitern: i'm running tests now with that change. how many times have they passed for you with that patch? [17:33] katco, just the removal of the timestamp? [17:33] dimitern: yeah [17:34] katco, completely - none, there are always some failures [17:34] dimitern: i believe those are the race conditions... if you run the failures alone, they will pass. [17:36] FAIL: multi_test.go:1: AmazonClientSuite.TestBucketList [17:36] this one always fails so far [17:37] dimitern: because it's a loop [17:37] dimitern: (essentially separate tests) [17:40] katco, I'm trying a few more things [17:41] dimitern: with this patch, they pass: http://pastebin.ubuntu.com/10490009/ [17:41] dimitern: i think they should for you as well [17:42] katco, I'll try it out in a bit [17:53] katco, why do you register AmazonClientSuite twice btw? [17:53] dimitern: i didn't think i messed with suite registration [17:53] dimitern: let me look [17:55] dimitern: yeah i think that was existing [17:55] dimitern: it's not in my diff [17:55] katco, you added the next one? AmazonDomainClientSuite? [17:55] dimitern: can you point me at the line? i don't recall adding that [17:56] katco, nope, just checked - you didn't [17:56] katco, but that's the most likely reason for the races [17:58] dimitern: it looks like they're attempting to test different regions [17:59] katco, yeah, but us-east-1 is tested twice [18:01] dimitern: i see this [18:01] var _ = Suite(&AmazonClientSuite{Region: aws.EUWest}) [18:01] var _ = Suite(&AmazonDomainClientSuite{Region: aws.USEast}) [18:02] oops and USEast for the AmazonClientSuite [18:02] katco, no var _ = Suite(&AmazonClientSuite{Region: aws.USEast}) before that? [18:02] no you're right, it's there [18:02] katco, so I've changed this to USWest and now testing [18:02] so yeah it looks like there will be overlap b/t AmazonClientSuite and AmazonDomainClientSuite [18:04] dimitern: are you testing with the shared credentials? i don't want to mess up your run [18:05] katco, no with mine [18:08] Bug #1426940 was filed: Juju-Windows upgrade-charm --force does not in place force an upgrade

[18:11] dimitern: i get failures when switching one of the tests to USWest [18:11] dimitern: i still have to land this into juju. what do you think about this: [18:12] dimitern: we received reports that ec2 was working in china with the last version of juju [18:12] dimitern: so maybe we clone the sign.go file and put it into s3 for now [18:12] dimitern: apply the datetime fix for the live tests [18:12] dimitern: and we can fix both those things when we have more time [18:13] katco, let me finish this first and I'll come back - 5m [18:13] dimitern: ok [18:23] dimitern, katco everything ok? [18:24] alexisb, yeah, just helping out katco with goamz stuff [18:24] ack, [18:24] alexisb: yeah we're just trying to get that landed into juju before the 1.23 cut [18:25] katco, ok, so my changes didn't seem to improve the failure rate; i'll try your patch with the date [18:25] dimitern: ok [18:38] katco, wow! [18:38] dimitern: ? [18:38] katco, still running, but nothing failed so far [18:38] dimitern: yay :) [18:38] dimitern: i am nervous about introducing this large a change right before we cut for v1.23, will there be time to correct any mistakes? [18:39] katco, there's always time :) [18:39] dimitern: lol [18:39] katco, so the v4 signing for ec2 won't be done, right? [18:39] dimitern: well, let me dig up the bug report. supposedly that was working fine [18:40] https://bugs.launchpad.net/juju-core/+bug/1415693 [18:40] Bug #1415693: Unable to bootstrap on cn-north-1

[18:40] https://github.com/go-amz/amz/issues/22 [18:40] katco, all passed! [18:40] woohoo! [18:41] katco, however, there are a bunch of leftover buckets I can see [18:41] dimitern: yeah [18:41] katco, ah, ok - let's add an issue for this as well [18:42] dimitern: ok i can add that later [18:42] katco, so one issue for the skipped v4 sign tests and one for cleaning up after live tests pass (or fail for that matter) [18:42] katco, ok, cheers [18:42] katco, I'll comment on the PR [18:43] dimitern: regarding the skipped v4 tests... looking at the original bug, i actually don't know if ec2 was ever working with v4 [18:44] dimitern: and i have no way of testing china [18:44] dimitern: i think the best i can do for now is to check to see if the region is china, and use v4. it may not work, and we'd have to try to backport fixes for it i think [18:44] katco, I haven't seen it working at least [18:44] dimitern: but you have more context on what's acceptable, any thoughts on how to proceed? [18:45] katco, if it's not testable, it's not fixable [18:45] dimitern: do you have access to china testing? [18:45] katco, so I'd comment on the bug about providing you with a cn-north account [18:45] katco, no [19:34] dimitern: PR is up with changes, Travis CI passes [19:34] katco, cool, I'm having a look === wgrant_ is now known as wgrant [22:38] davecheney: Could you re-review my ensure-availability changes? http://reviews.vapour.ws/r/1015/ [22:38] It seems to have also picked up some of the changes that anastasiamac did for a separate PR [23:01] cherylj: ok