From 2dada07dcbf3aacf927d20bf417cbfb7fe829be9 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Thu, 2 Jul 2020 14:41:54 +0200 Subject: [PATCH] openvidu-server: COMPOSED_QUICK_START fixes when stopping and launching container --- .../openvidu/server/core/SessionManager.java | 29 ++++--- .../ComposedQuickStartRecordingService.java | 33 ++------ .../service/ComposedRecordingService.java | 10 +-- .../recording/service/RecordingManager.java | 16 ++-- .../recording/service/RecordingService.java | 5 +- .../service/SingleStreamRecordingService.java | 2 +- .../server/rest/SessionRestController.java | 2 +- .../test/e2e/OpenViduTestAppE2eTest.java | 78 ++++++++++++++++++- 8 files changed, 114 insertions(+), 61 deletions(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java b/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java index 6e0dedb5..91dd8fd6 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java +++ b/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java @@ -32,7 +32,6 @@ import java.util.stream.Collectors; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; -import io.openvidu.java.client.Recording; import org.apache.commons.lang3.RandomStringUtils; import org.kurento.jsonrpc.message.Request; import org.slf4j.Logger; @@ -48,6 +47,7 @@ import io.openvidu.client.OpenViduException; import io.openvidu.client.OpenViduException.Code; import io.openvidu.client.internal.ProtocolElements; import io.openvidu.java.client.OpenViduRole; +import io.openvidu.java.client.Recording; import io.openvidu.java.client.SessionProperties; import io.openvidu.server.cdr.CDREventRecording; import io.openvidu.server.config.OpenviduConfig; @@ -543,21 +543,20 @@ public abstract class SessionManager { public void closeSessionAndEmptyCollections(Session session, EndReason reason, boolean stopRecording) { - if (openviduConfig.isRecordingModuleEnabled() && stopRecording - && this.recordingManager.sessionIsBeingRecorded(session.getSessionId())) { - try { - recordingManager.stopRecording(session, null, RecordingManager.finalReason(reason), true); - } catch (OpenViduException e) { - log.error("Error stopping recording of session {}: {}", session.getSessionId(), e.getMessage()); + if (openviduConfig.isRecordingModuleEnabled()) { + if (stopRecording && this.recordingManager.sessionIsBeingRecorded(session.getSessionId())) { + try { + recordingManager.stopRecording(session, null, RecordingManager.finalReason(reason)); + } catch (OpenViduException e) { + log.error("Error stopping recording of session {}: {}", session.getSessionId(), e.getMessage()); + } } - } else if(openviduConfig.isRecordingModuleEnabled() && stopRecording - && !this.recordingManager.sessionIsBeingRecorded(session.getSessionId()) - && session.getSessionProperties().defaultOutputMode().equals(Recording.OutputMode.COMPOSED_QUICK_START) - && this.recordingManager.getStartedRecording(session.getSessionId()) != null) { - try { - this.recordingManager.stopComposedQuickStartContainer(session, reason); - } catch (OpenViduException e) { - log.error("Error stopping COMPOSED_QUICK_START container of session {}", session.getSessionId()); + if (Recording.OutputMode.COMPOSED_QUICK_START.equals(session.getSessionProperties().defaultOutputMode())) { + try { + this.recordingManager.stopComposedQuickStartContainer(session, reason); + } catch (OpenViduException e) { + log.error("Error stopping COMPOSED_QUICK_START container of session {}", session.getSessionId()); + } } } diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java index 56ccd83a..1ea7a3b3 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedQuickStartRecordingService.java @@ -90,7 +90,7 @@ public class ComposedQuickStartRecordingService extends ComposedRecordingService } @Override - protected Recording stopRecordingWithVideo(Session session, Recording recording, EndReason reason, boolean hasSessionEnded) { + protected Recording stopRecordingWithVideo(Session session, Recording recording, EndReason reason) { log.info("Stopping COMPOSED_QUICK_START ({}) recording {} of session {}. Reason: {}", recording.hasAudio() ? "video + audio" : "audio-only", recording.getId(), recording.getSessionId(), RecordingManager.finalReason(reason)); @@ -106,31 +106,12 @@ public class ComposedQuickStartRecordingService extends ComposedRecordingService recording.getId()); } - if (hasSessionEnded) { - // Gracefully stop ffmpeg process - try { - dockerManager.runCommandInContainer(containerId, "./composed_quick_start.sh --stop-recording", 10); - } catch (InterruptedException e1) { - e1.printStackTrace(); - } - - try { - dockerManager.removeDockerContainer(containerId, true); - } catch (Exception e) { - failRecordingCompletion(recording, containerId, new OpenViduException(OpenViduException.Code.RECORDING_COMPLETION_ERROR_CODE, - "Can't remove COMPOSED_QUICK_START recording container from session" + session.getSessionId())); - } - - containers.remove(containerId); - sessionsContainers.remove(recording.getSessionId()); - } else { - try { - dockerManager.runCommandInContainer(containerId, "./composed_quick_start.sh --stop-recording", 10); - } catch (InterruptedException e1) { - cleanRecordingMaps(recording); - log.error("Error stopping recording for session id: {}", session.getSessionId()); - e1.printStackTrace(); - } + try { + dockerManager.runCommandInContainer(containerId, "./composed_quick_start.sh --stop-recording", 10); + } catch (InterruptedException e1) { + cleanRecordingMaps(recording); + log.error("Error stopping recording for session id: {}", session.getSessionId()); + e1.printStackTrace(); } recording = updateRecordingAttributes(recording); diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java index 7958ebe1..a645e6d2 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/ComposedRecordingService.java @@ -102,18 +102,18 @@ public class ComposedRecordingService extends RecordingService { } @Override - public Recording stopRecording(Session session, Recording recording, EndReason reason, boolean hasSessionEnded) { + public Recording stopRecording(Session session, Recording recording, EndReason reason) { recording = this.sealRecordingMetadataFileAsStopped(recording); if (recording.hasVideo()) { - return this.stopRecordingWithVideo(session, recording, reason, hasSessionEnded); + return this.stopRecordingWithVideo(session, recording, reason); } else { return this.stopRecordingAudioOnly(session, recording, reason, 0); } } - public Recording stopRecording(Session session, Recording recording, EndReason reason, long kmsDisconnectionTime, boolean hasSessionEnded) { + public Recording stopRecording(Session session, Recording recording, EndReason reason, long kmsDisconnectionTime) { if (recording.hasVideo()) { - return this.stopRecordingWithVideo(session, recording, reason, hasSessionEnded); + return this.stopRecordingWithVideo(session, recording, reason); } else { return this.stopRecordingAudioOnly(session, recording, reason, kmsDisconnectionTime); } @@ -228,7 +228,7 @@ public class ComposedRecordingService extends RecordingService { return recording; } - protected Recording stopRecordingWithVideo(Session session, Recording recording, EndReason reason, boolean hasSessionEnded) { + protected Recording stopRecordingWithVideo(Session session, Recording recording, EndReason reason) { log.info("Stopping composed ({}) recording {} of session {}. Reason: {}", recording.hasAudio() ? "video + audio" : "audio-only", recording.getId(), recording.getSessionId(), diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java index 311f4012..41b901f7 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingManager.java @@ -302,7 +302,7 @@ public class RecordingManager { } } - public Recording stopRecording(Session session, String recordingId, EndReason reason, boolean hasSessionEnded) { + public Recording stopRecording(Session session, String recordingId, EndReason reason) { Recording recording; if (session == null) { recording = this.startedRecordings.get(recordingId); @@ -316,14 +316,13 @@ public class RecordingManager { switch (recording.getOutputMode()) { case COMPOSED: - recording = this.composedRecordingService.stopRecording(session, recording, reason, hasSessionEnded); + recording = this.composedRecordingService.stopRecording(session, recording, reason); break; case COMPOSED_QUICK_START: - recording = this.composedQuickStartRecordingService.stopRecording(session, recording, reason, - hasSessionEnded); + recording = this.composedQuickStartRecordingService.stopRecording(session, recording, reason); break; case INDIVIDUAL: - recording = this.singleStreamRecordingService.stopRecording(session, recording, reason, hasSessionEnded); + recording = this.singleStreamRecordingService.stopRecording(session, recording, reason); break; } this.abortAutomaticRecordingStopThread(session, reason); @@ -335,8 +334,7 @@ public class RecordingManager { recording = this.sessionsRecordings.get(session.getSessionId()); switch (recording.getOutputMode()) { case COMPOSED: - recording = this.composedRecordingService.stopRecording(session, recording, reason, kmsDisconnectionTime, - true); + recording = this.composedRecordingService.stopRecording(session, recording, reason, kmsDisconnectionTime); if (recording.hasVideo()) { // Evict the recorder participant if composed recording with video this.sessionManager.evictParticipant( @@ -346,7 +344,7 @@ public class RecordingManager { break; case COMPOSED_QUICK_START: recording = this.composedQuickStartRecordingService.stopRecording(session, recording, reason, - kmsDisconnectionTime, true); + kmsDisconnectionTime); if (recording.hasVideo()) { // Evict the recorder participant if composed recording with video this.sessionManager.evictParticipant( @@ -557,7 +555,7 @@ public class RecordingManager { log.info( "Automatic stopping recording {}. There are users connected to session {}, but no one is publishing", recordingId, session.getSessionId()); - this.stopRecording(session, recordingId, EndReason.automaticStop, true); + this.stopRecording(session, recordingId, EndReason.automaticStop); } } finally { if (!alreadyUnlocked) { diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingService.java index fc5a2052..15317a07 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/RecordingService.java @@ -59,8 +59,7 @@ public abstract class RecordingService { public abstract Recording startRecording(Session session, RecordingProperties properties) throws OpenViduException; - public abstract Recording stopRecording(Session session, Recording recording, EndReason reason, - boolean hasSessionEnded); + public abstract Recording stopRecording(Session session, Recording recording, EndReason reason); /** * Generates metadata recording file (".recording.RECORDING_ID" JSON file to @@ -178,7 +177,7 @@ public abstract class RecordingService { recording.setStatus(io.openvidu.java.client.Recording.Status.failed); this.recordingManager.startingRecordings.remove(recording.getId()); this.recordingManager.sessionsRecordingsStarting.remove(session.getSessionId()); - this.stopRecording(session, recording, null, true); + this.stopRecording(session, recording, null); return new OpenViduException(Code.RECORDING_START_ERROR_CODE, errorMessage); } diff --git a/openvidu-server/src/main/java/io/openvidu/server/recording/service/SingleStreamRecordingService.java b/openvidu-server/src/main/java/io/openvidu/server/recording/service/SingleStreamRecordingService.java index cff12bdd..0e246e34 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/recording/service/SingleStreamRecordingService.java +++ b/openvidu-server/src/main/java/io/openvidu/server/recording/service/SingleStreamRecordingService.java @@ -138,7 +138,7 @@ public class SingleStreamRecordingService extends RecordingService { } @Override - public Recording stopRecording(Session session, Recording recording, EndReason reason, boolean hasSessionEnded) { + public Recording stopRecording(Session session, Recording recording, EndReason reason) { recording = this.sealRecordingMetadataFileAsStopped(recording); return this.stopRecording(session, recording, reason, 0); } diff --git a/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java b/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java index d44f8872..28da7b54 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java +++ b/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java @@ -608,7 +608,7 @@ public class SessionRestController { Session session = sessionManager.getSession(recording.getSessionId()); Recording stoppedRecording = this.recordingManager.stopRecording(session, recording.getId(), - EndReason.recordingStoppedByServer, false); + EndReason.recordingStoppedByServer); session.recordingManuallyStopped.set(true); diff --git a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java index 897cfe12..cb08b5c3 100644 --- a/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java +++ b/openvidu-test-e2e/src/test/java/io/openvidu/test/e2e/OpenViduTestAppE2eTest.java @@ -126,6 +126,7 @@ public class OpenViduTestAppE2eTest { private static final Logger log = LoggerFactory.getLogger(OpenViduTestAppE2eTest.class); private static final CommandLineExecutor commandLine = new CommandLineExecutor(); + private static final String RECORDING_IMAGE = "openvidu/openvidu-recording"; MyUser user; Collection otherUsers = new ArrayList<>(); @@ -255,6 +256,7 @@ public class OpenViduTestAppE2eTest { } }); if (isRecordingTest) { + removeAllRecordingContiners(); try { FileUtils.cleanDirectory(new File("/opt/openvidu/recordings")); } catch (IOException e) { @@ -1233,6 +1235,70 @@ public class OpenViduTestAppE2eTest { gracefullyLeaveParticipants(1); } + @Test + @DisplayName("Remote composed quick start record") + void remoteComposedQuickStartRecordTest() throws Exception { + isRecordingTest = true; + + setupBrowser("chrome"); + + log.info("Remote composed quick start record"); + + final String sessionName = "COMPOSED_QUICK_START_RECORDED_SESSION"; + + user.getDriver().findElement(By.id("add-user-btn")).click(); + user.getDriver().findElement(By.id("session-name-input-0")).clear(); + user.getDriver().findElement(By.id("session-name-input-0")).sendKeys(sessionName); + + user.getDriver().findElement(By.id("session-settings-btn-0")).click(); + Thread.sleep(1000); + user.getDriver().findElement(By.id("output-mode-select")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("option-COMPOSED_QUICK_START")).click(); + Thread.sleep(500); + user.getDriver().findElement(By.id("save-btn")).click(); + Thread.sleep(1000); + + // Join the subscriber user to the session + user.getDriver().findElement(By.cssSelector("#openvidu-instance-0 .publish-checkbox")).click(); + user.getDriver().findElement(By.className("join-btn")).click(); + user.getEventManager().waitUntilEventReaches("connectionCreated", 1); + + // Check the recording container is up and running but no ongoing recordings + checkDockerContainerRunning(RECORDING_IMAGE, 1); + Assert.assertEquals("Wrong number of recordings found", 0, OV.listRecordings().size()); + + // Join the publisher user to the session + user.getDriver().findElement(By.id("add-user-btn")).click(); + user.getDriver().findElement(By.id("session-name-input-1")).clear(); + user.getDriver().findElement(By.id("session-name-input-1")).sendKeys(sessionName); + user.getDriver().findElement(By.cssSelector("#openvidu-instance-1 .join-btn")).click(); + + user.getEventManager().waitUntilEventReaches("connectionCreated", 4); + user.getEventManager().waitUntilEventReaches("accessAllowed", 1); + user.getEventManager().waitUntilEventReaches("streamCreated", 2); + user.getEventManager().waitUntilEventReaches("streamPlaying", 2); + + // Start recording + OV.fetch(); + String recId = OV.startRecording(sessionName).getId(); + user.getEventManager().waitUntilEventReaches("recordingStarted", 2); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); + + Thread.sleep(1000); + + Assert.assertEquals("Wrong number of recordings found", 1, OV.listRecordings().size()); + OV.stopRecording(recId); + user.getEventManager().waitUntilEventReaches("recordingStopped", 2); + checkDockerContainerRunning("openvidu/openvidu-recording", 1); + + Assert.assertEquals("Wrong number of sessions", 1, OV.getActiveSessions().size()); + Session session = OV.getActiveSessions().get(0); + session.close(); + + checkDockerContainerRunning("openvidu/openvidu-recording", 0); + } + @Test @DisplayName("Remote individual record") void remoteIndividualRecordTest() throws Exception { @@ -2928,7 +2994,7 @@ public class OpenViduTestAppE2eTest { Assert.assertEquals("Wrong number of properties in event 'recordingStatusChanged'", 12 + 1, event.keySet().size()); Assert.assertEquals("Wrong recording status in webhook event", "ready", event.get("status").getAsString()); - Assert.assertTrue("Wrong recording outputMode in webhook event", event.get("size").getAsLong() > 0); + Assert.assertTrue("Wrong recording size in webhook event", event.get("size").getAsLong() > 0); Assert.assertTrue("Wrong recording outputMode in webhook event", event.get("duration").getAsLong() > 0); Assert.assertEquals("Wrong recording reason in webhook event", "sessionClosedByServer", event.get("reason").getAsString()); @@ -3463,4 +3529,14 @@ public class OpenViduTestAppE2eTest { this.startKms(); } + private void checkDockerContainerRunning(String imageName, int amount) { + int number = Integer.parseInt(commandLine.executeCommand("docker ps | grep " + imageName + " | wc -l")); + Assert.assertEquals("Wrong number of Docker containers for image " + imageName + " running", amount, number); + } + + private void removeAllRecordingContiners() { + commandLine.executeCommand("docker ps -a | awk '{ print $1,$2 }' | grep " + RECORDING_IMAGE + + " | awk '{print $1 }' | xargs -I {} docker rm -f {}"); + } + }