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.

Sign up for GitHub

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

Jump to bottom

tests: use copy of SocketUtil that does not use 127.x.y.255 addresses #3460

Merged
merged 6 commits into from Sep 3, 2020
Merged

tests: use copy of SocketUtil that does not use 127.x.y.255 addresses #3460

merged 6 commits into from Sep 3, 2020

Conversation

jrudolph
Copy link
Member

@jrudolph jrudolph commented Sep 3, 2020

It recently failed various times with this stack trace:

java.net.BindException: Cannot assign requested address
	at java.base/sun.nio.ch.Net.bind0(Native Method)
	at java.base/sun.nio.ch.Net.bind(Net.java:455)
	at java.base/sun.nio.ch.Net.bind(Net.java:447)
	at java.base/sun.nio.ch.ServerSocketChannelImpl.bind(ServerSocketChannelImpl.java:227)
	at java.base/sun.nio.ch.ServerSocketAdaptor.bind(ServerSocketAdaptor.java:80)
	at java.base/sun.nio.ch.ServerSocketAdaptor.bind(ServerSocketAdaptor.java:73)
	at akka.testkit.SocketUtil$.$anonfun$temporaryServerAddresses$1(SocketUtil.scala:97)
	at scala.collection.StrictOptimizedSeqFactory.fill(Factory.scala:330)
	at scala.collection.StrictOptimizedSeqFactory.fill$(Factory.scala:325)
	at scala.collection.immutable.Vector$.fill(Vector.scala:34)
	at akka.testkit.SocketUtil$.temporaryServerAddresses(SocketUtil.scala:81)
	at akka.testkit.SocketUtil$.temporaryServerAddress(SocketUtil.scala:74)
	at akka.testkit.SocketUtil$.temporaryServerHostnameAndPort(SocketUtil.scala:106)

It's unclear why this could fail, so we add this for a round of builds to find out what the address is when it fails.

@jrudolph jrudolph requested a review from raboof September 3, 2020 08:19
@akka-ci akka-ci added validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR that is currently being validated by Jenkins labels Sep 3, 2020
@akka-ci
Copy link

akka-ci commented Sep 3, 2020

Test FAILed.

!!! Couldn't read commit file !!!

@akka-ci akka-ci added validating PR that is currently being validated by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) labels Sep 3, 2020
raboof
raboof approved these changes Sep 3, 2020
View reviewed changes
akka-http-core/src/test/scala/akka/testkit/SocketUtil2.scala
}
catch {
case NonFatal(ex) =>
throw new RuntimeException(s"Binding to $addr failed with ${ex.getMessage}", ex)
Copy link
Member

Choose a reason for hiding this comment

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

👍

@akka-ci akka-ci added needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed validating PR that is currently being validated by Jenkins labels Sep 3, 2020
@akka-ci
Copy link

akka-ci commented Sep 3, 2020

Test FAILed.

!!! Couldn't read commit file !!!

@akka-ci akka-ci added validating PR that is currently being validated by Jenkins needs-attention Indicates a PR validation failure (set by CI infrastructure) and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR that is currently being validated by Jenkins labels Sep 3, 2020
@akka-ci
Copy link

akka-ci commented Sep 3, 2020

Test FAILed.

Pull request validation report

Failed Test Suites

Test result for akka-http-core / Pr-validation / ./ executeTests

[info] ScalaTest
[info] Run completed in 3 minutes, 19 seconds.
[info] Total number of tests run: 1041
[info] Suites: completed 77, aborted 0  Scopes: pending 1
[info] Tests: succeeded 1040, failed 1, canceled 0, ignored 2, pending 54
[info] *** 1 TEST FAILED ***
[error] Failed: Total 1041, Failed 1, Errors 0, Passed 1040, Ignored 2, Pending 54
[error] Failed tests:
[error] 	akka.http.scaladsl.ClientTransportWithCustomResolverSpec

@jrudolph
Copy link
Member Author

jrudolph commented Sep 3, 2020

Got a result right away:

java.lang.RuntimeException: Binding to /127.20.224.255:0 failed with Cannot assign requested address

@jrudolph
Copy link
Member Author

jrudolph commented Sep 3, 2020

I can reproduce it manually in the docker container that you cannot bind ports on 127.x.y.255.

@jrudolph
Copy link
Member Author

jrudolph commented Sep 3, 2020

I can reproduce it manually in the docker container that you cannot bind ports on 127.x.y.255.

Though nc actually can bind those, so it also seems to be related to the JDK in some way...

@akka-ci akka-ci added validating PR that is currently being validated by Jenkins tested PR that was successfully built and tested by Jenkins and removed needs-attention Indicates a PR validation failure (set by CI infrastructure) validating PR that is currently being validated by Jenkins labels Sep 3, 2020
@akka-ci
Copy link

akka-ci commented Sep 3, 2020

Test PASSed.

jrudolph
jrudolph commented Sep 3, 2020
View reviewed changes
akka-http-core/src/test/scala/akka/testkit/SocketUtil2.scala
case RANDOM_LOOPBACK_ADDRESS =>
// JDK limitation? You cannot bind on addresses matching the pattern 127.x.y.255,
// that's why the last component must be < 255
if (canBindOnAlternativeLoopbackAddresses) s"127.20.${Random.nextInt(256)}.${Random.nextInt(255)}"
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix here

@akka-ci akka-ci added validating PR that is currently being validated by Jenkins and removed tested PR that was successfully built and tested by Jenkins labels Sep 3, 2020
@jrudolph
Copy link
Member Author

jrudolph commented Sep 3, 2020

I'd say we merge it like this and if it holds water for a few days we can backport to Akka.

@akka-ci akka-ci added tested PR that was successfully built and tested by Jenkins and removed validating PR that is currently being validated by Jenkins labels Sep 3, 2020
@akka-ci
Copy link

akka-ci commented Sep 3, 2020

Test PASSed.

@jrudolph jrudolph changed the title tests: use copy of SocketUtil that gives more information if it fails tests: use copy of SocketUtil that does not use 127.x.y.255 addresses Sep 3, 2020
@jrudolph jrudolph merged commit d68eaaf into akka:master Sep 3, 2020
@jrudolph jrudolph deleted the better-errors-in-socket-util branch September 3, 2020 09:44
@jrudolph jrudolph mentioned this pull request Sep 3, 2020
Open
@jrudolph
Copy link
Member Author

jrudolph commented Sep 7, 2020

Seems to work fine on master and now needs a backport to release-10.1 and a fix in Akka itself.

jrudolph added a commit to jrudolph/akka-http that referenced this pull request Sep 7, 2020
@jrudolph
tests: use copy of SocketUtil that does not use 127.x.y.255 addresses (…
de7f5d8 
…akka#3460)

(cherry picked from commit d68eaaf)

# Conflicts:
#	akka-http-core/src/test/scala/akka/http/impl/engine/client/ConnectionPoolSpec.scala
#	akka-http-core/src/test/scala/akka/http/scaladsl/ClientServerSpec.scala
#	akka-http-core/src/test/scala/akka/http/scaladsl/ClientSpec.scala
#	akka-http-core/src/test/scala/akka/http/scaladsl/ClientTransportWithCustomResolverSpec.scala
#	akka-http-core/src/test/scala/akka/http/scaladsl/model/EntityDiscardingSpec.scala
#	docs/src/test/scala/docs/http/scaladsl/server/directives/CustomHttpMethodSpec.scala
#	docs/src/test/scala/docs/http/scaladsl/server/directives/TimeoutDirectivesExamplesSpec.scala
jrudolph added a commit to jrudolph/akka-http that referenced this pull request Sep 7, 2020
@jrudolph
[backport] tests: use copy of SocketUtil that does not use 127.x.y.25…
ea76249 
…5 addresses (akka#3460)

(cherry picked from commit d68eaaf)
This was referenced Sep 7, 2020
Closed
Merged
@jrudolph
Copy link
Member Author

jrudolph commented Sep 7, 2020

Seems to work fine on master and now needs a backport to release-10.1 and a fix in Akka itself.

10.1 backport: #3469

This was referenced Sep 7, 2020
Merged
Merged
@ignasi35 ignasi35 mentioned this pull request Oct 20, 2020
Closed
@jrudolph jrudolph mentioned this pull request Nov 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raboof raboof raboof approved these changes

Assignees
No one assigned
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jrudolph @akka-ci @raboof