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

Add flag --mock-port / -m to specify port for Mock #47

Closed
wants to merge 1 commit into from

Conversation

zbintliff
Copy link
Contributor

When running pact-go in containers we want to expose minimal number of
ports. Currently it randomly selects any port. This makes it
impossible to expose ports in a container.

This change will add a CLI option to specify a port to deploy the
mocking service on. This way we know exactly which port to expose.

When running pact-go in containers we want to expose minimal number of
ports. Currently it randomly selects _any_ port. This makes it
impossible to expose ports in a container.

This change will add a CLI option to specify a port to deploy the
mocking service on. This way we know exactly which port to expose.
@zbintliff
Copy link
Contributor Author

In my defense of not handling if they pass a used port: the GetFreePort() function will return error if it grabs a used port but there is no retry logic embedded in.

@zbintliff
Copy link
Contributor Author

To resolve #46

@mefellows
Copy link
Member

Thanks @zbintliff, and sorry I wasn't able to provide guidance on how to achieve this last week.

I think this is better controlled by the consumer DSL (via API), not via the daemon itself. The daemon is multi-threaded, and so it is able to spin up and manage multiple mock servers in its lifetime.

Having this specified on the command-line will limit it to just a single mock server at any point in time.

I'd look to add a new field to pact.go, perhaps something like MockServerPort to take an optional new argument for the port when creating a pact struct. You can then add a check to pact.Setup(...) to select a default port using utils.GetFreePort() if none is specified using the same mechanism that mock_service.go currently does, and remove the default --port flag it's adding. Essentially, moving that logic from the daemon to the DSL.

@zbintliff
Copy link
Contributor Author

Ok thanks for the direction. Just for my own understanding (still new to pact). When does the daemon spin up a new mock server? For every client or ever interaction?

@zbintliff
Copy link
Contributor Author

@mefellows just trying to get a better understanding before I adjust the PR. Can you comment on the above question?

@mefellows
Copy link
Member

The mock service is started once per client, as in it will stay up for all the interactions. It will shutdown the mock service (and write the pact for the consumer/provider pair) when pact.Teardown() is called or its process finishes. In theory, you could run multiple test suites in parallel and each could have its own mock.

@zbintliff
Copy link
Contributor Author

zbintliff commented Sep 29, 2017

Sorry for delay. I still want to specify a range of ports. We will have a few mock clients with different ports and I think putting that logic in the daemon makes more sense.

ENV variable PORTS_RANGE x-y (can be comma separated or range). Then instead of having Find port to grab any random port, it then goes to that range to find an unused port.

with this change we can also add some retry logic on getting the port, because we have already ran into cases where it has grabbed a used port

Edit: Nevermind, starting to grok what you said

@zbintliff
Copy link
Contributor Author

Since i started from scratch agian I opened a new pr: #49

@zbintliff zbintliff closed this Oct 2, 2017
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.

2 participants