From aa887096f9a305568dc3a846d0c9cdf7e45f1c67 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Tue, 25 Oct 2022 14:18:59 +0200 Subject: [PATCH] Correctly delete live files from object storage --- server/lib/live/live-utils.ts | 6 ++++-- server/lib/live/shared/muxing-session.ts | 4 ++-- .../shared/object-storage-helpers.ts | 9 +++++++-- server/lib/object-storage/videos.ts | 16 ++++++++++++++-- server/models/video/video.ts | 8 ++++---- server/tests/api/live/live-constraints.ts | 2 +- server/tests/api/live/live-permanent.ts | 12 ++++++++++++ server/tests/api/live/live-save-replay.ts | 18 +++++++++--------- server/tests/shared/live.ts | 17 ++++++++++++++++- 9 files changed, 69 insertions(+), 23 deletions(-) diff --git a/server/lib/live/live-utils.ts b/server/lib/live/live-utils.ts index d2b8e3a55..c0dec9829 100644 --- a/server/lib/live/live-utils.ts +++ b/server/lib/live/live-utils.ts @@ -3,7 +3,7 @@ import { basename, join } from 'path' import { logger } from '@server/helpers/logger' import { MStreamingPlaylist, MStreamingPlaylistVideo, MVideo } from '@server/types/models' import { VideoStorage } from '@shared/models' -import { listHLSFileKeysOf, removeHLSFileObjectStorage, removeHLSObjectStorage } from '../object-storage' +import { listHLSFileKeysOf, removeHLSFileObjectStorageByFullKey, removeHLSObjectStorage } from '../object-storage' import { getLiveDirectory } from '../paths' function buildConcatenatedName (segmentOrPlaylistPath: string) { @@ -77,11 +77,13 @@ async function cleanupTMPLiveFilesFromFilesystem (video: MVideo) { async function cleanupTMPLiveFilesFromObjectStorage (streamingPlaylist: MStreamingPlaylistVideo) { if (streamingPlaylist.storage !== VideoStorage.OBJECT_STORAGE) return + logger.info('Cleanup TMP live files from object storage for %s.', streamingPlaylist.Video.uuid) + const keys = await listHLSFileKeysOf(streamingPlaylist) for (const key of keys) { if (isTMPLiveFile(key)) { - await removeHLSFileObjectStorage(streamingPlaylist, key) + await removeHLSFileObjectStorageByFullKey(key) } } } diff --git a/server/lib/live/shared/muxing-session.ts b/server/lib/live/shared/muxing-session.ts index 64add2611..6ec126955 100644 --- a/server/lib/live/shared/muxing-session.ts +++ b/server/lib/live/shared/muxing-session.ts @@ -10,7 +10,7 @@ import { getLiveMuxingCommand, getLiveTranscodingCommand } from '@server/helpers import { logger, loggerTagsFactory, LoggerTagsFn } from '@server/helpers/logger' import { CONFIG } from '@server/initializers/config' import { MEMOIZE_TTL, VIDEO_LIVE } from '@server/initializers/constants' -import { removeHLSFileObjectStorage, storeHLSFileFromFilename, storeHLSFileFromPath } from '@server/lib/object-storage' +import { removeHLSFileObjectStorageByPath, storeHLSFileFromFilename, storeHLSFileFromPath } from '@server/lib/object-storage' import { VideoFileModel } from '@server/models/video/video-file' import { MStreamingPlaylistVideo, MUserId, MVideoLiveVideo } from '@server/types/models' import { VideoStorage } from '@shared/models' @@ -341,7 +341,7 @@ class MuxingSession extends EventEmitter { if (this.streamingPlaylist.storage === VideoStorage.OBJECT_STORAGE) { try { - await removeHLSFileObjectStorage(this.streamingPlaylist, segmentPath) + await removeHLSFileObjectStorageByPath(this.streamingPlaylist, segmentPath) } catch (err) { logger.error('Cannot remove segment %s from object storage', segmentPath, { err, ...this.lTags() }) } diff --git a/server/lib/object-storage/shared/object-storage-helpers.ts b/server/lib/object-storage/shared/object-storage-helpers.ts index d13c25798..3046d76bc 100644 --- a/server/lib/object-storage/shared/object-storage-helpers.ts +++ b/server/lib/object-storage/shared/object-storage-helpers.ts @@ -110,11 +110,15 @@ function updatePrefixACL (options: { function removeObject (objectStorageKey: string, bucketInfo: BucketInfo) { const key = buildKey(objectStorageKey, bucketInfo) - logger.debug('Removing file %s in bucket %s', key, bucketInfo.BUCKET_NAME, lTags()) + return removeObjectByFullKey(key, bucketInfo) +} + +function removeObjectByFullKey (fullKey: string, bucketInfo: BucketInfo) { + logger.debug('Removing file %s in bucket %s', fullKey, bucketInfo.BUCKET_NAME, lTags()) const command = new DeleteObjectCommand({ Bucket: bucketInfo.BUCKET_NAME, - Key: key + Key: fullKey }) return getClient().send(command) @@ -195,6 +199,7 @@ export { storeObject, removeObject, + removeObjectByFullKey, removePrefix, makeAvailable, diff --git a/server/lib/object-storage/videos.ts b/server/lib/object-storage/videos.ts index 003807826..b764e4b22 100644 --- a/server/lib/object-storage/videos.ts +++ b/server/lib/object-storage/videos.ts @@ -11,6 +11,7 @@ import { lTags, makeAvailable, removeObject, + removeObjectByFullKey, removePrefix, storeObject, updateObjectACL, @@ -76,10 +77,18 @@ function removeHLSObjectStorage (playlist: MStreamingPlaylistVideo) { return removePrefix(generateHLSObjectBaseStorageKey(playlist), CONFIG.OBJECT_STORAGE.STREAMING_PLAYLISTS) } -function removeHLSFileObjectStorage (playlist: MStreamingPlaylistVideo, filename: string) { +function removeHLSFileObjectStorageByFilename (playlist: MStreamingPlaylistVideo, filename: string) { return removeObject(generateHLSObjectStorageKey(playlist, filename), CONFIG.OBJECT_STORAGE.STREAMING_PLAYLISTS) } +function removeHLSFileObjectStorageByPath (playlist: MStreamingPlaylistVideo, path: string) { + return removeObject(generateHLSObjectStorageKey(playlist, basename(path)), CONFIG.OBJECT_STORAGE.STREAMING_PLAYLISTS) +} + +function removeHLSFileObjectStorageByFullKey (key: string) { + return removeObjectByFullKey(key, CONFIG.OBJECT_STORAGE.STREAMING_PLAYLISTS) +} + // --------------------------------------------------------------------------- function removeWebTorrentObjectStorage (videoFile: MVideoFile) { @@ -162,7 +171,10 @@ export { updateHLSFilesACL, removeHLSObjectStorage, - removeHLSFileObjectStorage, + removeHLSFileObjectStorageByFilename, + removeHLSFileObjectStorageByPath, + removeHLSFileObjectStorageByFullKey, + removeWebTorrentObjectStorage, makeWebTorrentFileAvailable, diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 9399712b8..2ff92cbf1 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -26,7 +26,7 @@ import { } from 'sequelize-typescript' import { getPrivaciesForFederation, isPrivacyForFederation, isStateForFederation } from '@server/helpers/video' import { LiveManager } from '@server/lib/live/live-manager' -import { removeHLSFileObjectStorage, removeHLSObjectStorage, removeWebTorrentObjectStorage } from '@server/lib/object-storage' +import { removeHLSFileObjectStorageByFilename, removeHLSObjectStorage, removeWebTorrentObjectStorage } from '@server/lib/object-storage' import { tracer } from '@server/lib/opentelemetry/tracing' import { getHLSDirectory, getHLSRedundancyDirectory, getHlsResolutionPlaylistFilename } from '@server/lib/paths' import { VideoPathManager } from '@server/lib/video-path-manager' @@ -1830,8 +1830,8 @@ export class VideoModel extends Model>> { await remove(VideoPathManager.Instance.getFSHLSOutputPath(this, resolutionFilename)) if (videoFile.storage === VideoStorage.OBJECT_STORAGE) { - await removeHLSFileObjectStorage(streamingPlaylist.withVideo(this), videoFile.filename) - await removeHLSFileObjectStorage(streamingPlaylist.withVideo(this), resolutionFilename) + await removeHLSFileObjectStorageByFilename(streamingPlaylist.withVideo(this), videoFile.filename) + await removeHLSFileObjectStorageByFilename(streamingPlaylist.withVideo(this), resolutionFilename) } } @@ -1840,7 +1840,7 @@ export class VideoModel extends Model>> { await remove(filePath) if (streamingPlaylist.storage === VideoStorage.OBJECT_STORAGE) { - await removeHLSFileObjectStorage(streamingPlaylist.withVideo(this), filename) + await removeHLSFileObjectStorageByFilename(streamingPlaylist.withVideo(this), filename) } } diff --git a/server/tests/api/live/live-constraints.ts b/server/tests/api/live/live-constraints.ts index 64ef73a2a..c82585a9e 100644 --- a/server/tests/api/live/live-constraints.ts +++ b/server/tests/api/live/live-constraints.ts @@ -49,7 +49,7 @@ describe('Test live constraints', function () { expect(video.duration).to.be.greaterThan(0) } - await checkLiveCleanup(servers[0], videoId, resolutions) + await checkLiveCleanup({ server: servers[0], permanent: false, videoUUID: videoId, savedResolutions: resolutions }) } function updateQuota (options: { total: number, daily: number }) { diff --git a/server/tests/api/live/live-permanent.ts b/server/tests/api/live/live-permanent.ts index 5d227200e..4203b1bfc 100644 --- a/server/tests/api/live/live-permanent.ts +++ b/server/tests/api/live/live-permanent.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */ import { expect } from 'chai' +import { checkLiveCleanup } from '@server/tests/shared' import { wait } from '@shared/core-utils' import { LiveVideoCreate, VideoPrivacy, VideoState } from '@shared/models' import { @@ -129,6 +130,8 @@ describe('Permanent live', function () { expect(videoDetails.streamingPlaylists).to.have.lengthOf(0) } + + await checkLiveCleanup({ server: servers[0], permanent: true, videoUUID }) }) it('Should have set this live to waiting for live state', async function () { @@ -186,6 +189,15 @@ describe('Permanent live', function () { } }) + it('Should remove the live and have cleaned up the directory', async function () { + this.timeout(60000) + + await servers[0].videos.remove({ id: videoUUID }) + await waitJobs(servers) + + await checkLiveCleanup({ server: servers[0], permanent: true, videoUUID }) + }) + after(async function () { await cleanupTests(servers) }) diff --git a/server/tests/api/live/live-save-replay.ts b/server/tests/api/live/live-save-replay.ts index 7014292d0..8f17b4566 100644 --- a/server/tests/api/live/live-save-replay.ts +++ b/server/tests/api/live/live-save-replay.ts @@ -186,7 +186,7 @@ describe('Save replay setting', function () { await checkVideoState(liveVideoUUID, VideoState.LIVE_ENDED) // No resolutions saved since we did not save replay - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) it('Should have appropriate ended session', async function () { @@ -220,7 +220,7 @@ describe('Save replay setting', function () { await wait(5000) await waitJobs(servers) - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) it('Should have blacklisted session error', async function () { @@ -238,7 +238,7 @@ describe('Save replay setting', function () { await publishLiveAndDelete({ permanent: false, replay: false }) await checkVideosExist(liveVideoUUID, false, HttpStatusCode.NOT_FOUND_404) - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) }) @@ -317,7 +317,7 @@ describe('Save replay setting', function () { }) it('Should have cleaned up the live files', async function () { - await checkLiveCleanup(servers[0], liveVideoUUID, [ 720 ]) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false, savedResolutions: [ 720 ] }) }) it('Should correctly terminate the stream on blacklist and blacklist the saved replay video', async function () { @@ -332,7 +332,7 @@ describe('Save replay setting', function () { await wait(5000) await waitJobs(servers) - await checkLiveCleanup(servers[0], liveVideoUUID, [ 720 ]) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false, savedResolutions: [ 720 ] }) }) it('Should correctly terminate the stream on delete and delete the video', async function () { @@ -341,7 +341,7 @@ describe('Save replay setting', function () { await publishLiveAndDelete({ permanent: false, replay: true }) await checkVideosExist(liveVideoUUID, false, HttpStatusCode.NOT_FOUND_404) - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) }) @@ -413,7 +413,7 @@ describe('Save replay setting', function () { }) it('Should have cleaned up the live files', async function () { - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) it('Should correctly terminate the stream on blacklist and blacklist the saved replay video', async function () { @@ -432,7 +432,7 @@ describe('Save replay setting', function () { await servers[1].videos.get({ id: videoId, expectedStatus: HttpStatusCode.NOT_FOUND_404 }) } - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) it('Should correctly terminate the stream on delete and not save the video', async function () { @@ -444,7 +444,7 @@ describe('Save replay setting', function () { expect(replay).to.not.exist await checkVideosExist(liveVideoUUID, false, HttpStatusCode.NOT_FOUND_404) - await checkLiveCleanup(servers[0], liveVideoUUID, []) + await checkLiveCleanup({ server: servers[0], videoUUID: liveVideoUUID, permanent: false }) }) }) diff --git a/server/tests/shared/live.ts b/server/tests/shared/live.ts index 78e29f575..47e0dc481 100644 --- a/server/tests/shared/live.ts +++ b/server/tests/shared/live.ts @@ -7,10 +7,25 @@ import { LiveVideo, VideoStreamingPlaylistType } from '@shared/models' import { ObjectStorageCommand, PeerTubeServer } from '@shared/server-commands' import { checkLiveSegmentHash, checkResolutionsInMasterPlaylist } from './streaming-playlists' -async function checkLiveCleanup (server: PeerTubeServer, videoUUID: string, savedResolutions: number[] = []) { +async function checkLiveCleanup (options: { + server: PeerTubeServer + videoUUID: string + permanent: boolean + savedResolutions?: number[] +}) { + const { server, videoUUID, permanent, savedResolutions = [] } = options + const basePath = server.servers.buildDirectory('streaming-playlists') const hlsPath = join(basePath, 'hls', videoUUID) + if (permanent) { + if (!await pathExists(hlsPath)) return + + const files = await readdir(hlsPath) + expect(files).to.have.lengthOf(0) + return + } + if (savedResolutions.length === 0) { return checkUnsavedLiveCleanup(server, videoUUID, hlsPath) }