Add --download-migration-logs option to generate-script command for g…

…ei and ado2gh (#382)

* Add --download-migration-logs option to generate-script.

* Wrap a couple commands in Exec in GenerateScriptCommand.

* Remove unused param to MigrateGithubRepoScript in GenerateScriptCommand.

* Add more tests for GenerateScriptCommand with migration logs.

* Remove extra param from MigrateGithubRepoScript calls.

* Add release notes for --download-migration-logs option.

* Add --download-migration-logs to ado2gh generate-script.

* Always check exit code of migrate-repo in parallel ADO scripts.

* Add tests for downloading migration logs to GenerateScriptCommandTests.

* Pass --download-migration-logs to integration tests.

* Fix possibly null variable in GenerateScriptCommandTests.

* Add DownloadMigrationLogs to GenerateScriptOptions in GenerateScriptCommand.

* Remove some unused parameters from GenerateScriptCommand.

* Include download-logs command ado2gh generate-script's --all.

* Add AssertMigrationLogFileExists integration test helper.

* Add ado2gh integration tests to ensure migration logs exist.

* Add gei integration tests to ensure migration logs exist.

* Use better log message for AssertMigrationLogFileExists helper.

* Use async Task in AssertMigrationLogFileExists helper.

* Add --wait option to download-logs command for gei.

* Add --wait option to download-logs command for ado2gh.

* Add --wait to download-logs in gei GenerateScriptCommand.

* Add --wait to download-logs in ado2gh GenerateScriptCommand.

* Remove unnessary assignments from DownloadLogsCommand classes.

* Fix style to ifs in DownloadLogsCommand classes.

* Un-hide download-logs commands from gei and ado2gh.

* Update gei DownloadLogsCommand to always retry with a timeout.

* Update ado2gh DownloadLogsCommand to always retry with a timeout.

* Add comment to timeoutMinutes var in DownloadLogsCommandTests.

* Check for --timeout-minutes param in DownloadLogsCommandTests.

* Don't pass --wait to download-logs in GenerateScriptCommand.

* Don't pass --wait to download-logs in ado2gh GenerateScriptCommand.

* Use Pascal case in names for all tests.

* Use var instead of explicit type in DownloadLogsCommand classes.

* Add readonly to DateTimeNow Funcs.

* Fix Pascal case on a couple DownloadLogsCommand tests.

* Use hard-coded timeout in gei DownloadLogsCommand.

* Remove unused variable in gei DownloadLogsCommandTests.

* Use hard-coded timeout in ado2gh DownloadLogsCommand.

* Use constants in DownloadLogsCommandTests test classes.

* Add newline in a test for gei DownloadLogsCommand.

* Update grammar in DownloadLogsCommand classes.

* Use syntactic sugar null check in DownloadLogsCommand classes.

* Remove async from AssertMigrationLogFileExists.

* Use BeTrue() in AssertMigrationLogFileExists.

* Use implicit type in for loops in DownloadLogsCommand classes.

* Remove redundant cast to string in DownloadLogsCommandTests classes.

* Add 24h availability warning to gei DownloadLogsCommand.

* Add 24h availability warning to ado2gh DownloadLogsCommand.

* Remove null params to Invoke in DownloadLogsCommandTests classes.

* Remove --download-migration-logs from ado2gh integration tests.

* Don't include org name in start of migration log output.

* Don't test for log output in DownloadLogsCommand classes.

* Call download-logs command after other ado2gh commands.

co-authored-by: Dylan Smith <[email protected]>

* Add DOWNLOAD MIGRATION LOGS output in ado2gh generate-script.

* adopted Polly for retry logic

* Refactor gei test for single ADO repo with migration logs.

* Refactor gei test for ADO server with single repo and migration logs.

* Refactor remaining gei GenerateScriptCommandTests.

* Join GetOsDistPath() in AssertMigrationLogFileExists.

* Make _httpRetryInterval readonly in RetryPolicy.

* Use Polly in gei DownloadLogsCommand.

* Remove unused "using" in DownloadLogsCommandTests.

* Fix broken tests from merging main.

* Add verbose logs to HttpDownloadService.

* Fix curly location in HttpDownloadService.

Co-authored-by: Dylan Smith <[email protected]>

RELEASENOTES.md

@@ -1 +1,3 @@
+- Add `download-logs` command to download migration logs
+- Add `--download-migration-logs` option to `generate-script` command
 - Log GitHub request id into the verbose log for each GitHub API call (this can be useful for GitHub support if something goes wrong)

src/Octoshift/GithubApi.cs

@@ -86,7 +86,7 @@ public virtual async Task<IEnumerable<string>> GetTeamMembers(string org, string
         {
             var url = $"{_apiUrl}/orgs/{org}/teams/{teamSlug}/members?per_page=100";
 
-            return await _retryPolicy.Retry(async () => await _client.GetAllAsync(url).Select(x => (string)x["login"]).ToListAsync(),
+            return await _retryPolicy.HttpRetry(async () => await _client.GetAllAsync(url).Select(x => (string)x["login"]).ToListAsync(),
                                             ex => ex.StatusCode == HttpStatusCode.NotFound);
         }
 
@@ -290,7 +290,7 @@ public virtual async Task<string> GetMigrationState(string migrationId)
 
             var payload = new { query = $"{query} {{ {gql} }}", variables = new { id = migrationId } };
 
-            var response = await _retryPolicy.Retry(async () => await _client.PostAsync(url, payload),
+            var response = await _retryPolicy.HttpRetry(async () => await _client.PostAsync(url, payload),
                                                     ex => ex.StatusCode == HttpStatusCode.BadGateway);
             var data = JObject.Parse(response);
 
@@ -349,7 +349,7 @@ public virtual async Task<string> GetMigrationFailureReason(string migrationId)
 
             var payload = new { query = $"{query} {{ {gql} }}", variables = new { id = migrationId } };
 
-            var response = await _retryPolicy.Retry(async () => await _client.PostAsync(url, payload),
+            var response = await _retryPolicy.HttpRetry(async () => await _client.PostAsync(url, payload),
                                                     ex => ex.StatusCode == HttpStatusCode.BadGateway);
             var data = JObject.Parse(response);
 

src/Octoshift/HttpDownloadService.cs

@@ -11,13 +11,22 @@ public class HttpDownloadService
     {
         internal Func<string, string, Task> WriteToFile = async (path, contents) => await File.WriteAllTextAsync(path, contents);
 
+        private readonly OctoLogger _log;
         private readonly HttpClient _httpClient;
 
-        public HttpDownloadService(HttpClient httpClient) => _httpClient = httpClient;
+        public HttpDownloadService(OctoLogger log, HttpClient httpClient)
+        {
+            _log = log;
+            _httpClient = httpClient;
+        }
 
         public virtual async Task Download(string url, string file)
         {
+            _log.LogVerbose($"HTTP GET: {url}");
+
             using var response = await _httpClient.GetAsync(url);
+            _log.LogVerbose($"RESPONSE ({response.StatusCode}): <truncated>");
+
             response.EnsureSuccessStatusCode();
 
             var contents = await response.Content.ReadAsStringAsync();

src/Octoshift/RetryPolicy.cs

@@ -8,21 +8,34 @@ namespace OctoshiftCLI
     public class RetryPolicy
     {
         private readonly OctoLogger _log;
+        internal readonly int _httpRetryInterval = 1000;
+        internal int _retryOnResultInterval = 4000;
 
         public RetryPolicy(OctoLogger log)
         {
             _log = log;
         }
 
-        public async Task<T> Retry<T>(Func<Task<T>> func, Func<HttpRequestException, bool> filter)
+        public async Task<T> HttpRetry<T>(Func<Task<T>> func, Func<HttpRequestException, bool> filter)
         {
             var policy = Policy.Handle(filter)
-                               .WaitAndRetryAsync(5, retry => retry * TimeSpan.FromMilliseconds(1000), (ex, _) =>
+                               .WaitAndRetryAsync(5, retry => retry * TimeSpan.FromMilliseconds(_httpRetryInterval), (ex, _) =>
                                {
                                    _log.LogVerbose($"Call failed with HTTP {((HttpRequestException)ex).StatusCode}, retrying...");
                                });
 
             return await policy.ExecuteAsync(func);
         }
+
+        public async Task<PolicyResult<T>> RetryOnResult<T>(Func<Task<T>> func, T resultFilter, string retryLogMessage)
+        {
+            var policy = Policy.HandleResult(resultFilter)
+                               .WaitAndRetryAsync(5, retry => retry * TimeSpan.FromMilliseconds(_retryOnResultInterval), (_, _) =>
+                               {
+                                   _log.LogVerbose(retryLogMessage ?? "Retrying...");
+                               });
+
+            return await policy.ExecuteAndCaptureAsync(func);
+        }
     }
 }

src/OctoshiftCLI.IntegrationTests/AdoToGithub.cs

@@ -90,6 +90,8 @@ public async Task Basic()
             await _helper.AssertPipelineRewired(adoOrg, teamProject2, pipeline2, githubOrg, $"{teamProject2}-{teamProject2}");
             await _helper.AssertBoardsIntegrationConfigured(adoOrg, teamProject1);
             await _helper.AssertBoardsIntegrationConfigured(adoOrg, teamProject2);
+            _helper.AssertMigrationLogFileExists(githubOrg, $"{teamProject1}-{teamProject1}");
+            _helper.AssertMigrationLogFileExists(githubOrg, $"{teamProject2}-{teamProject2}");
         }
 
         protected virtual void Dispose(bool disposing)

src/OctoshiftCLI.IntegrationTests/GithubToGithub.cs

@@ -47,12 +47,15 @@ public async Task Basic()
             await _helper.CreateGithubRepo(githubSourceOrg, repo1);
             await _helper.CreateGithubRepo(githubSourceOrg, repo2);
 
-            await _helper.RunGeiCliMigration($"generate-script --github-source-org {githubSourceOrg} --github-target-org {githubTargetOrg}");
+            await _helper.RunGeiCliMigration($"generate-script --github-source-org {githubSourceOrg} --github-target-org {githubTargetOrg} --download-migration-logs");
 
             await _helper.AssertGithubRepoExists(githubTargetOrg, repo1);
             await _helper.AssertGithubRepoExists(githubTargetOrg, repo2);
             await _helper.AssertGithubRepoInitialized(githubTargetOrg, repo1);
             await _helper.AssertGithubRepoInitialized(githubTargetOrg, repo2);
+
+            _helper.AssertMigrationLogFileExists(githubTargetOrg, repo1);
+            _helper.AssertMigrationLogFileExists(githubTargetOrg, repo2);
         }
 
         protected virtual void Dispose(bool disposing)

src/OctoshiftCLI.IntegrationTests/TestHelper.cs

@@ -585,5 +585,14 @@ public async Task AssertBoardsIntegrationConfigured(string adoOrg, string teamPr
             boardsConnection.Should().NotBeNull();
             boardsConnection.repoIds.Count().Should().Be(1);
         }
+
+        public void AssertMigrationLogFileExists(string githubOrg, string repo)
+        {
+            _output.WriteLine("Checking that the migration log was downloaded...");
+
+            var migrationLogFile = Path.Join(GetOsDistPath(), $"migration-log-{githubOrg}-{repo}.log");
+
+            File.Exists(migrationLogFile).Should().BeTrue();
+        }
     }
 }

src/OctoshiftCLI.Tests/HttpDownloadServiceTests.cs

@@ -11,6 +11,8 @@ namespace OctoshiftCLI.Tests
 {
     public class HttpDownloadServiceTests
     {
+        private readonly Mock<OctoLogger> _mockOctoLogger = TestHelpers.CreateMock<OctoLogger>();
+
         [Fact]
         public async Task Downloads_File()
         {
@@ -38,7 +40,7 @@ public async Task Downloads_File()
             using var httpClient = new HttpClient(mockHttpHandler.Object);
 
             // Act
-            var httpDownloadService = new HttpDownloadService(httpClient)
+            var httpDownloadService = new HttpDownloadService(_mockOctoLogger.Object, httpClient)
             {
                 WriteToFile = (_, contents) =>
                 {
@@ -76,7 +78,7 @@ public async Task Raises_Exception_When_File_Cannot_Be_Downloaded()
             using var httpClient = new HttpClient(mockHttpHandler.Object);
 
             // Act
-            var httpDownloadService = new HttpDownloadService(httpClient)
+            var httpDownloadService = new HttpDownloadService(_mockOctoLogger.Object, httpClient)
             {
                 WriteToFile = (_, contents) =>
                 {