Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Live Reloading the TimeLock Block, Part 3: Working with 0 Nodes #2647

Merged
merged 8 commits into from
Nov 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -275,6 +275,8 @@ private void checkLeaderAndTimelockBlocks() {
"If the leader block is present, then the lock and timestamp server blocks must both be absent.");
Preconditions.checkState(!timelock().isPresent(),
"If the leader block is present, then the timelock block must be absent.");
Preconditions.checkState(!leader().get().leaders().isEmpty(),
"Leader config must have at least one server.");
}

if (timelock().isPresent()) {
Expand All @@ -286,6 +288,14 @@ private void checkLeaderAndTimelockBlocks() {
private void checkLockAndTimestampBlocks() {
Preconditions.checkState(lock().isPresent() == timestamp().isPresent(),
"Lock and timestamp server blocks must either both be present or both be absent.");
checkServersListHasAtLeastOneServerIfPresent(lock());
checkServersListHasAtLeastOneServerIfPresent(timestamp());
}

private static void checkServersListHasAtLeastOneServerIfPresent(Optional<ServerListConfig> serverListOptional) {
serverListOptional.ifPresent(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this for Timelock or separate Timestamps and Lock blocks?

Have you verified this condition is actually true all of the time? Is it guaranteed by the config source?

Might be better to code defensively around all of the various ways the config could indicate no servers.

Copy link
Contributor Author

@jeremyk-91 jeremyk-91 Nov 13, 2017

Choose a reason for hiding this comment

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

This is only applied for the Timestamp and Lock blocks.

We want this condition to be true all of the time, because of that. The leader config does its own verification. The state of the world is thus:
leader with 0 --> fail in configuration
lock/timestamp with 0 --> fail here
timelock with 0 --> pass
nothing at all in the config (embedded on local node) --> pass

Didn't want to change the LeaderConfig, as that's used in other places e.g. TimeLockAgent where we don't support live reloading yet!

So we're targeting:
leader with 0 --> fail in configuration and here
lock/timestamp with 0 --> fail here
timelock with 0 --> pass

serverList -> Preconditions.checkState(serverList.hasAtLeastOneServer(),
"Server list must have at least one server."));
}

private String checkNamespaceConfigAndGetNamespace() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ public String getClientOrThrow() {
* for connecting to TimeLock.
*/
@Deprecated
public abstract ServerListConfig serversList();
@Value.Default
public ServerListConfig serversList() {
return ImmutableServerListConfig.builder().build();
}

public ServerListConfig toNamespacedServerList() {
return ServerListConfigs.namespaceUris(serversList(), getClientOrThrow());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,8 @@
@JsonDeserialize(as = ImmutableTimeLockRuntimeConfig.class)
@Value.Immutable
public abstract class TimeLockRuntimeConfig {
public abstract ServerListConfig serversList();
@Value.Default
public ServerListConfig serversList() {
return ImmutableServerListConfig.builder().build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ public class AtlasDbConfigTest {
.localServer("me")
.addLeaders("me")
.build();
private static final ServerListConfig DEFAULT_SERVER_LIST = ImmutableServerListConfig.builder()
private static final ServerListConfig SINGLETON_SERVER_LIST = ImmutableServerListConfig.builder()
.addServers("server")
.build();
private static final TimeLockClientConfig TIMELOCK_CONFIG = ImmutableTimeLockClientConfig.builder()
.client("client")
.serversList(DEFAULT_SERVER_LIST)
.serversList(SINGLETON_SERVER_LIST)
.build();
private static final Optional<SslConfiguration> SSL_CONFIG = Optional.of(mock(SslConfiguration.class));
private static final Optional<SslConfiguration> OTHER_SSL_CONFIG = Optional.of(mock(SslConfiguration.class));
Expand All @@ -61,13 +61,14 @@ public class AtlasDbConfigTest {
private static final TimeLockClientConfig TIMELOCK_CONFIG_WITH_OPTIONAL_EMPTY_CLIENT = ImmutableTimeLockClientConfig
.builder()
.client(Optional.empty())
.serversList(DEFAULT_SERVER_LIST)
.serversList(SINGLETON_SERVER_LIST)
.build();
private static final TimeLockClientConfig TIMELOCK_CONFIG_WITH_OTHER_CLIENT = ImmutableTimeLockClientConfig
.builder()
.client(OTHER_CLIENT)
.serversList(DEFAULT_SERVER_LIST)
.serversList(SINGLETON_SERVER_LIST)
.build();
private static final String CLIENT_NAMESPACE = "client";

@BeforeClass
public static void setUp() {
Expand Down Expand Up @@ -113,8 +114,8 @@ public void configWithTimelockBlockIsValid() {
public void remoteLockAndTimestampConfigIsValid() {
AtlasDbConfig config = ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.lock(DEFAULT_SERVER_LIST)
.timestamp(DEFAULT_SERVER_LIST)
.lock(SINGLETON_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
assertThat(config.getNamespaceString(), equalTo(TEST_NAMESPACE));
assertThat(config, not(nullValue()));
Expand All @@ -125,8 +126,8 @@ public void leaderBlockNotPermittedWithLockAndTimestampBlocks() {
ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.leader(LEADER_CONFIG)
.lock(DEFAULT_SERVER_LIST)
.timestamp(DEFAULT_SERVER_LIST)
.lock(SINGLETON_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
}

Expand All @@ -136,9 +137,9 @@ public void timelockBlockNotPermittedWithLockAndTimestampBlocks() {
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.timelock(ImmutableTimeLockClientConfig.builder()
.client("testClient")
.serversList(DEFAULT_SERVER_LIST).build())
.lock(DEFAULT_SERVER_LIST)
.timestamp(DEFAULT_SERVER_LIST)
.serversList(SINGLETON_SERVER_LIST).build())
.lock(SINGLETON_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
}

Expand All @@ -156,7 +157,7 @@ public void leaderBlockNotPermittedWithLockBlock() {
ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.leader(LEADER_CONFIG)
.lock(DEFAULT_SERVER_LIST)
.lock(SINGLETON_SERVER_LIST)
.build();
}

Expand All @@ -165,23 +166,23 @@ public void leaderBlockNotPermittedWithTimestampBlock() {
ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.leader(LEADER_CONFIG)
.timestamp(DEFAULT_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
}

@Test(expected = IllegalStateException.class)
public void lockBlockRequiresTimestampBlock() {
ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.lock(DEFAULT_SERVER_LIST)
.lock(SINGLETON_SERVER_LIST)
.build();
}

@Test(expected = IllegalStateException.class)
public void timestampBlockRequiresLockBlock() {
ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.timestamp(DEFAULT_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
}

Expand Down Expand Up @@ -316,8 +317,8 @@ public void addingFallbackSslAddsItToLeaderBlock() {
public void addingFallbackSslAddsItToLockBlock() {
AtlasDbConfig withoutSsl = ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.lock(DEFAULT_SERVER_LIST)
.timestamp(DEFAULT_SERVER_LIST)
.lock(SINGLETON_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
AtlasDbConfig withSsl = AtlasDbConfigs.addFallbackSslConfigurationToAtlasDbConfig(withoutSsl, SSL_CONFIG);
assertThat(withSsl.lock().get().sslConfiguration(), is(SSL_CONFIG));
Expand All @@ -338,8 +339,8 @@ public void addingFallbackSslAddsItToTimelockServersBlock() {
public void addingFallbackSslAddsItToTimestampBlock() {
AtlasDbConfig withoutSsl = ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITH_NAMESPACE)
.lock(DEFAULT_SERVER_LIST)
.timestamp(DEFAULT_SERVER_LIST)
.lock(SINGLETON_SERVER_LIST)
.timestamp(SINGLETON_SERVER_LIST)
.build();
AtlasDbConfig withSsl = AtlasDbConfigs.addFallbackSslConfigurationToAtlasDbConfig(withoutSsl, SSL_CONFIG);
assertThat(withSsl.timestamp().get().sslConfiguration(), is(SSL_CONFIG));
Expand Down Expand Up @@ -367,4 +368,35 @@ public void addingAbsentFallbackSslWhenItDoesntExistsLeavesItAsAbsent() {
AtlasDbConfig withSsl = AtlasDbConfigs.addFallbackSslConfigurationToAtlasDbConfig(withoutSsl, NO_SSL_CONFIG);
assertThat(withSsl.leader().get().sslConfiguration(), is(NO_SSL_CONFIG));
}

@Test
public void cannotSpecifyZeroServersIfUsingTimestampBlock() {
assertThatThrownBy(() -> ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITHOUT_NAMESPACE)
.namespace(CLIENT_NAMESPACE)
.timestamp(ImmutableServerListConfig.builder().build())
.lock(SINGLETON_SERVER_LIST)
.build()).isInstanceOf(IllegalStateException.class);
}

@Test
public void cannotSpecifyZeroServersIfUsingLockBlock() {
assertThatThrownBy(() -> ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITHOUT_NAMESPACE)
.namespace(CLIENT_NAMESPACE)
.timestamp(SINGLETON_SERVER_LIST)
.lock(ImmutableServerListConfig.builder().build())
.build()).isInstanceOf(IllegalStateException.class);
}

@Test
public void canSpecifyZeroServersIfUsingTimelockBlock() {
ImmutableAtlasDbConfig.builder()
.keyValueService(KVS_CONFIG_WITHOUT_NAMESPACE)
.namespace(CLIENT_NAMESPACE)
.timelock(ImmutableTimeLockClientConfig.builder()
.serversList(ImmutableServerListConfig.builder().build())
.build())
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,18 @@

import java.io.File;
import java.io.IOException;
import java.nio.file.Paths;

import org.junit.Test;

import com.palantir.remoting.api.config.ssl.SslConfiguration;

public class AtlasDbRuntimeConfigDeserializationTest {
private static final File TEST_RUNTIME_CONFIG_FILE = new File(
AtlasDbRuntimeConfigDeserializationTest.class.getResource("/runtime-config-block.yml").getPath());
private static final File TEST_RUNTIME_CONFIG_NO_SERVERS_FILE = new File(
AtlasDbRuntimeConfigDeserializationTest.class.getResource("/runtime-config-block-no-servers.yml")
.getPath());

@Test
public void canDeserializeRuntimeConfig() throws IOException {
Expand All @@ -39,5 +45,24 @@ public void canDeserializeRuntimeConfig() throws IOException {
"https://foo1:12345",
"https://foo2:8421",
"https://foo3:9421");
assertThat(runtimeConfig.timelockRuntime().get().serversList().sslConfiguration()).satisfies(
sslConfiguration -> sslConfiguration.ifPresent(this::assertSslConfigDeserializedCorrectly));
}

private void assertSslConfigDeserializedCorrectly(SslConfiguration sslConfiguration) {
assertThat(sslConfiguration.keyStorePassword()).hasValue("0987654321");
assertThat(sslConfiguration.keyStorePath()).hasValue(Paths.get("var", "security", "keyStore.jks"));
assertThat(sslConfiguration.trustStorePath()).isEqualTo(Paths.get("var", "security", "trustStore.jks"));
}

@Test
public void canDeserializeRuntimeConfigWithZeroServers() throws IOException {
AtlasDbRuntimeConfig runtimeConfig =
AtlasDbConfigs.OBJECT_MAPPER.readValue(TEST_RUNTIME_CONFIG_NO_SERVERS_FILE, AtlasDbRuntimeConfig.class);
assertThat(runtimeConfig.timestampClient().enableTimestampBatching()).isTrue();

assertThat(runtimeConfig.timelockRuntime()).isPresent();
assertThat(runtimeConfig.timelockRuntime().get().serversList().servers()).isEmpty();
assertThat(runtimeConfig.timelockRuntime().get().serversList().sslConfiguration()).isEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public class ServerListConfigsTest {
private static final ImmutableServerListConfig SERVERS_LIST_3 = ImmutableServerListConfig.builder()
.addServers("three/")
.build();
private static final ImmutableServerListConfig SERVERS_LIST_EMPTY = ImmutableServerListConfig.builder()
.build();

private static final TimeLockClientConfig INSTALL_CONFIG = ImmutableTimeLockClientConfig.builder()
.serversList(SERVERS_LIST_1)
Expand All @@ -60,6 +62,12 @@ public void namespacingCanDealWithTrailingSlash() {
assertThat(namespacedServersList.servers()).containsExactlyInAnyOrder("three/client");
}

@Test
public void namespacingCanDealWithServerListConfigsWithZeroNodes() {
ServerListConfig namespacedServersList = ServerListConfigs.namespaceUris(SERVERS_LIST_EMPTY, CLIENT);
assertThat(namespacedServersList.servers()).isEmpty();
}

@Test
public void prioritisesRuntimeConfigIfAvailable() {
ServerListConfig resolvedConfig = ServerListConfigs.parseInstallAndRuntimeConfigs(
Expand All @@ -69,6 +77,15 @@ public void prioritisesRuntimeConfigIfAvailable() {
assertThat(resolvedConfig.servers()).containsExactlyInAnyOrder("one/client", "two/client");
}

@Test
public void prioritisesRuntimeConfigEvenIfThatHasNoClients() {
ServerListConfig resolvedConfig = ServerListConfigs.parseInstallAndRuntimeConfigs(
INSTALL_CONFIG,
() -> Optional.of(ImmutableTimeLockRuntimeConfig.builder().build()),
CLIENT);
assertThat(resolvedConfig.servers()).isEmpty();
}

@Test
public void fallsBackToInstallConfigIfRuntimeConfigNotAvailable() {
ServerListConfig resolvedConfig = ServerListConfigs.parseInstallAndRuntimeConfigs(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

import javax.net.ssl.SSLSocketFactory;
import javax.ws.rs.GET;
Expand All @@ -49,7 +50,9 @@
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.Uninterruptibles;
import com.palantir.atlasdb.config.ImmutableServerListConfig;
import com.palantir.atlasdb.config.ServerListConfig;
import com.palantir.atlasdb.factory.ServiceCreator;
import com.palantir.common.remoting.ServiceNotAvailableException;
import com.palantir.remoting.api.config.service.ProxyConfiguration;
import com.palantir.remoting3.config.ssl.SslSocketFactories;

Expand Down Expand Up @@ -172,6 +175,43 @@ public void canLiveReloadServersList() {
unavailableServer.verify(getRequestedFor(urlMatching(TEST_ENDPOINT)));
}

@Test
public void httpProxyThrowsServiceNotAvailableExceptionIfConfiguredWithZeroNodes() {
TestResource testResource = AtlasDbHttpClients.createLiveReloadingProxyWithQuickFailoverForTesting(
() -> ImmutableServerListConfig.builder().build(),
SslSocketFactories::createSslSocketFactory,
proxyConfiguration -> ProxySelector.getDefault(),
TestResource.class,
UserAgents.DEFAULT_VALUE);

assertThatThrownBy(testResource::getTestNumber).isInstanceOf(ServiceNotAvailableException.class);
}

@Test
public void httpProxyCanBeCommissionedAndDecommissionedIfNodeAvailabilityChanges() {
AtomicReference<ServerListConfig> config = new AtomicReference<>(ImmutableServerListConfig.builder().build());

TestResource testResource = AtlasDbHttpClients.createLiveReloadingProxyWithQuickFailoverForTesting(
config::get,
SslSocketFactories::createSslSocketFactory,
proxyConfiguration -> ProxySelector.getDefault(),
TestResource.class,
UserAgents.DEFAULT_VALUE);

// At this point, there are zero nodes in the config, so we should get ServiceNotAvailable.
assertThatThrownBy(testResource::getTestNumber).isInstanceOf(ServiceNotAvailableException.class);

config.set(ImmutableServerListConfig.builder().addServers(getUriForPort(availablePort)).build());
Uninterruptibles.sleepUninterruptibly(
PollingRefreshable.DEFAULT_REFRESH_INTERVAL.getSeconds() + 1, TimeUnit.SECONDS);
assertThat(testResource.getTestNumber(), equalTo(TEST_NUMBER));

config.set(ImmutableServerListConfig.builder().build());
Uninterruptibles.sleepUninterruptibly(
PollingRefreshable.DEFAULT_REFRESH_INTERVAL.getSeconds() + 1, TimeUnit.SECONDS);
assertThatThrownBy(testResource::getTestNumber).isInstanceOf(ServiceNotAvailableException.class);
}

private static String getUriForPort(int port) {
return String.format("http://%s:%s", WireMockConfiguration.DEFAULT_BIND_ADDRESS, port);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
timestampClient:
enableTimestampBatching: true

timelockRuntime:
serversList:
servers: []
6 changes: 5 additions & 1 deletion atlasdb-config/src/test/resources/runtime-config-block.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ timelockRuntime:
servers:
- "https://foo1:12345"
- "https://foo2:8421"
- "https://foo3:9421"
- "https://foo3:9421"
sslConfiguration:
trustStorePath: var/security/trustStore.jks
keyStorePath: var/security/keyStore.jks
keyStorePassword: 0987654321
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import java.util.Optional;
import java.util.Set;

import javax.validation.constraints.Size;

import org.immutables.value.Value;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
Expand All @@ -31,11 +29,13 @@
@JsonSerialize(as = ImmutableServerListConfig.class)
@Value.Immutable
public interface ServerListConfig {

@Size(min = 1)
Set<String> servers();

Optional<SslConfiguration> sslConfiguration();

Optional<ProxyConfiguration> proxyConfiguration();

default boolean hasAtLeastOneServer() {
return servers().size() >= 1;
}
}
Loading