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 webapp startup if no docker IP's set. #8655

Merged
merged 4 commits into from
Feb 8, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,21 @@ public class SinglePortLabelsProvisioner implements ConfigurationProvisioner {

@Inject
public SinglePortLabelsProvisioner(
@Named("che.single.port") boolean isSinglePortEnabled,
@Nullable @Named("che.docker.ip") String internalIpOfContainers,
@Nullable @Named("che.docker.ip.external") String externalIpOfContainers,
@Nullable @Named("che.docker.network") String dockerNetwork,
@Nullable @Named("che.singleport.wildcard_domain.host") String wildcardHost) {
if (internalIpOfContainers == null && externalIpOfContainers == null) {
if (isSinglePortEnabled && internalIpOfContainers == null && externalIpOfContainers == null) {
throw new IllegalStateException(
Copy link
Member

Choose a reason for hiding this comment

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

I think ConfigurationException would be better here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree

"Value of both of the properties 'che.docker.ip' and 'che.docker.ip.external' is null,"
+ " which is unsuitable for the single-port mode");
}
this.hostnameBuilder =
new SinglePortHostnameBuilder(externalIpOfContainers, internalIpOfContainers, wildcardHost);
isSinglePortEnabled
? new SinglePortHostnameBuilder(
externalIpOfContainers, internalIpOfContainers, wildcardHost)
: null;
this.internalIpOfContainers = internalIpOfContainers;
this.externalIpOfContainers = externalIpOfContainers;
this.dockerNetwork = dockerNetwork;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,21 @@ public class SinglePortUrlRewriter implements URLRewriter {

@Inject
public SinglePortUrlRewriter(
@Named("che.single.port") boolean isSinglePortEnabled,
@Nullable @Named("che.docker.ip") String internalIpOfContainers,
@Named("che.port") int chePort,
@Nullable @Named("che.docker.ip.external") String externalIpOfContainers,
@Nullable @Named("che.singleport.wildcard_domain.host") String wildcardHost) {
if (internalIpOfContainers == null && externalIpOfContainers == null) {
if (isSinglePortEnabled && internalIpOfContainers == null && externalIpOfContainers == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this check into SinglePortHostnameBuilder?

throw new IllegalStateException(
"Value of both of the properties 'che.docker.ip' and 'che.docker.ip.external' is null,"
+ " which is unsuitable for the single-port mode");
}
this.hostnameBuilder =
new SinglePortHostnameBuilder(externalIpOfContainers, internalIpOfContainers, wildcardHost);
isSinglePortEnabled
? new SinglePortHostnameBuilder(
externalIpOfContainers, internalIpOfContainers, wildcardHost)
: null;
this.chePort = chePort;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void shouldRewriteURL(
String expectedURL)
throws Exception {
SinglePortUrlRewriter rewriter =
new SinglePortUrlRewriter(externalIP, 8080, internalIp, nioHost);
new SinglePortUrlRewriter(true, externalIP, 8080, internalIp, nioHost);

String rewrittenURL = rewriter.rewriteURL(identity, machineName, serverName, incomeURL);

Expand Down Expand Up @@ -97,7 +97,7 @@ public static Object[][] urlRewritingTestProvider() {
)
public void shouldThrowExceptionWhenRewritingFails() throws Exception {
SinglePortUrlRewriter rewriter =
new SinglePortUrlRewriter("127.0.0.1", 8080, "172.12.0.2", null);
new SinglePortUrlRewriter(true, "127.0.0.1", 8080, "172.12.0.2", null);
rewriter.rewriteURL(new RuntimeIdentityImpl("ws123", null, null), "machine1", "server", ":");
}
}