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

Added standalone,sentinel and cluster compose files and updated workflow #251

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
37 changes: 31 additions & 6 deletions .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,45 @@ jobs:
strategy:
matrix:
node-version: [18.x, 20.x]

services:
falkordb:
image: falkordb/falkordb:latest
ports:
- 6379:6379

steps:
- uses: actions/checkout@v4
- name: Run docker-compose for redis standalone instance
uses: hoverkraft-tech/[email protected]
with:
compose-file: ./docker/standalone-compose.yml

- name: Run docker-compose for redis replication instance
uses: hoverkraft-tech/[email protected]
with:
compose-file: ./docker/sentinel-compose.yml

- name: Run docker-compose for redis cluster instance
uses: hoverkraft-tech/[email protected]
with:
compose-file: ./docker/cluster-compose.yml
Comment on lines +23 to +36
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add service health checks before running tests

The workflow starts multiple Redis instances but doesn't verify if they're ready before proceeding. Add health checks to ensure services are fully operational.

     - name: Run docker-compose for redis standalone instance
       uses: hoverkraft-tech/[email protected]
       with:
         compose-file: ./docker/standalone-compose.yml
+    - name: Wait for standalone Redis to be ready
+      run: |
+        timeout 30s bash -c 'until redis-cli ping; do sleep 1; done'

     - name: Run docker-compose for redis replication instance
       uses: hoverkraft-tech/[email protected]
       with:
         compose-file: ./docker/sentinel-compose.yml
+    - name: Wait for sentinel setup to be ready
+      run: |
+        timeout 60s bash -c 'until redis-cli -p 26379 ping; do sleep 1; done'

     - name: Run docker-compose for redis cluster instance
       uses: hoverkraft-tech/[email protected]
       with:
         compose-file: ./docker/cluster-compose.yml
+    - name: Wait for cluster to be ready
+      run: |
+        timeout 60s bash -c 'until redis-cli cluster info; do sleep 1; done'

Committable suggestion skipped: line range outside the PR's diff.


- name: Add entries to the host file to correctly resolve service names
run: |
sudo echo "127.0.0.1 redis-standalone" | sudo tee -a /etc/hosts
sudo echo "127.0.0.1 sentinel-1" | sudo tee -a /etc/hosts
sudo echo "127.0.0.1 sentinel-2" | sudo tee -a /etc/hosts
sudo echo "127.0.0.1 sentinel-3" | sudo tee -a /etc/hosts
sudo echo "127.0.0.1 redis-server-1" | sudo tee -a /etc/hosts
sudo echo "127.0.0.1 redis-server-2" | sudo tee -a /etc/hosts
for i in node{0..5};do
echo "127.0.0.1 $i" | sudo tee -a /etc/hosts
done

- name: install docker-compose client
run: sudo apt update && sudo apt install docker-compose -y

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
cache: 'npm'

- run: npm ci
- run: npm run lint
- run: npm run build --if-present
Expand Down
146 changes: 146 additions & 0 deletions docker/cluster-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
services:
node0:
image: falkordb/falkordb:edge
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix critical issues in healthcheck configuration and image tag

  1. The edge tag is not recommended for production use as it may contain unstable changes. Consider using a specific version tag.
  2. The healthcheck has several issues:
    • The command || echo failed && exit 1 doesn't work as intended due to shell operator precedence
    • No dependency management between nodes
    • No wait for other nodes to be ready before cluster creation

Consider these fixes:

-    image: falkordb/falkordb:edge
+    image: falkordb/falkordb:latest  # or specific version tag
     # ... other configs ...
     healthcheck:
           test: |
-            redis-cli --cluster create node0:5000 node1:6000 \
-              node2:7000 node3:8000 node4:9000 node5:10000 --cluster-replicas 1 --cluster-yes || echo failed && exit 1
+            timeout 5 bash -c '
+              until redis-cli -h node1 -p 6000 ping && \
+                    redis-cli -h node2 -p 7000 ping && \
+                    redis-cli -h node3 -p 8000 ping && \
+                    redis-cli -h node4 -p 9000 ping && \
+                    redis-cli -h node5 -p 10000 ping; do
+                sleep 1
+              done
+              redis-cli --cluster create node0:5000 node1:6000 \
+                node2:7000 node3:8000 node4:9000 node5:10000 \
+                --cluster-replicas 1 --cluster-yes || exit 1'

Also applies to: 10-17

container_name: node0
hostname: node0
ports:
- 5000:5000
networks:
- redis-cluster
healthcheck:
test: |
redis-cli --cluster create node0:5000 node1:6000 \
node2:7000 node3:8000 node4:9000 node5:10000 --cluster-replicas 1 --cluster-yes || echo failed && exit 1
interval: 3s
timeout: 10s
retries: 10
start_period: 2s
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 5000" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "cluster-preferred-endpoint-type hostname" >> /data/node.conf
echo "cluster-port 16379" >> /data/node.conf
echo "cluster-enabled yes" >> /data/node.conf
echo "cluster-config-file /data/nodes.conf" >> /data/node.conf
echo "cluster-node-timeout 5000" >> /data/node.conf
echo "cluster-announce-hostname node0" >> /data/node.conf
redis-server /data/node.conf
Comment on lines +22 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve configuration management and production readiness

  1. The node configurations are heavily duplicated. Consider using Docker Compose extensions to define a common template.
  2. Missing important production configurations:
    • No volume mounts for data persistence
    • No restart policy defined
    • No resource limits

Consider refactoring using this pattern:

x-node-template: &node-template
  image: falkordb/falkordb:latest
  networks:
    - redis-cluster
  restart: unless-stopped
  volumes:
    - redis-data:/data
  entrypoint: /bin/bash
  command:
    - -c
    - |
      touch /data/node.conf
      cat > /data/node.conf << EOF
      port ${PORT}
      loadmodule /FalkorDB/bin/src/falkordb.so
      cluster-preferred-endpoint-type hostname
      cluster-port 16379
      cluster-enabled yes
      cluster-config-file /data/nodes.conf
      cluster-node-timeout 5000
      cluster-announce-hostname ${HOSTNAME}
      EOF
      redis-server /data/node.conf

services:
  node0:
    <<: *node-template
    environment:
      - PORT=5000
    ports:
      - "${PORT}:${PORT}"
    # ... other specific configs

volumes:
  redis-data:

Also applies to: 44-53, 66-75, 88-97, 110-119, 133-142

node1:
image: falkordb/falkordb:edge
container_name: node1
hostname: node1
ports:
- 6000:6000
networks:
- redis-cluster
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 6000" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "cluster-preferred-endpoint-type hostname" >> /data/node.conf
echo "cluster-port 16379" >> /data/node.conf
echo "cluster-enabled yes" >> /data/node.conf
echo "cluster-config-file /data/nodes.conf" >> /data/node.conf
echo "cluster-node-timeout 5000" >> /data/node.conf
echo "cluster-announce-hostname node1" >> /data/node.conf
redis-server /data/node.conf
node2:
image: falkordb/falkordb:edge
container_name: node2
hostname: node2
ports:
- 7000:7000
networks:
- redis-cluster
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 7000" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "cluster-preferred-endpoint-type hostname" >> /data/node.conf
echo "cluster-port 16379" >> /data/node.conf
echo "cluster-enabled yes" >> /data/node.conf
echo "cluster-config-file /data/nodes.conf" >> /data/node.conf
echo "cluster-node-timeout 5000" >> /data/node.conf
echo "cluster-announce-hostname node2" >> /data/node.conf
redis-server /data/node.conf
node3:
image: falkordb/falkordb:edge
container_name: node3
hostname: node3
ports:
- 8000:8000
networks:
- redis-cluster
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 8000" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "cluster-preferred-endpoint-type hostname" >> /data/node.conf
echo "cluster-port 16379" >> /data/node.conf
echo "cluster-enabled yes" >> /data/node.conf
echo "cluster-config-file /data/nodes.conf" >> /data/node.conf
echo "cluster-node-timeout 5000" >> /data/node.conf
echo "cluster-announce-hostname node3" >> /data/node.conf
redis-server /data/node.conf
node4:
image: falkordb/falkordb:edge
container_name: node4
hostname: node4
ports:
- 9000:9000
networks:
- redis-cluster
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 9000" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "cluster-preferred-endpoint-type hostname" >> /data/node.conf
echo "cluster-port 16379" >> /data/node.conf
echo "cluster-enabled yes" >> /data/node.conf
echo "cluster-config-file /data/nodes.conf" >> /data/node.conf
echo "cluster-node-timeout 5000" >> /data/node.conf
echo "cluster-announce-hostname node4" >> /data/node.conf
redis-server /data/node.conf

node5:
image: falkordb/falkordb:edge
container_name: node5
hostname: node5
ports:
- 10000:10000
networks:
- redis-cluster
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 10000" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "cluster-preferred-endpoint-type hostname" >> /data/node.conf
echo "cluster-port 16379" >> /data/node.conf
echo "cluster-enabled yes" >> /data/node.conf
echo "cluster-config-file /data/nodes.conf" >> /data/node.conf
echo "cluster-node-timeout 5000" >> /data/node.conf
echo "cluster-announce-hostname node5" >> /data/node.conf
redis-server /data/node.conf


networks:
redis-cluster:
108 changes: 108 additions & 0 deletions docker/sentinel-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
services:
sentinel-1:
image: falkordb/falkordb:edge
container_name: sentinel-1
hostname: sentinel-1
restart: always
networks:
- redis-sentinel
ports:
- 26379:26379
entrypoint: /bin/bash
command:
- -c
- |
touch /data/sentinel.conf
echo "sentinel monitor master redis-server-1 6380 2" >> /data/sentinel.conf
echo "sentinel announce-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-ip sentinel-1" >> /data/sentinel.conf
echo "sentinel resolve-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-port 26379" >> /data/sentinel.conf
echo "port 26379" >> /data/sentinel.conf
redis-server /data/sentinel.conf --sentinel
Comment on lines +15 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance Sentinel configuration with additional parameters

The current Sentinel configuration lacks important parameters for production deployment:

  • down-after-milliseconds: Timeout to mark node as down
  • failover-timeout: Time before triggering failover
  • parallel-syncs: Number of replicas that can sync simultaneously
         touch /data/sentinel.conf
         echo "sentinel monitor master redis-server-1 6380 2" >> /data/sentinel.conf
+        echo "sentinel down-after-milliseconds master 5000"  >> /data/sentinel.conf
+        echo "sentinel failover-timeout master 60000"        >> /data/sentinel.conf
+        echo "sentinel parallel-syncs master 1"             >> /data/sentinel.conf
         echo "sentinel announce-hostnames yes"               >> /data/sentinel.conf
         echo "sentinel announce-ip sentinel-1"               >> /data/sentinel.conf
         echo "sentinel resolve-hostnames yes"                >> /data/sentinel.conf
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
touch /data/sentinel.conf
echo "sentinel monitor master redis-server-1 6380 2" >> /data/sentinel.conf
echo "sentinel announce-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-ip sentinel-1" >> /data/sentinel.conf
echo "sentinel resolve-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-port 26379" >> /data/sentinel.conf
echo "port 26379" >> /data/sentinel.conf
redis-server /data/sentinel.conf --sentinel
touch /data/sentinel.conf
echo "sentinel monitor master redis-server-1 6380 2" >> /data/sentinel.conf
echo "sentinel down-after-milliseconds master 5000" >> /data/sentinel.conf
echo "sentinel failover-timeout master 60000" >> /data/sentinel.conf
echo "sentinel parallel-syncs master 1" >> /data/sentinel.conf
echo "sentinel announce-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-ip sentinel-1" >> /data/sentinel.conf
echo "sentinel resolve-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-port 26379" >> /data/sentinel.conf
echo "port 26379" >> /data/sentinel.conf
redis-server /data/sentinel.conf --sentinel


sentinel-2:
image: falkordb/falkordb:edge
container_name: sentinel-2
hostname: sentinel-2
restart: always
networks:
- redis-sentinel
ports:
- 26380:26380
entrypoint: /bin/bash
command:
- -c
- |
touch /data/sentinel.conf
echo "sentinel monitor master redis-server-1 6380 2" >> /data/sentinel.conf
echo "sentinel announce-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-ip sentinel-2" >> /data/sentinel.conf
echo "sentinel resolve-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-port 26380" >> /data/sentinel.conf
echo "port 26380" >> /data/sentinel.conf
redis-server /data/sentinel.conf --sentinel

sentinel-3:
image: falkordb/falkordb:edge
container_name: sentinel-3
hostname: sentinel-3
restart: always
networks:
- redis-sentinel
ports:
- 26381:26381
entrypoint: /bin/bash
command:
- -c
- |
touch /data/sentinel.conf
echo "sentinel monitor master redis-server-1 6380 2" >> /data/sentinel.conf
echo "sentinel announce-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-ip sentinel-3" >> /data/sentinel.conf
echo "sentinel resolve-hostnames yes" >> /data/sentinel.conf
echo "sentinel announce-port 26381" >> /data/sentinel.conf
echo "port 26381" >> /data/sentinel.conf
redis-server /data/sentinel.conf --sentinel

redis-server-1:
image: falkordb/falkordb:edge
hostname: redis-server-1
container_name: redis-server-1
ports:
- 6380:6380
networks:
- redis-sentinel
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 6380" >> /data/node.conf
echo "replica-announce-ip redis-server-1" >> /data/node.conf
echo "replica-announce-port 6380" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
redis-server /data/node.conf
Comment on lines +68 to +85
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add persistence and security configurations for Redis master

The Redis master configuration needs additional settings for durability and security:

  • Data persistence
  • Password protection
  • Memory limits
         touch /data/node.conf
         echo "port 6380" >> /data/node.conf 
+        echo "requirepass your_redis_password" >> /data/node.conf
+        echo "masterauth your_redis_password" >> /data/node.conf
+        echo "maxmemory 1gb" >> /data/node.conf
+        echo "maxmemory-policy allkeys-lru" >> /data/node.conf
+        echo "appendonly yes" >> /data/node.conf
+        echo "appendfsync everysec" >> /data/node.conf
         echo "replica-announce-ip redis-server-1"       >> /data/node.conf
         echo "replica-announce-port 6380"               >> /data/node.conf
         echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf

Also, add volume configuration:

     image: falkordb/falkordb:edge
     hostname: redis-server-1
     container_name: redis-server-1
+    volumes:
+      - redis-master-data:/data
     ports:
       - 6380:6380
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
redis-server-1:
image: falkordb/falkordb:edge
hostname: redis-server-1
container_name: redis-server-1
ports:
- 6380:6380
networks:
- redis-sentinel
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 6380" >> /data/node.conf
echo "replica-announce-ip redis-server-1" >> /data/node.conf
echo "replica-announce-port 6380" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
redis-server /data/node.conf
redis-server-1:
image: falkordb/falkordb:edge
hostname: redis-server-1
container_name: redis-server-1
volumes:
- redis-master-data:/data
ports:
- 6380:6380
networks:
- redis-sentinel
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 6380" >> /data/node.conf
echo "requirepass your_redis_password" >> /data/node.conf
echo "masterauth your_redis_password" >> /data/node.conf
echo "maxmemory 1gb" >> /data/node.conf
echo "maxmemory-policy allkeys-lru" >> /data/node.conf
echo "appendonly yes" >> /data/node.conf
echo "appendfsync everysec" >> /data/node.conf
echo "replica-announce-ip redis-server-1" >> /data/node.conf
echo "replica-announce-port 6380" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
redis-server /data/node.conf
🧰 Tools
🪛 yamllint (1.35.1)

[error] 81-81: trailing spaces

(trailing-spaces)


redis-server-2:
image: falkordb/falkordb:edge
hostname: redis-server-2
container_name: redis-server-2
ports:
- 6381:6381
networks:
- redis-sentinel
entrypoint: /bin/bash
command:
- -c
- |
touch /data/node.conf
echo "port 6381" >> /data/node.conf
echo "replica-announce-ip redis-server-2" >> /data/node.conf
echo "replica-announce-port 6381" >> /data/node.conf
echo "loadmodule /FalkorDB/bin/src/falkordb.so" >> /data/node.conf
echo "replicaof redis-server-1 6380" >> /data/node.conf
redis-server /data/node.conf

networks:
redis-sentinel:
7 changes: 7 additions & 0 deletions docker/standalone-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
services:
redis-standalone:
container_name: redis-standalone
hostname: redis-standalone
image: falkordb/falkordb:edge
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Consider using the stable version v4.4.1 instead of 'edge'

Using the edge tag can lead to unexpected behavior as it always pulls the latest build. Based on the available tags from Docker Hub, I recommend using v4.4.1 which is the latest stable version. This will ensure better stability and reproducibility of your deployment.

Example:

image: falkordb/falkordb:v4.4.1
🔗 Analysis chain

Consider using a specific version tag instead of 'edge'

Using the edge tag can lead to unexpected behavior as it always pulls the latest build. Consider using a specific version tag for better stability and reproducibility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check available FalkorDB tags
curl -s https://registry.hub.docker.com/v2/repositories/falkordb/falkordb/tags | jq -r '.results[].name'

Length of output: 206

ports:
- 6379:6379
Comment on lines +1 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add essential container configurations for production readiness

The standalone Redis configuration needs several important additions for production deployment:

  1. Volume persistence to prevent data loss
  2. Health check for container orchestration
  3. Resource limits to prevent resource exhaustion
  4. Restart policy for automatic recovery
 services:
   redis-standalone:
     container_name: redis-standalone
     hostname: redis-standalone
     image: falkordb/falkordb:edge
     ports:
       - 6379:6379
+    volumes:
+      - redis-data:/data
+    healthcheck:
+      test: ["CMD", "redis-cli", "ping"]
+      interval: 10s
+      timeout: 5s
+      retries: 3
+    deploy:
+      resources:
+        limits:
+          memory: 1G
+    restart: unless-stopped
+
+volumes:
+  redis-data:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
services:
redis-standalone:
container_name: redis-standalone
hostname: redis-standalone
image: falkordb/falkordb:edge
ports:
- 6379:6379
services:
redis-standalone:
container_name: redis-standalone
hostname: redis-standalone
image: falkordb/falkordb:edge
ports:
- 6379:6379
volumes:
- redis-data:/data
healthcheck:
test: ["CMD", "redis-cli", "ping"]
interval: 10s
timeout: 5s
retries: 3
deploy:
resources:
limits:
memory: 1G
restart: unless-stopped
volumes:
redis-data:

Loading