Revert "Be down initially"

6 files changed, 101 insertions, 170 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java b/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java
deleted file mode 100644
index 19617b46a84..00000000000
--- a/container-core/src/main/java/com/yahoo/container/handler/ClustersStatus.java
+++ /dev/null
@@ -1,75 +0,0 @@
-package com.yahoo.container.handler;
-
-import com.google.inject.Inject;
-import com.yahoo.component.AbstractComponent;
-
-import java.util.HashMap;
-import java.util.Map;
-
-/**
- * A component which tracks the up/down status of any clusters which should influence
- * the up down status of this container itself, as well as the separate fact that such clusters are present.
- *
- * This is a separate component which has <b>no dependencies</b> such that the status tracked in this
- * will survive reconfiguration events and inform other components even immediately after a reconfiguration
- * (where the true statue of clusters may not yet be available).
- *
- * @author bratseth
- */
-public class ClustersStatus extends AbstractComponent {
-
-    // NO DEPENDENCIES: Do not add dependencies here
-    @Inject
-    public ClustersStatus() { }
-
-    /** Are there any (in-service influencing) clusters in this container? */
-    private boolean containerHasClusters;
-
-    /** If we have no clusters, what should we answer? */
-    private boolean receiveTrafficByDefault;
-
-    private final Object mutex = new Object();
-
-    /** The status of clusters, when known. Note that clusters may exist for which there is no knowledge yet. */
-    private final Map<Object, Boolean> clusterStatus = new HashMap<>();
-
-    public void setContainerHasClusters(boolean containerHasClusters) {
-        synchronized (mutex) {
-            this.containerHasClusters = containerHasClusters;
-            if ( ! containerHasClusters)
-                clusterStatus.clear(); // forget container clusters which was configured away
-        }
-    }
-
-    public void setReceiveTrafficByDefault(boolean receiveTrafficByDefault) {
-        synchronized (mutex) {
-            this.receiveTrafficByDefault = receiveTrafficByDefault;
-        }
-    }
-
-    public void setUp(Object clusterIdentifier) {
-        synchronized (mutex) {
-            clusterStatus.put(clusterIdentifier, Boolean.TRUE);
-        }
-    }
-
-    public void setDown(Object clusterIdentifier) {
-        synchronized (mutex) {
-            clusterStatus.put(clusterIdentifier, Boolean.FALSE);
-        }
-    }
-
-    /** Returns whether this container should receive traffic based on the state of this */
-    public boolean containerShouldReceiveTraffic() {
-        synchronized (mutex) {
-            if (containerHasClusters) {
-                // Should receive traffic when at least one cluster is up
-                return clusterStatus.values().stream().anyMatch(status -> status==true);
-            }
-            else {
-                return receiveTrafficByDefault;
-            }
-        }
-    }
-
-}
diff --git a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java
index be386b1b84e..d7457140dae 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java
@@ -15,56 +15,70 @@ import com.yahoo.container.core.VipStatusConfig;
  */
 public class VipStatus {
 
-    private final ClustersStatus clustersStatus;
-
-    /** If this is non-null, its value decides whether this container is in rotation */
-    private Boolean inRotationOverride;
+    private final Map<Object, Boolean> clusters = new IdentityHashMap<>();
+    private final VipStatusConfig vipStatusConfig;
 
     public VipStatus() {
-        this(new QrSearchersConfig(new QrSearchersConfig.Builder()),
-             new VipStatusConfig(new VipStatusConfig.Builder()),
-             new ClustersStatus());
+        this(null, new VipStatusConfig(new VipStatusConfig.Builder()));
     }
 
     public VipStatus(QrSearchersConfig dispatchers) {
-        this(dispatchers, new VipStatusConfig(new VipStatusConfig.Builder()), new ClustersStatus());
-    }
-
-    public VipStatus(ClustersStatus clustersStatus) {
-        this.clustersStatus = clustersStatus;
+        this(dispatchers, new VipStatusConfig(new VipStatusConfig.Builder()));
     }
 
+    // TODO: Why use QrSearchersConfig here? Remove and inject ComponentRegistry<ClusterSearcher> instead?
     @Inject
-    public VipStatus(QrSearchersConfig dispatchers, VipStatusConfig vipStatusConfig, ClustersStatus clustersStatus) {
-        this.clustersStatus = clustersStatus;
-        clustersStatus.setReceiveTrafficByDefault(vipStatusConfig.initiallyInRotation());
-        clustersStatus.setContainerHasClusters(! dispatchers.searchcluster().isEmpty());
+    public VipStatus(QrSearchersConfig dispatchers, VipStatusConfig vipStatusConfig) {
+        // the config is not used for anything, it's just a dummy to create a
+        // dependency link to which dispatchers are used
+        this.vipStatusConfig = vipStatusConfig;
     }
 
     /**
-     * Explicitly set this container in or out of rotation
+     * Set a service or cluster into rotation.
      *
-     * @param inRotation true to set this in rotation regardless of any clusters and of the default value,
-     *                   false to set it out, and null to make this decision using the usual cluster-dependent logic
+     * @param clusterIdentifier
+     *            an object where the object identity will serve to identify the
+     *            cluster or service
      */
-    public void setInRotation(Boolean inRotation) {
-        this.inRotationOverride = inRotation;
-    }
-
-    /** Note that a cluster (which influences up/down state) is up */
     public void addToRotation(Object clusterIdentifier) {
-        clustersStatus.setUp(clusterIdentifier);
+        synchronized (clusters) {
+            clusters.put(clusterIdentifier, Boolean.TRUE);
+        }
     }
 
-    /** Note that a cluster (which influences up/down state) is down */
+    /**
+     * Set a service or cluster out of rotation.
+     *
+     * @param clusterIdentifier
+     *            an object where the object identity will serve to identify the
+     *            cluster or service
+     */
     public void removeFromRotation(Object clusterIdentifier) {
-        clustersStatus.setDown(clusterIdentifier);
+        synchronized (clusters) {
+            clusters.put(clusterIdentifier, Boolean.FALSE);
+        }
     }
 
-    /** Returns whether this container should receive traffic at this time */
+    /**
+     * Tell whether the container is connected to any active services at all.
+     *
+     * @return true if at least one service or cluster is up, or value is taken from config if no services
+     *         are registered (yet)
+     */
     public boolean isInRotation() {
-        if (inRotationOverride != null) return inRotationOverride;
-        return clustersStatus.containerShouldReceiveTraffic();
+        synchronized (clusters) {
+            // if no stored state, use config to decide whether to serve or not
+            if (clusters.size() == 0) {
+                return vipStatusConfig.initiallyInRotation();
+            }
+            for (Boolean inRotation : clusters.values()) {
+                if (inRotation) {
+                    return true;
+                }
+            }
+        }
+        return false;
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java b/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java
index 60affddeb60..b7977e7832d 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/VipStatusHandler.java
@@ -49,7 +49,7 @@ public final class VipStatusHandler extends ThreadedHttpRequestHandler {
     class StatusResponse extends HttpResponse {
 
         static final String COULD_NOT_FIND_STATUS_FILE = "Could not find status file.\n";
-        static final String NO_SEARCH_BACKENDS = "No search backends available, VIP status disabled."; // TODO: Generify
+        static final String NO_SEARCH_BACKENDS = "No search backends available, VIP status disabled.";
         private static final String TEXT_HTML = "text/html";
         private String contentType = TEXT_HTML;
         private byte[] data = null;
diff --git a/container-core/src/main/resources/configdefinitions/vip-status.def b/container-core/src/main/resources/configdefinitions/vip-status.def
index 6aa10ff1b90..1e364419ab8 100644
--- a/container-core/src/main/resources/configdefinitions/vip-status.def
+++ b/container-core/src/main/resources/configdefinitions/vip-status.def
@@ -4,7 +4,6 @@ namespace=container.core
 ## If there is a Vespa search backend connected to this container, and that
 ## backend is out of service, automatically remove this container from VIP
 ## rotation, ignoring any status file.
-## TODO VESPA 7: This is always true and can be removed
 noSearchBackendsImpliesOutOfService bool default=true
 
 ## Whether to return hard-coded reply or serve "status.html" from disk
@@ -14,5 +13,5 @@ accessdisk bool default=false
 ## If the path is relative vespa home is prepended
 statusfile string default="share/qrsdocs/status.html"
 
-## The default rotation state when there are no configured clusters to decide rotation state
+## The initial rotation state when no information is known about backend clusters
 initiallyInRotation bool default=true
diff --git a/container-core/src/test/java/com/yahoo/container/handler/VipStatusHandlerTestCase.java b/container-core/src/test/java/com/yahoo/container/handler/VipStatusHandlerTestCase.java
index f8de32ee3ff..ef51a3c0f51 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/VipStatusHandlerTestCase.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/VipStatusHandlerTestCase.java
@@ -35,16 +35,16 @@ import static org.junit.Assert.fail;
  * care about the incoming URI, that's 100% handled in JDIsc by the binding
  * pattern.
  *
- * @author Steinar Knutsen
+ * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
  */
 public class VipStatusHandlerTestCase {
 
-    public static class MockResponseHandler implements ResponseHandler {
-
+    public static final class MockResponseHandler implements ResponseHandler {
         final ReadableContentChannel channel = new ReadableContentChannel();
 
         @Override
-        public ContentChannel handleResponse(com.yahoo.jdisc.Response response) {
+        public ContentChannel handleResponse(
+                final com.yahoo.jdisc.Response response) {
             return channel;
         }
     }
@@ -52,44 +52,45 @@ public class VipStatusHandlerTestCase {
     Metric metric = Mockito.mock(Metric.class);
 
     @Test
-    public void testHandleRequest() {
-        VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(false)
+    public final void testHandleRequest() {
+        final VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(false)
                 .noSearchBackendsImpliesOutOfService(false));
-        VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config, metric);
-        MockResponseHandler responseHandler = new MockResponseHandler();
-        HttpRequest request = createRequest();
-        BufferedContentChannel requestContent = createChannel();
+        final VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config, metric);
+        final MockResponseHandler responseHandler = new MockResponseHandler();
+        final HttpRequest request = createRequest();
+        final BufferedContentChannel requestContent = createChannel();
         handler.handleRequest(request, requestContent, responseHandler);
-        ByteBuffer b = responseHandler.channel.read();
-        byte[] asBytes = new byte[b.remaining()];
+        final ByteBuffer b = responseHandler.channel.read();
+        final byte[] asBytes = new byte[b.remaining()];
         b.get(asBytes);
         assertEquals(VipStatusHandler.OK_MESSAGE, Utf8.toString(asBytes));
     }
 
-    public static class NotFoundResponseHandler implements ResponseHandler {
-
+    public static final class NotFoundResponseHandler implements
+            ResponseHandler {
         final ReadableContentChannel channel = new ReadableContentChannel();
 
         @Override
-        public ContentChannel handleResponse(com.yahoo.jdisc.Response response) {
-            assertEquals(com.yahoo.jdisc.Response.Status.NOT_FOUND, response.getStatus());
+        public ContentChannel handleResponse(
+                final com.yahoo.jdisc.Response response) {
+            assertEquals(com.yahoo.jdisc.Response.Status.NOT_FOUND,
+                    response.getStatus());
             return channel;
         }
-
     }
 
     @Test
-    public void testFileNotFound() {
-        VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(true)
+    public final void testFileNotFound() {
+        final VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(true)
                 .statusfile("/VipStatusHandlerTestCaseFileThatReallyReallyShouldNotExist")
                 .noSearchBackendsImpliesOutOfService(false));
-        VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config, metric);
-        NotFoundResponseHandler responseHandler = new NotFoundResponseHandler();
-        HttpRequest request = createRequest();
-        BufferedContentChannel requestContent = createChannel();
+        final VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config, metric);
+        final NotFoundResponseHandler responseHandler = new NotFoundResponseHandler();
+        final HttpRequest request = createRequest();
+        final BufferedContentChannel requestContent = createChannel();
         handler.handleRequest(request, requestContent, responseHandler);
-        ByteBuffer b = responseHandler.channel.read();
-        byte[] asBytes = new byte[b.remaining()];
+        final ByteBuffer b = responseHandler.channel.read();
+        final byte[] asBytes = new byte[b.remaining()];
         b.get(asBytes);
         assertEquals(
                 VipStatusHandler.StatusResponse.COULD_NOT_FIND_STATUS_FILE,
@@ -97,22 +98,23 @@ public class VipStatusHandlerTestCase {
     }
 
     @Test
-    public void testFileFound() throws IOException {
-        File statusFile = File.createTempFile("VipStatusHandlerTestCase", null);
+    public final void testFileFound() throws IOException {
+        final File statusFile = File.createTempFile("VipStatusHandlerTestCase",
+                null);
         try {
-            FileWriter writer = new FileWriter(statusFile);
-            String OK = "OK\n";
+            final FileWriter writer = new FileWriter(statusFile);
+            final String OK = "OK\n";
             writer.write(OK);
             writer.close();
-            VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(true)
+            final VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(true)
                     .statusfile(statusFile.getAbsolutePath()).noSearchBackendsImpliesOutOfService(false));
-            VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config, metric);
-            MockResponseHandler responseHandler = new MockResponseHandler();
-            HttpRequest request = createRequest();
-            BufferedContentChannel requestContent = createChannel();
+            final VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config, metric);
+            final MockResponseHandler responseHandler = new MockResponseHandler();
+            final HttpRequest request = createRequest();
+            final BufferedContentChannel requestContent = createChannel();
             handler.handleRequest(request, requestContent, responseHandler);
-            ByteBuffer b = responseHandler.channel.read();
-            byte[] asBytes = new byte[b.remaining()];
+            final ByteBuffer b = responseHandler.channel.read();
+            final byte[] asBytes = new byte[b.remaining()];
             b.get(asBytes);
             assertEquals(OK, Utf8.toString(asBytes));
         } finally {
@@ -121,34 +123,34 @@ public class VipStatusHandlerTestCase {
     }
 
     @Test
-    public void testExplicitlyRotationControl() {
+    public final void testProgrammaticallyRemovedFromRotation() throws IOException {
         VipStatus vipStatus = new VipStatus();
-        VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(false)
+        final VipStatusConfig config = new VipStatusConfig(new VipStatusConfig.Builder().accessdisk(false)
                 .noSearchBackendsImpliesOutOfService(true));
-        VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config,  metric, vipStatus);
+        final VipStatusHandler handler = new VipStatusHandler(Executors.newCachedThreadPool(), config,  metric, vipStatus);
 
-        vipStatus.setInRotation(false);
+        vipStatus.removeFromRotation(this);
 
         {
-            MockResponseHandler responseHandler = new MockResponseHandler();
-            HttpRequest request = createRequest();
-            BufferedContentChannel requestContent = createChannel();
+            final MockResponseHandler responseHandler = new MockResponseHandler();
+            final HttpRequest request = createRequest();
+            final BufferedContentChannel requestContent = createChannel();
             handler.handleRequest(request, requestContent, responseHandler);
-            ByteBuffer b = responseHandler.channel.read();
-            byte[] asBytes = new byte[b.remaining()];
+            final ByteBuffer b = responseHandler.channel.read();
+            final byte[] asBytes = new byte[b.remaining()];
             b.get(asBytes);
             assertEquals(VipStatusHandler.StatusResponse.NO_SEARCH_BACKENDS, Utf8.toString(asBytes));
         }
 
-        vipStatus.setInRotation(true);
+        vipStatus.addToRotation(this);
 
         {
-            MockResponseHandler responseHandler = new MockResponseHandler();
-            HttpRequest request = createRequest();
-            BufferedContentChannel requestContent = createChannel();
+            final MockResponseHandler responseHandler = new MockResponseHandler();
+            final HttpRequest request = createRequest();
+            final BufferedContentChannel requestContent = createChannel();
             handler.handleRequest(request, requestContent, responseHandler);
-            ByteBuffer b = responseHandler.channel.read();
-            byte[] asBytes = new byte[b.remaining()];
+            final ByteBuffer b = responseHandler.channel.read();
+            final byte[] asBytes = new byte[b.remaining()];
             b.get(asBytes);
             assertEquals(VipStatusHandler.OK_MESSAGE, Utf8.toString(asBytes));
         }
diff --git a/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java b/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java
index e54f968f41d..725f8256ba3 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java
@@ -13,22 +13,13 @@ import org.junit.Test;
 public class VipStatusTestCase {
 
     @Test
-    public void testVipStatusWorksWithClusters() {
-        ClustersStatus clustersStatus = new ClustersStatus();
-        clustersStatus.setContainerHasClusters(true);
-        VipStatus v = new VipStatus(clustersStatus);
-
+    public final void testSmoke() {
         Object cluster1 = new Object();
         Object cluster2 = new Object();
         Object cluster3 = new Object();
-
+        VipStatus v = new VipStatus();
         // initial state
-        assertFalse(v.isInRotation());
-
-        // one cluster becomes up
-        v.addToRotation(cluster1);
         assertTrue(v.isInRotation());
-
         // all clusters down
         v.removeFromRotation(cluster1);
         v.removeFromRotation(cluster2);