From c50cf02f7dbaac266e1c922cc7417f86298bd7dc Mon Sep 17 00:00:00 2001 From: benbierens Date: Fri, 31 Mar 2023 10:00:44 +0200 Subject: [PATCH 1/2] Simplifies node-metrics interface --- CodexDistTestCore/CodexNodeLog.cs | 7 ++++++- CodexDistTestCore/DistTest.cs | 20 +------------------- CodexDistTestCore/K8sManager.cs | 19 +++++++++++++++++++ CodexDistTestCore/MetricsAccess.cs | 25 +++++++++++++++---------- CodexDistTestCore/MetricsAggregator.cs | 15 ++++++--------- CodexDistTestCore/OnlineCodexNode.cs | 6 ++++-- Tests/BasicTests/SimpleTests.cs | 12 ++++-------- 7 files changed, 55 insertions(+), 49 deletions(-) diff --git a/CodexDistTestCore/CodexNodeLog.cs b/CodexDistTestCore/CodexNodeLog.cs index b4cae47..1a0572a 100644 --- a/CodexDistTestCore/CodexNodeLog.cs +++ b/CodexDistTestCore/CodexNodeLog.cs @@ -2,7 +2,12 @@ namespace CodexDistTestCore { - public class CodexNodeLog + public interface ICodexNodeLog + { + void AssertLogContains(string expectedString); + } + + public class CodexNodeLog : ICodexNodeLog { private readonly LogFile logFile; diff --git a/CodexDistTestCore/DistTest.cs b/CodexDistTestCore/DistTest.cs index c1d5c70..db32409 100644 --- a/CodexDistTestCore/DistTest.cs +++ b/CodexDistTestCore/DistTest.cs @@ -9,7 +9,6 @@ namespace CodexDistTestCore private TestLog log = null!; private FileManager fileManager = null!; private K8sManager k8sManager = null!; - private MetricsAggregator metricsAggregator = null!; [OneTimeSetUp] public void GlobalSetup() @@ -49,7 +48,6 @@ namespace CodexDistTestCore fileManager = new FileManager(log); k8sManager = new K8sManager(log, fileManager); - metricsAggregator = new MetricsAggregator(log, k8sManager); } } @@ -80,22 +78,6 @@ namespace CodexDistTestCore return new OfflineCodexNodes(k8sManager, numberOfNodes); } - public MetricsAccess GatherMetrics(ICodexNodeGroup group) - { - return GatherMetrics(group.ToArray()); - } - - public MetricsAccess GatherMetrics(params IOnlineCodexNode[] nodes) - { - var onlineNodes = nodes.Cast().ToArray(); - - Assert.That(onlineNodes.All(n => n.Group.Origin.MetricsEnabled), - "Incorrect test setup: Metrics were not enabled on (all) provided OnlineCodexNodes. " + - "To use metrics, please use 'EnableMetrics()' when setting up Codex nodes."); - - return metricsAggregator.BeginCollectingMetricsFor(onlineNodes); - } - private void IncludeLogsAndMetricsOnTestFailure() { var result = TestContext.CurrentContext.Result; @@ -105,7 +87,7 @@ namespace CodexDistTestCore { log.Log("Downloading all CodexNode logs and metrics because of test failure..."); k8sManager.ForEachOnlineGroup(DownloadLogs); - metricsAggregator.DownloadAllMetrics(); + k8sManager.DownloadAllMetrics(); } else { diff --git a/CodexDistTestCore/K8sManager.cs b/CodexDistTestCore/K8sManager.cs index f9dace8..f92c628 100644 --- a/CodexDistTestCore/K8sManager.cs +++ b/CodexDistTestCore/K8sManager.cs @@ -14,11 +14,13 @@ private readonly KnownK8sPods knownPods = new KnownK8sPods(); private readonly TestLog log; private readonly IFileManager fileManager; + private readonly MetricsAggregator metricsAggregator; public K8sManager(TestLog log, IFileManager fileManager) { this.log = log; this.fileManager = fileManager; + metricsAggregator = new MetricsAggregator(log, this); } public ICodexNodeGroup BringOnline(OfflineCodexNodes offline) @@ -29,6 +31,11 @@ log.Log($"{online.Describe()} online."); + if (offline.MetricsEnabled) + { + BringOnlineMetrics(online); + } + return online; } @@ -67,6 +74,18 @@ return info!; } + public void DownloadAllMetrics() + { + metricsAggregator.DownloadAllMetrics(); + } + + private void BringOnlineMetrics(CodexNodeGroup group) + { + var onlineNodes = group.Nodes.Cast().ToArray(); + + metricsAggregator.BeginCollectingMetricsFor(onlineNodes); + } + private CodexNodeGroup CreateOnlineCodexNodes(OfflineCodexNodes offline) { var containers = CreateContainers(offline); diff --git a/CodexDistTestCore/MetricsAccess.cs b/CodexDistTestCore/MetricsAccess.cs index dbda2a8..feb3fe7 100644 --- a/CodexDistTestCore/MetricsAccess.cs +++ b/CodexDistTestCore/MetricsAccess.cs @@ -5,27 +5,32 @@ namespace CodexDistTestCore { public interface IMetricsAccess { - void AssertThat(IOnlineCodexNode node, string metricName, IResolveConstraint constraint, string message = ""); + void AssertThat(string metricName, IResolveConstraint constraint, string message = ""); + } + + public class MetricsUnavailable : IMetricsAccess + { + public void AssertThat(string metricName, IResolveConstraint constraint, string message = "") + { + Assert.Fail("Incorrect test setup: Metrics were not enabled for this group of Codex nodes. Add 'EnableMetrics()' after 'SetupCodexNodes()' to enable it."); + throw new InvalidOperationException(); + } } public class MetricsAccess : IMetricsAccess { private readonly MetricsQuery query; - private readonly OnlineCodexNode[] nodes; + private readonly OnlineCodexNode node; - public MetricsAccess(MetricsQuery query, OnlineCodexNode[] nodes) + public MetricsAccess(MetricsQuery query, OnlineCodexNode node) { this.query = query; - this.nodes = nodes; + this.node = node; } - public void AssertThat(IOnlineCodexNode node, string metricName, IResolveConstraint constraint, string message = "") + public void AssertThat(string metricName, IResolveConstraint constraint, string message = "") { - var n = (OnlineCodexNode)node; - CollectionAssert.Contains(nodes, n, "Incorrect test setup: Attempt to get metrics for OnlineCodexNode from the wrong MetricsAccess object. " + - "(This CodexNode is tracked by a different instance.)"); - - var metricSet = GetMetricWithTimeout(metricName, n); + var metricSet = GetMetricWithTimeout(metricName, node); var metricValue = metricSet.Values[0].Value; Assert.That(metricValue, constraint, message); } diff --git a/CodexDistTestCore/MetricsAggregator.cs b/CodexDistTestCore/MetricsAggregator.cs index 7b08f3b..59e884d 100644 --- a/CodexDistTestCore/MetricsAggregator.cs +++ b/CodexDistTestCore/MetricsAggregator.cs @@ -16,15 +16,8 @@ namespace CodexDistTestCore this.k8sManager = k8sManager; } - public MetricsAccess BeginCollectingMetricsFor(OnlineCodexNode[] nodes) + public void BeginCollectingMetricsFor(OnlineCodexNode[] nodes) { - var alreadyStartedNodes = nodes.Where(n => activePrometheuses.Values.Any(v => v.Contains(n))); - if (alreadyStartedNodes.Any()) - { - Assert.Fail("Incorrect test setup: 'GatherMetrics' was already called on one or more of these OnlineCodexNodes."); - throw new InvalidOperationException(); - } - log.Log($"Starting metrics collecting for {nodes.Length} nodes..."); var config = GeneratePrometheusConfig(nodes); @@ -33,7 +26,11 @@ namespace CodexDistTestCore activePrometheuses.Add(query, nodes); log.Log("Metrics service started."); - return new MetricsAccess(query, nodes); + + foreach(var node in nodes) + { + node.Metrics = new MetricsAccess(query, node); + } } public void DownloadAllMetrics() diff --git a/CodexDistTestCore/OnlineCodexNode.cs b/CodexDistTestCore/OnlineCodexNode.cs index f8cf90f..a9cf511 100644 --- a/CodexDistTestCore/OnlineCodexNode.cs +++ b/CodexDistTestCore/OnlineCodexNode.cs @@ -9,7 +9,8 @@ namespace CodexDistTestCore ContentId UploadFile(TestFile file); TestFile? DownloadContent(ContentId contentId); void ConnectToPeer(IOnlineCodexNode node); - CodexNodeLog DownloadLog(); + ICodexNodeLog DownloadLog(); + IMetricsAccess Metrics { get; } } public class OnlineCodexNode : IOnlineCodexNode @@ -30,6 +31,7 @@ namespace CodexDistTestCore public CodexNodeContainer Container { get; } public CodexNodeGroup Group { get; internal set; } = null!; + public IMetricsAccess Metrics { get; set; } = new MetricsUnavailable(); public string GetName() { @@ -80,7 +82,7 @@ namespace CodexDistTestCore Log($"Successfully connected to peer {peer.GetName()}."); } - public CodexNodeLog DownloadLog() + public ICodexNodeLog DownloadLog() { return Group.DownloadLog(this); } diff --git a/Tests/BasicTests/SimpleTests.cs b/Tests/BasicTests/SimpleTests.cs index 9a8c5c8..efb410a 100644 --- a/Tests/BasicTests/SimpleTests.cs +++ b/Tests/BasicTests/SimpleTests.cs @@ -37,13 +37,9 @@ namespace Tests.BasicTests .EnableMetrics() .BringOnline(); - var metrics = GatherMetrics(group); - var group2 = SetupCodexNodes(2) - .EnableMetrics() - .BringOnline(); - - var metrics2 = GatherMetrics(group2); + .EnableMetrics() + .BringOnline(); var primary = group[0]; var secondary = group[1]; @@ -55,8 +51,8 @@ namespace Tests.BasicTests Thread.Sleep(TimeSpan.FromMinutes(5)); - metrics.AssertThat(primary, "libp2p_peers", Is.EqualTo(1)); - metrics2.AssertThat(primary2, "libp2p_peers", Is.EqualTo(1)); + primary.Metrics.AssertThat("libp2p_peers", Is.EqualTo(1)); + primary2.Metrics.AssertThat("libp2p_peers", Is.EqualTo(1)); } [Test] From 6af0890571b1fb196b63f27da91567c2b06084d3 Mon Sep 17 00:00:00 2001 From: benbierens Date: Fri, 31 Mar 2023 10:37:55 +0200 Subject: [PATCH 2/2] Ignores two-location test for now --- Tests/BasicTests/SimpleTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/BasicTests/SimpleTests.cs b/Tests/BasicTests/SimpleTests.cs index efb410a..ae72183 100644 --- a/Tests/BasicTests/SimpleTests.cs +++ b/Tests/BasicTests/SimpleTests.cs @@ -91,6 +91,7 @@ namespace Tests.BasicTests } [Test] + [Ignore("Requires Location map to be configured for k8s cluster.")] public void TwoClientsTwoLocationsTest() { var primary = SetupCodexNodes(1)