Merge pull request #528 from iExecBlockchainComputing/hotfix/8.1.1

Hotfix/8.1.1

Author: jbern0rd

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to this project will be documented in this file.
 
+## [[8.1.1]](https://github.com/iExecBlockchainComputing/iexec-worker/releases/tag/v8.1.1) 2023-06-15
+
+### Bug Fixes
+- Do not throw exceptions from `LasService#start` method. (#528)
+- Add `synchronized` keyword on `LasService#start` method to avoid race conditions. (#528)
+
 ## [[8.1.0]](https://github.com/iExecBlockchainComputing/iexec-worker/releases/tag/v8.1.0) 2023-06-12
 
 ### New Features

gradle.properties

@@ -1,4 +1,4 @@
-version=8.1.0
+version=8.1.1
 iexecCommonVersion=8.2.0
 iexecCommonsContainersVersion=1.0.2
 iexecCommonsPocoVersion=3.0.2

src/main/java/com/iexec/worker/tee/scone/LasService.java

@@ -38,7 +38,7 @@ public LasService(String containerName,
         this.dockerService = dockerService;
     }
 
-    boolean start() {
+    synchronized boolean start() {
         if (isStarted) {
             return true;
         }
@@ -53,9 +53,9 @@ boolean start() {
                 .maxExecutionTime(0)
                 .build();
         if (!imageUri.contains(sconeConfig.getRegistryName())) {
-            // FIXME: throw an IllegalArgumentException
-            throw new RuntimeException(String.format("LAS image (%s) is not " +
-                    "from a known registry (%s)", imageUri, sconeConfig.getRegistryName()));
+            log.error("LAS image ({}) is not from a known registry ({})",
+                    imageUri, sconeConfig.getRegistryName());
+            return false;
         }
         DockerClientInstance client;
         try {
@@ -64,9 +64,8 @@ boolean start() {
                     sconeConfig.getRegistryUsername(),
                     sconeConfig.getRegistryPassword());
         } catch (Exception e) {
-            log.error("", e);
-            // FIXME: throw another, more specific, Exception
-            throw new RuntimeException("Failed to get Docker authenticated client to run LAS");
+            log.error("Failed to get Docker authenticated client to run LAS", e);
+            return false;
         }
         if (client == null) {
             log.error("Docker client with credentials is required to enable TEE support");

src/main/java/com/iexec/worker/tee/scone/LasServicesManager.java

@@ -36,7 +36,8 @@ public class LasServicesManager implements Purgeable {
      */
     private final Map<String, LasService> lasImageUriToLasService = new HashMap<>();
 
-    public LasServicesManager(SconeConfiguration sconeConfiguration,
+    public LasServicesManager(
+            SconeConfiguration sconeConfiguration,
             TeeServicesPropertiesService teeServicesPropertiesService,
             WorkerConfigurationService workerConfigService,
             SgxService sgxService,

src/test/java/com/iexec/worker/tee/scone/LasServiceTests.java

@@ -16,7 +16,8 @@
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;
 
@@ -71,7 +72,7 @@ void shouldStartLasService() {
                 .thenReturn(DockerRunResponse.builder().finalStatus(DockerRunFinalStatus.SUCCESS).build());
         when(sgxService.getSgxDriverMode()).thenReturn(SgxDriverMode.LEGACY);
 
-        Assertions.assertThat(lasService.start()).isTrue();
+        assertTrue(lasService.start());
         verify(dockerService).run(dockerRunRequestArgumentCaptor.capture());
         DockerRunRequest dockerRunRequest = dockerRunRequestArgumentCaptor.getValue();
         Assertions.assertThat(dockerRunRequest).isEqualTo(
@@ -85,6 +86,20 @@ void shouldStartLasService() {
         assertTrue(lasService.isStarted());
     }
 
+    @Test
+    void shouldStartLasServiceOnlyOnce() throws Exception {
+        when(dockerClientInstanceMock.pullImage(IMAGE_URI)).thenReturn(true);
+        when(dockerService.run(any()))
+                .thenReturn(DockerRunResponse.builder().finalStatus(DockerRunFinalStatus.SUCCESS).build());
+        when(sgxService.getSgxDriverMode()).thenReturn(SgxDriverMode.LEGACY);
+
+        assertTrue(lasService.start());
+        assertTrue(lasService.start());
+        assertTrue(lasService.isStarted());
+        verify(dockerService).getClient(REGISTRY_NAME, REGISTRY_USERNAME, REGISTRY_PASSWORD);
+        verify(dockerService).run(any());
+    }
+
     @Test
     void shouldNotStartLasServiceSinceUnknownRegistry() {
         LasService lasService = new LasService(
@@ -95,12 +110,8 @@ void shouldNotStartLasServiceSinceUnknownRegistry() {
                 sgxService,
                 dockerService
         );
-        Exception exception = assertThrows(
-                RuntimeException.class,
-                lasService::start
-        );
 
-        Assertions.assertThat(exception.getMessage()).contains("not from a known registry");
+        assertFalse(lasService.start());
         assertFalse(lasService.isStarted());
     }
 
@@ -109,15 +120,24 @@ void shouldNotStartLasServiceSinceClientError() throws Exception {
         when(dockerService.getClient(REGISTRY_NAME, REGISTRY_USERNAME, REGISTRY_PASSWORD))
                 .thenReturn(null);
 
-        Assertions.assertThat(lasService.start()).isFalse();
+        assertFalse(lasService.start());
+        assertFalse(lasService.isStarted());
+    }
+
+    @Test
+    void shouldNotStartLasServiceSinceClientException() throws Exception {
+        when(dockerService.getClient(REGISTRY_NAME, REGISTRY_USERNAME, REGISTRY_PASSWORD))
+                .thenThrow(Exception.class);
+
+        assertFalse(lasService.start());
         assertFalse(lasService.isStarted());
     }
 
     @Test
     void shouldNotStartLasServiceSinceCannotPullImage() {
         when(dockerClientInstanceMock.pullImage(IMAGE_URI)).thenReturn(false);
 
-        Assertions.assertThat(lasService.start()).isFalse();
+        assertFalse(lasService.start());
         assertFalse(lasService.isStarted());
     }
 
@@ -127,7 +147,7 @@ void shouldNotStartLasServiceSinceCannotRunDockerContainer() {
         when(dockerService.run(any()))
                 .thenReturn(DockerRunResponse.builder().finalStatus(DockerRunFinalStatus.FAILED).build());
 
-        Assertions.assertThat(lasService.start()).isFalse();
+        assertFalse(lasService.start());
         assertFalse(lasService.isStarted());
     }
     // endregion