-
Notifications
You must be signed in to change notification settings - Fork 90
Add Warp-specific IBM Cloud VM nightly runs tooling #9230
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
Conversation
WalkthroughAdds a GitHub cloud-init config to bootstrap an IBM Warp Runner VM, introduces orchestration and Slack notifier scripts to run Warp tests and upload logs to IBM COS, updates Makefile and container wrapper scripts to forward CLI args, and adds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CloudInit as Cloud-Init (VM)
participant Runner as run_containerized_warp_on_cloud_runner.sh
participant Docker as Docker Daemon
participant TesterImg as NooBaa Tester Image
participant Container as Test Container
participant Warp as Warp CLI (in container)
participant COS as IBM COS
participant Slack as Slack Webhook
CloudInit->>Runner: Start runner script
activate Runner
Runner->>Docker: Build tester image (make tester)
Docker-->>TesterImg: Image available
Runner->>Docker: Run container with WARP_ARGS (make test-warp)
Docker->>Container: Start test container
Container->>Warp: Execute tests
alt Success
Warp-->>Runner: Exit 0
Runner->>COS: Upload test logs to s3://${WARP_LOGS_BUCKET}/${log_dir}/
Runner->>Slack: Send success notification
else Failure
Warp-->>Runner: Non-zero exit
Runner->>COS: Upload logs/cloud-init to s3://${WARP_LOGS_BUCKET}/${log_dir}/
Runner->>Slack: Send failure notification
end
Runner->>CloudInit: Trigger VM shutdown (EXIT trap)
deactivate Runner
sequenceDiagram
autonumber
participant Make as Make (test-warp / test-nc-warp)
participant Shell as bash -c
participant Wrapper as run_*_warp_on_test_container.sh
participant Node as node run_warp.js
participant Warp as Warp CLI
Make->>Shell: Invoke wrapper via bash -c with $(WARP_ARGS)
Shell->>Wrapper: Execute wrapper script with "$@"
Wrapper->>Node: node run_warp.js "$@"
Node->>Warp: warp ... [includes --obj.randsize if --obj-randsize passed]
Warp-->>Node: Exit code
Node-->>Wrapper: Propagate exit
Wrapper-->>Shell: Propagate exit
Shell-->>Make: Propagate exit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (7)
tools/ibm_runner_helpers/slack_notifier.js (1)
23-28: Consider logging to stderr and using non-zero exit code on failure.The script currently logs errors to stderr but always exits with code 0, even on failure. This prevents calling scripts from detecting notification failures.
Apply this diff to enable failure detection:
} catch (error) { console.error('Notification failed:', error.message); + process.exit(1); } - - process.exit(0); }If silent failures are intentional (e.g., to prevent CI pipeline failures due to notification issues), consider adding a command-line flag like
--fail-on-errorto make this behavior configurable..github/ibm-warp-runner-config.yaml (1)
3-3: Consider pinning package versions for reproducibility.Installing packages without version constraints can lead to inconsistent behavior across VM instances if upstream packages are updated. This is particularly important for Docker, Node.js, and npm, which have breaking changes between major versions.
Consider specifying versions in the format
package=version, e.g.:-packages: [docker.io, git, make, unzip, jq, curl, ca-certificates, nodejs, npm] +packages: [docker.io=24.0.5-0ubuntu1, git, make, unzip, jq, curl, ca-certificates, nodejs=18.x, npm]Alternatively, document the tested/expected versions in a comment for reference.
tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh (5)
9-12: Consider using UTC explicitly for timestamp consistency.The timestamp uses the system's default timezone, which should be UTC on cloud VMs but isn't explicitly guaranteed. For consistency and clarity across different environments, consider using UTC explicitly.
-timestamp=$(date +"%Y-%m-%d-%H-%M-%S") +timestamp=$(date -u +"%Y-%m-%d-%H-%M-%S")
14-22: Consider logging notification failures for debugging.The
|| truepattern correctly prevents notification failures from stopping the script. However, silent failures make it difficult to diagnose why notifications weren't received.send_failure_notification() { local msg="$1" - node /usr/local/bin/slack_notifier.js "${SLACK_NIGHTLY_RESULTS_URL}" failure "Warp failed: ${msg}" || true + node /usr/local/bin/slack_notifier.js "${SLACK_NIGHTLY_RESULTS_URL}" failure "Warp failed: ${msg}" || echo "Warning: Failed to send Slack notification" } send_success_notification() { local log_location="$1" - node /usr/local/bin/slack_notifier.js "${SLACK_NIGHTLY_RESULTS_URL}" success "Warp success - logs available at: ${log_location}" || true + node /usr/local/bin/slack_notifier.js "${SLACK_NIGHTLY_RESULTS_URL}" success "Warp success - logs available at: ${log_location}" || echo "Warning: Failed to send Slack notification" }
31-36: Consider uploading test logs on failure as well.Currently,
handle_failureonly uploads cloud-init logs. If the test fails after producing partial logs, those logs would be valuable for debugging but aren't uploaded.handle_failure() { echo "Uploading cloud-init logs to IBM COS s3://${WARP_LOGS_BUCKET}/${log_dir}/" aws s3 cp "/var/log/cloud-init-output.log" "s3://${WARP_LOGS_BUCKET}/${log_dir}/" --no-progress --endpoint-url "${IBM_COS_ENDPOINT}" + # Upload test logs if they exist + if [ -d "${test_logs_path}" ] && [ "$(ls -A ${test_logs_path})" ]; then + echo "Uploading partial test logs to IBM COS s3://${WARP_LOGS_BUCKET}/${log_dir}/" + aws s3 cp "${test_logs_path}" "s3://${WARP_LOGS_BUCKET}/${log_dir}/test-logs/" --recursive --no-progress --endpoint-url "${IBM_COS_ENDPOINT}" || true + fi echo "Successfully uploaded cloud-init logs to IBM COS s3://${WARP_LOGS_BUCKET}/${log_dir}/" send_failure_notification "Run failed, logs available at s3://${WARP_LOGS_BUCKET}/${log_dir}/" }
38-41: Unnecessary sudo in shutdown_vm.The script runs as root (via cloud-init), so
sudois unnecessary. However, this is harmless and doesn't affect functionality.shutdown_vm() { echo "Shutting down VM..." - sudo shutdown -P now + shutdown -P now }
56-56: Consider parameterizing WARP_ARGS for flexibility.The PR objectives mention "enabling dynamic test parameters," but WARP_ARGS is hardcoded here. If different test configurations are needed (e.g., shorter duration for testing, different object sizes), the script would need to be modified.
Consider sourcing WARP_ARGS from
/etc/warp.envor defaulting to current values:+# Use environment variable if set, otherwise use defaults +WARP_ARGS="${WARP_ARGS:---duration 3h --obj-size 10m --obj-randsize}" + echo "Running Warp tests..." -make test-warp -o tester WARP_ARGS="--duration 3h --obj-size 10m --obj-randsize" +make test-warp -o tester WARP_ARGS="${WARP_ARGS}"Then add
WARP_ARGS=${WARP_ARGS}to the cloud-init config's/etc/warp.envfile.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/ibm-warp-runner-config.yaml(1 hunks)Makefile(1 hunks)src/test/external_tests/warp/run_nc_warp_on_test_container.sh(1 hunks)src/test/external_tests/warp/run_warp.js(3 hunks)src/test/external_tests/warp/run_warp_on_test_container.sh(1 hunks)tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh(1 hunks)tools/ibm_runner_helpers/slack_notifier.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/test/**/*.*
⚙️ CodeRabbit configuration file
src/test/**/*.*: Ensure that the PR includes tests for the changes.
Files:
src/test/external_tests/warp/run_warp.jssrc/test/external_tests/warp/run_warp_on_test_container.shsrc/test/external_tests/warp/run_nc_warp_on_test_container.sh
🧠 Learnings (2)
📚 Learning: 2025-09-18T07:11:55.053Z
Learnt from: Neon-White
PR: noobaa/noobaa-core#9196
File: tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh:3-3
Timestamp: 2025-09-18T07:11:55.053Z
Learning: In the NooBaa project's IBM Cloud Warp testing infrastructure, `/etc/warp.env` is created by cloud-init during VM provisioning with mode 0600 (owner read/write only). This file contains environment variables and secrets passed from GitHub Actions. The file creation is part of a controlled internal flow with proper permissions set automatically, so additional permission checks before sourcing are not necessary in this context.
Applied to files:
.github/ibm-warp-runner-config.yaml
📚 Learning: 2025-09-18T07:11:55.053Z
Learnt from: Neon-White
PR: noobaa/noobaa-core#9196
File: tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh:3-3
Timestamp: 2025-09-18T07:11:55.053Z
Learning: In the NooBaa project's IBM Cloud Warp testing infrastructure, `/etc/warp.env` is created by cloud-init during VM provisioning with permissions 0600 (owner read/write only). This file contains environment variables and secrets passed from GitHub Actions. The file creation is part of a controlled internal flow with proper permissions set automatically by cloud-init, so additional permission checks before sourcing are not necessary in this context.
Applied to files:
.github/ibm-warp-runner-config.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Noobaa Image
- GitHub Check: run-package-lock-validation
- GitHub Check: run-jest-unit-tests
🔇 Additional comments (15)
src/test/external_tests/warp/run_nc_warp_on_test_container.sh (1)
73-73: LGTM! Correct argument forwarding implementation.The use of
"$@"properly forwards all command-line arguments to the Node.js runner while preserving argument boundaries and handling spaces correctly.src/test/external_tests/warp/run_warp_on_test_container.sh (1)
73-73: LGTM! Correct argument forwarding implementation.The use of
"$@"properly forwards all command-line arguments to the Node.js runner while preserving argument boundaries.src/test/external_tests/warp/run_warp.js (3)
31-35: LGTM! Clear documentation for the new flag.The usage documentation correctly describes the new
--obj-randsizeflag with appropriate formatting and default value.
78-78: LGTM! Proper default value handling.The variable declaration correctly uses
argv.obj_randsizewith a falsy default, consistent with other boolean flags in the script.
92-92: No change needed:--obj.randsizeis a valid Warp CLI flag. Confirmed via the official Minio Warp documentation.tools/ibm_runner_helpers/slack_notifier.js (2)
1-11: LGTM! Clean argument parsing and graceful early exit.The script correctly parses command-line arguments and exits gracefully when no webhook URL is provided, which is appropriate for optional notification scenarios.
13-21: LGTM! Simple and correct Slack message construction.The emoji-based status indicator and text-only payload are appropriate for notifications. The webhook integration is straightforward and correct.
Makefile (2)
360-360: Quote WARP_ARGS to handle spaces and special characters safely.The current implementation expands
$(WARP_ARGS)without quoting, which could cause issues ifWARP_ARGScontains spaces or special characters. While the downstream script uses"$@"correctly, the Makefile should quote the variable expansion.Apply this diff to add proper quoting:
- $(CONTAINER_ENGINE) run $(CPUSET) --privileged --user root --network noobaa-net --name noobaa_$(GIT_COMMIT)_$(NAME_POSTFIX) --env "SUPPRESS_LOGS=$(SUPPRESS_LOGS)" --env "POSTGRES_HOST=coretest-postgres-$(GIT_COMMIT)-$(NAME_POSTFIX)" --env "POSTGRES_USER=noobaa" --env "DB_TYPE=postgres" --env "POSTGRES_DBNAME=coretest" -v $(PWD)/logs:/logs $(TESTER_TAG) bash -c "./src/test/external_tests/warp/run_warp_on_test_container.sh $(WARP_ARGS)" + $(CONTAINER_ENGINE) run $(CPUSET) --privileged --user root --network noobaa-net --name noobaa_$(GIT_COMMIT)_$(NAME_POSTFIX) --env "SUPPRESS_LOGS=$(SUPPRESS_LOGS)" --env "POSTGRES_HOST=coretest-postgres-$(GIT_COMMIT)-$(NAME_POSTFIX)" --env "POSTGRES_USER=noobaa" --env "DB_TYPE=postgres" --env "POSTGRES_DBNAME=coretest" -v $(PWD)/logs:/logs $(TESTER_TAG) bash -c './src/test/external_tests/warp/run_warp_on_test_container.sh $(WARP_ARGS)'This uses single quotes around the bash -c command string, allowing
$(WARP_ARGS)to be expanded by bash at runtime, which better handles spaces and preserves the shell's argument parsing.Likely an incorrect or invalid review comment.
368-368: Quote WARP_ARGS to handle spaces and special characters safely.The same quoting issue applies here as in the test-warp target.
Apply this diff:
- $(CONTAINER_ENGINE) run $(CPUSET) --privileged --user root --name noobaa_$(GIT_COMMIT)_$(NAME_POSTFIX) --env "SUPPRESS_LOGS=$(SUPPRESS_LOGS)" -v $(PWD)/logs:/logs $(TESTER_TAG) bash -c "./src/test/external_tests/warp/run_nc_warp_on_test_container.sh $(WARP_ARGS)" + $(CONTAINER_ENGINE) run $(CPUSET) --privileged --user root --name noobaa_$(GIT_COMMIT)_$(NAME_POSTFIX) --env "SUPPRESS_LOGS=$(SUPPRESS_LOGS)" -v $(PWD)/logs:/logs $(TESTER_TAG) bash -c './src/test/external_tests/warp/run_nc_warp_on_test_container.sh $(WARP_ARGS)'Likely an incorrect or invalid review comment.
.github/ibm-warp-runner-config.yaml (4)
5-14: LGTM - secrets handling follows established pattern.The permissions (0600) and file location (/etc/warp.env) align with the established pattern documented in the learnings. The variable substitution syntax is correct for cloud-init.
Based on learnings.
18-18: Verify shutdown timing buffer is sufficient.The VM is scheduled to shut down after 240 minutes (4 hours), but the Warp test runs for 3 hours. This leaves only 1 hour for:
- VM provisioning and setup (docker, AWS CLI, git clone, image build)
- Log upload to COS
- Slack notification
Consider whether this buffer is sufficient, especially if the tester image build is slow or network operations are delayed.
You can validate the typical setup and teardown time by reviewing historical run logs or adding timing instrumentation.
34-38: LGTM - script installation and execution flow is correct.The scripts are installed with appropriate permissions and ownership. The main script execution on line 38 runs as root (via cloud-init), which correctly allows it to read
/etc/warp.env(mode 0600, owned by root) and perform privileged operations like VM shutdown.
22-22: Verify Docker daemon readiness in GitHub runnerWhile
systemctl enable --now dockerstarts the service, Docker may not immediately accept connections under runner images wheretimeoutisn’t available. Before invokingmake testeron line 38, add an explicit readiness loop:#!/bin/bash # Poll for Docker readiness up to 60s for i in {1..60}; do docker info &>/dev/null && break sleep 1 done docker info &>/dev/null || { echo "Docker daemon failed to start" exit 1 }Insert this check into the main script (or as a preceding
runcmd) to avoid build races.tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh (2)
1-7: LGTM - environment setup follows best practices.The strict bash options (
-euxo pipefail) ensure errors are caught and the script fails fast. Theset -apattern correctly exports all variables from/etc/warp.env. Setting HOME to/home/ubuntuis appropriate since cloud-init runs this as root.Based on learnings.
43-58: LGTM - error handling and execution flow are robust.The trap setup ensures that:
- Any command failure triggers
handle_failure(ERR trap)- The VM always shuts down (EXIT trap), even if error handling fails
- The execution flow is straightforward and easy to follow
The
-o testerflag correctly optimizes by skipping the rebuild after line 49.
45e2cae to
876d2c4
Compare
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tools/ibm_runner_helpers/slack_notifier.js (2)
23-25: Consider logging the full error and exiting with a non-zero code on failure.The current implementation logs only
error.messageand always exits with code 0, even when the notification fails. This makes debugging difficult and prevents the caller from detecting failures.Apply this diff to improve error visibility:
} catch (error) { - console.error('Notification failed:', error.message); + console.error('Notification failed:', error); + process.exit(1); } - - process.exit(0);Alternatively, if graceful failure is intentional (to avoid blocking workflows), consider adding a comment explaining the rationale for exiting with code 0.
27-28: Remove redundantprocess.exit(0)call.The explicit
process.exit(0)on line 27 is unnecessary because Node.js will naturally exit when the asyncmain()function completes and there are no pending operations. This explicit exit can prevent proper cleanup of resources.Apply this diff to remove the redundant exit:
} catch (error) { console.error('Notification failed:', error.message); } - - process.exit(0); }If the explicit exit is intentional to ensure the script terminates immediately (e.g., to avoid hanging on unclosed connections), consider adding a comment explaining the rationale.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/ibm-warp-runner-config.yaml(1 hunks)Makefile(1 hunks)src/test/external_tests/warp/run_nc_warp_on_test_container.sh(1 hunks)src/test/external_tests/warp/run_warp.js(3 hunks)src/test/external_tests/warp/run_warp_on_test_container.sh(1 hunks)tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh(1 hunks)tools/ibm_runner_helpers/slack_notifier.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/test/external_tests/warp/run_warp_on_test_container.sh
- src/test/external_tests/warp/run_nc_warp_on_test_container.sh
- .github/ibm-warp-runner-config.yaml
- Makefile
- src/test/external_tests/warp/run_warp.js
- tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Neon-White
PR: noobaa/noobaa-core#9229
File: .github/workflows/ibm-nightly-provision-dispatcher.yaml:13-13
Timestamp: 2025-09-30T08:56:55.452Z
Learning: In the noobaa-core repository, PR #9229 (nightly IBM VM provision dispatcher) has a dependency on `.github/ibm-warp-runner-config.yaml` which is provided in PR #9230, requiring PR #9230 to be merged first.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run-package-lock-validation
- GitHub Check: Build Noobaa Image
- GitHub Check: run-jest-unit-tests
🔇 Additional comments (1)
tools/ibm_runner_helpers/slack_notifier.js (1)
13-21: LGTM!The webhook initialization and message sending logic is correct. The status-based emoji selection and text construction are straightforward and appropriate for a notification tool.
65ce483 to
a605615
Compare
alphaprinz
left a comment
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.
LGTM :)
Signed-off-by: Ben <[email protected]>
a605615 to
76f0d8f
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tools/ibm_runner_helpers/slack_notifier.js (1)
8-10: Exit with non-zero code on validation failure.The script exits with code 0 when required arguments are missing. This prevents callers from detecting misconfiguration. Exit with code 1 to signal an error.
Apply this diff:
if (!webhookUrl || !status || !message) { console.error('Usage: node slack_notifier.js <webhook_url> <status> <message>'); - process.exit(0); + process.exit(1); }.github/ibm-warp-runner-config.yaml (1)
24-24: Consider pinning the npm package version.Installing
@slack/webhookwithout a version specifier pulls the latest version, which could introduce breaking changes in future runs. While the VM is ephemeral, pinning reduces unexpected failures.Apply this diff to pin to a specific version (check current latest and update accordingly):
- - [ npm, -g, install, '@slack/webhook' ] + - [ npm, -g, install, '@slack/[email protected]' ]Or use a semver range like
'@slack/webhook@^7.0.0'to allow patch updates.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/ibm-warp-runner-config.yaml(1 hunks)Makefile(1 hunks)src/test/external_tests/warp/run_nc_warp_on_test_container.sh(1 hunks)src/test/external_tests/warp/run_warp.js(3 hunks)src/test/external_tests/warp/run_warp_on_test_container.sh(1 hunks)tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh(1 hunks)tools/ibm_runner_helpers/slack_notifier.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/test/external_tests/warp/run_warp_on_test_container.sh
- Makefile
- tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh
- src/test/external_tests/warp/run_warp.js
- src/test/external_tests/warp/run_nc_warp_on_test_container.sh
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Neon-White
PR: noobaa/noobaa-core#9229
File: .github/workflows/ibm-nightly-provision-dispatcher.yaml:13-13
Timestamp: 2025-09-30T08:56:55.478Z
Learning: In the noobaa-core repository, PR #9229 (nightly IBM VM provision dispatcher) has a dependency on `.github/ibm-warp-runner-config.yaml` which is provided in PR #9230, requiring PR #9230 to be merged first.
Learnt from: Neon-White
PR: noobaa/noobaa-core#9230
File: tools/ibm_runner_helpers/slack_notifier.js:4-4
Timestamp: 2025-09-30T10:27:57.996Z
Learning: In the noobaa-core repository, the `tools/ibm_runner_helpers/slack_notifier.js` script depends on `slack/webhook`, which is installed globally via `npm -g install 'slack/webhook'` in `.github/ibm-warp-runner-config.yaml` (cloud-init configuration) rather than being declared in package.json, since the script runs on IBM Cloud VMs provisioned by cloud-init.
📚 Learning: 2025-09-30T10:27:57.996Z
Learnt from: Neon-White
PR: noobaa/noobaa-core#9230
File: tools/ibm_runner_helpers/slack_notifier.js:4-4
Timestamp: 2025-09-30T10:27:57.996Z
Learning: In the noobaa-core repository, the `tools/ibm_runner_helpers/slack_notifier.js` script depends on `slack/webhook`, which is installed globally via `npm -g install 'slack/webhook'` in `.github/ibm-warp-runner-config.yaml` (cloud-init configuration) rather than being declared in package.json, since the script runs on IBM Cloud VMs provisioned by cloud-init.
Applied to files:
.github/ibm-warp-runner-config.yamltools/ibm_runner_helpers/slack_notifier.js
📚 Learning: 2025-09-30T08:56:55.478Z
Learnt from: Neon-White
PR: noobaa/noobaa-core#9229
File: .github/workflows/ibm-nightly-provision-dispatcher.yaml:13-13
Timestamp: 2025-09-30T08:56:55.478Z
Learning: In the noobaa-core repository, PR #9229 (nightly IBM VM provision dispatcher) has a dependency on `.github/ibm-warp-runner-config.yaml` which is provided in PR #9230, requiring PR #9230 to be merged first.
Applied to files:
.github/ibm-warp-runner-config.yamltools/ibm_runner_helpers/slack_notifier.js
📚 Learning: 2025-09-18T07:11:55.053Z
Learnt from: Neon-White
PR: noobaa/noobaa-core#9196
File: tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh:3-3
Timestamp: 2025-09-18T07:11:55.053Z
Learning: In the NooBaa project's IBM Cloud Warp testing infrastructure, `/etc/warp.env` is created by cloud-init during VM provisioning with mode 0600 (owner read/write only). This file contains environment variables and secrets passed from GitHub Actions. The file creation is part of a controlled internal flow with proper permissions set automatically, so additional permission checks before sourcing are not necessary in this context.
Applied to files:
.github/ibm-warp-runner-config.yaml
📚 Learning: 2025-09-18T07:11:55.053Z
Learnt from: Neon-White
PR: noobaa/noobaa-core#9196
File: tools/ibm_runner_helpers/run_containerized_warp_on_cloud_runner.sh:3-3
Timestamp: 2025-09-18T07:11:55.053Z
Learning: In the NooBaa project's IBM Cloud Warp testing infrastructure, `/etc/warp.env` is created by cloud-init during VM provisioning with permissions 0600 (owner read/write only). This file contains environment variables and secrets passed from GitHub Actions. The file creation is part of a controlled internal flow with proper permissions set automatically by cloud-init, so additional permission checks before sourcing are not necessary in this context.
Applied to files:
.github/ibm-warp-runner-config.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run-package-lock-validation
- GitHub Check: Build Noobaa Image
- GitHub Check: run-jest-unit-tests
🔇 Additional comments (1)
tools/ibm_runner_helpers/slack_notifier.js (1)
23-27: Silent failure: notification errors don't propagate.The script exits with code 0 even when the webhook send fails. This prevents callers from detecting notification failures. If notification failure should be silent (so it doesn't break the main job), this is acceptable. Otherwise, exit with code 1 on error.
Current behavior: notification failures are logged but don't fail the script. Is this intentional?
If notification failures should propagate:
} catch (error) { console.error('Notification failed:', error.message); + process.exit(1); } - - process.exit(0);
Explain the Changes
bash -cwrapper to properly handleWARP_ARGSexpansion, allowing to pass dynamic test parameters torun_warp.jsthrough the Makefile targetsIssues:
Testing Instructions:
Test local warp execution: Run
make test-warp WARP_ARGS="--duration 1m --obj-size 1KB"to verify the new argument passing mechanism worksTest Slack notifications: Verify the
slack_notifier.jsscript works with:node tools/ibm_runner_helpers/slack_notifier.js <webhook_url> success "Test message"Validate cloud-init config: Use
cloud-init schema --config-file .github/ibm-warp-runner-config.yamlto verify configuration syntaxDoc added/updated
Summary by CodeRabbit
New Features
Tests
Chores