Blackbox tests for gorouter

[combor]

What

I noticed that gorouter tests doesn't do any blackbox testing . This is a problem as code relies on the functionality from external libraries and is assuming that they work properly which is not always true.

For instance https://github.com/cloudfoundry/gorouter/blob/master/main.go#L132 is using ifrit to handle OS signals and pass them to relevant gorouter specific code.
Now, https://github.com/cloudfoundry/gorouter/blob/master/router/router.go#L255 handles incoming signals and runs DrainAndStop() to wait for existing connections to finish and discards any other signals.

Unfortunately ifrit implemented handling of multiple signals here: tedsuo/ifrit@a335a27#diff-5b925779355908120e85f9c9f98af666 on Dec 15, 2015 and routing-release points to the commit that was created on Apr 2015.

So how gorouter behaves when two USR1 signals are send to it and ifrit fix is not included? It just dies immediately without proper connection draining.

Interestingly, the gorouter drain script https://github.com/cloudfoundry-incubator/routing-release/blob/develop/jobs/gorouter/templates/drain#L19 sends USR1 signal every 5 seconds to check if process is still alive. Of course second check (second USR1 signal) kills gorouter after 5 seconds not allowing for proper connection drain.

How to reproduce?

Compile gorouter according to the documentation:

git clone https://github.com/cloudfoundry-incubator/routing-release
cd routing-release
./scripts/update
cd src/github.com/cloudfoundry/gorouter
go build 

run it with a config that specifies drain_wait: 30:

./gorouter -c ./gorouter.yml

Send USR1 twice

pkill -USR1 gorouter ; sleep 2 ; pkill -USR1 gorouter

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/123012431

The labels on this github issue will be updated when the story is started.

[crhino]

@combor We have black box testing of the gorouter server here, but as you mention did not have a test for draining connections and thus failed to catch our mistake with the ifrit library.

The PR here fixes the ifrit library issue, and we have recently backfilled a drain test in the main_test file in order catch this case if it should ever occur.

Thanks for the report! I am going to close this issue since the PR above has solved the drain issue.