-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Load Redis test endpoints from config file or env vars #294
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,54 @@ | ||
using StackExchange.Redis; | ||
using System.Text.Json; | ||
|
||
namespace NRedisStack.Tests; | ||
|
||
public class EndpointConfig | ||
{ | ||
public List<string>? endpoints { get; set; } | ||
|
||
public bool tls { get; set; } | ||
|
||
public string? password { get; set; } | ||
|
||
public int? bdb_id { get; set; } | ||
|
||
public object? raw_endpoints { get; set; } | ||
|
||
public ConnectionMultiplexer CreateConnection(ConfigurationOptions configurationOptions) | ||
{ | ||
configurationOptions.EndPoints.Clear(); | ||
|
||
foreach (var endpoint in endpoints!) | ||
{ | ||
configurationOptions.EndPoints.Add(endpoint); | ||
} | ||
|
||
if (password != null) | ||
{ | ||
configurationOptions.Password = password; | ||
} | ||
|
||
// TODO(imalinovskiy): Add support for TLS | ||
// TODO(imalinovskiy): Add support for Discovery/Sentinel API | ||
|
||
return ConnectionMultiplexer.Connect(configurationOptions); | ||
} | ||
} | ||
|
||
|
||
public class RedisFixture : IDisposable | ||
{ | ||
// Set the environment variable to specify your own alternate host and port: | ||
private readonly string redisStandalone = Environment.GetEnvironmentVariable("REDIS") ?? "localhost:6379"; | ||
private readonly string? redisCluster = Environment.GetEnvironmentVariable("REDIS_CLUSTER"); | ||
private readonly string? numRedisClusterNodesEnv = Environment.GetEnvironmentVariable("NUM_REDIS_CLUSTER_NODES"); | ||
|
||
private readonly string defaultEndpointId = Environment.GetEnvironmentVariable("REDIS_DEFAULT_ENDPOINT_ID") ?? "standalone"; | ||
private readonly string? redisEndpointsPath = Environment.GetEnvironmentVariable("REDIS_ENDPOINTS_CONFIG_PATH"); | ||
private Dictionary<string, EndpointConfig> redisEndpoints = new(); | ||
|
||
|
||
public bool isEnterprise = Environment.GetEnvironmentVariable("IS_ENTERPRISE") == "true"; | ||
public bool isOSSCluster; | ||
|
||
|
@@ -18,7 +59,42 @@ public RedisFixture() | |
AsyncTimeout = 10000, | ||
SyncTimeout = 10000 | ||
}; | ||
Redis = Connect(clusterConfig, out isOSSCluster); | ||
|
||
if (redisEndpointsPath != null && File.Exists(redisEndpointsPath)) | ||
{ | ||
string json = File.ReadAllText(redisEndpointsPath); | ||
var parsedEndpoints = JsonSerializer.Deserialize<Dictionary<string, EndpointConfig>>(json); | ||
|
||
redisEndpoints = parsedEndpoints ?? throw new Exception("Failed to parse the Redis endpoints configuration."); | ||
} | ||
else | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just an idea, would it work to have a default JSON file to load, so we don't do any config from code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can replace it with a default JSON file. I did it only to keep it backward compatible with the existing GitHub workflows. |
||
{ | ||
redisEndpoints.Add("standalone", | ||
new EndpointConfig { endpoints = new List<string> { redisStandalone } }); | ||
|
||
if (redisCluster != null) | ||
{ | ||
string[] parts = redisCluster!.Split(':'); | ||
string host = parts[0]; | ||
int startPort = int.Parse(parts[1]); | ||
|
||
var endpoints = new List<string>(); | ||
int numRedisClusterNodes = int.Parse(numRedisClusterNodesEnv!); | ||
for (int i = 0; i < numRedisClusterNodes; i++) | ||
{ | ||
endpoints.Add($"{host}:{startPort + i}"); | ||
} | ||
|
||
redisEndpoints.Add("cluster", | ||
new EndpointConfig { endpoints = endpoints }); | ||
|
||
// Set the default endpoint to the cluster to keep the tests consistent | ||
defaultEndpointId = "cluster"; | ||
isOSSCluster = true; | ||
} | ||
} | ||
|
||
Redis = GetConnectionById(clusterConfig, defaultEndpointId); | ||
} | ||
|
||
public void Dispose() | ||
|
@@ -28,39 +104,18 @@ public void Dispose() | |
|
||
public ConnectionMultiplexer Redis { get; } | ||
|
||
public ConnectionMultiplexer CustomRedis(ConfigurationOptions configurationOptions, out bool isOssCluster) | ||
public ConnectionMultiplexer GetConnectionById(ConfigurationOptions configurationOptions, string id) | ||
{ | ||
return Connect(configurationOptions, out isOssCluster); | ||
} | ||
|
||
private ConnectionMultiplexer Connect(ConfigurationOptions configurationOptions, out bool isOssCluster) | ||
{ | ||
// Redis Cluster | ||
if (redisCluster != null && numRedisClusterNodesEnv != null) | ||
if (!redisEndpoints.ContainsKey(id)) | ||
{ | ||
// Split to host and port | ||
string[] parts = redisCluster!.Split(':'); | ||
string host = parts[0]; | ||
int startPort = int.Parse(parts[1]); | ||
|
||
var endpoints = new EndPointCollection(); // TODO: check if needed | ||
|
||
configurationOptions.EndPoints.Clear(); | ||
int numRedisClusterNodes = int.Parse(numRedisClusterNodesEnv!); | ||
for (int i = 0; i < numRedisClusterNodes; i++) | ||
{ | ||
configurationOptions.EndPoints.Add(host, startPort + i); | ||
} | ||
|
||
isOssCluster = true; | ||
return ConnectionMultiplexer.Connect(configurationOptions); | ||
throw new Exception($"The connection with id '{id}' is not configured."); | ||
} | ||
|
||
// Redis Standalone | ||
configurationOptions.EndPoints.Clear(); | ||
configurationOptions.EndPoints.Add($"{redisStandalone}"); | ||
return redisEndpoints[id].CreateConnection(configurationOptions); | ||
} | ||
|
||
isOssCluster = false; | ||
return ConnectionMultiplexer.Connect(configurationOptions); | ||
public bool IsTargetConnectionExist(string id) | ||
{ | ||
return redisEndpoints.ContainsKey(id); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we perhaps list somewhere the environments we would like to have?
I think the existing tests (which make sense to us) are a good reference as to what we need.
Everybody can suggest additions / deletions to this list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list of required environments depends on the current test coverage. For example, in Jedis, we need a huge list of Redis endpoints https://github.com/redis/jedis/pull/3836/files#diff-c731b96d111ab92ab2f88dcbe2e36abef53c269a71f96598463e6cc3b70b7acd and it doesn't yet cover the Sentinel and Cluster endpoints. As a first stage, I suggest identifying those endpoints and collecting them in endpoints.json file in each repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is what I meant, gather a list so we know what we need. Thanks!