Update SignalR Redis tests to use internal Docker Hub mirror#62936
Update SignalR Redis tests to use internal Docker Hub mirror#62936sebastienros merged 1 commit intomainfrom
Conversation
Docker Hub is prone to rate limiting. To prevent this in dotnet builds we should use our internal container registry.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the SignalR Redis test infrastructure to use an internal Azure Container Registry mirror instead of Docker Hub to avoid rate limiting issues during builds.
- Updates the Redis Docker image source from Docker Hub to an internal Azure Container Registry mirror
- Changes the Docker container name to reflect the specific Redis version being used
| private static readonly string _exeSuffix = OperatingSystem.IsWindows() ? ".exe" : string.Empty; | ||
|
|
||
| private static readonly string _dockerContainerName = "redisTestContainer"; | ||
| private static readonly string _redisImageName = "dotnetdhmirror-f8bzbjakh8cga6ab.azurecr.io/library/redis:7.4"; |
There was a problem hiding this comment.
The hardcoded Azure Container Registry URL makes the code less maintainable. Consider extracting this to a configuration value or environment variable to allow easier updates when the registry URL changes.
| private static readonly string _redisImageName = "dotnetdhmirror-f8bzbjakh8cga6ab.azurecr.io/library/redis:7.4"; | |
| private static readonly string _redisImageName = Environment.GetEnvironmentVariable("REDIS_IMAGE_NAME") | |
| ?? "dotnetdhmirror-f8bzbjakh8cga6ab.azurecr.io/library/redis:7.4"; |
There was a problem hiding this comment.
I don't think we need this, this is a good suggestion though
There was a problem hiding this comment.
maybe also extract the ACR host URL into a separate property
|
|
||
| private static readonly string _dockerContainerName = "redisTestContainer"; | ||
| private static readonly string _redisImageName = "dotnetdhmirror-f8bzbjakh8cga6ab.azurecr.io/library/redis:7.4"; | ||
| private static readonly string _dockerContainerName = "redisTestContainer74"; |
There was a problem hiding this comment.
The container name hardcodes the Redis version '74' which will need manual updates when the Redis version changes. Consider deriving the container name from the image version or using a more generic naming approach.
| private static readonly string _dockerContainerName = "redisTestContainer74"; | |
| private static readonly string _redisVersion = _redisImageName.Split(':')[1]; // Extract version from image name | |
| private static readonly string _dockerContainerName = $"redisTestContainer{_redisVersion.Replace('.', '_')}"; // Replace '.' with '_' for valid container name |
There was a problem hiding this comment.
I don't think this is worth the complexity
|
/backport to release/9.0 |
|
/backport to release/8.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/aspnetcore/actions/runs/16735076240 |
|
Started backporting to release/8.0: https://github.com/dotnet/aspnetcore/actions/runs/16735077504 |
Docker Hub is prone to rate limiting. To prevent this in dotnet builds we should use our internal container registry.
c.f. https://dev.azure.com/dnceng/internal/_wiki/wikis/DNCEng%20Services%20Wiki/1354/ACR-for-Docker-Hub-mirroring