Use config server from ConfigserverConfig in DuperModel for controller

3 files changed, 62 insertions, 35 deletions

diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/application/DuperModel.java b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/application/DuperModel.java
index b28b37006ed..f74d436bb2e 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/application/DuperModel.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/monitor/application/DuperModel.java
@@ -40,45 +40,39 @@ public class DuperModel implements DuperModelInfraApi {
             controllerHostApplication)
             .collect(Collectors.toMap(InfraApplication::getApplicationId, Function.identity()));
 
-    private final boolean containsInfra;
-    private final boolean useConfigserverConfig;
+    private final FeatureFlag containsInfra;
+    private final FeatureFlag useConfigserverConfig;
+    private final boolean multitenant;
 
     // Each of the above infrastructure applications may be active, in case their ApplicationInfo is present here
     private final ConcurrentHashMap<ApplicationId, ApplicationInfo> infraInfos =
             new ConcurrentHashMap<>(2 * supportedInfraApplications.size());
 
-    // ApplicationInfo known at construction time
-    private final List<ApplicationInfo> staticInfos = new ArrayList<>();
+    private final ApplicationInfo configServerApplicationFromConfig;
 
     @Inject
     public DuperModel(ConfigserverConfig configServerConfig, FlagSource flagSource) {
         this(
-                // Whether to include activate infrastructure applications in the DuperModel.
-                new FeatureFlag("dupermodel-contains-infra", true, flagSource).value(),
-
-                // Whether to base the ApplicationInfo for the config server on ConfigserverConfig or InfrastructureProvisioner:
-                // - ConfigserverConfig: The list of config servers comes from VESPA_CONFIGSERVERS environment variable.
-                // - InfrastructureProvisioner: The list of config servers comes from the node repository.
-                //
-                // The goal is to use InfrastructureProvisioner like other infrastructure applications.
-                new FeatureFlag("dupermodel-use-configserverconfig", true, flagSource).value(),
+                // Whether to include activate infrastructure applications (except from controller/config apps - see below).
+                new FeatureFlag("dupermodel-contains-infra", true, flagSource),
+
+                // For historical reasons, the ApplicationInfo in the DuperModel for controllers and config servers
+                // is based on the ConfigserverConfig (this flag is true). We want to transition to use the
+                // infrastructure application activated by the InfrastructureProvisioner once that supports health.
+                new FeatureFlag("dupermodel-use-configserverconfig", true, flagSource),
                 configServerConfig.multitenant(),
                 configServerApplication.makeApplicationInfoFromConfig(configServerConfig));
     }
 
     /** For testing */
-    public DuperModel(boolean containsInfra,
-                      boolean useConfigserverConfig,
+    public DuperModel(FeatureFlag containsInfra,
+                      FeatureFlag useConfigserverConfig,
                       boolean multitenant,
                       ApplicationInfo configServerApplicationInfo) {
         this.containsInfra = containsInfra;
         this.useConfigserverConfig = useConfigserverConfig;
-
-        // Single-tenant applications have the config server as part of the application model.
-        // TODO: Add health monitoring for config server when part of application model.
-        if (useConfigserverConfig && multitenant) {
-            staticInfos.add(configServerApplicationInfo);
-        }
+        this.multitenant = multitenant;
+        this.configServerApplicationFromConfig = configServerApplicationInfo;
     }
 
     public ConfigServerApplication getConfigServerApplication() {
@@ -118,10 +112,6 @@ public class DuperModel implements DuperModelInfraApi {
             throw new IllegalArgumentException("There is no infrastructure application with ID '" + applicationId + "'");
         }
 
-        if (useConfigserverConfig && application.equals(configServerApplication)) {
-            return;
-        }
-
         infraInfos.put(application.getApplicationId(), application.makeApplicationInfo(hostnames));
     }
 
@@ -131,10 +121,33 @@ public class DuperModel implements DuperModelInfraApi {
     }
 
     public List<ApplicationInfo> getApplicationInfos(SuperModel superModelSnapshot) {
-        List<ApplicationInfo> allApplicationInfos = new ArrayList<>();
-        allApplicationInfos.addAll(staticInfos);
-        if (containsInfra) allApplicationInfos.addAll(infraInfos.values());
-        allApplicationInfos.addAll(superModelSnapshot.getAllApplicationInfos());
-        return allApplicationInfos;
+        List<ApplicationInfo> infos = new ArrayList<>();
+
+        // Single-tenant applications have the config server as part of the application model.
+        if (multitenant) {
+            if (useConfigserverConfig.value()) {
+                infos.add(configServerApplicationFromConfig);
+            } else {
+                ApplicationInfo info = infraInfos.get(controllerApplication.getApplicationId());
+                if (info == null) {
+                    info = infraInfos.get(configServerApplication.getApplicationId());
+                }
+
+                if (info != null) {
+                    infos.add(info);
+                }
+            }
+        }
+
+        if (containsInfra.value()) {
+            // All infra apps, excluding controller and config server which handled above.
+            infraInfos.values().stream()
+                    .filter(info -> !info.getApplicationId().equals(controllerApplication.getApplicationId()) &&
+                            !info.getApplicationId().equals(configServerApplication.getApplicationId()))
+                    .forEach(infos::add);
+        }
+
+        infos.addAll(superModelSnapshot.getAllApplicationInfos());
+        return infos;
     }
 }
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/application/DuperModelTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/application/DuperModelTest.java
index 40204c0de8e..9801f80acc9 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/application/DuperModelTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/application/DuperModelTest.java
@@ -7,6 +7,7 @@ import com.yahoo.config.model.api.SuperModel;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
+import com.yahoo.vespa.flags.FeatureFlag;
 import com.yahoo.vespa.service.monitor.ServiceStatusProvider;
 import com.yahoo.vespa.service.monitor.internal.ConfigserverUtil;
 import org.junit.Before;
@@ -35,6 +36,8 @@ public class DuperModelTest {
     private final ConfigserverConfig configserverConfig = ConfigserverUtil.createExampleConfigserverConfig();
     private final ApplicationInfo configServerApplicationInfo = new ConfigServerApplication().makeApplicationInfoFromConfig(configserverConfig);
     private final SuperModel superModel = mock(SuperModel.class);
+    private final FeatureFlag containsInfra = mock(FeatureFlag.class);
+    private final FeatureFlag useConfigserverConfig = mock(FeatureFlag.class);
 
     @Before
     public void setUp() {
@@ -43,7 +46,9 @@ public class DuperModelTest {
 
     @Test
     public void toApplicationInstance() {
-        DuperModel duperModel = new DuperModel(false, true, true, configServerApplicationInfo);
+        when(containsInfra.value()).thenReturn(false);
+        when(useConfigserverConfig.value()).thenReturn(true);
+        DuperModel duperModel = new DuperModel(containsInfra, useConfigserverConfig, true, configServerApplicationInfo);
         ApplicationInfo superModelApplicationInfo = mock(ApplicationInfo.class);
         when(superModel.getAllApplicationInfos()).thenReturn(Collections.singletonList(superModelApplicationInfo));
         List<ApplicationInfo> applicationInfos = duperModel.getApplicationInfos(superModel);
@@ -54,7 +59,9 @@ public class DuperModelTest {
 
     @Test
     public void toApplicationInstanceInSingleTenantMode() {
-        DuperModel duperModel = new DuperModel(false, true, false, configServerApplicationInfo);
+        when(containsInfra.value()).thenReturn(false);
+        when(useConfigserverConfig.value()).thenReturn(true);
+        DuperModel duperModel = new DuperModel(containsInfra, useConfigserverConfig, false, configServerApplicationInfo);
         ApplicationInfo superModelApplicationInfo = mock(ApplicationInfo.class);
         when(superModel.getAllApplicationInfos()).thenReturn(Collections.singletonList(superModelApplicationInfo));
         List<ApplicationInfo> applicationInfos = duperModel.getApplicationInfos(superModel);
@@ -64,7 +71,9 @@ public class DuperModelTest {
 
     @Test
     public void testInfraApplications() {
-        DuperModel duperModel = new DuperModel(true, true, true, configServerApplicationInfo);
+        when(containsInfra.value()).thenReturn(true);
+        when(useConfigserverConfig.value()).thenReturn(true);
+        DuperModel duperModel = new DuperModel(containsInfra, useConfigserverConfig, true, configServerApplicationInfo);
         ApplicationInfo infraApplicationInfo = mock(ApplicationInfo.class);
         when(superModel.getAllApplicationInfos()).thenReturn(Collections.emptyList());
 
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ModelGeneratorTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ModelGeneratorTest.java
index 275fa8568c2..6574f592b60 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ModelGeneratorTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/monitor/internal/ModelGeneratorTest.java
@@ -12,9 +12,10 @@ import com.yahoo.vespa.applicationmodel.ApplicationInstanceReference;
 import com.yahoo.vespa.applicationmodel.ServiceCluster;
 import com.yahoo.vespa.applicationmodel.ServiceInstance;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
-import com.yahoo.vespa.service.monitor.application.DuperModel;
+import com.yahoo.vespa.flags.FeatureFlag;
 import com.yahoo.vespa.service.monitor.ServiceModel;
 import com.yahoo.vespa.service.monitor.application.ConfigServerApplication;
+import com.yahoo.vespa.service.monitor.application.DuperModel;
 import com.yahoo.vespa.service.monitor.internal.slobrok.SlobrokMonitorManagerImpl;
 import org.junit.Test;
 
@@ -39,7 +40,11 @@ public class ModelGeneratorTest {
 
         ConfigserverConfig config = ConfigserverUtil.createExampleConfigserverConfig();
         ApplicationInfo configServerInfo = new ConfigServerApplication().makeApplicationInfoFromConfig(config);
-        DuperModel duperModel = new DuperModel(false, true, true, configServerInfo);
+        FeatureFlag containsInfra = mock(FeatureFlag.class);
+        when(containsInfra.value()).thenReturn(false);
+        FeatureFlag useConfigserverConfig = mock(FeatureFlag.class);
+        when(useConfigserverConfig.value()).thenReturn(true);
+        DuperModel duperModel = new DuperModel(containsInfra, useConfigserverConfig, true, configServerInfo);
         ModelGenerator modelGenerator = new ModelGenerator();
 
         Zone zone = new Zone(Environment.from(ENVIRONMENT), RegionName.from(REGION));