From d58cb38c79346d5f953c3dbb4cee79cab3ea0eae Mon Sep 17 00:00:00 2001 From: benbierens Date: Wed, 10 May 2023 09:55:36 +0200 Subject: [PATCH] Cleanup duplicated retry logic. Adds layered and chain tests --- DistTestCore/DistTest.cs | 14 +++- DistTestCore/Http.cs | 20 +---- .../Marketplace/ContainerInfoExtractor.cs | 29 ++----- DistTestCore/Timing.cs | 21 +++-- .../LayeredDiscoveryTests.cs | 51 ++++++++++++ .../PeerDiscoveryTests/PeerDiscoveryTests.cs | 77 +++---------------- Tests/PeerDiscoveryTests/PeerTestHelpers.cs | 57 ++++++++++++++ Utils/Time.cs | 69 +++++++++++++++++ 8 files changed, 214 insertions(+), 124 deletions(-) create mode 100644 Tests/PeerDiscoveryTests/LayeredDiscoveryTests.cs create mode 100644 Tests/PeerDiscoveryTests/PeerTestHelpers.cs diff --git a/DistTestCore/DistTest.cs b/DistTestCore/DistTest.cs index e5ee885..f0280b1 100644 --- a/DistTestCore/DistTest.cs +++ b/DistTestCore/DistTest.cs @@ -128,16 +128,26 @@ namespace DistTestCore return Get().CodexStarter.BringOnline((CodexSetup)codexSetup); } + public IEnumerable GetAllOnlineCodexNodes() + { + return Get().CodexStarter.RunningGroups.SelectMany(g => g.Nodes); + } + + protected BaseLog GetTestLog() + { + return Get().Log; + } + protected void Log(string msg) { TestContext.Progress.WriteLine(msg); - Get().Log.Log(msg); + GetTestLog().Log(msg); } protected void Debug(string msg) { TestContext.Progress.WriteLine(msg); - Get().Log.Debug(msg); + GetTestLog().Debug(msg); } private TestLifecycle Get() diff --git a/DistTestCore/Http.cs b/DistTestCore/Http.cs index 3dfee9d..bd9f34c 100644 --- a/DistTestCore/Http.cs +++ b/DistTestCore/Http.cs @@ -107,25 +107,7 @@ namespace DistTestCore private T Retry(Func operation) { - var retryCounter = 0; - - while (true) - { - try - { - return operation(); - } - catch (Exception exception) - { - timeSet.HttpCallRetryDelay(); - retryCounter++; - if (retryCounter > timeSet.HttpCallRetryCount()) - { - Assert.Fail(exception.ToString()); - throw; - } - } - } + return Time.Retry(operation, timeSet.HttpCallRetryTimeout(), timeSet.HttpCallRetryDelay()); } private static T TryJsonDeserialize(string json) diff --git a/DistTestCore/Marketplace/ContainerInfoExtractor.cs b/DistTestCore/Marketplace/ContainerInfoExtractor.cs index f99827b..5f6db11 100644 --- a/DistTestCore/Marketplace/ContainerInfoExtractor.cs +++ b/DistTestCore/Marketplace/ContainerInfoExtractor.cs @@ -56,30 +56,6 @@ namespace DistTestCore.Marketplace return marketplaceAbi; } - private string Retry(Func fetch) - { - var result = string.Empty; - Time.WaitUntil(() => - { - result = Catch(fetch); - return !string.IsNullOrEmpty(result); - }, TimeSpan.FromMinutes(1), TimeSpan.FromSeconds(3)); - - return result; - } - - private string Catch(Func fetch) - { - try - { - return fetch(); - } - catch - { - return string.Empty; - } - } - private string FetchAccountsCsv() { return workflow.ExecuteCommand(container, "cat", GethContainerRecipe.AccountsFilename); @@ -116,6 +92,11 @@ namespace DistTestCore.Marketplace var privateKey = tokens[1]; return new GethAccount(account, privateKey); } + + private static string Retry(Func fetch) + { + return Time.Retry(fetch); + } } public class PubKeyFinder : LogHandler, ILogHandler diff --git a/DistTestCore/Timing.cs b/DistTestCore/Timing.cs index 9771767..e3f7414 100644 --- a/DistTestCore/Timing.cs +++ b/DistTestCore/Timing.cs @@ -1,5 +1,4 @@ using NUnit.Framework; -using Utils; namespace DistTestCore { @@ -11,8 +10,8 @@ namespace DistTestCore public interface ITimeSet { TimeSpan HttpCallTimeout(); - int HttpCallRetryCount(); - void HttpCallRetryDelay(); + TimeSpan HttpCallRetryTimeout(); + TimeSpan HttpCallRetryDelay(); TimeSpan WaitForK8sServiceDelay(); TimeSpan K8sOperationTimeout(); TimeSpan WaitForMetricTimeout(); @@ -25,14 +24,14 @@ namespace DistTestCore return TimeSpan.FromSeconds(10); } - public int HttpCallRetryCount() + public TimeSpan HttpCallRetryTimeout() { - return 5; + return TimeSpan.FromSeconds(10); } - public void HttpCallRetryDelay() + public TimeSpan HttpCallRetryDelay() { - Time.Sleep(TimeSpan.FromSeconds(3)); + return TimeSpan.FromSeconds(3); } public TimeSpan WaitForK8sServiceDelay() @@ -58,14 +57,14 @@ namespace DistTestCore return TimeSpan.FromHours(2); } - public int HttpCallRetryCount() + public TimeSpan HttpCallRetryTimeout() { - return 2; + return TimeSpan.FromHours(5); } - public void HttpCallRetryDelay() + public TimeSpan HttpCallRetryDelay() { - Time.Sleep(TimeSpan.FromMinutes(5)); + return TimeSpan.FromMinutes(5); } public TimeSpan WaitForK8sServiceDelay() diff --git a/Tests/PeerDiscoveryTests/LayeredDiscoveryTests.cs b/Tests/PeerDiscoveryTests/LayeredDiscoveryTests.cs new file mode 100644 index 0000000..0be9db1 --- /dev/null +++ b/Tests/PeerDiscoveryTests/LayeredDiscoveryTests.cs @@ -0,0 +1,51 @@ +using DistTestCore; +using NUnit.Framework; + +namespace Tests.PeerDiscoveryTests +{ + [TestFixture] + public class LayeredDiscoveryTests : DistTest + { + [Test] + public void TwoLayersTest() + { + var root = SetupCodexNode(); + var l1Source = SetupCodexNode(s => s.WithBootstrapNode(root)); + var l1Node = SetupCodexNode(s => s.WithBootstrapNode(root)); + var l2Target = SetupCodexNode(s => s.WithBootstrapNode(l1Node)); + + AssertAllNodesConnected(); + } + + [Test] + public void ThreeLayersTest() + { + var root = SetupCodexNode(); + var l1Source = SetupCodexNode(s => s.WithBootstrapNode(root)); + var l1Node = SetupCodexNode(s => s.WithBootstrapNode(root)); + var l2Node = SetupCodexNode(s => s.WithBootstrapNode(l1Node)); + var l3Target = SetupCodexNode(s => s.WithBootstrapNode(l2Node)); + + AssertAllNodesConnected(); + } + + [TestCase(3)] + [TestCase(5)] + [TestCase(10)] + public void NodeChainTest(int chainLength) + { + var node = SetupCodexNode(); + for (var i = 1; i < chainLength; i++) + { + node = SetupCodexNode(s => s.WithBootstrapNode(node)); + } + + AssertAllNodesConnected(); + } + + private void AssertAllNodesConnected() + { + PeerTestHelpers.AssertFullyConnected(GetAllOnlineCodexNodes(), GetTestLog()); + } + } +} diff --git a/Tests/PeerDiscoveryTests/PeerDiscoveryTests.cs b/Tests/PeerDiscoveryTests/PeerDiscoveryTests.cs index 2b3e4ff..9d5c735 100644 --- a/Tests/PeerDiscoveryTests/PeerDiscoveryTests.cs +++ b/Tests/PeerDiscoveryTests/PeerDiscoveryTests.cs @@ -1,7 +1,5 @@ -using DistTestCore.Codex; -using DistTestCore; +using DistTestCore; using NUnit.Framework; -using Utils; namespace Tests.PeerDiscoveryTests { @@ -11,9 +9,9 @@ namespace Tests.PeerDiscoveryTests [Test] public void TwoNodes() { - var node = SetupCodexNode(); + SetupCodexNode(); - AssertKnowEachother(BootstrapNode, node); + AssertAllNodesConnected(); } [TestCase(2)] @@ -21,9 +19,9 @@ namespace Tests.PeerDiscoveryTests [TestCase(10)] public void VariableNodes(int number) { - var nodes = SetupCodexNodes(number); + SetupCodexNodes(number); - AssertFullyConnected(nodes); + AssertAllNodesConnected(); } [TestCase(2)] @@ -31,74 +29,17 @@ namespace Tests.PeerDiscoveryTests [TestCase(10)] public void VariableNodesInPods(int number) { - var bootstrap = SetupCodexBootstrapNode(); - - var nodes = new List(); for (var i = 0; i < number; i++) { - nodes.Add(SetupCodexNode(s => s.WithBootstrapNode(bootstrap))); + SetupCodexNode(); } - AssertFullyConnected(nodes); + AssertAllNodesConnected(); } - private void AssertFullyConnected(IEnumerable nodes) + private void AssertAllNodesConnected() { - Retry(() => - { - var array = nodes.ToArray(); - - foreach (var node in array) AssertKnowEachother(node, BootstrapNode); - - for (var x = 0; x < array.Length; x++) - { - for (var y = x + 1; y < array.Length; y++) - { - AssertKnowEachother(array[x], array[y]); - } - } - }); - } - - private static void Retry(Action action) - { - try - { - action(); - return; - } - catch - { - Time.Sleep(TimeSpan.FromMinutes(1)); - } - - action(); - } - - private void AssertKnowEachother(IOnlineCodexNode a, IOnlineCodexNode b) - { - AssertKnowEachother(a.GetDebugInfo(), b.GetDebugInfo()); - } - - private void AssertKnowEachother(CodexDebugResponse a, CodexDebugResponse b) - { - AssertKnows(a, b); - AssertKnows(b, a); - } - - private void AssertKnows(CodexDebugResponse a, CodexDebugResponse b) - { - //var enginePeers = string.Join(",", a.enginePeers.Select(p => p.peerId)); - //var switchPeers = string.Join(",", a.switchPeers.Select(p => p.peerId)); - var tableNodes = string.Join(",", a.table.nodes.Select(n => n.nodeId)); - - //Debug($"{a.id} is looking for {b.id} in engine-peers [{enginePeers}]"); - //Debug($"{a.id} is looking for {b.id} in switch-peers [{switchPeers}]"); - Debug($"{a.table.localNode.nodeId} is looking for {b.table.localNode.nodeId} in table-nodes [{tableNodes}]"); - - //Assert.That(a.enginePeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in engine-peers [{enginePeers}] but it was not found."); - //Assert.That(a.switchPeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in switch-peers [{switchPeers}] but it was not found."); - Assert.That(a.table.nodes.Any(n => n.nodeId == b.table.localNode.nodeId), $"{a.table.localNode.nodeId} was looking for '{b.table.localNode.nodeId}' in table-nodes [{tableNodes}] but it was not found."); + PeerTestHelpers.AssertFullyConnected(GetAllOnlineCodexNodes(), GetTestLog()); } } } diff --git a/Tests/PeerDiscoveryTests/PeerTestHelpers.cs b/Tests/PeerDiscoveryTests/PeerTestHelpers.cs new file mode 100644 index 0000000..6b33ddd --- /dev/null +++ b/Tests/PeerDiscoveryTests/PeerTestHelpers.cs @@ -0,0 +1,57 @@ +using DistTestCore.Codex; +using DistTestCore; +using NUnit.Framework; +using Utils; +using Logging; + +namespace Tests.PeerDiscoveryTests +{ + public static class PeerTestHelpers + { + public static void AssertFullyConnected(IEnumerable nodes, BaseLog? log = null) + { + AssertFullyConnected(log, nodes.ToArray()); + } + + public static void AssertFullyConnected(BaseLog? log = null, params IOnlineCodexNode[] nodes) + { + Time.Retry(() => + { + for (var x = 0; x < nodes.Length; x++) + { + for (var y = x + 1; y < nodes.Length; y++) + { + AssertKnowEachother(nodes[x], nodes[y], log); + } + } + }); + } + + private static void AssertKnowEachother(IOnlineCodexNode a, IOnlineCodexNode b, BaseLog? log) + { + AssertKnowEachother(a.GetDebugInfo(), b.GetDebugInfo(), log); + } + + private static void AssertKnowEachother(CodexDebugResponse a, CodexDebugResponse b, BaseLog? log) + { + AssertKnows(a, b, log); + AssertKnows(b, a, log); + } + + private static void AssertKnows(CodexDebugResponse a, CodexDebugResponse b, BaseLog? log) + { + //var enginePeers = string.Join(",", a.enginePeers.Select(p => p.peerId)); + //var switchPeers = string.Join(",", a.switchPeers.Select(p => p.peerId)); + var tableNodes = string.Join(",", a.table.nodes.Select(n => n.nodeId)); + + if (log != null) + { + log.Debug($"{a.table.localNode.nodeId} is looking for {b.table.localNode.nodeId} in table-nodes [{tableNodes}]"); + } + + //Assert.That(a.enginePeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in engine-peers [{enginePeers}] but it was not found."); + //Assert.That(a.switchPeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in switch-peers [{switchPeers}] but it was not found."); + Assert.That(a.table.nodes.Any(n => n.nodeId == b.table.localNode.nodeId), $"{a.table.localNode.nodeId} was looking for '{b.table.localNode.nodeId}' in table-nodes [{tableNodes}] but it was not found."); + } + } +} diff --git a/Utils/Time.cs b/Utils/Time.cs index 0f4f71b..2002b9c 100644 --- a/Utils/Time.cs +++ b/Utils/Time.cs @@ -38,5 +38,74 @@ state = predicate(); } } + + public static void Retry(Action action) + { + Retry(action, TimeSpan.FromMinutes(1)); + } + + public static T Retry(Func action) + { + return Retry(action, TimeSpan.FromMinutes(1)); + } + + public static void Retry(Action action, TimeSpan timeout) + { + Retry(action, timeout, TimeSpan.FromSeconds(1)); + } + + public static T Retry(Func action, TimeSpan timeout) + { + return Retry(action, timeout, TimeSpan.FromSeconds(1)); + } + + public static void Retry(Action action, TimeSpan timeout, TimeSpan retryTime) + { + var start = DateTime.UtcNow; + var exceptions = new List(); + while (true) + { + if (DateTime.UtcNow - start > timeout) + { + throw new TimeoutException("Retry timed out.", new AggregateException(exceptions)); + } + + try + { + action(); + return; + } + catch (Exception ex) + { + exceptions.Add(ex); + } + + Sleep(retryTime); + } + } + + public static T Retry(Func action, TimeSpan timeout, TimeSpan retryTime) + { + var start = DateTime.UtcNow; + var exceptions = new List(); + while (true) + { + if (DateTime.UtcNow - start > timeout) + { + throw new TimeoutException("Retry timed out.", new AggregateException(exceptions)); + } + + try + { + return action(); + } + catch (Exception ex) + { + exceptions.Add(ex); + } + + Sleep(retryTime); + } + } } }