Prevent rc.docker from trying to start up non-unRAID containers#2610
Prevent rc.docker from trying to start up non-unRAID containers#2610andrebrait wants to merge 1 commit intounraid:masterfrom
Conversation
* Docker Compose handles its own startup * Both unRAID and Compose use the `net.unraid.docker.managed` label, but with different values * unRAID trying to start Compose containers may cause them to fail to start * This commit fixes the behavior above by only attempting to manage dockerman-managed containers
WalkthroughUpdated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@etc/rc.d/rc.docker`:
- Around line 160-165: The docker listing commands are using grep (which does
substring matches) causing false positives; update both docker container ls
invocations (the one before the "Running containers" comment and the
running_containers() function) to use Docker's label filter instead of grep: add
--filter 'label=net.unraid.docker.managed=dockerman' to docker container ls,
remove the grep and keep/adjust the --format to output just '{{.Names}}' (so you
can drop the awk '{print $1}' step), ensuring exact key=value label matching and
no substring matches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| docker container ls -a --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=dockerman' | awk '{print $1}' | ||
| } | ||
|
|
||
| # Running containers | ||
| running_containers(){ | ||
| docker ps --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=' | awk '{print $1}' | ||
| docker container ls --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=dockerman' | awk '{print $1}' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify grep regex pitfalls with current pattern:
set -euo pipefail
pattern='net.unraid.docker.managed=dockerman'
printf '%s\n' \
'ok net.unraid.docker.managed=dockerman,foo=bar' \
'bad1 netXunraidXdockerXmanaged=dockerman,foo=bar' \
'bad2 net.unraid.docker.managed=dockerman-extra,foo=bar' \
| grep "$pattern" || true
echo "Expected: current grep matches all 3 lines; only first should be considered managed."
echo "Using docker --filter label=... avoids this class of mismatch."Repository: unraid/webgui
Length of output: 355
🏁 Script executed:
cd /root/webgui && wc -l etc/rc.d/rc.dockerRepository: unraid/webgui
Length of output: 119
🏁 Script executed:
cd /root/webgui && sed -n '155,170p' etc/rc.d/rc.dockerRepository: unraid/webgui
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Test Docker label filtering behavior (simulation with echo):
# Simulate docker container ls output with labels
docker_output='
container1
container2
container3
'
echo "=== Testing grep with current pattern ==="
printf '%s\n' \
'mycontainer net.unraid.docker.managed=dockerman,other=value' \
'badcontainer net.unraid.docker.managed=dockerman-extra,other=value' \
'othercontainer some.other.label=value' \
| grep 'net.unraid.docker.managed=dockerman' && echo "Matched above (should be 2 but may be 1)" || true
echo ""
echo "=== Testing grep with -F (literal) flag ==="
printf '%s\n' \
'mycontainer net.unraid.docker.managed=dockerman,other=value' \
'badcontainer net.unraid.docker.managed=dockerman-extra,other=value' \
'othercontainer some.other.label=value' \
| grep -F 'net.unraid.docker.managed=dockerman' && echo "Matched above" || true
echo ""
echo "=== Checking if Docker supports --filter with label= ==="
docker container ls --help 2>/dev/null | grep -A 5 '\--filter' | head -20Repository: unraid/webgui
Length of output: 510
🏁 Script executed:
git ls-files | grep -E '(rc\.docker|etc/rc\.d)' | head -20Repository: unraid/webgui
Length of output: 423
🏁 Script executed:
git ls-files | xargs grep -l 'all_containers\|running_containers' 2>/dev/null | head -5Repository: unraid/webgui
Length of output: 75
🏁 Script executed:
cat -n etc/rc.d/rc.docker | sed -n '155,170p'Repository: unraid/webgui
Length of output: 611
🏁 Script executed:
# Test Docker --filter with label syntax
docker container ls --help 2>&1 | grep -A 10 'filter' | head -20Repository: unraid/webgui
Length of output: 39
🌐 Web query:
Docker container ls --filter label exact match behavior documentation
💡 Result:
For docker container ls / docker ps, the label filter only supports:
- Key present:
--filter "label=color"matches containers that have thecolorlabel, regardless of value. [1] - Exact key=value:
--filter "label=color=blue"matches containers whosecolorlabel value is exactlyblue(it’s not documented as doing partial/substring or regex matching for label values). [1]
Docker labels are stored/compared as strings (Docker doesn’t deserialize structured values like JSON for “querying/filtering by label value”). [3]
If you need substring/regex-like behavior, Docker’s general filter docs note that some filters support partial/regex matching but this is command/filter-specific—so you generally have to post-filter the output (e.g., --format … | grep/jq) when the label filter isn’t expressive enough. [2]
Sources
[1] Docker CLI reference: docker container ls (Filtering / label) (docs.docker.com)
[2] Docker Docs: “Filter commands” (some filters exact vs partial vs regex; command-specific) (docs.docker.com)
[3] Docker Docs: “Docker object labels” (labels are strings; no deserialization for querying/filtering) (docs.docker.com)
Citations:
- 1: https://docs.docker.com/reference/cli/docker/container/ls
- 2: https://docs.docker.com/config/filter/
- 3: https://docs.docker.com/engine/manage-resources/labels/
Use Docker label filters instead of grep to avoid substring matching false positives.
Lines 160 and 165 use grep 'net.unraid.docker.managed=dockerman' which matches substrings (e.g., dockerman-extra). This can re-include unintended containers, undermining the PR's goal.
Docker's native --filter 'label=net.unraid.docker.managed=dockerman' performs exact key=value matching and is the correct approach.
Proposed fix
all_containers(){
- docker container ls -a --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=dockerman' | awk '{print $1}'
+ docker container ls -a --filter 'label=net.unraid.docker.managed=dockerman' --format='{{.Names}}' 2>/dev/null
}
# Running containers
running_containers(){
- docker container ls --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=dockerman' | awk '{print $1}'
+ docker container ls --filter 'label=net.unraid.docker.managed=dockerman' --format='{{.Names}}' 2>/dev/null
}📝 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.
| docker container ls -a --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=dockerman' | awk '{print $1}' | |
| } | |
| # Running containers | |
| running_containers(){ | |
| docker ps --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=' | awk '{print $1}' | |
| docker container ls --format='{{.Names}} {{.Labels}}' 2>/dev/null | grep 'net.unraid.docker.managed=dockerman' | awk '{print $1}' | |
| all_containers(){ | |
| docker container ls -a --filter 'label=net.unraid.docker.managed=dockerman' --format='{{.Names}}' 2>/dev/null | |
| } | |
| # Running containers | |
| running_containers(){ | |
| docker container ls --filter 'label=net.unraid.docker.managed=dockerman' --format='{{.Names}}' 2>/dev/null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@etc/rc.d/rc.docker` around lines 160 - 165, The docker listing commands are
using grep (which does substring matches) causing false positives; update both
docker container ls invocations (the one before the "Running containers" comment
and the running_containers() function) to use Docker's label filter instead of
grep: add --filter 'label=net.unraid.docker.managed=dockerman' to docker
container ls, remove the grep and keep/adjust the --format to output just
'{{.Names}}' (so you can drop the awk '{print $1}' step), ensuring exact
key=value label matching and no substring matches.
|
I found a bug when restoring user networks. Will update the script with the fix. |
1 participant
net.unraid.docker.managedlabel, but with different valuesSummary by CodeRabbit