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

fix(outputs/graylog): fix failing test due to port already in use #10074

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Nov 8, 2021

Direct link: https://app.circleci.com/pipelines/github/influxdata/telegraf/7370/workflows/dd5cc7c4-9b38-4f91-9f6d-86040c4fca5a/jobs/127224/tests

The error message from the CI:

graylog_test.go:167: 
     Error Trace:	graylog_test.go:167
      	            		asm_amd64.s:1581
     Error:      	Received unexpected error:
      	           listen udp 127.0.0.1:12201: bind: address already in use
     Test:       	TestWriteUDP/UDP

Description:

The output plugin graylog tests are failing due to trying to bind to a port already in use, because the previous test didn't fully unbind yet. I found this package: https://github.com/libp2p/go-reuseport that makes it easy to call a syscall SO_REUSEPORT so that the bind can be reused even if something else was already bound to it. Originally I was hoping to just use bytes.Buffer instead to avoid having to start a UDP and TCP server in a test, but it doesn't cover the connection logic. This approach introduces the fewest changes, so decided this would be better.

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Nov 8, 2021
@powersj
Copy link
Contributor

powersj commented Nov 8, 2021

Is it possible to change the port each test uses? That way each test has a unique port number.

@sspaink
Copy link
Contributor Author

sspaink commented Nov 8, 2021

@powersj probably, but I think you do run the risk of another test using the same port number, the tests can run in parallel and in random order so you might end up with a sneaky "already bound" issue. I guess you could keep trying a new port until you find one?

edit: Using ":0" could work as well: https://stackoverflow.com/questions/43424787/how-to-use-next-available-port-in-http-listenandserve but I guess, it could pick a free port and another test could pick the same port. Not sure how to prevent collisions. Maybe these tests should be marked as integration tests.

@sspaink sspaink added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 15, 2021
@sspaink sspaink merged commit 4e4a330 into master Nov 16, 2021
@sspaink sspaink deleted the fixgraylogoutputtest branch November 16, 2021 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants