diff options
Diffstat (limited to 'clustercontroller-apps')
4 files changed, 26 insertions, 46 deletions
diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java index eba3b35fcb2..9020765f777 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterController.java @@ -44,16 +44,16 @@ public class ClusterController extends AbstractComponent } - public void setOptions(String clusterName, FleetControllerOptions options, Metric metricImpl) throws Exception { + public void setOptions(FleetControllerOptions options, Metric metricImpl) throws Exception { metricWrapper.updateMetricImplementation(metricImpl); verifyThatZooKeeperWorks(options); synchronized (controllers) { - FleetController controller = controllers.get(clusterName); + FleetController controller = controllers.get(options.clusterName); if (controller == null) { StatusHandler.ContainerStatusPageServer statusPageServer = new StatusHandler.ContainerStatusPageServer(); controller = FleetController.create(options, statusPageServer, metricWrapper); - controllers.put(clusterName, controller); - status.put(clusterName, statusPageServer); + controllers.put(options.clusterName, controller); + status.put(options.clusterName, statusPageServer); } else { controller.updateOptions(options, 0); } diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java index c95d814eb99..ad65435c770 100644 --- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java +++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java @@ -31,9 +31,8 @@ public class ClusterControllerClusterConfigurer { configure(fleetcontrollerConfig); configure(slobroksConfig); configure(zookeepersConfig); - checkIfZooKeeperNeeded(); if (controller != null) { - controller.setOptions(options.clusterName, options, metricImpl); + controller.setOptions(options, metricImpl); } } @@ -79,7 +78,7 @@ public class ClusterControllerClusterConfigurer { } private void configure(SlobroksConfig config) { - String specs[] = new String[config.slobrok().size()]; + String[] specs = new String[config.slobrok().size()]; for (int i = 0; i < config.slobrok().size(); i++) { specs[i] = config.slobrok().get(i).connectionspec(); } @@ -87,22 +86,14 @@ public class ClusterControllerClusterConfigurer { } private void configure(ZookeepersConfig config) { - options.zooKeeperServerAddress = config.zookeeperserverlist(); + options.zooKeeperServerAddress = verifyZooKeeperAddress(config.zookeeperserverlist()); } - private void checkIfZooKeeperNeeded() { - // For legacy (testing, presumably) reasons, support running 1 instance - // without a ZK cluster. This is really a Horrible Thing(tm) since we - // violate cluster state versioning invariants when the controller is - // restarted. - if (options.zooKeeperServerAddress == null || "".equals(options.zooKeeperServerAddress)) { - if (options.fleetControllerCount > 1) { - throw new IllegalArgumentException( - "Must set zookeeper server with multiple fleetcontrollers"); - } else { - options.zooKeeperServerAddress = null; // Force null - } + private String verifyZooKeeperAddress(String zooKeeperServerAddress) { + if (zooKeeperServerAddress == null || "".equals(zooKeeperServerAddress)) { + throw new IllegalArgumentException("zookeeper server address must be set, was '" + zooKeeperServerAddress + "'"); } + return zooKeeperServerAddress; } } diff --git a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java index 330174f0313..37131349602 100644 --- a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java +++ b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurerTest.java @@ -58,19 +58,8 @@ public class ClusterControllerClusterConfigurerTest { assertTrue(configurer.getOptions() != null); assertEquals(0.123, configurer.getOptions().minNodeRatioPerGroup, 0.01); - // Oki with no zookeeper if one node - zookeepersConfig.zookeeperserverlist(""); - new ClusterControllerClusterConfigurer( - controller, - new StorDistributionConfig(distributionConfig), - new FleetcontrollerConfig(fleetcontrollerConfig), - new SlobroksConfig(slobroksConfig), - new ZookeepersConfig(zookeepersConfig), - metric - ); - try{ - fleetcontrollerConfig.fleet_controller_count(5); + zookeepersConfig.zookeeperserverlist(""); new ClusterControllerClusterConfigurer( controller, new StorDistributionConfig(distributionConfig), @@ -81,7 +70,7 @@ public class ClusterControllerClusterConfigurerTest { ); fail("Should not get here"); } catch (Exception e) { - assertEquals("Must set zookeeper server with multiple fleetcontrollers", e.getMessage()); + assertEquals("zookeeper server address must be set, was ''", e.getMessage()); } } diff --git a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java index b86d9a561d3..f4df8a4e2a3 100644 --- a/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java +++ b/clustercontroller-apps/src/test/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerTest.java @@ -1,8 +1,4 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/** - * Doesn't really test cluster controller, but runs some lines of code. - * System tests verifies that container can load it.. - */ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.clustercontroller.apps.clustercontroller; import com.yahoo.jdisc.Metric; @@ -13,14 +9,18 @@ import org.junit.Test; import java.util.Map; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +/** + * Doesn't really test cluster controller, but runs some lines of code. + * System tests verifies that container can load it.. + */ public class ClusterControllerTest { private FleetControllerOptions options = new FleetControllerOptions("storage"); - private Metric metric = new Metric() { + private final Metric metric = new Metric() { @Override public void set(String s, Number number, Context context) {} @Override @@ -42,13 +42,13 @@ public class ClusterControllerTest { // Cluster controller object keeps state and should never be remade, so should // inject nothing ClusterController cc = new ClusterController(); - cc.setOptions("storage", options, metric); - cc.setOptions("storage", options, metric); + cc.setOptions(options, metric); + cc.setOptions(options, metric); cc.getFleetControllers(); cc.getAll(); - assertTrue(cc.get("storage") != null); - assertFalse(cc.get("music") != null); + assertNotNull(cc.get("storage")); + assertNull(cc.get("music")); cc.deconstruct(); } @@ -59,7 +59,7 @@ public class ClusterControllerTest { throw new Exception("Foo"); } }; - cc.setOptions("storage", options, metric); + cc.setOptions(options, metric); cc.deconstruct(); } |