From e27369686ba293426b558091c6b49525c84779c9 Mon Sep 17 00:00:00 2001 From: ta264 Date: Thu, 4 Apr 2019 16:15:43 +0100 Subject: [PATCH] A remote path mapping health check (#617) --- frontend/src/System/Status/Health/Health.js | 1 + src/NzbDrone.Common/EnvironmentInfo/OsInfo.cs | 9 +- .../Checks/HealthCheckFixtureExtensions.cs | 7 +- .../Checks/RemotePathMappingCheckFixture.cs | 171 ++++++++++++++++++ .../NzbDrone.Core.Test.csproj | 1 + .../Checks/RemotePathMappingCheck.cs | 128 +++++++++++++ .../HealthCheck/HealthCheckService.cs | 20 +- .../HealthCheck/IProvideHealthCheck.cs | 7 + .../DownloadedTracksImportService.cs | 19 +- .../TrackImport/ImportApprovedTracks.cs | 2 + src/NzbDrone.Core/NzbDrone.Core.csproj | 1 + .../RemotePathMappingRepository.cs | 9 +- 12 files changed, 360 insertions(+), 15 deletions(-) create mode 100644 src/NzbDrone.Core.Test/HealthCheck/Checks/RemotePathMappingCheckFixture.cs create mode 100644 src/NzbDrone.Core/HealthCheck/Checks/RemotePathMappingCheck.cs diff --git a/frontend/src/System/Status/Health/Health.js b/frontend/src/System/Status/Health/Health.js index 361b6cb10..fdb4655c8 100644 --- a/frontend/src/System/Status/Health/Health.js +++ b/frontend/src/System/Status/Health/Health.js @@ -27,6 +27,7 @@ function getInternalLink(source) { ); case 'DownloadClientCheck': case 'ImportMechanismCheck': + case 'RemotePathMappingCheck': return ( Os == Os.Linux; public static bool IsOsx => Os == Os.Osx; public static bool IsWindows => Os == Os.Windows; - + public static bool IsDocker { get; } + public string Version { get; } public string Name { get; } public string FullName { get; } @@ -44,6 +45,10 @@ namespace NzbDrone.Common.EnvironmentInfo else { Os = Os.Linux; + if (File.Exists("/proc/1/cgroup") && File.ReadAllText("/proc/1/cgroup").Contains("/docker/")) + { + IsDocker = true; + } } break; } @@ -101,4 +106,4 @@ namespace NzbDrone.Common.EnvironmentInfo Linux, Osx } -} \ No newline at end of file +} diff --git a/src/NzbDrone.Core.Test/HealthCheck/Checks/HealthCheckFixtureExtensions.cs b/src/NzbDrone.Core.Test/HealthCheck/Checks/HealthCheckFixtureExtensions.cs index 5654fa388..9bc851fe4 100644 --- a/src/NzbDrone.Core.Test/HealthCheck/Checks/HealthCheckFixtureExtensions.cs +++ b/src/NzbDrone.Core.Test/HealthCheck/Checks/HealthCheckFixtureExtensions.cs @@ -21,7 +21,7 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks } } - public static void ShouldBeError(this Core.HealthCheck.HealthCheck result, string message = null) + public static void ShouldBeError(this Core.HealthCheck.HealthCheck result, string message = null, string wikiFragment = null) { result.Type.Should().Be(HealthCheckResult.Error); @@ -29,6 +29,11 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks { result.Message.Should().Contain(message); } + + if (wikiFragment.IsNotNullOrWhiteSpace()) + { + result.WikiUrl.Fragment.Should().Be(wikiFragment); + } } } } diff --git a/src/NzbDrone.Core.Test/HealthCheck/Checks/RemotePathMappingCheckFixture.cs b/src/NzbDrone.Core.Test/HealthCheck/Checks/RemotePathMappingCheckFixture.cs new file mode 100644 index 000000000..a82bcd47c --- /dev/null +++ b/src/NzbDrone.Core.Test/HealthCheck/Checks/RemotePathMappingCheckFixture.cs @@ -0,0 +1,171 @@ +using System; +using System.Collections.Generic; +using System.IO; +using Moq; +using NUnit.Framework; +using NzbDrone.Common.Disk; +using NzbDrone.Core.Download; +using NzbDrone.Core.HealthCheck.Checks; +using NzbDrone.Core.MediaFiles; +using NzbDrone.Core.MediaFiles.Events; +using NzbDrone.Core.Parser.Model; +using NzbDrone.Core.Test.Framework; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.HealthCheck.Checks +{ + [TestFixture] + public class RemotePathMappingCheckFixture : CoreTest + { + private string downloadRootPath = @"c:\Test".AsOsAgnostic(); + private string downloadItemPath = @"c:\Test\item".AsOsAgnostic(); + + private DownloadClientInfo clientStatus; + private DownloadClientItem downloadItem; + + [SetUp] + public void Setup() + { + downloadItem = new DownloadClientItem { + DownloadClient = "Test", + DownloadId = "TestId", + OutputPath = new OsPath(downloadItemPath) + }; + + clientStatus = new DownloadClientInfo { + IsLocalhost = true, + OutputRootFolders = new List { new OsPath(downloadRootPath) } + }; + + var downloadClient = Mocker.GetMock(); + downloadClient.Setup(s => s.Definition) + .Returns(new DownloadClientDefinition { Name = "Test" }); + + downloadClient.Setup(s => s.GetItems()) + .Returns(new List { downloadItem }); + + downloadClient.Setup(s => s.GetStatus()) + .Returns(clientStatus); + + Mocker.GetMock() + .Setup(s => s.GetDownloadClients()) + .Returns(new IDownloadClient[] { downloadClient.Object }); + + Mocker.GetMock() + .Setup(x => x.FolderExists(It.IsAny())) + .Returns(false); + + Mocker.GetMock() + .Setup(x => x.FileExists(It.IsAny())) + .Returns(false); + } + + private void GivenFolderExists(string folder) + { + Mocker.GetMock() + .Setup(x => x.FolderExists(folder)) + .Returns(true); + } + + private void GivenFileExists(string file) + { + Mocker.GetMock() + .Setup(x => x.FileExists(file)) + .Returns(true); + } + + + private void GivenDocker() + { + // + } + + [Test] + public void should_return_ok_if_setup_correctly() + { + GivenFolderExists(downloadRootPath); + + Subject.Check().ShouldBeOk(); + } + + [Test] + public void should_return_permissions_error_if_local_client_download_root_missing() + { + Subject.Check().ShouldBeError(wikiFragment: "permissions-error"); + } + + [Test] + public void should_return_path_mapping_error_if_remote_client_download_root_missing() + { + clientStatus.IsLocalhost = false; + + Subject.Check().ShouldBeError(wikiFragment: "bad-remote-path-mapping"); + } + + [Test] + [Explicit("Only works if running inside a docker container")] + public void should_return_docker_path_mapping_error_if_on_docker_and_root_missing() + { + Subject.Check().ShouldBeError(wikiFragment: "docker-bad-remote-path-mapping"); + } + + [Test] + public void should_return_ok_on_track_imported_event() + { + GivenFolderExists(downloadRootPath); + var importEvent = new TrackImportedEvent(new LocalTrack(), new TrackFile(), new List(), true, new DownloadClientItem()); + + Subject.Check(importEvent).ShouldBeOk(); + } + + [Test] + public void should_return_permissions_error_on_track_import_failed_event_if_file_exists() + { + var localTrack = new LocalTrack { + Path = Path.Combine(downloadItemPath, "file.mp3") + }; + GivenFileExists(localTrack.Path); + + var importEvent = new TrackImportFailedEvent(new Exception(), localTrack, true, new DownloadClientItem()); + + Subject.Check(importEvent).ShouldBeError(wikiFragment: "permissions-error"); + } + + [Test] + public void should_return_permissions_error_on_track_import_failed_event_if_folder_exists() + { + GivenFolderExists(downloadItemPath); + + var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem); + + Subject.Check(importEvent).ShouldBeError(wikiFragment: "permissions-error"); + } + + [Test] + public void should_return_permissions_error_on_track_import_failed_event_for_local_client_if_folder_does_not_exist() + { + var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem); + + Subject.Check(importEvent).ShouldBeError(wikiFragment: "permissions-error"); + } + + [Test] + public void should_return_mapping_error_on_track_import_failed_event_for_remote_client_if_folder_does_not_exist() + { + clientStatus.IsLocalhost = false; + var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem); + + Subject.Check(importEvent).ShouldBeError(wikiFragment: "bad-remote-path-mapping"); + } + + [Test] + [Explicit("Only works if running inside a docker container")] + public void should_return_docker_mapping_error_on_track_import_failed_event_inside_docker_if_folder_does_not_exist() + { + clientStatus.IsLocalhost = false; + var importEvent = new TrackImportFailedEvent(null, null, true, downloadItem); + + Subject.Check(importEvent).ShouldBeError(wikiFragment: "docker-bad-remote-path-mapping"); + } + } +} diff --git a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj index 11b1045e4..d3431bf64 100644 --- a/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj +++ b/src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj @@ -224,6 +224,7 @@ + diff --git a/src/NzbDrone.Core/HealthCheck/Checks/RemotePathMappingCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/RemotePathMappingCheck.cs new file mode 100644 index 000000000..c7fb2a6e0 --- /dev/null +++ b/src/NzbDrone.Core/HealthCheck/Checks/RemotePathMappingCheck.cs @@ -0,0 +1,128 @@ +using System.Linq; +using NLog; +using NzbDrone.Common.Disk; +using NzbDrone.Common.EnvironmentInfo; +using NzbDrone.Common.Extensions; +using NzbDrone.Common.Messaging; +using NzbDrone.Core.Datastore.Events; +using NzbDrone.Core.Download; +using NzbDrone.Core.MediaFiles.Events; +using NzbDrone.Core.RemotePathMappings; +using NzbDrone.Core.ThingiProvider.Events; + +namespace NzbDrone.Core.HealthCheck.Checks +{ + [CheckOn(typeof(ProviderAddedEvent))] + [CheckOn(typeof(ProviderUpdatedEvent))] + [CheckOn(typeof(ProviderDeletedEvent))] + [CheckOn(typeof(ModelEvent))] + [CheckOn(typeof(TrackImportedEvent), CheckOnCondition.FailedOnly)] + [CheckOn(typeof(TrackImportFailedEvent), CheckOnCondition.SuccessfulOnly)] + public class RemotePathMappingCheck : HealthCheckBase, IProvideHealthCheckWithMessage + { + private readonly IDiskProvider _diskProvider; + private readonly IProvideDownloadClient _downloadClientProvider; + private readonly Logger _logger; + + public RemotePathMappingCheck(IDiskProvider diskProvider, + IProvideDownloadClient downloadClientProvider, + Logger logger) + { + _diskProvider = diskProvider; + _downloadClientProvider = downloadClientProvider; + _logger = logger; + } + + public override HealthCheck Check() + { + var clients = _downloadClientProvider.GetDownloadClients(); + + foreach (var client in clients) + { + var folders = client.GetStatus().OutputRootFolders; + if (folders != null) + { + foreach (var folder in folders) + { + if (!_diskProvider.FolderExists(folder.FullPath)) + { + if (OsInfo.IsDocker) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"You are using docker; download client {client.Definition.Name} places downloads in {folder.FullPath} but this directory does not appear to exist inside the container. Review your remote path mappings and container volume settings.", "#docker-bad-remote-path-mapping"); + } + else if (!client.GetStatus().IsLocalhost) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Remote download client {client.Definition.Name} places downloads in {folder.FullPath} but this directory does not appear to exist. Likely missing or incorrect remote path mapping.", "#bad-remote-path-mapping"); + } + else + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Download client {client.Definition.Name} places downloads in {folder.FullPath} but Lidarr cannot see this directory. You may need to adjust the folder's permissions.", "#permissions-error"); + } + } + } + } + } + return new HealthCheck(GetType()); + } + + public HealthCheck Check(IEvent message) + { + if (typeof(TrackImportFailedEvent).IsAssignableFrom(message.GetType())) + { + var failureMessage = (TrackImportFailedEvent) message; + + // if we can see the file exists but the import failed then likely a permissions issue + if (failureMessage.TrackInfo != null) + { + var trackPath = failureMessage.TrackInfo.Path; + if (_diskProvider.FileExists(trackPath)) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Lidarr can see but not access downloaded track {trackPath}. Likely permissions error.", "#permissions-error"); + } + else + { + // If the file doesn't exist but TrackInfo is not null then the message is coming from + // ImportApprovedTracks and the file must have been removed part way through processing + return new HealthCheck(GetType(), HealthCheckResult.Error, $"File {trackPath} was removed part way though procesing."); + } + } + + // If the previous case did not match then the failure occured in DownloadedTracksImportService, + // while trying to locate the files reported by the download client + var client = _downloadClientProvider.GetDownloadClients().FirstOrDefault(x => x.Definition.Name == failureMessage.DownloadClient); + var dlpath = client?.GetItems().FirstOrDefault(x => x.DownloadId == failureMessage.DownloadId)?.OutputPath.FullPath; + + // If dlpath is null then there's not much useful we can report. Give a generic message so + // that the user realises something is wrong. + if (dlpath.IsNullOrWhiteSpace()) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Lidarr failed to import a track. Check your logs for details."); + } + + if (_diskProvider.FolderExists(dlpath)) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Lidarr can see but not access download directory {dlpath}. Likely permissions error.", "#permissions-error"); + } + + // if it's a remote client/docker, likely missing path mappings + if (OsInfo.IsDocker) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"You are using docker; download client {client.Definition.Name} reported files in {dlpath} but this directory does not appear to exist inside the container. Review your remote path mappings and container volume settings.", "#docker-bad-remote-path-mapping"); + } + else if (!client.GetStatus().IsLocalhost) + { + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Remote download client {client.Definition.Name} reported files in {dlpath} but this directory does not appear to exist. Likely missing remote path mapping.", "#bad-remote-path-mapping"); + } + else + { + // path mappings shouldn't be needed locally so probably a permissions issue + return new HealthCheck(GetType(), HealthCheckResult.Error, $"Download client {client.Definition.Name} reported files in {dlpath} but Lidarr cannot see this directory. You may need to adjust the folder's permissions.", "#permissions-error"); + } + } + else + { + return Check(); + } + } + } +} diff --git a/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs b/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs index 5ab1c5416..f31fa7efd 100644 --- a/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs +++ b/src/NzbDrone.Core/HealthCheck/HealthCheckService.cs @@ -66,10 +66,21 @@ namespace NzbDrone.Core.HealthCheck .ToDictionary(g => g.Key, g => g.ToArray()); } - private void PerformHealthCheck(IProvideHealthCheck[] healthChecks) + private void PerformHealthCheck(IProvideHealthCheck[] healthChecks, IEvent message = null) { - var results = healthChecks.Select(c => c.Check()) - .ToList(); + var results = new List(); + + foreach (var healthCheck in healthChecks) + { + if (healthCheck is IProvideHealthCheckWithMessage && message != null) + { + results.Add(((IProvideHealthCheckWithMessage)healthCheck).Check(message)); + } + else + { + results.Add(healthCheck.Check()); + } + } foreach (var result in results) { @@ -151,8 +162,7 @@ namespace NzbDrone.Core.HealthCheck } // TODO: Add debounce - - PerformHealthCheck(filteredChecks.ToArray()); + PerformHealthCheck(filteredChecks.ToArray(), message); } } } diff --git a/src/NzbDrone.Core/HealthCheck/IProvideHealthCheck.cs b/src/NzbDrone.Core/HealthCheck/IProvideHealthCheck.cs index d71f2653f..46e7048ef 100644 --- a/src/NzbDrone.Core/HealthCheck/IProvideHealthCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/IProvideHealthCheck.cs @@ -1,3 +1,5 @@ +using NzbDrone.Common.Messaging; + namespace NzbDrone.Core.HealthCheck { public interface IProvideHealthCheck @@ -6,4 +8,9 @@ namespace NzbDrone.Core.HealthCheck bool CheckOnStartup { get; } bool CheckOnSchedule { get; } } + + public interface IProvideHealthCheckWithMessage : IProvideHealthCheck + { + HealthCheck Check(IEvent message); + } } diff --git a/src/NzbDrone.Core/MediaFiles/DownloadedTracksImportService.cs b/src/NzbDrone.Core/MediaFiles/DownloadedTracksImportService.cs index 9f94163a0..6e8abeb4c 100644 --- a/src/NzbDrone.Core/MediaFiles/DownloadedTracksImportService.cs +++ b/src/NzbDrone.Core/MediaFiles/DownloadedTracksImportService.cs @@ -6,7 +6,9 @@ using NLog; using NzbDrone.Common.Disk; using NzbDrone.Core.DecisionEngine; using NzbDrone.Core.Download; +using NzbDrone.Core.MediaFiles.Events; using NzbDrone.Core.MediaFiles.TrackImport; +using NzbDrone.Core.Messaging.Events; using NzbDrone.Core.Music; using NzbDrone.Core.Parser; using NzbDrone.Core.Parser.Model; @@ -28,15 +30,17 @@ namespace NzbDrone.Core.MediaFiles private readonly IParsingService _parsingService; private readonly IMakeImportDecision _importDecisionMaker; private readonly IImportApprovedTracks _importApprovedTracks; + private readonly IEventAggregator _eventAggregator; private readonly Logger _logger; public DownloadedTracksImportService(IDiskProvider diskProvider, - IDiskScanService diskScanService, - IArtistService artistService, - IParsingService parsingService, - IMakeImportDecision importDecisionMaker, - IImportApprovedTracks importApprovedTracks, - Logger logger) + IDiskScanService diskScanService, + IArtistService artistService, + IParsingService parsingService, + IMakeImportDecision importDecisionMaker, + IImportApprovedTracks importApprovedTracks, + IEventAggregator eventAggregator, + Logger logger) { _diskProvider = diskProvider; _diskScanService = diskScanService; @@ -44,6 +48,7 @@ namespace NzbDrone.Core.MediaFiles _parsingService = parsingService; _importDecisionMaker = importDecisionMaker; _importApprovedTracks = importApprovedTracks; + _eventAggregator = eventAggregator; _logger = logger; } @@ -93,6 +98,8 @@ namespace NzbDrone.Core.MediaFiles } _logger.Error("Import failed, path does not exist or is not accessible by Lidarr: {0}", path); + _eventAggregator.PublishEvent(new TrackImportFailedEvent(null, null, true, downloadClientItem)); + return new List(); } diff --git a/src/NzbDrone.Core/MediaFiles/TrackImport/ImportApprovedTracks.cs b/src/NzbDrone.Core/MediaFiles/TrackImport/ImportApprovedTracks.cs index 5f672e8cf..27eadafe2 100644 --- a/src/NzbDrone.Core/MediaFiles/TrackImport/ImportApprovedTracks.cs +++ b/src/NzbDrone.Core/MediaFiles/TrackImport/ImportApprovedTracks.cs @@ -231,6 +231,8 @@ namespace NzbDrone.Core.MediaFiles.TrackImport catch (UnauthorizedAccessException e) { _logger.Warn(e, "Couldn't import track " + localTrack); + _eventAggregator.PublishEvent(new TrackImportFailedEvent(e, localTrack, !localTrack.ExistingFile, downloadClientItem)); + importResults.Add(new ImportResult(importDecision, "Failed to import track, Permissions error")); } catch (Exception e) diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index afc14efda..9a8e1f2af 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -504,6 +504,7 @@ + diff --git a/src/NzbDrone.Core/RemotePathMappings/RemotePathMappingRepository.cs b/src/NzbDrone.Core/RemotePathMappings/RemotePathMappingRepository.cs index a7df3c35a..4a20f1576 100644 --- a/src/NzbDrone.Core/RemotePathMappings/RemotePathMappingRepository.cs +++ b/src/NzbDrone.Core/RemotePathMappings/RemotePathMappingRepository.cs @@ -16,6 +16,13 @@ namespace NzbDrone.Core.RemotePathMappings { } + public new void Delete(int id) + { + var model = Get(id); + base.Delete(id); + ModelDeleted(model); + } + protected override bool PublishModelEvents => true; } -} \ No newline at end of file +}