diff options
9 files changed, 166 insertions, 190 deletions
diff --git a/configserver-flags/pom.xml b/configserver-flags/pom.xml index a55a8db5f56..8c96512c4c0 100644 --- a/configserver-flags/pom.xml +++ b/configserver-flags/pom.xml @@ -63,6 +63,12 @@ <!-- test --> <dependency> <groupId>com.yahoo.vespa</groupId> + <artifactId>defaults</artifactId> + <version>${project.version}</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> <artifactId>testutil</artifactId> <version>${project.version}</version> <scope>test</scope> diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java index d452d8a4aad..b1ffc05e70c 100644 --- a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSource.java @@ -2,16 +2,23 @@ package com.yahoo.vespa.configserver.flags; import com.google.inject.Inject; +import com.yahoo.vespa.configserver.flags.db.BootstrapFlagSource; import com.yahoo.vespa.configserver.flags.db.ZooKeeperFlagSource; -import com.yahoo.vespa.flags.FileFlagSource; import com.yahoo.vespa.flags.OrderedFlagSource; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; + /** * @author hakonhall */ public class ConfigServerFlagSource extends OrderedFlagSource { @Inject public ConfigServerFlagSource(FlagsDb flagsDb) { - super(new FileFlagSource(), new ZooKeeperFlagSource(flagsDb)); + this(FileSystems.getDefault(), flagsDb); + } + + ConfigServerFlagSource(FileSystem fileSystem, FlagsDb flagsDb) { + super(new BootstrapFlagSource(fileSystem), new ZooKeeperFlagSource(flagsDb)); } } diff --git a/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/BootstrapFlagSource.java b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/BootstrapFlagSource.java new file mode 100644 index 00000000000..0c467a0e7c8 --- /dev/null +++ b/configserver-flags/src/main/java/com/yahoo/vespa/configserver/flags/db/BootstrapFlagSource.java @@ -0,0 +1,36 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.configserver.flags.db; + +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.FlagSource; +import com.yahoo.vespa.flags.RawFlag; +import com.yahoo.vespa.flags.file.FlagDbFile; +import com.yahoo.vespa.flags.json.FlagData; + +import java.nio.file.FileSystem; +import java.util.Map; +import java.util.Optional; + +/** + * A flag source that can be used in emergencies, see test for path and format of file. + * + * <p>Use file if the ZooKeeper flag makes it difficult to start the config server, would make irreparable damage, + * or is impossible to change through ZooKeeper due to a bug. This flag source should take precedence over ZooKeeper + * whenever it has specified the value for a flag. + * + * @author hakonhall + */ +public class BootstrapFlagSource implements FlagSource { + private final Map<FlagId, FlagData> flagData; + + public BootstrapFlagSource(FileSystem fileSystem) { + // The flags on disk is read once now and never again. + this.flagData = new FlagDbFile(fileSystem).read(); + } + + @Override + public Optional<RawFlag> fetch(FlagId id, FetchVector vector) { + return Optional.ofNullable(flagData.get(id)).flatMap(data -> data.resolve(vector)); + } +} diff --git a/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java new file mode 100644 index 00000000000..d0d1d61628c --- /dev/null +++ b/configserver-flags/src/test/java/com/yahoo/vespa/configserver/flags/ConfigServerFlagSourceTest.java @@ -0,0 +1,106 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.configserver.flags; + +import com.yahoo.vespa.defaults.Defaults; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.FetchVector; +import com.yahoo.vespa.flags.FlagId; +import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.RawFlag; +import com.yahoo.vespa.test.file.TestFileSystem; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Optional; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author hakonhall + */ +public class ConfigServerFlagSourceTest { + private final String vespaHome = Defaults.getDefaults().vespaHome(); + private final FileSystem fileSystem = TestFileSystem.create(); + private final FlagsDb flagsDb = mock(FlagsDb.class); + private final FlagId flagId = new FlagId("fid"); + + private Flags.Replacer flagsReplacer; + private ConfigServerFlagSource flagSource; + private BooleanFlag flag; + + @Before + public void setUp() { + flagsReplacer = Flags.clearFlagsForTesting(); + } + + @After + public void tearDown() { + flagsReplacer.close(); + } + + private void initialize() { + flagSource = new ConfigServerFlagSource(fileSystem, flagsDb); + flag = Flags.defineFeatureFlag(flagId.toString(), false, "", "").bindTo(flagSource); + } + + @Test + public void testAbsentInFileSystemForwardsToFlagsDb() { + initialize(); + + when(flagsDb.getValue(flagId)).thenReturn(Optional.empty()); + Optional<RawFlag> rawFlag = flagSource.fetch(flagId, new FetchVector()); + assertFalse(rawFlag.isPresent()); + verify(flagsDb, times(1)).getValue(flagId); + } + + @Test + public void testAvoidingZooKeeperWhenOverridingInFile() throws IOException { + // Here is how to set the value of a flag, such that ZooKeeper will NOT be queried when getting that value: + // - Make a flag.db file with the override + Path flagPath = fileSystem.getPath(vespaHome + "/var/vespa/flag.db"); + String json = "{\n" + + " \"flags\": [\n" + + " {\n" + + " \"id\" : \"fid\",\n" + + " \"rules\" : [\n" + + " {\n" + + " \"value\" : true\n" + + " }\n" + + " ]\n" + + " }\n" + + " ]\n" + + "}"; + Files.createDirectories(flagPath.getParent()); + Files.write(flagPath, json.getBytes(StandardCharsets.UTF_8)); + + // Alright, verify we have accomplished overriding that flag and avoid flagDb/ZooKeeper + + initialize(); + + Optional<RawFlag> rawFlag = flagSource.fetch(flagId, new FetchVector()); + assertTrue(rawFlag.isPresent()); + assertTrue(flag.value()); + + verify(flagsDb, times(0)).getValue(any()); + + // Other flags DO hit flagDb/ZooKeeper + + FlagId flagId2 = new FlagId("fooId"); + when(flagsDb.getValue(flagId2)).thenReturn(Optional.empty()); + Optional<RawFlag> rawFlag2 = flagSource.fetch(flagId2, new FetchVector()); + assertFalse(rawFlag2.isPresent()); + verify(flagsDb, times(1)).getValue(flagId2); + } +}
\ No newline at end of file diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java index 2463cca190a..d6d2bfbe89f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/Application.java @@ -25,9 +25,9 @@ import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.config.util.ConfigUtils; import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FetchVector; -import com.yahoo.vespa.flags.FileFlagSource; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; +import com.yahoo.vespa.flags.InMemoryFlagSource; import java.util.Objects; import java.util.Set; @@ -53,7 +53,7 @@ public class Application implements ModelResult { public Application(Model model, ServerCache cache, long appGeneration, boolean internalRedeploy, Version vespaVersion, MetricUpdater metricUpdater, ApplicationId app) { - this(model, cache, appGeneration, internalRedeploy, vespaVersion, metricUpdater, app, new FileFlagSource()); + this(model, cache, appGeneration, internalRedeploy, vespaVersion, metricUpdater, app, new InMemoryFlagSource()); } public Application(Model model, ServerCache cache, long appGeneration, boolean internalRedeploy, diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java index b2e911c47a4..476f77ae1db 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/InjectedGlobalComponentRegistryTest.java @@ -14,11 +14,12 @@ import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.monitoring.Metrics; import com.yahoo.vespa.config.server.provision.HostProvisionerProvider; import com.yahoo.vespa.config.server.rpc.RpcServer; -import com.yahoo.vespa.config.server.session.*; -import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.config.server.session.SessionPreparer; +import com.yahoo.vespa.config.server.session.SessionTest; import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.flags.FileFlagSource; +import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.model.VespaModelFactory; import org.junit.Before; import org.junit.Rule; @@ -75,7 +76,7 @@ public class InjectedGlobalComponentRegistryTest { globalComponentRegistry = new InjectedGlobalComponentRegistry(curator, configCurator, metrics, modelFactoryRegistry, sessionPreparer, rpcServer, configserverConfig, generationCounter, defRepo, permanentApplicationPackage, hostRegistries, hostProvisionerProvider, zone, - new ConfigServerDB(configserverConfig), new FileFlagSource()); + new ConfigServerDB(configserverConfig), new InMemoryFlagSource()); } @Test diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java b/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java index 83b1fdd009f..ef1f0f380e3 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/TestComponentRegistry.java @@ -18,10 +18,9 @@ import com.yahoo.vespa.config.server.session.SessionPreparer; import com.yahoo.vespa.config.server.tenant.MockTenantListener; import com.yahoo.vespa.config.server.tenant.TenantListener; import com.yahoo.vespa.config.server.tenant.TenantRequestHandlerTest; +import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; -import com.yahoo.vespa.flags.FileFlagSource; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.model.VespaModelFactory; @@ -200,7 +199,7 @@ public class TestComponentRegistry implements GlobalComponentRegistry { @Override public ConfigServerDB getConfigServerDB() { return configServerDB;} @Override - public FlagSource getFlagSource() { return new FileFlagSource(); } + public FlagSource getFlagSource() { return new InMemoryFlagSource(); } public FileDistributionFactory getFileDistributionFactory() { return fileDistributionFactory; } diff --git a/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java b/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java deleted file mode 100644 index 1a31ecd713e..00000000000 --- a/flags/src/main/java/com/yahoo/vespa/flags/FileFlagSource.java +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags; - -import com.google.inject.Inject; -import com.yahoo.vespa.flags.json.FlagData; -import com.yahoo.vespa.flags.json.Rule; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.FileSystem; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.NoSuchFileException; -import java.nio.file.Path; -import java.util.Collections; -import java.util.Optional; - -/** - * @author hakonhall - */ -public class FileFlagSource implements FlagSource { - static final String FLAGS_DIRECTORY = "/etc/vespa/flags"; - - private final Path flagsDirectory; - - @Inject - public FileFlagSource() { - this(FileSystems.getDefault()); - } - - public FileFlagSource(FileSystem fileSystem) { - this(fileSystem.getPath(FLAGS_DIRECTORY)); - } - - public FileFlagSource(Path flagsDirectory) { - this.flagsDirectory = flagsDirectory; - } - - @Override - public Optional<RawFlag> fetch(FlagId flagId, FetchVector vector) { - return getResolver(flagId).resolve(vector); - } - - private FlagData getResolver(FlagId flagId) { - Optional<String> v2String = getString(flagId, ".2"); - if (v2String.isPresent()) { - return FlagData.deserialize(v2String.get()); - } - - Optional<String> v1String = getString(flagId, ""); - if (v1String.isPresent()) { - // version 1: File contains value as a JSON - // version 2: File contains FileResolver as a JSON (which may contain many values, one for each rule) - // version 1 files should probably be discontinued - Rule rule = new Rule(Optional.of(JsonNodeRawFlag.fromJson(v1String.get())), Collections.emptyList()); - return new FlagData(flagId, new FetchVector(), Collections.singletonList(rule)); - } - - // Will eventually resolve to empty RawFlag - return new FlagData(flagId); - } - - private Optional<String> getString(FlagId id, String suffix) { - return getBytes(id, suffix).map(bytes -> new String(bytes, StandardCharsets.UTF_8)); - } - - private Optional<byte[]> getBytes(FlagId id, String suffix) { - try { - return Optional.of(Files.readAllBytes(getPath(id, suffix))); - } catch (NoSuchFileException e) { - return Optional.empty(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - private Path getPath(FlagId id, String suffix) { - return flagsDirectory.resolve(id.toString() + suffix); - } - - @Override - public String toString() { - return "FileFlagSource{" + flagsDirectory + '}'; - } -} diff --git a/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java b/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java deleted file mode 100644 index e19436069b5..00000000000 --- a/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.flags; - -import com.yahoo.vespa.test.file.TestFileSystem; -import org.junit.Test; - -import java.io.IOException; -import java.io.UncheckedIOException; -import java.nio.file.FileSystem; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Optional; - -import static org.hamcrest.CoreMatchers.containsString; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -public class FileFlagSourceTest { - private final FileSystem fileSystem = TestFileSystem.create(); - private final FileFlagSource source = new FileFlagSource(fileSystem); - private final FlagId id = new FlagId("foo"); - - @Test - public void testFeatureLikeFlags() throws IOException { - BooleanFlag booleanFlag = new UnboundBooleanFlag(id).bindTo(source); - BooleanFlag byDefaultTrue = new UnboundBooleanFlag(id, true).bindTo(source); - - assertFalse(booleanFlag.value()); - assertTrue(byDefaultTrue.value()); - - writeFlag(id.toString(), "true\n"); - - assertTrue(booleanFlag.value()); - assertTrue(byDefaultTrue.value()); - - writeFlag(id.toString(), "false\n"); - - assertFalse(booleanFlag.value()); - assertFalse(byDefaultTrue.value()); - } - - @Test - public void testIntegerLikeFlags() throws IOException { - IntFlag intFlag = new UnboundIntFlag(id, -1).bindTo(source); - LongFlag longFlag = new UnboundLongFlag(id, -2L).bindTo(source); - - assertFalse(fetch().isPresent()); - assertFalse(fetch().isPresent()); - assertEquals(-1, intFlag.value()); - assertEquals(-2L, longFlag.value()); - - writeFlag(id.toString(), "1\n"); - - assertTrue(fetch().isPresent()); - assertTrue(fetch().isPresent()); - assertEquals(1, intFlag.value()); - assertEquals(1L, longFlag.value()); - } - - @Test - public void testStringFlag() throws IOException { - StringFlag stringFlag = new UnboundStringFlag(id, "default").bindTo(source); - assertFalse(fetch().isPresent()); - assertEquals("default", stringFlag.value()); - - writeFlag(id.toString(), "\"1\\n\"\n"); - assertEquals("1\n", stringFlag.value()); - } - - @Test - public void parseFailure() throws IOException { - BooleanFlag booleanFlag = new UnboundBooleanFlag(id).bindTo(source); - writeFlag(booleanFlag.id().toString(), "garbage"); - - try { - booleanFlag.value(); - } catch (UncheckedIOException e) { - assertThat(e.getMessage(), containsString("garbage")); - } - } - - private Optional<RawFlag> fetch() { - return source.fetch(id, new FetchVector()); - } - - private void writeFlag(String flagId, String value) throws IOException { - Path featurePath = fileSystem.getPath(FileFlagSource.FLAGS_DIRECTORY).resolve(flagId); - Files.createDirectories(featurePath.getParent()); - Files.write(featurePath, value.getBytes()); - } -}
\ No newline at end of file |