Read override file flags from file only once for config server

Up until now, every lookup of a flag in the ConfigServerFlagSource would
 1. try to read 2 flag files under /etc/vespa/flags, causing exceptions
    because they are typically not set, and
 2. then read flag from ZooKeeper through ZooKeeperFlagSource

Optimization was deliberately held off until later (now). This PR fixes (1).

Changes the ConfigServerFlagSource to:
 1'. Read VESPA_HOME/var/vespa/flag.db once during component graph
     construction.  As before, if a flag is defined on file, the flag is not
     looked up in ZK, which may be useful in emergencies.
 2.  As before.

Also, removes the last usages of FileFlagSource and its reading of flags in
/etc/vespa/flags.

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