.github/workflows/load-tests.yaml (1)

Line range hint 149-157: Consider enhancing error handling and test status monitoring

Instead of using fixed sleep durations for test bootstrap and completion, consider implementing a more robust solution:

1. Poll the k6 operator status to confirm test bootstrap
2. Monitor the test execution status instead of sleeping
3. Add timeout and failure handling

This would make the workflow more reliable and provide better feedback on test progress.

Would you like me to provide an example implementation of these improvements?

.github/workflows/merge.yaml (1)

84-89: LGTM! Consider documenting the new load testing parameters.

The changes to load testing parameters align well with the websocket implementation:

• cluster_nodes: core for core node testing
• topology: many2many for multiple concurrent connections
• workload: constant for sustained load
• vu parameter for virtual user simulation

Consider adding documentation about these new parameters and their expected values in the repository's testing documentation.

🧰 Tools

🪛 actionlint

87-87: input "duration" is typed as number by reusable workflow "./.github/workflows/load-tests.yaml". string value cannot be assigned

(expression)

---

88-88: input "vu" is typed as number by reusable workflow "./.github/workflows/load-tests.yaml". string value cannot be assigned

(expression)

---

89-89: input "rate" is typed as number by reusable workflow "./.github/workflows/load-tests.yaml". string value cannot be assigned

(expression)

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Search for any remaining usage of the old parameter name
rg "nodes:" .github/workflows/ -A 5 -B 5

#!/bin/bash
# Description: Check for direct references to this workflow and its network parameter
# to ensure all callers explicitly specify the network when needed

# Search for workflow references
echo "Checking for workflow references:"
rg -A 5 "uses: .*build-dappnode\.yaml"

# Search for manual workflow dispatch events in the UI
echo "Checking for workflow_dispatch events:"
rg "workflow_dispatch.*build-dappnode"

#!/bin/bash
# Let's check the workflow file itself to understand how the network parameter is used
# and what safeguards might already be in place

# Check the workflow file content
echo "Checking workflow file content:"
cat .github/workflows/build-dappnode.yaml

# Also check for any other workflow files that might be related
echo -e "\nChecking other workflow files:"
fd -e yaml -e yml . .github/workflows/

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Search for connection setup and wait logic
rg -A 10 "connection.*setup|bootstrap" ./k6

#!/bin/bash
# Look for websocket-related changes in other workflow files
rg -A 5 "websocket" .github/workflows/
# Check for any load test configuration files that might have been modified
fd -g "*load-test*.{yaml,json}" | xargs cat

#!/bin/bash
# Check for any other workflow files that might be affected by this network change
rg -l "network.*dufour" .github/workflows/
# Check for any documentation that might need updating
fd -g "*.md" | xargs rg "dufour.*default"

#!/bin/bash
# Let's check for any references to network configurations and their defaults
rg -A 3 "network.*default" .
# Also check for any references to 'rotsee' to understand the context
rg -A 3 "rotsee" .
# Check for any configuration files that might define networks
fd -g "*config*.{yaml,json,ts,js}" | xargs cat

.github/workflows/build-dappnode.yaml (1)

109-110: LGTM! Consider standardizing commit hash length

Good improvement to version handling for non-standard branches. Using the commit hash provides better traceability and allows for unique versioning of development builds.

Consider using a fixed length for the commit hash (e.g., --short=8) to ensure consistent version string lengths:

-last_commit=$(git rev-parse --short HEAD)
+last_commit=$(git rev-parse --short=8 HEAD)

Configuration used: .coderabbit.yaml
Review profile: CHILL

.github/workflows/load-tests.yaml (5)

Line range hint 13-30: Add default values to workflow_call inputs

The workflow_call inputs lack default values that are present in workflow_dispatch. This inconsistency could cause issues when the workflow is called programmatically.

Add default values to match workflow_dispatch:

      rate:
        type: number
        required: true
+       default: 50
      vu:
        type: number
        required: true
+       default: 1

---

45-53: Document topology options with descriptions

The topology choices would benefit from descriptions explaining their purpose and impact on the test setup.

Add descriptions like:

      topology:
        type: choice
        options:
          - many2many  # Tests message passing between all nodes
          - sender     # Tests dedicated sender nodes
          - relayer    # Tests intermediate relay nodes
          - receiver   # Tests dedicated receiver nodes
        required: true
-       description: 'Nodes channel topology'
+       description: 'Node channel topology (many2many: all-to-all communication, sender: dedicated senders, relayer: intermediate nodes, receiver: dedicated receivers)'

---

96-114: Reduce duplication in environment variable setup

The environment variable setup is duplicated between manual and programmatic triggers. This could be simplified to reduce maintenance overhead.

Consider refactoring to:

      - name: Set environment variables
        id: vars
        run: |
+         # Determine input source based on trigger type
+         input_source="${{ github.event.inputs.test_id }}" && source="github.event.inputs" || source="inputs"
+         
          {
-           if [ -z "${{ github.event.inputs.test_id }}" ]; then
-             echo "The workflow is triggered by other pipeline"
-             {
-               echo "TESTID=${{ inputs.test_id }}"
-               echo "K6_CLUSTER_NODES=${{ inputs.cluster_nodes }}"
-               echo "K6_TOPOLOGY_NAME=${{ inputs.topology }}"
-               echo "K6_WORKLOAD_NAME=${{ inputs.workload }}"
-               echo "K6_TEST_DURATION=${{ inputs.duration }}"
-               echo "K6_VU_PER_ROUTE=${{ inputs.vu }}"
-               echo "K6_REQUESTS_PER_SECOND_PER_VU=${{inputs.rate }}"
-             } >> $GITHUB_ENV
-           else
-             echo "The workflow is triggered manually"
-             {
-               echo "TESTID=${{ github.event.inputs.test_id }}"
-               echo "K6_CLUSTER_NODES=${{ github.event.inputs.cluster_nodes }}"
-               echo "K6_TOPOLOGY_NAME=${{ github.event.inputs.topology }}"
-               echo "K6_WORKLOAD_NAME=${{ github.event.inputs.workload }}"
-               echo "K6_TEST_DURATION=${{ github.event.inputs.duration }}"
-               echo "K6_VU_PER_ROUTE=${{ github.event.inputs.vu }}"
-               echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ github.event.inputs.rate }}"
-             } >> $GITHUB_ENV
-           fi
+           echo "TESTID=${{ ${source}.test_id }}"
+           echo "K6_CLUSTER_NODES=${{ ${source}.cluster_nodes }}"
+           echo "K6_TOPOLOGY_NAME=${{ ${source}.topology }}"
+           echo "K6_WORKLOAD_NAME=${{ ${source}.workload }}"
+           echo "K6_TEST_DURATION=${{ ${source}.duration }}"
+           echo "K6_VU_PER_ROUTE=${{ ${source}.vu }}"
+           echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ ${source}.rate }}"
          } >> $GITHUB_ENV

---

Line range hint 153-157: Replace fixed sleep with dynamic test completion check

Using fixed sleep durations is fragile and could lead to premature termination or unnecessary waiting.

Consider implementing a loop that checks for test completion:

-         echo "Waiting for test to bootstrap..."
-         sleep 2m # Wait for test to bootstrap
-         echo "Waiting for test to finish..."
-         sleep ${{ steps.vars.outputs.DURATION }}m # wait for the test to finish
+         echo "Waiting for test to bootstrap..."
+         kubectl wait --for=condition=initialized k6 k6-test --timeout=5m
+         echo "Waiting for test to finish..."
+         kubectl wait --for=condition=complete k6 k6-test --timeout=$(( ${{ steps.vars.outputs.DURATION }} + 5 ))m

---

Line range hint 160-165: Add error handling for test results collection

The results collection step should validate the test's success and handle potential failures.

Add error handling:

      - name: Collect Load test results
        run: |
+         # Check if test completed successfully
+         if ! kubectl get k6 k6-test -o json | jq -e '.status.stage == "finished"' > /dev/null; then
+           echo "::error::Load test failed or did not complete properly"
+           exit 1
+         fi
          kubectl delete -f k6-test-results.yaml 2> /dev/null || true
          kubectl apply -f k6-test-results.yaml
+         # Verify results were collected
+         if ! kubectl wait --for=condition=complete k6-test-results --timeout=2m; then
+           echo "::error::Failed to collect test results"
+           exit 1
+         fi

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

.github/workflows/build.yaml (1)

42-42: Remove trailing whitespace.

There's a trailing whitespace at the end of line 42.

-    needs: 
+    needs:

🧰 Tools

🪛 yamllint

[error] 42-42: trailing spaces

(trailing-spaces)

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Description: Verify the workflow dependencies and related changes
# Check for any other workflow files that might need similar dependencies

# Look for other workflow files that might reference build-docker or build-dappnode
rg -l "build-docker|build-dappnode" .github/workflows/

#!/bin/bash
# Let's verify the build.yaml workflow to understand the complete job dependency chain
rg -A 10 "jobs:" .github/workflows/build.yaml

# Also check if build-docker job exists and its dependencies
rg -A 15 "build-docker:" .github/workflows/build.yaml

# Check if there are any other jobs that might need to depend on build-docker
rg "needs.*build-docker" .github/workflows/

#!/bin/bash
# Let's check the build-dappnode job configuration and its relationship with Docker builds
rg -A 10 "build-dappnode:" .github/workflows/build.yaml

# Check if there are any reusable workflow files that might be relevant
fd 'build.*ya?ml' .github/workflows/

# Check the content of build-docker.yaml to understand the complete workflow
rg -l "build-docker.yaml" .github/workflows/ | xargs cat

Configuration used: .coderabbit.yaml
Review profile: CHILL

.github/workflows/load-tests.yaml (5)

13-15: Add descriptions to workflow_call inputs

While workflow_dispatch inputs have helpful descriptions, the corresponding workflow_call inputs lack descriptions. Consider adding descriptions to improve maintainability.

Note: I see from the learnings that the enum property isn't supported in workflow_call, so the different validation approaches are expected.

Also applies to: 37-44

---

26-30: Add default values to workflow_call inputs

The workflow_dispatch inputs have default values for vu (1) and rate (50), but these defaults are missing in the workflow_call inputs. Consider adding default values to ensure consistent behavior.

Also applies to: 69-78

---

96-103: Consider extracting common environment variables to a function

The environment variable setup is duplicated between the manual and pipeline triggers. Consider extracting this to a shell function to reduce duplication and improve maintainability.

      - name: Set environment variables
        id: vars
        run: |
+         setup_env_vars() {
+           local prefix=$1
+           {
+             echo "TESTID=$prefix${{ github.event.inputs.test_id }}"
+             echo "K6_CLUSTER_NODES=$prefix${{ github.event.inputs.cluster_nodes }}"
+             echo "K6_TOPOLOGY_NAME=$prefix${{ github.event.inputs.topology }}"
+             echo "K6_WORKLOAD_NAME=$prefix${{ github.event.inputs.workload }}"
+             echo "K6_TEST_DURATION=$prefix${{ github.event.inputs.duration }}"
+             echo "K6_VU_PER_ROUTE=$prefix${{ github.event.inputs.vu }}"
+             echo "K6_REQUESTS_PER_SECOND_PER_VU=$prefix${{ github.event.inputs.rate }}"
+           } >> $GITHUB_ENV
+         }
+         
          if [ -z "${{ github.event.inputs.test_id }}" ]; then
            echo "The workflow is triggered by other pipeline"
-           {
-             echo "TESTID=${{ inputs.test_id }}"
-             echo "K6_CLUSTER_NODES=${{ inputs.cluster_nodes }}"
-             echo "K6_TOPOLOGY_NAME=${{ inputs.topology }}"
-             echo "K6_WORKLOAD_NAME=${{ inputs.workload }}"
-             echo "K6_TEST_DURATION=${{ inputs.duration }}"
-             echo "K6_VU_PER_ROUTE=${{ inputs.vu }}"
-             echo "K6_REQUESTS_PER_SECOND_PER_VU=${{inputs.rate }}"
-           } >> $GITHUB_ENV
+           setup_env_vars "\${{ inputs."
          else
            echo "The workflow is triggered manually"
-           {
-             echo "TESTID=${{ github.event.inputs.test_id }}"
-             echo "K6_CLUSTER_NODES=${{ github.event.inputs.cluster_nodes }}"
-             echo "K6_TOPOLOGY_NAME=${{ github.event.inputs.topology }}"
-             echo "K6_WORKLOAD_NAME=${{ github.event.inputs.workload }}"
-             echo "K6_TEST_DURATION=${{ github.event.inputs.duration }}"
-             echo "K6_VU_PER_ROUTE=${{ github.event.inputs.vu }}"
-             echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ github.event.inputs.rate }}"
-           } >> $GITHUB_ENV
+           setup_env_vars "\${{ github.event.inputs."
          fi

Also applies to: 107-114

---

Line range hint 153-157: Replace fixed sleep with dynamic wait

Using a fixed sleep duration (2m) for test bootstrap might be unreliable. Consider implementing a polling mechanism to check the test status.

-         echo "Waiting for test to bootstrap..."
-         sleep 2m # Wait for test to bootstrap
+         echo "Waiting for test to bootstrap..."
+         kubectl wait --for=condition=ready k6 k6-test --timeout=5m
          echo "Waiting for test to finish..."
          sleep ${{ steps.vars.outputs.DURATION }}m # wait for the test to finish

---

Line range hint 160-165: Add error handling for kubectl operations

The kubectl operations lack proper error handling and status checks. Consider adding validation to ensure the operations completed successfully.

      - name: Collect Load test results
        run: |
          kubectl delete -f k6-test-results.yaml 2> /dev/null || true
          kubectl apply -f k6-test-results.yaml
+         if ! kubectl wait --for=condition=complete k6 k6-test-results --timeout=2m; then
+           echo "Failed to collect test results"
+           exit 1
+         fi

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

.github/workflows/load-tests.yaml (1)

102-102: Fix spacing in variable reference

There's inconsistent spacing in the variable reference.

-              echo "K6_REQUESTS_PER_SECOND_PER_VU=${{inputs.rate }}"
+              echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ inputs.rate }}"

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

.github/workflows/load-tests.yaml (2)

102-102: Fix inconsistent spacing in environment variable assignment

There's an extra space between inputs. and rate that should be removed for consistency.

-              echo "K6_REQUESTS_PER_SECOND_PER_VU=${{inputs.rate }}"
+              echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ inputs.rate }}"

---

141-142: Improve shell script safety with proper quoting

The shell script should use proper quoting to prevent word splitting and handle special characters in variable values.

-          hoprdApiToken=$(kubectl get secret -n k6-operator-system -o json hoprd-api-tokens | jq --arg entry "${K6_CLUSTER_NODES}" -r '.data.${entry}' | base64 -d)
-          echo "HOPRD_API_TOKEN=${hoprdApiToken}" >> $GITHUB_ENV
+          hoprdApiToken=$(kubectl get secret -n k6-operator-system -o json hoprd-api-tokens | jq --arg entry "${K6_CLUSTER_NODES}" -r '.data[$entry]' | base64 -d)
+          echo "HOPRD_API_TOKEN=${hoprdApiToken}" >> "$GITHUB_ENV"

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

-          ref: ausias/weboscket-bidirectional # main
+          ref: main

#!/bin/bash
# Description: Check if there are any timing-related issues reported in past test runs

# Search for timing-related issues in test results
kubectl logs -n k6-operator-system -l app=k6-operator --tail=1000 | grep -i "timeout\|duration\|exceeded"

# Check if any tests were terminated before completion
kubectl get k6 -n k6-operator-system -o json | jq '.items[] | select(.status.stage == "terminated") | {name: .metadata.name, stage: .status.stage, reason: .status.reason}'

#!/bin/bash
# Let's analyze the workflow file and related test configurations to understand timing settings

# Check the workflow file for timing-related configurations
rg -A 2 -B 2 "duration|sleep|timeout" .github/workflows/load-tests.yaml

# Look for test configuration files that might contain timing settings
fd -e json -e js -e yaml -e yml . | xargs rg -l "duration|timeout"

# Search for any timing-related constants or configurations in test setup files
fd setup | xargs rg -l "duration|timeout|sleep"

#!/bin/bash
# Let's check the test configurations and k6 scripts to understand the actual test requirements

# Check k6 test scripts for timing configurations
fd -e js . k6/ | xargs rg -A 3 -B 3 "duration|sleep"

# Look for topology-specific setup scripts
fd setup k6/ | xargs cat

# Check k6 test configuration template
cat k6/k6-test-run.yaml

#!/bin/bash
# Let's check the workflow file for test duration and sleep configurations

# Look for test duration and sleep configurations in the workflow file
rg -A 5 -B 5 "K6_TEST_DURATION|sleep" .github/workflows/load-tests.yaml

# Check for any topology-specific setup scripts that might affect timing
fd -e sh -e js -e yaml -e yml . | xargs rg -l "setup|bootstrap|duration|sleep"

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

-          ref: ausias/weboscket-bidirectional # main
+          ref: main

.github/workflows/load-tests.yaml (1)

102-102: Fix spacing in variable reference

There's an extra space between inputs and the dot operator.

-              echo "K6_REQUESTS_PER_SECOND_PER_VU=${{inputs.rate }}"
+              echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ inputs.rate }}"

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.

-          npm run setup:${K6_TOPOLOGY_NAME}
+          npm run "setup:${K6_TOPOLOGY_NAME}"

.github/workflows/load-tests.yaml (2)

95-114: Consider reducing code duplication in environment variable setup

The current implementation duplicates the environment variable assignments between pipeline and manual triggers. Consider refactoring to reduce duplication.

- if [ -z "${{ github.event.inputs.test_id }}" ]; then
-   echo "The workflow is triggered by other pipeline"
-   {
-     echo "TESTID=${{ inputs.test_id }}"
-     echo "K6_CLUSTER_NODES=${{ inputs.cluster_nodes }}"
-     echo "K6_TOPOLOGY_NAME=${{ inputs.topology }}"
-     echo "K6_WORKLOAD_NAME=${{ inputs.workload }}"
-     echo "K6_TEST_DURATION=${{ inputs.duration }}"
-     echo "K6_VU_PER_ROUTE=${{ inputs.vu }}"
-     echo "K6_REQUESTS_PER_SECOND_PER_VU=${{inputs.rate }}"
-   } >> $GITHUB_ENV
- else
-   echo "The workflow is triggered manually"
-   {
-     echo "TESTID=${{ github.event.inputs.test_id }}"
-     echo "K6_CLUSTER_NODES=${{ github.event.inputs.cluster_nodes }}"
-     echo "K6_TOPOLOGY_NAME=${{ github.event.inputs.topology }}"
-     echo "K6_WORKLOAD_NAME=${{ github.event.inputs.workload }}"
-     echo "K6_TEST_DURATION=${{ github.event.inputs.duration }}"
-     echo "K6_VU_PER_ROUTE=${{ github.event.inputs.vu }}"
-     echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ github.event.inputs.rate }}"
-   } >> $GITHUB_ENV
- fi
+ PREFIX="${{ github.event.inputs.test_id != '' && 'github.event.inputs.' || 'inputs.' }}"
+ {
+   echo "TESTID=${{ github.event.inputs.test_id != '' && github.event.inputs.test_id || inputs.test_id }}"
+   echo "K6_CLUSTER_NODES=${{ github.event.inputs.test_id != '' && github.event.inputs.cluster_nodes || inputs.cluster_nodes }}"
+   echo "K6_TOPOLOGY_NAME=${{ github.event.inputs.test_id != '' && github.event.inputs.topology || inputs.topology }}"
+   echo "K6_WORKLOAD_NAME=${{ github.event.inputs.test_id != '' && github.event.inputs.workload || inputs.workload }}"
+   echo "K6_TEST_DURATION=${{ github.event.inputs.test_id != '' && github.event.inputs.duration || inputs.duration }}"
+   echo "K6_VU_PER_ROUTE=${{ github.event.inputs.test_id != '' && github.event.inputs.vu || inputs.vu }}"
+   echo "K6_REQUESTS_PER_SECOND_PER_VU=${{ github.event.inputs.test_id != '' && github.event.inputs.rate || inputs.rate }}"
+ } >> $GITHUB_ENV

---

Line range hint 152-157: Consider making bootstrap wait time configurable

The hardcoded 2-minute bootstrap wait time could be insufficient for larger deployments or slower environments. Consider making this configurable through an input parameter.

+ bootstrap_duration:
+   type: number
+   default: 2
+   required: false
+   description: 'Bootstrap wait time in minutes'

- sleep 2m # Wait for test to bootstrap
+ sleep "${BOOTSTRAP_DURATION:-2}m" # Wait for test to bootstrap

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: ausias-armesto
PR: hoprnet/hoprnet#6640
File: .github/workflows/load-tests.yaml:13-15
Timestamp: 2024-11-18T10:01:11.871Z
Learning: In GitHub Actions workflows, inputs under `workflow_call` do not support the `enum` property; this property is only available under `workflow_dispatch`.