simplify states

2 files changed, 50 insertions, 48 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java b/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java
index a9cd7466b73..78c89e3a7d4 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java
@@ -11,12 +11,8 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
-import java.time.Duration;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
-import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -39,9 +35,13 @@ public class DataplaneProxyService extends AbstractComponent {
     private final ScheduledThreadPoolExecutor executorService;
     private final Path root;
 
-    enum NginxState {INITIALIZING, STARTING, RUNNING, RELOAD_REQUIRED, CONFIG_CHANGE_IN_PROGRESS, STOPPED};
+    enum NginxState {INITIALIZING, RUNNING, RELOAD_REQUIRED};
     private NginxState state;
 
+    private DataplaneProxyConfig cfg;
+    private Path proxyCredentialsCert;
+    private Path proxyCredentialsKey;
+
 
     @Inject
     public DataplaneProxyService() {
@@ -63,36 +63,10 @@ public class DataplaneProxyService extends AbstractComponent {
     }
 
     public void reconfigure(DataplaneProxyConfig config, DataplaneProxyCredentials credentialsProvider) {
-        NginxState prevState = state;
-        changeState(NginxState.CONFIG_CHANGE_IN_PROGRESS);
-        try {
-
-            String serverCert = config.serverCertificate();
-            String serverKey = config.serverKey();
-
-            boolean configChanged = false;
-            configChanged |= writeFile(serverCertificateFile, serverCert);
-            configChanged |= writeFile(serverKeyFile, serverKey);
-            configChanged |= writeFile(nginxConf,
-                      nginxConfig(
-                              configTemplate,
-                              credentialsProvider.certificateFile(),
-                              credentialsProvider.keyFile(),
-                              serverCertificateFile,
-                              serverKeyFile,
-                              config.port(),
-                              root
-                      ));
-            if (prevState == NginxState.INITIALIZING) {
-                changeState(NginxState.STARTING);
-            } else if (configChanged && prevState == NginxState.RUNNING) {
-                changeState(NginxState.RELOAD_REQUIRED);
-            } else {
-                changeState(prevState);
-            }
-        } catch (IOException e) {
-            changeState(prevState);
-            throw new RuntimeException("Error reconfiguring data plane proxy", e);
+        synchronized (this) {
+            this.cfg = config;
+            this.proxyCredentialsCert = credentialsProvider.certificateFile();
+            this.proxyCredentialsKey = credentialsProvider.keyFile();
         }
     }
 
@@ -101,9 +75,46 @@ public class DataplaneProxyService extends AbstractComponent {
     }
 
     void startOrReloadNginx() {
-        if (state == NginxState.CONFIG_CHANGE_IN_PROGRESS) {
-            return;
-        } else if (state == NginxState.STARTING) {
+        DataplaneProxyConfig config;
+        Path proxyCredentialsCert;
+        Path proxyCredentialsKey;
+        synchronized (this) {
+            config = cfg;
+            proxyCredentialsCert = this.proxyCredentialsCert;
+            proxyCredentialsKey = this.proxyCredentialsKey;
+            this.cfg = null;
+            this.proxyCredentialsCert = null;
+            this.proxyCredentialsKey = null;
+        }
+
+        boolean configChanged = false;
+        if (config != null) {
+            try {
+
+                String serverCert = config.serverCertificate();
+                String serverKey = config.serverKey();
+
+                configChanged |= writeFile(serverCertificateFile, serverCert);
+                configChanged |= writeFile(serverKeyFile, serverKey);
+                configChanged |= writeFile(nginxConf,
+                                           nginxConfig(
+                                                   configTemplate,
+                                                   proxyCredentialsCert,
+                                                   proxyCredentialsKey,
+                                                   serverCertificateFile,
+                                                   serverKeyFile,
+                                                   config.port(),
+                                                   root
+                                           ));
+                if (configChanged && state == NginxState.RUNNING) {
+                    changeState(NginxState.RELOAD_REQUIRED);
+                }
+            } catch (IOException e) {
+                throw new RuntimeException("Error reconfiguring data plane proxy", e);
+            }
+        }
+
+        if (state == NginxState.INITIALIZING) {
             try {
                 proxyCommands.start(nginxConf);
                 changeState(NginxState.RUNNING);
@@ -131,7 +142,6 @@ public class DataplaneProxyService extends AbstractComponent {
         }
         try {
             proxyCommands.stop();
-            changeState(NginxState.STOPPED);
         } catch (Exception e) {
             logger.log(Level.WARNING, "Failed shutting down nginx");
         }
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/DataplaneProxyServiceTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/DataplaneProxyServiceTest.java
index 95c05e9a1c4..b71dfbba88a 100644
--- a/container-disc/src/test/java/com/yahoo/container/jdisc/DataplaneProxyServiceTest.java
+++ b/container-disc/src/test/java/com/yahoo/container/jdisc/DataplaneProxyServiceTest.java
@@ -27,12 +27,10 @@ public class DataplaneProxyServiceTest {
 
     @Test
     public void starts_and_reloads_if_no_errors() throws IOException {
-        DataplaneProxyService.ProxyCommands proxyCommands = Mockito.mock(DataplaneProxyService.ProxyCommands.class);
         DataplaneProxyService service = dataplaneProxyService(proxyCommands);
 
         assertEquals(DataplaneProxyService.NginxState.INITIALIZING, service.state());
         service.reconfigure(proxyConfig(), credentials(fileSystem));
-        assertEquals(DataplaneProxyService.NginxState.STARTING, service.state());
 
         // Simulate executor next tick
         service.startOrReloadNginx();
@@ -40,32 +38,27 @@ public class DataplaneProxyServiceTest {
 
         // Trigger reload by recreating the proxy config (generates new server cert)
         service.reconfigure(proxyConfig(), credentials(fileSystem));
-        assertEquals(DataplaneProxyService.NginxState.RELOAD_REQUIRED, service.state());
-
         service.startOrReloadNginx();
         assertEquals(DataplaneProxyService.NginxState.RUNNING, service.state());
     }
 
     @Test
     public void retries_startup_errors() throws IOException {
-        DataplaneProxyService.ProxyCommands proxyCommands = Mockito.mock(DataplaneProxyService.ProxyCommands.class);
         Mockito.doThrow(new RuntimeException("IO error")).doNothing().when(proxyCommands).start(any());
         DataplaneProxyService service = dataplaneProxyService(proxyCommands);
 
         assertEquals(DataplaneProxyService.NginxState.INITIALIZING, service.state());
         service.reconfigure(proxyConfig(), credentials(fileSystem));
-        assertEquals(DataplaneProxyService.NginxState.STARTING, service.state());
 
         // Start nginx,
         service.startOrReloadNginx();
-        assertEquals(DataplaneProxyService.NginxState.STARTING, service.state());
+        assertEquals(DataplaneProxyService.NginxState.INITIALIZING, service.state());
         service.startOrReloadNginx();
         assertEquals(DataplaneProxyService.NginxState.RUNNING, service.state());
     }
 
     @Test
     public void retries_reload_errors() throws IOException {
-        DataplaneProxyService.ProxyCommands proxyCommands = Mockito.mock(DataplaneProxyService.ProxyCommands.class);
         Mockito.doThrow(new RuntimeException("IO error")).doNothing().when(proxyCommands).reload();
         DataplaneProxyService service = dataplaneProxyService(proxyCommands);
 
@@ -76,7 +69,6 @@ public class DataplaneProxyServiceTest {
 
         // Trigger reload, verifies 2nd attempt succeeds
         service.reconfigure(proxyConfig(), credentials(fileSystem));
-        assertEquals(DataplaneProxyService.NginxState.RELOAD_REQUIRED, service.state());
         service.startOrReloadNginx();
         assertEquals(DataplaneProxyService.NginxState.RELOAD_REQUIRED, service.state());
         service.startOrReloadNginx();