Build: Simplify Docker layout, fix Envoy, and upgrade tests (#2774)

It seems the Envoy apts have gone missing breaking out build, so instead of relying on the install let's docker compose their image as a proxy against the Redis supervisor instance as a simpler and faster-to-start setup that also works. This rearranged some things to simplify the Docker story overall. A move to AzDO or just GitHub builds would simplify everything further, but we need to figure out Windows testing against a Docker setup in CI. Note: we still can't use Linux containers on a Windows GitHub Actions host (actions/runner#904), so this remains much more complicated and not-really-testing-the-real-thing in the Windows front.
StackExchange · Aug 18, 2024 · 3701b50 · 3701b50
1 parent e208905
commit 3701b50
Show file tree

Hide file tree

Showing 26 changed files with 130 additions and 138 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -24,12 +24,12 @@ jobs:
       with:
         dotnet-version: | 
           6.0.x
-          7.0.x
+          8.0.x
     - name: .NET Build
       run: dotnet build Build.csproj -c Release /p:CI=true
     - name: Start Redis Services (docker-compose)
       working-directory: ./tests/RedisConfigs
-      run: docker compose -f docker-compose.yml up -d
+      run: docker compose -f docker-compose.yml up -d --wait
     - name: StackExchange.Redis.Tests
       run: dotnet test tests/StackExchange.Redis.Tests/StackExchange.Redis.Tests.csproj -c Release --logger trx --logger GitHubActions --results-directory ./test-results/ /p:CI=true
     - uses: dorny/test-reporter@v1
@@ -49,6 +49,7 @@ jobs:
       NUGET_CERT_REVOCATION_MODE: offline # Disabling signing because of massive perf hit, see https://github.com/NuGet/Home/issues/11548
       DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION: "1" # Note this doesn't work yet for Windows - see https://github.com/dotnet/runtime/issues/68340
       TERM: xterm
+      DOCKER_BUILDKIT: 1
     steps:
     - name: Checkout code
       uses: actions/checkout@v1
@@ -57,9 +58,13 @@ jobs:
     #   with:
     #     dotnet-version: | 
     #       6.0.x
-    #       7.0.x
+    #       8.0.x
     - name: .NET Build
       run: dotnet build Build.csproj -c Release /p:CI=true
+    # We can't do this combination - see https://github.com/actions/runner/issues/904
+    # - name: Start Redis Services (docker-compose)
+    #   working-directory: .\tests\RedisConfigs
+    #   run: docker compose -f docker-compose.yml up -d --wait
     - name: Start Redis Services (v3.0.503)
       working-directory: .\tests\RedisConfigs\3.0.503
       run: |

diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -10,21 +10,21 @@
 
     <!-- Packages only used in the solution, upgrade at will -->
     <PackageVersion Include="BenchmarkDotNet" Version="0.13.1" />
-    <PackageVersion Include="GitHubActionsTestLogger" Version="2.0.0-alpha" />
+    <PackageVersion Include="GitHubActionsTestLogger" Version="2.4.1" />
     <PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4" />
     <PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.0" />
     <PackageVersion Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.2" />
-    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
-    <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="1.1.1" />
-    <PackageVersion Include="Nerdbank.GitVersioning" Version="3.4.255" />
-    <PackageVersion Include="Newtonsoft.Json" Version="13.0.1" />
-    <PackageVersion Include="NSubstitute" Version="5.0.0" />
+    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
+    <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
+    <PackageVersion Include="Nerdbank.GitVersioning" Version="3.6.141" />
+    <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
+    <PackageVersion Include="NSubstitute" Version="5.1.0" />
     <PackageVersion Include="StackExchange.Redis" Version="2.6.96" />
     <PackageVersion Include="StyleCop.Analyzers" Version="1.2.0-beta.556" />
     <!-- For binding redirect testing, main package gets this transitively -->
     <PackageVersion Include="System.IO.Pipelines" Version="5.0.1" />
     <PackageVersion Include="System.Runtime.Caching" Version="5.0.0" />
-    <PackageVersion Include="xunit" Version="2.5.0" />
-    <PackageVersion Include="xunit.runner.visualstudio" Version="2.5.0" />
+    <PackageVersion Include="xunit" Version="2.9.0" />
+    <PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2" />
   </ItemGroup>
 </Project>
diff --git a/src/StackExchange.Redis/Maintenance/AzureMaintenanceEvent.cs b/src/StackExchange.Redis/Maintenance/AzureMaintenanceEvent.cs
@@ -15,7 +15,7 @@ public sealed class AzureMaintenanceEvent : ServerMaintenanceEvent
     {
         private const string PubSubChannelName = "AzureRedisEvents";
 
-        internal AzureMaintenanceEvent(string azureEvent)
+        internal AzureMaintenanceEvent(string? azureEvent)
         {
             if (azureEvent == null)
             {

diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs
@@ -526,7 +526,7 @@ internal sealed class ScriptLoadProcessor : ResultProcessor<byte[]>
 
             private static readonly Regex sha1 = new Regex("^[0-9a-f]{40}$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
 
-            internal static bool IsSHA1(string script) => script is not null && script.Length == SHA1Length && sha1.IsMatch(script);
+            internal static bool IsSHA1(string? script) => script is not null && script.Length == SHA1Length && sha1.IsMatch(script);
 
             internal const int Sha1HashLength = 20;
             internal static byte[] ParseSHA1(byte[] value)

diff --git a/tests/RedisConfigs/.docker/Envoy/Dockerfile b/tests/RedisConfigs/.docker/Envoy/Dockerfile
@@ -0,0 +1,6 @@
+FROM envoyproxy/envoy:v1.31-latest
+
+COPY envoy.yaml /etc/envoy/envoy.yaml
+RUN chmod go+r /etc/envoy/envoy.yaml
+
+EXPOSE 7015
diff --git a/tests/RedisConfigs/.docker/Envoy/envoy.yaml b/tests/RedisConfigs/.docker/Envoy/envoy.yaml
@@ -0,0 +1,35 @@
+admin:
+  address: { socket_address: { protocol: TCP, address: 0.0.0.0, port_value: 8001 } }
+static_resources:
+  listeners:
+  - name: redis_listener
+    address: { socket_address: { protocol: TCP, address: 0.0.0.0, port_value: 7015 } }
+    filter_chains:
+    - filters:
+      - name: envoy.filters.network.redis_proxy
+        typed_config:
+          "@type": type.googleapis.com/envoy.extensions.filters.network.redis_proxy.v3.RedisProxy
+          stat_prefix: envoy_redis_stats
+          settings:
+            op_timeout: 3s
+            dns_cache_config:
+              name: dynamic_forward_proxy_cache_config
+              dns_lookup_family: V4_ONLY
+          prefix_routes:
+            catch_all_route:
+              cluster: redis_cluster
+  clusters:
+  - name: redis_cluster
+    connect_timeout: 3s
+    type: STRICT_DNS
+    dns_lookup_family: V4_ONLY
+    load_assignment:
+      cluster_name: redis_cluster
+      endpoints:
+      - lb_endpoints:
+        - endpoint: { address: { socket_address: { address: redis, port_value: 7000 } } }
+        - endpoint: { address: { socket_address: { address: redis, port_value: 7001 } } }
+        - endpoint: { address: { socket_address: { address: redis, port_value: 7002 } } }
+        - endpoint: { address: { socket_address: { address: redis, port_value: 7003 } } }
+        - endpoint: { address: { socket_address: { address: redis, port_value: 7004 } } }
+        - endpoint: { address: { socket_address: { address: redis, port_value: 7005 } } }
diff --git a/tests/RedisConfigs/.docker/Redis/Dockerfile b/tests/RedisConfigs/.docker/Redis/Dockerfile
@@ -0,0 +1,23 @@
+FROM redis:7.4-rc1
+
+COPY --from=configs ./Basic /data/Basic/
+COPY --from=configs ./Failover /data/Failover/
+COPY --from=configs ./Cluster /data/Cluster/
+COPY --from=configs ./Sentinel /data/Sentinel/
+COPY --from=configs ./Certs /Certs/
+
+RUN chown -R redis:redis /data
+RUN chown -R redis:redis /Certs
+
+COPY docker-entrypoint.sh /usr/local/bin/
+RUN chmod +x /usr/local/bin/docker-entrypoint.sh
+
+RUN apt-get -y update && apt-get install supervisor -y
+
+RUN apt-get clean
+
+ADD supervisord.conf /etc/
+
+ENTRYPOINT ["docker-entrypoint.sh"]
+
+EXPOSE 6379 6380 6381 6382 6383 6384 7000 7001 7002 7003 7004 7005 7010 7011 26379 26380 26381
diff --git a/.../RedisConfigs/Docker/docker-entrypoint.sh → ...onfigs/.docker/Redis/docker-entrypoint.sh b/.../RedisConfigs/Docker/docker-entrypoint.sh → ...onfigs/.docker/Redis/docker-entrypoint.sh
diff --git a/tests/RedisConfigs/Docker/supervisord.conf → ...disConfigs/.docker/Redis/supervisord.conf b/tests/RedisConfigs/Docker/supervisord.conf → ...disConfigs/.docker/Redis/supervisord.conf
@@ -99,13 +99,6 @@ stdout_logfile=/var/log/supervisor/%(program_name)s.log
 stderr_logfile=/var/log/supervisor/%(program_name)s.log
 autorestart=true
 
-[program:redis-7015]
-command=/usr/bin/envoy -c /envoy/envoy.yaml
-directory=/envoy
-stdout_logfile=/var/log/supervisor/%(program_name)s.log
-stderr_logfile=/var/log/supervisor/%(program_name)s.log
-autorestart=true
-
 [program:sentinel-26379]
 command=/usr/local/bin/redis-server /data/Sentinel/sentinel-26379.conf --sentinel
 directory=/data/Sentinel

diff --git a/tests/RedisConfigs/Docker/install-envoy.sh b/tests/RedisConfigs/Docker/install-envoy.sh
diff --git a/tests/RedisConfigs/Dockerfile b/tests/RedisConfigs/Dockerfile
diff --git a/tests/RedisConfigs/Envoy/envoy.yaml b/tests/RedisConfigs/Envoy/envoy.yaml
diff --git a/tests/RedisConfigs/docker-compose.yml b/tests/RedisConfigs/docker-compose.yml
@@ -1,16 +1,28 @@
-version: '2.6'
+version: '2.7'
 
 services:
   redis:
     build:
-      context: .
-    image: stackexchange/redis-tests:latest
+      context: .docker/Redis
+      additional_contexts:
+         configs: .
     platform: linux
     ports:
-      - 6379-6384:6379-6384
-      - 7000-7006:7000-7006
-      - 7010-7011:7010-7011
-      - 7015:7015
-      - 26379-26381:26379-26381
+      - 6379-6384:6379-6384     # Misc
+      - 7000-7006:7000-7006     # Cluster
+      - 7010-7011:7010-7011     # Sentinel Controllers
+      - 26379-26381:26379-26381 # Sentinel Data
     sysctls :
       net.core.somaxconn: '511'
+  envoy:
+    build:
+      context: .docker/Envoy
+    platform: linux
+    environment:
+      loglevel: warning
+    depends_on:
+      redis:
+        condition: service_started
+    ports:
+      - 7015:7015 # Cluster
+      - 8001:8001 # Admin
diff --git a/tests/StackExchange.Redis.Tests/AzureMaintenanceEventTests.cs b/tests/StackExchange.Redis.Tests/AzureMaintenanceEventTests.cs
@@ -29,7 +29,7 @@ public AzureMaintenanceEventTests(ITestOutputHelper output) : base(output) { }
     [InlineData("NonSSLPort |", AzureNotificationType.Unknown, null, false, null, 0, 0)]
     [InlineData("StartTimeInUTC|thisisthestart", AzureNotificationType.Unknown, null, false, null, 0, 0)]
     [InlineData(null, AzureNotificationType.Unknown, null, false, null, 0, 0)]
-    public void TestAzureMaintenanceEventStrings(string message, AzureNotificationType expectedEventType, string expectedStart, bool expectedIsReplica, string expectedIP, int expectedSSLPort, int expectedNonSSLPort)
+    public void TestAzureMaintenanceEventStrings(string? message, AzureNotificationType expectedEventType, string? expectedStart, bool expectedIsReplica, string? expectedIP, int expectedSSLPort, int expectedNonSSLPort)
     {
         DateTime? expectedStartTimeUtc = null;
         if (expectedStart != null && DateTime.TryParseExact(expectedStart, "s", CultureInfo.InvariantCulture, DateTimeStyles.AssumeUniversal, out DateTime startTimeUtc))

diff --git a/tests/StackExchange.Redis.Tests/ClusterTests.cs b/tests/StackExchange.Redis.Tests/ClusterTests.cs
@@ -365,7 +365,7 @@ public void TransactionWithSameSlotKeys()
     [InlineData(null, 100)]
     [InlineData("abc", 10)]
     [InlineData("abc", 100)]
-    public void Keys(string pattern, int pageSize)
+    public void Keys(string? pattern, int pageSize)
     {
         using var conn = Create(allowAdmin: true);
 

diff --git a/tests/StackExchange.Redis.Tests/EnvoyTests.cs b/tests/StackExchange.Redis.Tests/EnvoyTests.cs
@@ -35,15 +35,15 @@ public void TestBasicEnvoyConnection()
         }
         catch (TimeoutException ex) when (ex.Message == "Connect timeout" || sb.ToString().Contains("Returned, but incorrectly"))
         {
-            Skip.Inconclusive("Envoy server not found.");
+            Skip.Inconclusive($"Envoy server not found: {ex}.");
         }
-        catch (AggregateException)
+        catch (AggregateException ex)
         {
-            Skip.Inconclusive("Envoy server not found.");
+            Skip.Inconclusive($"Envoy server not found: {ex}.");
         }
-        catch (RedisConnectionException) when (sb.ToString().Contains("It was not possible to connect to the redis server(s)"))
+        catch (RedisConnectionException ex) when (sb.ToString().Contains("It was not possible to connect to the redis server(s)"))
         {
-            Skip.Inconclusive("Envoy server not found.");
+            Skip.Inconclusive($"Envoy server not found: {ex}.");
         }
     }
 }
diff --git a/tests/StackExchange.Redis.Tests/Helpers/Attributes.cs b/tests/StackExchange.Redis.Tests/Helpers/Attributes.cs
@@ -251,16 +251,14 @@ public static IEnumerable<IXunitTestCase> Expand(this ITestMethod testMethod, Fu
             {
                 protocols = RunPerProtocol.AllProtocols;
             }
-            var results = new List<IXunitTestCase>();
             foreach (var protocol in protocols)
             {
-                results.Add(generator(protocol));
+                yield return generator(protocol);
             }
-            return results;
         }
         else
         {
-            return new[] { generator(RedisProtocol.Resp2) };
+            yield return generator(RedisProtocol.Resp2);
         }
     }
 }
@@ -270,7 +268,7 @@ public static IEnumerable<IXunitTestCase> Expand(this ITestMethod testMethod, Fu
 /// <see cref="Thread.CurrentThread" /> and <see cref="CultureInfo.CurrentCulture" /> with another culture.
 /// </summary>
 /// <remarks>
-/// Based on: https://bartwullems.blogspot.com/2022/03/xunit-change-culture-during-your-test.html
+/// Based on: https://bartwullems.blogspot.com/2022/03/xunit-change-culture-during-your-test.html.
 /// </remarks>
 [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
 public class TestCultureAttribute : BeforeAfterTestAttribute
@@ -288,7 +286,7 @@ public class TestCultureAttribute : BeforeAfterTestAttribute
     /// Stores the current <see cref="Thread.CurrentPrincipal" /> and <see cref="CultureInfo.CurrentCulture" />
     /// and replaces them with the new cultures defined in the constructor.
     /// </summary>
-    /// <param name="methodUnderTest">The method under test</param>
+    /// <param name="methodUnderTest">The method under test.</param>
     public override void Before(MethodInfo methodUnderTest)
     {
         originalCulture = Thread.CurrentThread.CurrentCulture;
@@ -299,7 +297,7 @@ public override void Before(MethodInfo methodUnderTest)
     /// <summary>
     /// Restores the original <see cref="CultureInfo.CurrentCulture" /> to <see cref="Thread.CurrentPrincipal" />.
     /// </summary>
-    /// <param name="methodUnderTest">The method under test</param>
+    /// <param name="methodUnderTest">The method under test.</param>
     public override void After(MethodInfo methodUnderTest)
     {
         if (originalCulture is not null)

diff --git a/tests/StackExchange.Redis.Tests/Issues/Issue2653.cs b/tests/StackExchange.Redis.Tests/Issues/Issue2653.cs
@@ -11,6 +11,6 @@ public class Issue2653
     [InlineData("abc.def", "abc.def")]
     [InlineData("abc d \t  ef", "abc-d-ef")]
     [InlineData("  abc\r\ndef\n", "abc-def")]
-    public void CheckLibraySanitization(string input, string expected)
+    public void CheckLibraySanitization(string? input, string expected)
         => Assert.Equal(expected, ServerEndPoint.ClientInfoSanitize(input));
 }
diff --git a/tests/StackExchange.Redis.Tests/PubSubTests.cs b/tests/StackExchange.Redis.Tests/PubSubTests.cs
@@ -56,7 +56,7 @@ await UntilConditionAsync(
     [InlineData(null, true, "d")]
     [InlineData("", true, "e")]
     [InlineData("Foo:", true, "f")]
-    public async Task TestBasicPubSub(string channelPrefix, bool wildCard, string breaker)
+    public async Task TestBasicPubSub(string? channelPrefix, bool wildCard, string breaker)
     {
         using var conn = Create(channelPrefix: channelPrefix, shared: false, log: Writer);