Fix Target throws an NRE when querying port name on container with IP…

… and no ports (#230) * Fix Target throws an NRE when querying port name on container with IP and no ports * Fix code and spec * Fix spec network port collision
akkadotnet · Oct 29, 2021 · 4445e59 · 4445e59
1 parent e3cb964
commit 4445e59
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 18 deletions.
diff --git a/...cluster.bootstrap/Akka.Management.Cluster.Bootstrap.Tests/ClusterBootstrapSettingsSpec.cs b/...cluster.bootstrap/Akka.Management.Cluster.Bootstrap.Tests/ClusterBootstrapSettingsSpec.cs
@@ -13,16 +13,15 @@
 
 namespace Akka.Management.Cluster.Bootstrap.Tests
 {
-    public class ClusterBootstrapSettingsSpec : TestKit.Xunit2.TestKit
+    public class ClusterBootstrapSettingsSpec
     {
-        private readonly Config _config = Config.Empty
-            .WithFallback(ClusterBootstrap.DefaultConfiguration())
-            .WithFallback(AkkaManagementProvider.DefaultConfiguration());
-
         [Fact(DisplayName = "ClusterBootstrapSettings should have expected defaults")]
         public void HaveExpectedDefaults()
         {
-            var settings = new ClusterBootstrapSettings(_config, NoLogger.Instance);
+            var config = ClusterBootstrap.DefaultConfiguration()
+                .WithFallback(AkkaManagementProvider.DefaultConfiguration());
+
+            var settings = new ClusterBootstrapSettings(config, NoLogger.Instance);
             settings.NewClusterEnabled.Should().BeTrue();
 
             settings.ContactPointDiscovery.ServiceName.Should().BeNull();

diff --git a/...nagement.Cluster.Bootstrap.Tests/ContactPoint/ClusterBootstrapAutostartIntegrationSpec.cs b/...nagement.Cluster.Bootstrap.Tests/ContactPoint/ClusterBootstrapAutostartIntegrationSpec.cs
@@ -28,6 +28,8 @@ namespace Akka.Management.Cluster.Bootstrap.Tests.ContactPoint
 {
     public class ClusterBootstrapAutostartIntegrationSpec : TestKit.Xunit2.TestKit
     {
+        private static readonly Config BaseConfig = 
+            ConfigurationFactory.ParseString("akka.remote.dot-netty.tcp.port = 0");
         private const int ClusterSize = 3;
         private const int ScaledSize = 3;
 
@@ -43,6 +45,7 @@ public class ClusterBootstrapAutostartIntegrationSpec : TestKit.Xunit2.TestKit
         private readonly int _terminatedSystemCount;
 
         public ClusterBootstrapAutostartIntegrationSpec(ITestOutputHelper output)
+            : base(BaseConfig, nameof(ClusterBootstrapAutostartIntegrationSpec), output)
         {
             _output = output;
 
@@ -144,7 +147,7 @@ private void HoconShouldBeInjected()
         {
             var exception = Record.Exception(() =>
             {
-                var settings = new ClusterBootstrapSettings(_systems[0].Settings.Config, Log);
+                var settings = new ClusterBootstrapSettings(_systems[0].Settings.Config, NoLogger.Instance);
             });
             exception.Should().BeNull();
         }

diff --git a/src/cluster.bootstrap/Akka.Management.Cluster.Bootstrap.Tests/InactiveAkkaManagementSpec.cs b/src/cluster.bootstrap/Akka.Management.Cluster.Bootstrap.Tests/InactiveAkkaManagementSpec.cs
@@ -25,6 +25,7 @@ public class InactiveAkkaManagementSpec : TestKit.Xunit2.TestKit
 akka.loglevel = DEBUG
 akka.actor.provider = cluster
 akka.discovery.method = config
+akka.remote.dot-netty.tcp.port = 0
 ")
             .WithFallback(DiscoveryProvider.DefaultConfiguration());
 

diff --git a/...er.bootstrap/Akka.Management.Cluster.Bootstrap.Tests/Internal/BootstrapCoordinatorSpec.cs b/...er.bootstrap/Akka.Management.Cluster.Bootstrap.Tests/Internal/BootstrapCoordinatorSpec.cs
@@ -29,6 +29,7 @@ public class BootstrapCoordinatorSpec : TestKit.Xunit2.TestKit
         private const string ServiceName = "bootstrap-coordinator-test-service";
         private static readonly Config Config = ConfigurationFactory.ParseString($@"
             akka.actor.provider = cluster
+            akka.remote.dot-netty.tcp.port = 0
             akka.management.cluster.bootstrap {{
                 contact-point-discovery.service-name = {ServiceName}
             }}")

diff --git a/src/coordination/kubernetes/Akka.Coordination.KubernetesApi.Tests/KubernetesApiSpec.cs b/src/coordination/kubernetes/Akka.Coordination.KubernetesApi.Tests/KubernetesApiSpec.cs
@@ -37,10 +37,12 @@ public class KubernetesApiSpec : TestKit.Xunit2.TestKit
         private const string ApiPath = "/apis/akka.io/v1/namespaces/lease/leases";
         private const string LeaseApiPath = ApiPath + "/" + LeaseName;
 
-        private static Config Config()
-            => ConfigurationFactory.ParseString("akka.loglevel=DEBUG");
+        private static readonly Config BaseConfig = 
+            ConfigurationFactory.ParseString(@"
+                akka.loglevel=DEBUG
+                akka.remote.dot-netty.tcp.port = 0");
 
-        public KubernetesApiSpec(ITestOutputHelper output) : base(Config(), nameof(KubernetesApiSpec), output)
+        public KubernetesApiSpec(ITestOutputHelper output) : base(BaseConfig, nameof(KubernetesApiSpec), output)
         {
             _wireMockServer = WireMockServer.Start();
 
@@ -60,9 +62,9 @@ public KubernetesApiSpec(ITestOutputHelper output) : base(Config(), nameof(Kuber
             _underTest = new MockKubernetesApi(Sys, _settings);
         }
 
-        protected override void Dispose(bool disposing)
+        protected override void AfterAll()
         {
-            base.Dispose(disposing);
+            base.AfterAll();
             _wireMockServer.Stop();
             Environment.SetEnvironmentVariable("KUBERNETES_SERVICE_HOST", null);
             Environment.SetEnvironmentVariable("KUBERNETES_SERVICE_PORT", null);

diff --git a/src/coordination/kubernetes/Akka.Coordination.KubernetesApi.Tests/LeaseActorSpec.cs b/src/coordination/kubernetes/Akka.Coordination.KubernetesApi.Tests/LeaseActorSpec.cs
@@ -571,7 +571,8 @@ private static Config Config()
             => ConfigurationFactory.ParseString(@"
                 akka.loglevel=DEBUG
                 akka.stdout-loglevel=DEBUG
-                akka.actor.debug.fsm=true")
+                akka.actor.debug.fsm=true
+                akka.remote.dot-netty.tcp.port = 0")
                 .WithFallback(Discovery.DiscoveryProvider.DefaultConfiguration())
                 .WithFallback(KubernetesLease.DefaultConfiguration);
 

diff --git a/src/discovery/aws/Akka.Discovery.AwsApi.Integration.Tests/AwsIntegrationSpec.cs b/src/discovery/aws/Akka.Discovery.AwsApi.Integration.Tests/AwsIntegrationSpec.cs
@@ -23,6 +23,7 @@ public class AwsIntegrationSpec : TestKit.Xunit2.TestKit
         private static Configuration.Config Config(LocalStackFixture fixture) => ConfigurationFactory.ParseString($@"
 akka {{
   actor.provider = ""cluster""
+  remote.dot-netty.tcp.port = 0
   discovery {{
     method = ""aws-api-ec2-tag-based""
     aws-api-ec2-tag-based {{

diff --git a/...covery/kubernetes/Akka.Discovery.KubernetesApi.Tests/KubernetesApiServiceDiscoverySpec.cs b/...covery/kubernetes/Akka.Discovery.KubernetesApi.Tests/KubernetesApiServiceDiscoverySpec.cs
@@ -64,6 +64,68 @@ public void TargetsCalculateCorrectResolvedTargetList()
             });
         }
 
+        [Fact(DisplayName = "Issue #223: Targets should ignore containers with IP with no ports if port name is queried")]
+        public void TargetsIgnoreContainersWithNoPorts()
+        {
+            var podList = new V1PodList(new List<V1Pod>
+            {
+                new V1Pod(
+                    spec: new V1PodSpec(new List<V1Container>
+                    {
+                        new V1Container("akka-cluster-tooling-example", ports:new List<V1ContainerPort>
+                        {
+                            new V1ContainerPort(10000, name:"akka-remote"),
+                            new V1ContainerPort(10001, name:"management"),
+                            new V1ContainerPort(10002, name:"http"),
+                        }),
+                        // Issue #223. If a pod container with an IP does not expose any port and port name is queried, Target will throw an NRE 
+                        new V1Container("akka-cluster-sidecar")
+                    }),
+                    status: new V1PodStatus(podIP: "172.17.0.4", phase:"Running", containerStatuses: new List<V1ContainerStatus>
+                    {
+                        new V1ContainerStatus
+                        {
+                            Name = "akka-cluster-tooling-example",
+                            Ready = true,
+                            State = new V1ContainerState(running:new V1ContainerStateRunning())
+                        },
+                        new V1ContainerStatus
+                        {
+                            Name = "akka-cluster-sidecar",
+                            Ready = true,
+                            State = new V1ContainerState(running:new V1ContainerStateRunning())
+                        },
+                    }),
+                    metadata: new V1ObjectMeta()),
+                new V1Pod(
+                    spec: new V1PodSpec(new List<V1Container>
+                    {
+                        // Issue #223. If a pod container with an IP does not expose any port and port name is queried, Target will throw an NRE
+                        new V1Container("akka-cluster-tooling-example")
+                    }),
+                    status: new V1PodStatus(podIP: "172.17.0.5", phase:"Running", containerStatuses: new List<V1ContainerStatus>
+                    {
+                        new V1ContainerStatus
+                        {
+                            Name = "akka-cluster-tooling-example",
+                            Ready = true,
+                            State = new V1ContainerState(running:new V1ContainerStateRunning())
+                        },
+                    }),
+                    metadata: new V1ObjectMeta()),
+            });
+
+            var result =
+                KubernetesApiServiceDiscovery.Targets(podList, "management", "default", "cluster.local", false, "akka-cluster-tooling-example");
+            result.Should().BeEquivalentTo(new List<ServiceDiscovery.ResolvedTarget>
+            {
+                new ServiceDiscovery.ResolvedTarget(
+                    host: "172-17-0-4.default.pod.cluster.local",
+                    port: 10001,
+                    address: IPAddress.Parse("172.17.0.4"))
+            });
+        }
+
         [Fact(DisplayName = "Targets should ignore deleted pods")]
         public void TargetsIgnoreDeletedPods()
         {
@@ -258,6 +320,8 @@ private string LoadResource(string resourceName)
             var assembly = this.GetType().Assembly;
             using(var stream = assembly.GetManifestResourceStream(resourceName))
             {
+                if(stream == null)
+                    throw new Exception($"Invalid test resource name, could not load {resourceName} from manifest");
                 using (var reader = new StreamReader(stream))
                 {
                     return reader.ReadToEnd();

diff --git a/src/discovery/kubernetes/Akka.Discovery.KubernetesApi/KubernetesApiServiceDiscovery.cs b/src/discovery/kubernetes/Akka.Discovery.KubernetesApi/KubernetesApiServiceDiscovery.cs
@@ -17,6 +17,7 @@
 using k8s;
 using k8s.Authentication;
 using k8s.Models;
+using Microsoft.Rest;
 
 #nullable enable
 namespace Akka.Discovery.KubernetesApi
@@ -153,9 +154,16 @@ public override async Task<Resolved> Lookup(Lookup lookup, TimeSpan resolveTimeo
                 int? maybePort = null;
                 if (portName != null)
                 {
-                    maybePort = itemSpec.Containers
+                    // Bugfix #223, container might not expose ports, therefore should be excluded if port name is queried
+                    var validContainers = itemSpec.Containers.Where(c => c.Ports != null); 
+                    var validPort = validContainers
                         .SelectMany(c => c.Ports)
-                        .FirstOrDefault(p => p.Name?.Contains(portName) ?? false)?.ContainerPort;
+                        .FirstOrDefault(p => p.Name?.Contains(portName) ?? false);
+
+                    if (validPort == null)
+                        continue;
+
+                    maybePort = validPort.ContainerPort;
                 }
 
                 var hostOrIp = rawIp ? ip : $"{ip.Replace('.', '-')}.{podNamespace}.pod.{podDomain}";

diff --git a/src/management/Akka.Http.Shim.Tests/HttpSpec.cs b/src/management/Akka.Http.Shim.Tests/HttpSpec.cs
@@ -25,7 +25,10 @@ namespace Akka.Http.Shim.Tests
 {
     public class HttpSpec : TestKit.Xunit2.TestKit
     {
-        public HttpSpec(ITestOutputHelper output) : base(Config.Empty, nameof(HttpSpec), output)
+        private static readonly Config BaseConfig = 
+            ConfigurationFactory.ParseString("akka.remote.dot-netty.tcp.port = 0");
+
+        public HttpSpec(ITestOutputHelper output) : base(BaseConfig, nameof(HttpSpec), output)
         { }
 
         [Fact]

diff --git a/src/management/Akka.Management.Tests/HealthCheckRoutesSpec.cs b/src/management/Akka.Management.Tests/HealthCheckRoutesSpec.cs
@@ -27,7 +27,7 @@ namespace Akka.Management.Tests
 {
     public class HealthCheckRoutesSpec : TestKit.Xunit2.TestKit
     {
-        private static Config Config = Config.Empty
+        private static Config Config = ConfigurationFactory.ParseString("akka.remote.dot-netty.tcp.port = 0")
             .WithFallback(Akka.Http.Dsl.Http.DefaultConfig())
             .WithFallback(AkkaManagementProvider.DefaultConfiguration());
 

diff --git a/src/management/Akka.Management.Tests/HealthCheckSpec.cs b/src/management/Akka.Management.Tests/HealthCheckSpec.cs
@@ -109,7 +109,10 @@ public Task<bool> Execute(CancellationToken token)
 
         private readonly ExtendedActorSystem _eas;
 
-        public HealthCheckSpec(ITestOutputHelper helper) : base(nameof(HealthCheckSpec), helper)
+        private static readonly Config BaseConfig = 
+            ConfigurationFactory.ParseString("akka.remote.dot-netty.tcp.port = 0");
+
+        public HealthCheckSpec(ITestOutputHelper helper) : base(BaseConfig, nameof(HealthCheckSpec), helper)
         {
             _eas = (ExtendedActorSystem) Sys;
         }