Merge pull request #16527 from vespa-engine/jonmv/create-only-one-cluster-controller

Avoid recreation of ClusterController when config changes

3 files changed, 19 insertions, 11 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 8ec93b99aff..df0e7b7d0b5 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
@@ -31,16 +31,13 @@ public class ClusterController extends AbstractComponent
     private final Map<String, StatusHandler.ContainerStatusPageServer> status = new TreeMap<>();
 
     /**
-     * Dependency injection constructor for controller. {@link VespaZooKeeperServer} argument given
-     * to ensure that zookeeper has started before we start polling it.
+     * Dependency injection constructor for controller. A {@link VespaZooKeeperServer} argument is required
+     * for all its users, to ensure that zookeeper has started before we start polling it, but
+     * should not be injected here, as that causes recreation of the cluster controller, and old and new
+     * will run master election, etc., concurrently, which breaks everything.
      */
-    @SuppressWarnings("unused")
     @Inject
-    public ClusterController(VespaZooKeeperServer ignored) {
-        this();
-    }
-
-    ClusterController() {
+    public ClusterController() {
         metricWrapper = new JDiscMetricWrapper(null);
     }
 
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 4cb6c5d222a..c6a2ecc0c1c 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
@@ -1,6 +1,7 @@
 // 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.google.inject.Inject;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vdslib.distribution.Distribution;
 import com.yahoo.vdslib.state.NodeType;
@@ -9,6 +10,7 @@ import com.yahoo.vespa.config.content.FleetcontrollerConfig;
 import com.yahoo.cloud.config.SlobroksConfig;
 import com.yahoo.vespa.config.content.StorDistributionConfig;
 import com.yahoo.cloud.config.ZookeepersConfig;
+import com.yahoo.vespa.zookeeper.VespaZooKeeperServer;
 
 import java.time.Duration;
 import java.util.Map;
@@ -21,12 +23,19 @@ public class ClusterControllerClusterConfigurer {
 
     private final FleetControllerOptions options;
 
+    /**
+     * The {@link VespaZooKeeperServer} argument is required by the injected {@link ClusterController},
+     * to ensure that zookeeper has started before it starts polling it. It must be done here to avoid
+     * duplicates being created by the dependency injection framework.
+     */
+    @Inject
     public ClusterControllerClusterConfigurer(ClusterController controller,
                                               StorDistributionConfig distributionConfig,
                                               FleetcontrollerConfig fleetcontrollerConfig,
                                               SlobroksConfig slobroksConfig,
                                               ZookeepersConfig zookeepersConfig,
-                                              Metric metricImpl) throws Exception {
+                                              Metric metricImpl,
+                                              VespaZooKeeperServer started) throws Exception {
         this.options = configure(distributionConfig, fleetcontrollerConfig, slobroksConfig, zookeepersConfig);
         if (controller != null) {
             controller.setOptions(options, metricImpl);
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 76eff0066b1..16b978b3ce9 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
@@ -57,7 +57,8 @@ public class ClusterControllerClusterConfigurerTest {
                 new FleetcontrollerConfig(fleetcontrollerConfig),
                 new SlobroksConfig(slobroksConfig),
                 new ZookeepersConfig(zookeepersConfig),
-                metric
+                metric,
+                null
         );
         assertTrue(configurer.getOptions() != null);
         assertEquals(0.123, configurer.getOptions().minNodeRatioPerGroup, 0.01);
@@ -74,7 +75,8 @@ public class ClusterControllerClusterConfigurerTest {
                     new FleetcontrollerConfig(fleetcontrollerConfig),
                     new SlobroksConfig(slobroksConfig),
                     new ZookeepersConfig(zookeepersConfig),
-                    metric
+                    metric,
+                    null
             );
             fail("Should not get here");
         } catch (Exception e) {