Skip to content

Conversation

@daniel-santos
Copy link
Contributor

@daniel-santos daniel-santos commented May 27, 2025

It's common to have forks of openwisp-monitoring due to use of $(some_command), so this can return more than one PID. Further, by calling kill "$pid" we just end up with an error about the parameter not being an integer.

Checklist

  • [no] I have read the OpenWISP Contributing Guidelines.
  • [yes] I have manually tested the changes proposed in this pull request.
  • [n/a] I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • [n/a] I have updated the documentation.

Reference to Existing Issue

Closes #157

Description of Changes

Only pick process whose parent is init, ignore others

daniel-santos and others added 2 commits May 27, 2025 02:11
It's common to have forks of openwisp-monitoring due to use of
$(some_command), so this can return more than one PID. Further, by
calling kill "$pid" we just end up with an error about the parameter not
being an integer.
@nemesifier
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

The shell monitoring agent updates its process lookup calls to add -P 1 to pgrep, narrowing matches to processes whose parent is PID 1. Specifically, save_data now uses pgrep -P 1 -f "openwisp-monitoring.*--mode send", and the error path in send_data uses pgrep -P 1 -f "openwisp-monitoring.*--mode collect". The kill commands and their argument quoting remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix bad pgrep / kill' is concise and directly relates to the main change, which restricts pgrep output to processes with init as parent to fix kill failures.
Description check ✅ Passed The description explains the issue and solution but omits required checklist items and lacks detailed reasoning. The description is mostly complete but could be more thorough.
Linked Issues check ✅ Passed The changes address the core objective of #157 by restricting pgrep to processes with init as parent, preventing multiple PIDs that cause kill failures.
Out of Scope Changes check ✅ Passed All changes are in-scope: two pgrep calls now use -P 1 flag to select only processes with init as parent, directly addressing the linked issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e707092 and ca9355f.

📒 Files selected for processing (1)
  • openwisp-monitoring/files/monitoring.agent
🚧 Files skipped from review as they are similar to previous changes (1)
  • openwisp-monitoring/files/monitoring.agent
⏰ 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). (2)
  • GitHub Check: Build and upload package as artifacts
  • GitHub Check: QA-Checks and Tests

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nemesifier
Copy link
Member

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jan 23, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 23, 2026
@nemesifier nemesifier merged commit 425916c into openwisp:master Jan 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] collector fails to send USR1 to messenger instance

2 participants