Revert "Add 'disabled' option for filedistribution"

5 files changed, 37 insertions, 78 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 0160978773d..b58406e1935 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,39 +7,28 @@ 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 BinaryScaledAmount uploadBitRate = new BinaryScaledAmount();
-    private BinaryScaledAmount downloadBitRate = new BinaryScaledAmount();
-    private boolean disabled = false;
+    private FileDistributionOptions() {}
 
+    private BinaryScaledAmount uploadbitrate = new BinaryScaledAmount();
+    private BinaryScaledAmount downloadbitrate = new BinaryScaledAmount();
 
-    public void downloadBitRate(BinaryScaledAmount amount) {
+    //Called through reflection
+    public void downloadbitrate(BinaryScaledAmount amount) {
         ensureNonNegative(amount);
-        downloadBitRate = amount;
+        downloadbitrate = amount;
     }
 
-    public void uploadBitRate(BinaryScaledAmount amount) {
+    //Called through reflection
+    public void uploadbitrate(BinaryScaledAmount amount) {
         ensureNonNegative(amount);
-        uploadBitRate = amount;
-    }
-
-    public void disabled(boolean value) {
-        disabled = value;
-    }
-
-    public boolean disabled() {
-        return disabled;
+        uploadbitrate = amount;
     }
 
     private void ensureNonNegative(BinaryScaledAmount amount) {
@@ -49,12 +38,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 8a5d6846a64..55c3f569900 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,42 +6,47 @@ import com.yahoo.text.XML;
 import com.yahoo.vespa.model.admin.FileDistributionOptions;
 import org.w3c.dom.Element;
 
-import java.util.Optional;
-
 /**
- * Builds file distribution options.
- *
- * @author Tony Vaagenes
- * @author hmusum
+ * Builds a file distribution options.
+ * @author tonytv
  */
 public class DomFileDistributionOptionsBuilder {
-
     private static void throwExceptionForElementInFileDistribution(String subElement, String reason) {
         throw new RuntimeException("In element '" + subElement + "' contained in 'filedistribution': " + reason);
     }
 
-    private Optional<BinaryScaledAmount> getAmount(String name, Element fileDistributionElement) {
-        Element optionElement = XML.getChild(fileDistributionElement, name);
+    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) {
         try {
+            Element optionElement = XML.getChild(fileDistributionElement, name);
             if (optionElement != null) {
                 String valueString = XML.getValue(optionElement);
-                return Optional.of(BinaryScaledAmountParser.parse(valueString));
+                BinaryScaledAmount amount = BinaryScaledAmountParser.parse(valueString);
+                callSetter(options, name, amount);
             }
         } 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) {
-            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)));
-            }
+            setIfPresent(options, "uploadbitrate", fileDistributionElement);
+            setIfPresent(options, "downloadbitrate", fileDistributionElement);
         }
         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 98e230cb8ba..6c2da51f3e3 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,10 +54,8 @@ public class FileDistributorService extends AbstractService implements
 
     @Override
     public String getStartupCommand() {
-        // 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();
+        return "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 1b0d487a388..cdf26807bcb 100644
--- a/config-model/src/main/resources/schema/admin.rnc
+++ b/config-model/src/main/resources/schema/admin.rnc
@@ -83,8 +83,7 @@ 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 disabled { xsd:boolean }?
+   element downloadbitrate { xsd:string { pattern = "\d+(\.\d*)?\s*[kmgKMG]?" } }?
 }
 
 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 946624a1cdb..b5375ccbce4 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,36 +275,4 @@ 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
-    }
 }