diff options
author | Morten Tokle <mortent@yahooinc.com> | 2023-06-19 13:30:59 +0200 |
---|---|---|
committer | Morten Tokle <mortent@yahooinc.com> | 2023-06-20 09:08:06 +0200 |
commit | ae5b5deda1253e7ea75d06d09be063eb8b4419cd (patch) | |
tree | c5029af6d133441c03fa43129fdba04ff9ab30cb /container-disc | |
parent | d7b3239e3acbd42ab7733fe868407786a93cea3f (diff) |
simplify states
Diffstat (limited to 'container-disc')
-rw-r--r-- | container-disc/src/main/java/com/yahoo/container/jdisc/DataplaneProxyService.java | 88 | ||||
-rw-r--r-- | container-disc/src/test/java/com/yahoo/container/jdisc/DataplaneProxyServiceTest.java | 10 |
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(); |