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

Improve origins reload visibility #108

Merged
merged 5 commits into from
Mar 13, 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
Prev Previous commit
Next Next commit
Prevent Styx from starting when initial origins configuration fails t…
…o load.

Origins reload command to return 500 Internal Server Error (as opposed to 400 Bad Request)
when the origins file failed to reload.
  • Loading branch information
mikkokar committed Mar 12, 2018
commit f72ea1d002c0c69a06de38c32d7b795d9ff68874
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (C) 2013-2017 Expedia Inc.
* Copyright (C) 2013-2018 Expedia Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -72,6 +72,8 @@ private void reload(Subscriber<? super HttpResponse> subscriber) {
} else if (result.outcome() == UNCHANGED) {
subscriber.onNext(okResponse(format("Origins were not reloaded because %s.\n", result.message())));
subscriber.onCompleted();
} else {
subscriber.onError(mapError(result));
}
} else {
subscriber.onNext(errorResponse(exception));
Expand All @@ -81,6 +83,14 @@ private void reload(Subscriber<? super HttpResponse> subscriber) {
});
}

private Throwable mapError(Registry.ReloadResult result) {
if (result.cause().isPresent()) {
return new RuntimeException(result.cause().get());
} else {
return new RuntimeException("Reload failure");
}
}

private HttpResponse okResponse(String content) {
return response(OK)
.contentType(PLAIN_TEXT_UTF_8)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static com.google.common.base.Throwables.propagate;
import static com.hotels.styx.api.io.ResourceFactory.newResource;
import static com.hotels.styx.client.applications.BackendServices.newBackendServices;
import static com.hotels.styx.infrastructure.Registry.Outcome.FAILED;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -102,7 +103,12 @@ protected CompletableFuture<Void> startService() {
return x;
}
return this.fileBackedRegistry.reload()
.thenAccept(result -> logReloadAttempt("Initial load", result));
.thenApply(result -> logReloadAttempt("Initial load", result))
.thenAccept(result -> {
if (result.outcome() == FAILED) {
throw new RuntimeException(result.cause().orElse(null));
}
});
}

@Override
Expand All @@ -125,7 +131,7 @@ private ReloadResult logReloadAttempt(String reason, ReloadResult outcome) {
String fileName = this.fileBackedRegistry.fileName();
if (outcome.outcome() == Outcome.RELOADED || outcome.outcome() == Outcome.UNCHANGED) {
LOGGER.info("Backend services reloaded. reason='{}', {}, file='{}'", new Object[]{reason, outcome.message(), fileName});
} else if (outcome.outcome() == Outcome.FAILED) {
} else if (outcome.outcome() == FAILED) {
LOGGER.error("Backend services reload failed. reason='{}', {}, file='{}'",
new Object[]{reason, outcome.message(), fileName, outcome.cause().get()});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ExecutionException;

import static ch.qos.logback.classic.Level.ERROR;
import static ch.qos.logback.classic.Level.INFO;
Expand Down Expand Up @@ -192,8 +193,9 @@ public void serviceStarts_logsInfoWhenReloadSucceeds() throws Exception {
));
}

@Test
public void serviceStarts_logsInfoWhenReloadFails() throws Exception {
@Test(expectedExceptions = ExecutionException.class,
expectedExceptionsMessageRegExp = "java.lang.RuntimeException: java.lang.RuntimeException: something went wrong")
public void serviceStarts_failsToStartWhenReloadFails() throws Exception {
FileBackedRegistry<BackendService> delegate = mock(FileBackedRegistry.class);
when(delegate.fileName()).thenReturn("/monitored/origins.yml");
when(delegate.reload()).thenReturn(
Expand All @@ -203,14 +205,32 @@ public void serviceStarts_logsInfoWhenReloadFails() throws Exception {

registry = new FileBackedBackendServicesRegistry(delegate);
registry.startService().get();
}

assertThat(log.lastMessage(), is(
loggingEvent(
ERROR,
"Backend services reload failed. reason='Initial load', md5-hash: 9034890345289043, Failed to load file, file='/monitored/origins.yml'",
RuntimeException.class,
"something went wrong")
));
@Test
public void serviceStarts_logsInfoWhenReloadFails() {
FileBackedRegistry<BackendService> delegate = mock(FileBackedRegistry.class);
when(delegate.fileName()).thenReturn("/monitored/origins.yml");
when(delegate.reload()).thenReturn(
completedFuture(failed(
"md5-hash: 9034890345289043, Failed to load file",
new RuntimeException("something went wrong"))));

registry = new FileBackedBackendServicesRegistry(delegate);

try {
registry.startService().get();
} catch (Throwable any) {
// pass
} finally {
assertThat(log.lastMessage(), is(
loggingEvent(
ERROR,
"Backend services reload failed. reason='Initial load', md5-hash: 9034890345289043, Failed to load file, file='/monitored/origins.yml'",
RuntimeException.class,
"something went wrong")
));
}
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import java.nio.file.StandardCopyOption.REPLACE_EXISTING
import java.nio.file.{Files, Paths}

import com.google.common.io.Files.createTempDir
import com.hotels.styx.api.HttpRequest
import com.hotels.styx.api.messages.HttpResponseStatus.BAD_REQUEST
import com.hotels.styx.api.HttpRequest.Builder.post
import com.hotels.styx.api.messages.HttpResponseStatus.INTERNAL_SERVER_ERROR
import com.hotels.styx.proxy.backends.file.FileBackedBackendServicesRegistry
import com.hotels.styx.support.ResourcePaths.fixturesHome
import com.hotels.styx.support.configuration._
Expand All @@ -44,16 +44,16 @@ class OriginsReloadCommandSpec extends FunSpec
val styxOriginsFile = Paths.get(tempDir.toString, "origins.yml")
var styxServer: StyxServer = _

it("Responds with BAD_REQUEST when the origins cannot be read") {
it("Responds with INTERNAL_SERVER_ERROR when the origins cannot be read") {
val fileBasedBackendsRegistry = FileBackedBackendServicesRegistry.create(styxOriginsFile.toString)
styxServer = StyxConfig().startServer(fileBasedBackendsRegistry)

styxServer.isRunning should be(true)

Files.copy(originsNok, styxOriginsFile, REPLACE_EXISTING)

val resp = decodedRequest(HttpRequest.Builder.post(styxServer.adminURL("/admin/tasks/origins/reload")).build())
resp.status() should be(BAD_REQUEST)
val resp = decodedRequest(post(styxServer.adminURL("/admin/tasks/origins/reload")).build())
resp.status() should be(INTERNAL_SERVER_ERROR)

BackendService.fromJava(fileBasedBackendsRegistry.get().asScala.head) should be(
BackendService(
Expand Down