Remove MONITOR_TENANT_HOST_HEALTH flag

5 files changed, 24 insertions, 90 deletions

diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java
index ad9a50f8127..7601cfd2e95 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/HealthMonitorManager.java
@@ -9,8 +9,6 @@ import com.yahoo.vespa.applicationmodel.ConfigId;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
 import com.yahoo.vespa.applicationmodel.ServiceStatusInfo;
 import com.yahoo.vespa.applicationmodel.ServiceType;
-import com.yahoo.vespa.flags.FlagSource;
-import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.service.duper.DuperModelManager;
 import com.yahoo.vespa.service.duper.ZoneApplication;
 import com.yahoo.vespa.service.executor.RunletExecutorImpl;
@@ -54,34 +52,26 @@ public class HealthMonitorManager implements MonitorManager, HealthMonitorApi {
 
     private final ConcurrentHashMap<ApplicationId, ApplicationHealthMonitor> healthMonitors = new ConcurrentHashMap<>();
     private final DuperModelManager duperModel;
-    private final boolean monitorTenantHostHealth;
     private final ApplicationHealthMonitorFactory applicationHealthMonitorFactory;
 
     @Inject
-    public HealthMonitorManager(DuperModelManager duperModel, FlagSource flagSource) {
-        this(duperModel, Flags.MONITOR_TENANT_HOST_HEALTH.bindTo(flagSource).value());
-    }
-
-    private HealthMonitorManager(DuperModelManager duperModel, boolean monitorTenantHostHealth) {
-        this(duperModel, monitorTenantHostHealth,
+    public HealthMonitorManager(DuperModelManager duperModel) {
+        this(duperModel,
                 new StateV1HealthModel(
                         TARGET_HEALTH_STALENESS,
                         HEALTH_REQUEST_TIMEOUT,
                         KEEP_ALIVE,
-                        new RunletExecutorImpl(THREAD_POOL_SIZE),
-                        monitorTenantHostHealth));
+                        new RunletExecutorImpl(THREAD_POOL_SIZE)));
     }
 
-    private HealthMonitorManager(DuperModelManager duperModel, boolean monitorTenantHostHealth, StateV1HealthModel healthModel) {
-        this(duperModel, monitorTenantHostHealth, id -> new ApplicationHealthMonitor(id, healthModel));
+    private HealthMonitorManager(DuperModelManager duperModel, StateV1HealthModel healthModel) {
+        this(duperModel, id -> new ApplicationHealthMonitor(id, healthModel));
     }
 
     /** Default access due to testing. */
     HealthMonitorManager(DuperModelManager duperModel,
-                         boolean monitorTenantHostHealth,
                          ApplicationHealthMonitorFactory applicationHealthMonitorFactory) {
         this.duperModel = duperModel;
-        this.monitorTenantHostHealth = monitorTenantHostHealth;
         this.applicationHealthMonitorFactory = applicationHealthMonitorFactory;
     }
 
@@ -109,17 +99,11 @@ public class HealthMonitorManager implements MonitorManager, HealthMonitorApi {
                                        ConfigId configId) {
         ApplicationHealthMonitor monitor = healthMonitors.get(applicationId);
 
-        if (!monitorTenantHostHealth && ZoneApplication.isNodeAdminService(applicationId, clusterId, serviceType)) {
-            // Legacy: The zone app is not health monitored (monitor == null), but the node-admin cluster's services
-            // are hard-coded to be UP
-            return new ServiceStatusInfo(ServiceStatus.UP);
-        }
-
         if (monitor == null) {
             return new ServiceStatusInfo(ServiceStatus.NOT_CHECKED);
         }
 
-        if (monitorTenantHostHealth && applicationId.equals(ZoneApplication.getApplicationId())) {
+        if (applicationId.equals(ZoneApplication.getApplicationId())) {
             // New: The zone app is health monitored (monitor != null), possibly even the routing cluster
             // which is a normal jdisc container (unnecessary but harmless), but the node-admin cluster
             // are tenant Docker hosts running host admin that are monitored via /state/v1/health.
@@ -134,15 +118,7 @@ public class HealthMonitorManager implements MonitorManager, HealthMonitorApi {
     }
 
     private boolean wouldMonitor(ApplicationId id) {
-        if (duperModel.isSupportedInfraApplication(id)) {
-            return true;
-        }
-
-        if (monitorTenantHostHealth && id.equals(ZoneApplication.getApplicationId())) {
-            return true;
-        }
-
-        return false;
+        return duperModel.isSupportedInfraApplication(id) || id.equals(ZoneApplication.getApplicationId());
     }
 
     @Override
@@ -154,7 +130,7 @@ public class HealthMonitorManager implements MonitorManager, HealthMonitorApi {
     public Map<ServiceId, ServiceStatusInfo> getServices(ApplicationId applicationId) {
         ApplicationHealthMonitor applicationHealthMonitor = healthMonitors.get(applicationId);
         if (applicationHealthMonitor == null) {
-            return Collections.emptyMap();
+            return Map.of();
         }
 
         return applicationHealthMonitor.getAllServiceStatuses();
diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java
index 9f51e0692a0..8e3780744f6 100644
--- a/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java
+++ b/service-monitor/src/main/java/com/yahoo/vespa/service/health/StateV1HealthModel.java
@@ -33,18 +33,15 @@ public class StateV1HealthModel implements AutoCloseable {
     private final Duration requestTimeout;
     private final Duration connectionKeepAlive;
     private final RunletExecutor executor;
-    private final boolean monitorTenantHostHealth;
 
     StateV1HealthModel(Duration targetHealthStaleness,
                        Duration requestTimeout,
                        Duration connectionKeepAlive,
-                       RunletExecutor executor,
-                       boolean monitorTenantHostHealth) {
+                       RunletExecutor executor) {
         this.targetHealthStaleness = targetHealthStaleness;
         this.requestTimeout = requestTimeout;
         this.connectionKeepAlive = connectionKeepAlive;
         this.executor = executor;
-        this.monitorTenantHostHealth = monitorTenantHostHealth;
     }
 
     Map<ServiceId, HealthEndpoint> extractHealthEndpoints(ApplicationInfo application) {
@@ -57,7 +54,7 @@ public class StateV1HealthModel implements AutoCloseable {
             for (ServiceInfo serviceInfo : hostInfo.getServices()) {
 
                 boolean isNodeAdmin = false;
-                if (monitorTenantHostHealth && isZoneApplication) {
+                if (isZoneApplication) {
                     if (ZoneApplication.isNodeAdminServiceInfo(application.getApplicationId(), serviceInfo)) {
                         isNodeAdmin = true;
                     } else {
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java
index 0c0a644165d..89bcda05074 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/HealthMonitorManagerTest.java
@@ -6,7 +6,6 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.applicationmodel.ConfigId;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
 import com.yahoo.vespa.applicationmodel.ServiceStatusInfo;
-import com.yahoo.vespa.service.duper.ConfigServerApplication;
 import com.yahoo.vespa.service.duper.ControllerHostApplication;
 import com.yahoo.vespa.service.duper.DuperModelManager;
 import com.yahoo.vespa.service.duper.InfraApplication;
@@ -14,6 +13,7 @@ import com.yahoo.vespa.service.duper.ProxyHostApplication;
 import com.yahoo.vespa.service.duper.TestZoneApplication;
 import com.yahoo.vespa.service.duper.ZoneApplication;
 import com.yahoo.vespa.service.monitor.ConfigserverUtil;
+import org.junit.Before;
 import org.junit.Test;
 
 import java.util.List;
@@ -29,21 +29,13 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class HealthMonitorManagerTest {
-    private final ConfigServerApplication configServerApplication = new ConfigServerApplication();
     private final DuperModelManager duperModel = mock(DuperModelManager.class);
     private final ApplicationHealthMonitor monitor = mock(ApplicationHealthMonitor.class);
     private final ApplicationHealthMonitorFactory monitorFactory = mock(ApplicationHealthMonitorFactory.class);
-    private HealthMonitorManager manager;
-
-    public void setUp(boolean monitorTenantHostHealth) {
-        manager = new HealthMonitorManager(duperModel, monitorTenantHostHealth, monitorFactory);
-        when(duperModel.getConfigServerApplication()).thenReturn(configServerApplication);
-        when(monitorFactory.create(any())).thenReturn(monitor);
-    }
+    private final HealthMonitorManager manager = new HealthMonitorManager(duperModel, monitorFactory);
 
     @Test
     public void addAndRemove() {
-        setUp(false);
         ApplicationInfo applicationInfo = ConfigserverUtil.makeExampleConfigServer();
         when(duperModel.isSupportedInfraApplication(applicationInfo.getApplicationId())).thenReturn(true);
 
@@ -57,29 +49,7 @@ public class HealthMonitorManagerTest {
     }
 
     @Test
-    public void withHostAdmin() {
-        setUp(false);
-        ServiceStatus status = manager.getStatus(
-                ZoneApplication.getApplicationId(),
-                ZoneApplication.getNodeAdminClusterId(),
-                ZoneApplication.getNodeAdminServiceType(),
-                new ConfigId("config-id-1")).serviceStatus();
-        assertEquals(ServiceStatus.UP, status);
-    }
-
-    @Test
-    public void verifyZoneApplicationIsNotMonitoredByDefault() {
-        verifyZoneApplicationIsMonitored(false, false);
-    }
-
-    @Test
     public void verifyZoneApplicationIsMonitored() {
-        verifyZoneApplicationIsMonitored(true, true);
-    }
-
-    private void verifyZoneApplicationIsMonitored(boolean monitorTenantHostHealth, boolean isMonitored) {
-        setUp(monitorTenantHostHealth);
-
         ApplicationInfo zoneApplicationInfo = new TestZoneApplication.Builder()
                 .addNodeAdminCluster("h1", "h2")
                 .addRoutingCluster("r1")
@@ -89,18 +59,13 @@ public class HealthMonitorManagerTest {
         verify(monitorFactory, times(0)).create(zoneApplicationInfo.getApplicationId());
         verify(monitor, times(0)).monitor(any());
         manager.applicationActivated(zoneApplicationInfo);
-        verify(monitorFactory, times(isMonitored ? 1 : 0)).create(zoneApplicationInfo.getApplicationId());
-        verify(monitor, times(isMonitored ? 1 : 0)).monitor(any());
+        verify(monitorFactory).create(zoneApplicationInfo.getApplicationId());
+        verify(monitor).monitor(any());
 
         when(monitor.getStatus(any(), any(), any(), any())).thenReturn(new ServiceStatusInfo(ServiceStatus.DOWN));
         verifyNodeAdminGetStatus(0);
-        if (isMonitored) {
-            assertEquals(ServiceStatus.DOWN, getNodeAdminStatus());
-            verifyNodeAdminGetStatus(1);
-        } else {
-            assertEquals(ServiceStatus.UP, getNodeAdminStatus());
-            verifyNodeAdminGetStatus(0);
-        }
+        assertEquals(ServiceStatus.DOWN, getNodeAdminStatus());
+        verifyNodeAdminGetStatus(1);
 
         verifyRoutingGetStatus(0);
         assertEquals(ServiceStatus.NOT_CHECKED, getRoutingStatus());
@@ -141,7 +106,6 @@ public class HealthMonitorManagerTest {
 
     @Test
     public void infrastructureApplication() {
-        setUp(false);
         ProxyHostApplication proxyHostApplication = new ProxyHostApplication();
         when(duperModel.isSupportedInfraApplication(proxyHostApplication.getApplicationId())).thenReturn(true);
         List<HostName> hostnames = Stream.of("proxyhost1", "proxyhost2").map(HostName::from).collect(Collectors.toList());
@@ -160,10 +124,14 @@ public class HealthMonitorManagerTest {
 
     @Test
     public void threadPoolSize() {
-        setUp(false);
         assertEquals(9, HealthMonitorManager.THREAD_POOL_SIZE);
     }
 
+    @Before
+    public void setup() {
+        when(monitorFactory.create(any())).thenReturn(monitor);
+    }
+
     private void assertStatus(ServiceStatus expected, int verifyTimes, InfraApplication infraApplication, String hostname) {
         ServiceStatus actual = manager.getStatus(
                 infraApplication.getApplicationId(),
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java
index 9803fb75423..3fce1cca899 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/health/StateV1HealthModelTest.java
@@ -43,14 +43,11 @@ public class StateV1HealthModelTest {
     private final List<HostName> hostnames = Stream.of("host1", "host2").map(HostName::from).collect(Collectors.toList());
     private final ApplicationInfo proxyHostApplicationInfo = proxyHostApplication.makeApplicationInfo(hostnames);
 
-    private StateV1HealthModel model;
-    private Map<ServiceId, HealthEndpoint> endpoints;
+    private final StateV1HealthModel model = new StateV1HealthModel(healthStaleness, requestTimeout, keepAlive, executor);
 
     @Test
     public void test() {
-        model = new StateV1HealthModel(healthStaleness, requestTimeout, keepAlive, executor, false);
-        endpoints = model.extractHealthEndpoints(proxyHostApplicationInfo);
-
+        Map<ServiceId, HealthEndpoint> endpoints = model.extractHealthEndpoints(proxyHostApplicationInfo);
         assertEquals(2, endpoints.size());
 
         ApplicationId applicationId = ApplicationId.from("hosted-vespa", "proxy-host", "default");
@@ -75,14 +72,13 @@ public class StateV1HealthModelTest {
 
     @Test
     public void testMonitoringTenantHostHealth() {
-        model = new StateV1HealthModel(healthStaleness, requestTimeout, keepAlive, executor, true);
         ApplicationInfo zoneApplicationInfo = new TestZoneApplication.Builder()
                 .addNodeAdminCluster("h1")
                 .addRoutingCluster("r1")
                 .build()
                 .makeApplicationInfo();
 
-        endpoints = model.extractHealthEndpoints(zoneApplicationInfo);
+        Map<ServiceId, HealthEndpoint> endpoints = model.extractHealthEndpoints(zoneApplicationInfo);
         assertEquals(1, endpoints.size());
         HealthEndpoint endpoint = endpoints.values().iterator().next();
         assertEquals("http://h1:8080/state/v1/health", endpoint.description());
diff --git a/service-monitor/src/test/java/com/yahoo/vespa/service/slobrok/SlobrokMonitorManagerImplTest.java b/service-monitor/src/test/java/com/yahoo/vespa/service/slobrok/SlobrokMonitorManagerImplTest.java
index 0b8a13a8869..e8953b57f79 100644
--- a/service-monitor/src/test/java/com/yahoo/vespa/service/slobrok/SlobrokMonitorManagerImplTest.java
+++ b/service-monitor/src/test/java/com/yahoo/vespa/service/slobrok/SlobrokMonitorManagerImplTest.java
@@ -7,7 +7,6 @@ import com.yahoo.vespa.applicationmodel.ClusterId;
 import com.yahoo.vespa.applicationmodel.ConfigId;
 import com.yahoo.vespa.applicationmodel.ServiceStatus;
 import com.yahoo.vespa.applicationmodel.ServiceType;
-import com.yahoo.vespa.service.duper.ConfigServerApplication;
 import com.yahoo.vespa.service.duper.DuperModelManager;
 import org.junit.Before;
 import org.junit.Test;
@@ -27,7 +26,6 @@ public class SlobrokMonitorManagerImplTest {
     @SuppressWarnings("unchecked")
     private final Supplier<SlobrokMonitor> slobrokMonitorFactory = mock(Supplier.class);
 
-    private final ConfigServerApplication configServerApplication = new ConfigServerApplication();
     private final DuperModelManager duperModelManager = mock(DuperModelManager.class);
     private final SlobrokMonitorManagerImpl slobrokMonitorManager =
             new SlobrokMonitorManagerImpl(slobrokMonitorFactory, duperModelManager);
@@ -38,7 +36,6 @@ public class SlobrokMonitorManagerImplTest {
 
     @Before
     public void setup() {
-        when(duperModelManager.getConfigServerApplication()).thenReturn(configServerApplication);
         when(slobrokMonitorFactory.get()).thenReturn(slobrokMonitor);
         when(application.getApplicationId()).thenReturn(applicationId);
     }