Skip to content
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

Fix a bug that happens when sentinels report peers with IP addresses #273

Merged
merged 3 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,43 +9,42 @@ on:
jobs:
test:
name: Test (Elixir ${{matrix.elixir}} | Erlang/OTP ${{matrix.otp}})
runs-on: ubuntu-20.04
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
include:
- otp: "27.0"
elixir: "1.17.0"
version-type: strict
elixir: "1.17"
os: ubuntu-latest
dialyzer: true
lint: true

- otp: "25.3"
elixir: "1.14.3"
os: ubuntu-20.04
coverage: true
version-type: loose

- otp: "23.0"
elixir: "1.12"
version-type: loose
os: ubuntu-20.04

env:
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
MIX_ENV: test

steps:
- name: Clone the repository
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Start Docker
run: docker-compose up --detach
run: docker compose up --detach

- name: Install OTP and Elixir
uses: erlef/setup-beam@v1
with:
otp-version: ${{matrix.otp}}
elixir-version: ${{matrix.elixir}}
version-type: ${{matrix.version-type}}
otp-version: ${{ matrix.otp }}
elixir-version: ${{ matrix.elixir }}

- name: Cache dependencies
id: cache-deps
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ Redix is low-level, but it's still built to handle most things thrown at it. For

## Contributing

To run the Redix test suite you will have to have Redis running locally. Redix requires a somewhat complex setup for running tests (because it needs a few instances running, for pub/sub and sentinel). For this reason, in this repository you'll find a `docker-compose.yml` file so that you can use [Docker][docker] and [docker-compose][] to spin up all the necessary Redis instances with just one command. Make sure you have Docker installed and then just run:
To run the Redix test suite you will have to have Redis running locally. Redix requires a somewhat complex setup for running tests (because it needs a few instances running, for pub/sub and sentinel). For this reason, in this repository you'll find a `docker-compose.yml` file so that you can use [Docker][docker] and [docker compose][docker-compose] to spin up all the necessary Redis instances with just one command. Make sure you have Docker installed and then just run:

```bash
docker-compose up
docker compose up
```

Now, you're ready to run tests with the `$ mix test` command.
Expand Down
20 changes: 17 additions & 3 deletions lib/redix/connector.ex
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,16 @@ defmodule Redix.Connector do
Logger.debug(fn ->
"Sentinel reported #{sentinel_opts[:role]}: #{server_host}:#{server_port}"
end),
{:ok, server_socket, _address} <-
server_host = string_address_to_erlang(server_host),
{:ok, server_socket, address} <-
connect_directly(
String.to_charlist(server_host),
server_host,
String.to_integer(server_port),
opts
),
:ok <- verify_server_role(server_socket, opts, sentinel_opts) do
:ok = transport.close(sent_socket)
{:ok, server_socket, "#{server_host}:#{server_port}"}
{:ok, server_socket, address}
else
{cause, reason} when cause in [:error, :stop] ->
:telemetry.execute([:redix, :failed_connection], %{}, %{
Expand All @@ -171,6 +172,19 @@ defmodule Redix.Connector do
end
end

defp string_address_to_erlang(address) when is_binary(address) do
address = String.to_charlist(address)

case :inet.parse_address(address) do
{:ok, ip} -> ip
{:error, :einval} -> address
end
end

defp string_address_to_erlang(address) do
address
end

defp connect_to_sentinel(sentinel, sentinel_opts, transport) do
host = Keyword.fetch!(sentinel, :host)
port = Keyword.fetch!(sentinel, :port)
Expand Down
12 changes: 11 additions & 1 deletion lib/redix/format.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule Redix.Format do
# Used for formatting things to print or log or anything like that.

@spec format_host_and_port(host, :inet.port_number()) :: String.t()
when host: {:local, String.t()} | binary()
when host: {:local, String.t()} | charlist() | binary() | :inet.ip_address()
def format_host_and_port(host, port)

def format_host_and_port({:local, path}, 0) when is_binary(path), do: path
Expand All @@ -14,4 +14,14 @@ defmodule Redix.Format do

def format_host_and_port(host, port) when is_list(host),
do: format_host_and_port(IO.chardata_to_string(host), port)

def format_host_and_port(host, port) when is_tuple(host) do
case :inet.ntoa(host) do
{:error, :einval} ->
raise ArgumentError, "invalid host: #{inspect(host)}"

host ->
format_host_and_port(host, port)
end
end
end
2 changes: 1 addition & 1 deletion test/redix/pubsub_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule Redix.PubSubTest do

@moduletag :pubsub

# See docker-compose.
# See docker-compose.yml.
@port 6380

setup do
Expand Down
1 change: 1 addition & 0 deletions test/redix/sentinel_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ defmodule Redix.SentinelTest do
assert_receive {:redix_pubsub, ^pubsub, ^ref, :message, %{channel: "foo", payload: "hello"}}
end

@tag :capture_log
test "when no sentinels are reachable" do
Process.flag(:trap_exit, true)

Expand Down
Loading