Merge pull request #4081 from vespa-engine/hmusum/add-option-to-disable-filedistributor

Add 'disabled' option for filedistribution

5 files changed, 78 insertions, 37 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java
index b58406e1935..0160978773d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/FileDistributionOptions.java
@@ -7,28 +7,39 @@ import com.yahoo.cloud.config.filedistribution.FiledistributorConfig;
 
 /**
  * Options for controlling the behavior of the file distribution services.
+ *
  * @author tonytv
  */
 public class FileDistributionOptions implements FiledistributorConfig.Producer {
+
     public static FileDistributionOptions defaultOptions() {
         return new FileDistributionOptions();
     }
 
-    private FileDistributionOptions() {}
+    private FileDistributionOptions() {
+    }
+
+    private BinaryScaledAmount uploadBitRate = new BinaryScaledAmount();
+    private BinaryScaledAmount downloadBitRate = new BinaryScaledAmount();
+    private boolean disabled = false;
 
-    private BinaryScaledAmount uploadbitrate = new BinaryScaledAmount();
-    private BinaryScaledAmount downloadbitrate = new BinaryScaledAmount();
 
-    //Called through reflection
-    public void downloadbitrate(BinaryScaledAmount amount) {
+    public void downloadBitRate(BinaryScaledAmount amount) {
         ensureNonNegative(amount);
-        downloadbitrate = amount;
+        downloadBitRate = amount;
     }
 
-    //Called through reflection
-    public void uploadbitrate(BinaryScaledAmount amount) {
+    public void uploadBitRate(BinaryScaledAmount amount) {
         ensureNonNegative(amount);
-        uploadbitrate = amount;
+        uploadBitRate = amount;
+    }
+
+    public void disabled(boolean value) {
+        disabled = value;
+    }
+
+    public boolean disabled() {
+        return disabled;
     }
 
     private void ensureNonNegative(BinaryScaledAmount amount) {
@@ -38,12 +49,12 @@ public class FileDistributionOptions implements FiledistributorConfig.Producer {
 
     private int byteRate(BinaryScaledAmount bitRate) {
         BinaryScaledAmount byteRate = bitRate.divide(8);
-        return (int)byteRate.as(BinaryPrefix.unit);
+        return (int) byteRate.as(BinaryPrefix.unit);
     }
 
     @Override
     public void getConfig(FiledistributorConfig.Builder builder) {
-        builder.maxuploadspeed((double)byteRate(uploadbitrate));
-        builder.maxdownloadspeed((double)byteRate(downloadbitrate));
+        builder.maxuploadspeed((double) byteRate(uploadBitRate));
+        builder.maxdownloadspeed((double) byteRate(downloadBitRate));
     }
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java
index 55c3f569900..8a5d6846a64 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomFileDistributionOptionsBuilder.java
@@ -6,47 +6,42 @@ import com.yahoo.text.XML;
 import com.yahoo.vespa.model.admin.FileDistributionOptions;
 import org.w3c.dom.Element;
 
+import java.util.Optional;
+
 /**
- * Builds a file distribution options.
- * @author tonytv
+ * Builds file distribution options.
+ *
+ * @author Tony Vaagenes
+ * @author hmusum
  */
 public class DomFileDistributionOptionsBuilder {
+
     private static void throwExceptionForElementInFileDistribution(String subElement, String reason) {
         throw new RuntimeException("In element '" + subElement + "' contained in 'filedistribution': " + reason);
     }
 
-    private static void callSetter(FileDistributionOptions options, String name, BinaryScaledAmount amount) {
-        try {
-            options.getClass().getMethod(name, BinaryScaledAmountParser.class).invoke(options, amount);
-        } catch (IllegalArgumentException e) {
-            throwExceptionForElementInFileDistribution(name, e.getMessage());
-        }
-        catch (Exception e) {
-            if (e instanceof RuntimeException)
-                throw (RuntimeException)e;
-            else
-                throw new RuntimeException(e);
-        }
-    }
-
-    private static void setIfPresent(FileDistributionOptions options, String name, Element fileDistributionElement) {
+    private Optional<BinaryScaledAmount> getAmount(String name, Element fileDistributionElement) {
+        Element optionElement = XML.getChild(fileDistributionElement, name);
         try {
-            Element optionElement = XML.getChild(fileDistributionElement, name);
             if (optionElement != null) {
                 String valueString = XML.getValue(optionElement);
-                BinaryScaledAmount amount = BinaryScaledAmountParser.parse(valueString);
-                callSetter(options, name, amount);
+                return Optional.of(BinaryScaledAmountParser.parse(valueString));
             }
         } catch (NumberFormatException e) {
             throwExceptionForElementInFileDistribution(name, "Expected a valid number. (Message = " + e.getMessage() + ").");
         }
+        return Optional.empty();
     }
 
     public FileDistributionOptions build(Element fileDistributionElement) {
         FileDistributionOptions options = FileDistributionOptions.defaultOptions();
         if (fileDistributionElement != null) {
-            setIfPresent(options, "uploadbitrate", fileDistributionElement);
-            setIfPresent(options, "downloadbitrate", fileDistributionElement);
+            getAmount("uploadbitrate", fileDistributionElement).ifPresent(options::uploadBitRate);
+            getAmount("downloadbitrate", fileDistributionElement).ifPresent(options::downloadBitRate);
+            Element disable = XML.getChild(fileDistributionElement, "disabled");
+            if (disable != null) {
+                options.disabled(Boolean.valueOf(XML.getValue(disable)));
+            }
         }
         return options;
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java
index 6c2da51f3e3..98e230cb8ba 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/FileDistributorService.java
@@ -54,8 +54,10 @@ public class FileDistributorService extends AbstractService implements
 
     @Override
     public String getStartupCommand() {
-        return "exec $ROOT/sbin/vespa-filedistributor"
-                + " --configid " + getConfigId();
+        // If disabled config proxy should act as file distributor, so don't start this service
+        return (fileDistributionOptions.disabled())
+                ? null
+                : "exec $ROOT/sbin/vespa-filedistributor" + " --configid " + getConfigId();
     }
 
     @Override
diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc
index cdf26807bcb..1b0d487a388 100644
--- a/config-model/src/main/resources/schema/admin.rnc
+++ b/config-model/src/main/resources/schema/admin.rnc
@@ -83,7 +83,8 @@ LogServer = element logserver {
 
 FileDistribution = element filedistribution {
    element uploadbitrate { xsd:string {   pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }? &
-   element downloadbitrate { xsd:string { pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }?
+   element downloadbitrate { xsd:string { pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }? &
+   element disabled { xsd:boolean }?
 }
 
 Metrics = element metrics {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java
index b5375ccbce4..946624a1cdb 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/AdminTestCase.java
@@ -275,4 +275,36 @@ public class AdminTestCase {
         // 1 logforwarder on each host
         assertTrue(configIds.toString(), configIds.contains("admin/logforwarder.0"));
     }
+
+    @Test
+    public void disableFiledistributorService() throws Exception {
+        String hosts = "<hosts>"
+                + "  <host name=\"localhost\">"
+                + "    <alias>node0</alias>"
+                + "  </host>"
+                + "</hosts>";
+
+        String services = "<services>" +
+                "  <admin version='2.0'>" +
+                "    <adminserver hostalias='node0' />" +
+                "    <filedistribution>" +
+                "      <disabled>true</disabled>" +
+                "    </filedistribution>" +
+                "  </admin>" +
+                "</services>";
+
+        VespaModel vespaModel = new VespaModelCreatorWithMockPkg(hosts, services).create();
+        String localhost = HostName.getLocalhost();
+        String localhostConfigId = "hosts/" + localhost;
+
+        // Verify services in the sentinel config
+        SentinelConfig.Builder b = new SentinelConfig.Builder();
+        vespaModel.getConfig(b, localhostConfigId);
+        SentinelConfig sentinelConfig = new SentinelConfig(b);
+        assertThat(sentinelConfig.service().size(), is(3));
+        assertThat(sentinelConfig.service(0).name(), is("logserver"));
+        assertThat(sentinelConfig.service(1).name(), is("slobrok"));
+        assertThat(sentinelConfig.service(2).name(), is("logd"));
+        // No filedistributor service
+    }
 }