Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

semaphore.sh, go: add semaphore to release-3.1 branch, upgrade release-3.1 to go 1.8.5 #8807

Merged
merged 2 commits into from
Nov 2, 2017

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Nov 2, 2017

Pending approval of #8805. This adds semaphore to 3.1 branch, like 2f74456 did for the 3.2 branch.

This also upgrades the 3.1 branch from go 1.8.3 to 1.8.5, primarly because we already have gcr.io/etcd-development/etcd-test:go1.8.5 in the registry and don't see any point adding 1.8.3 when we can more easily upgrade go by a couple patch versions.

@gyuho
Copy link
Contributor

gyuho commented Nov 2, 2017

lgtm, after CI greens.

Thanks!

@fanminshi
Copy link
Member

lgtm

@gyuho
Copy link
Contributor

gyuho commented Nov 2, 2017

Seems like we also need backport 6825ffe for Go 1.8+.

@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 2, 2017

Thanks @gyuho, I'll cherry pick that as well.

@gyuho
Copy link
Contributor

gyuho commented Nov 2, 2017

Hmm, nvm. Seems like we already have that in release-3.1.

Looks like it's something else

--- FAIL: TestV2NoRetryNoLeader (0.00s)
expected "no leader", got invalid URL port "123.4686.sock"

UPDATE: we need change this as well?

https://github.com/coreos/etcd/blob/0520cb9304cb2385f7e72b8bc02d6e4d3257158a/client/integration/client_test.go#L65-L69

@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 2, 2017

Yep, cherry-pick gave me a "empty commit" response-- it's already on 3.1. I'll poke around tomorrow and figure out where .sock came from.

@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 2, 2017

Thanks @gyuho Looks like 22c52b6 is what we need.

Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. Thanks!

@gyuho
Copy link
Contributor

gyuho commented Nov 2, 2017

Yeah maybe we can merge this first, and cherry-pick those in a separate PR?
UPDATE: nvm, patch is already pushed :)

@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 2, 2017

I've split them out as two separate commits in this PR. That way we can verify that semaphore passes before committing any of this.

@jpbetz
Copy link
Contributor Author

jpbetz commented Nov 2, 2017

Only remaining test failure:

github.com/coreos/etcd/clientv3:

Too many goroutines running after all test(s).
2 instances of:
net.runtime_pollWait(...)
	/usr/local/go/src/runtime/netpoll.go:164 +0x4b
net.(*pollDesc).wait(...)
	/usr/local/go/src/net/fd_poll_runtime.go:75 +0x2b
net.(*pollDesc).waitRead(...)
	/usr/local/go/src/net/fd_poll_runtime.go:80 +0x29
net.(*netFD).Read(...)
	/usr/local/go/src/net/fd_unix.go:250 +0x154
net.(*conn).Read(...)
	/usr/local/go/src/net/net.go:181 +0x56
net/http.(*persistConn).Read(...)
	/usr/local/go/src/net/http/transport.go:1316 +0x15d
bufio.(*Reader).fill(...)
	/usr/local/go/src/bufio/bufio.go:97 +0xd5
bufio.(*Reader).Peek(...)
	/usr/local/go/src/bufio/bufio.go:129 +0x51
net/http.(*persistConn).readLoop(...)
	/usr/local/go/src/net/http/transport.go:1474 +0x185
created by net/http.(*Transport).dialConn
	/usr/local/go/src/net/http/transport.go:1117 +0x88b
2 instances of:
net/http.(*persistConn).writeLoop(...)
	/usr/local/go/src/net/http/transport.go:1704 +0x37f
created by net/http.(*Transport).dialConn
	/usr/local/go/src/net/http/transport.go:1118 +0x8ac

@gyuho
Copy link
Contributor

gyuho commented Nov 2, 2017

Yeah, release-3.1 branch has been flaky with clientv3 tests.
We can address them when we backport clientv3 fixes.
Shouldn't block mvcc patches.

@fanminshi
Copy link
Member

the semaphoreci is not running. @gyuho do you know why?

@gyuho
Copy link
Contributor

gyuho commented Nov 2, 2017

@fanminshi It's running?

I just reran a few times https://semaphoreci.com/coreos/etcd/branches/pull-request-8807/builds/5, to see if it passes.

@fanminshi
Copy link
Member

@gyuho you are right. I didn't noticed that you re-run the test.

@fanminshi
Copy link
Member

similar to this failure #6933. not sure if they are related. @gyuho

@xiang90 xiang90 merged commit ee76b2b into etcd-io:release-3.1 Nov 2, 2017
@xiang90
Copy link
Contributor

xiang90 commented Nov 2, 2017

@jpbetz thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants