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

Don't restart the ddev-router all the time, fixes #1236 #1280

Merged
merged 3 commits into from
Nov 27, 2018

Conversation

rfay
Copy link
Member

@rfay rfay commented Nov 19, 2018

The Problem/Issue/Bug:

OP #1236 - We stop the router and check for potential port conflicts with every single ddev start, and it takes a few seconds in the start. It's unnecessary.

How this PR Solves The Problem:

  • Calculate required ports to check for conflicts and only check new ports.
  • Just let docker-compose up do the job of deciding whether to recreate ddev-router.

Manual Testing Instructions:

Use a few projects with different ports.
Use ddev start on them and monitor ddev-router behavior.

You can force the occupy-port test with the nc tool, like nc -l 8080 to force occupation of port 8080 to see if the port check does the right thing on ddev start.

Automated Testing Overview:

TestGetExposedContainerPorts() was added for the new GetExposedContainerPorts() utility.
Our existing tests should do the job of testing port overrides and such, as the behavior mostly stays on parallel.

Related Issue Link(s):

OP #1236

@rfay rfay self-assigned this Nov 19, 2018
@rfay rfay force-pushed the 20181118_dont_restart_router branch from f1654c3 to 355ff22 Compare November 21, 2018 00:34
Copy link
Contributor

@andrewfrench andrewfrench left a comment

Choose a reason for hiding this comment

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

I was able to test this on Windows 10 Pro and MacOS.

When re-using ports, new projects are started with ddev-router is up-to-date.

When a project is configured to use a previously unused port, the router is restarted when the project is started.

When a port is already in use (nc -l 8080, ddev config --http-port 8080) and the project is started, the command fails with Unable to listen on required ports, port 8080 is already in use.

@rfay rfay force-pushed the 20181118_dont_restart_router branch 2 times, most recently from be7f799 to a35f76c Compare November 27, 2018 00:06
@rfay rfay force-pushed the 20181118_dont_restart_router branch from a35f76c to 179fe6b Compare November 27, 2018 00:47
@rfay rfay merged commit c664fa1 into ddev:master Nov 27, 2018
@rfay rfay deleted the 20181118_dont_restart_router branch November 27, 2018 03:27
@dclear dclear added this to the v1.5.0 milestone Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants