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 a "defaultPortMapping" option which exposes all Mesos provided ports #1036

Merged
merged 5 commits into from May 17, 2016

Conversation

@stevenschlansker
Copy link
Contributor

stevenschlansker commented May 12, 2016

Our apps generally ask for 1 or 2 ports. It's a pain to repeatedly write

      "portMappings": [
        {"hostPort": 0, "hostPortType": "FROM_OFFER", "containerPort": 0, "containerPortType": "FROM_OFFER"},
        {"hostPort": 1, "hostPortType": "FROM_OFFER", "containerPort": 1, "containerPortType": "FROM_OFFER"}
      ]

so why not just let Singularity fill it in for you?

@@ -239,6 +239,14 @@ private void prepareContainerInfo(final Offer offer, final SingularityTaskId tas
dockerInfoBuilder.addPortMappings(maybePortMapping.get());
}
}
} else if (configuration.getNetworkConfiguration().isDefaultPortMapping() && dockerInfo.get().getPortMappings().isEmpty() && ports.isPresent()) {

This comment has been minimized.

Copy link
@ssalinas

ssalinas May 13, 2016

Member

Might also need to check that network mode is BRIDGE here, if it's HOST or NONE and you specify mappings believe that will end up at least in a TASK_FAILED if not a TASK_ERROR. Other than that looks good

This comment has been minimized.

Copy link
@stevenschlansker

stevenschlansker May 13, 2016

Author Contributor

Fixed.

@ssalinas ssalinas modified the milestone: 0.6.0 May 13, 2016
@ssalinas ssalinas added the hs_staging label May 13, 2016
@ssalinas
Copy link
Member

ssalinas commented May 13, 2016

Tested this in our staging infra and looks good 👍

@stevenschlansker
Copy link
Contributor Author

stevenschlansker commented May 13, 2016

Test failure looks transient, timed out on the openjdk7 build.

@tpetr
Copy link
Member

tpetr commented May 17, 2016

LGTM

@ssalinas ssalinas merged commit 0408b28 into HubSpot:master May 17, 2016
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@stevenschlansker stevenschlansker deleted the stevenschlansker:default-network-mapping branch Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.