cleanup: regex not being evaluated in quoted expression #605
Conversation
ryansavino
force-pushed
the
cleanup-rm-bug
branch
from
b835815
to
1438d6b
Compare
ryansavino
force-pushed
the
cleanup-rm-bug
branch
from
1438d6b
to
bed97a2
Compare
misc/snapshotter/snapshotter.sh
Outdated
| @@ -141,14 +141,14 @@ EOF | |||
| "${CONTAINER_RUNTIME_CONFIG}".bak | |||
| else | |||
| sed -i '/\[plugins\..*\.containerd\]/a\disable_snapshot_annotations = false' \ | |||
| "${CONTAINER_RUNTIME_CONFIG}".bak | |||
| "${CONTAINER_RUNTIME_CONFIG}".bak | |||
There was a problem hiding this comment.
Can you make this one and the change on line 151 to follow what's in the line 148?
misc/snapshotter/snapshotter.sh
Outdated
| bash -c "rm -f ${NYDUS_BINARY_DIR}/nydus*" | ||
| bash -c "rm -rf ${NYDUS_CONFIG_DIR}/*" | ||
| bash -c "rm -rf ${SNAPSHOTTER_SCRYPT_DIR}/*" | ||
| bash -c "rm -rf ${NYDUS_LIB_DIR}/*" |
There was a problem hiding this comment.
This doesn't change anything with respect to pathname expansion : ${NYDUS_LIB_DIR}/* is expanded all the same above and in the original code. The Regex not evaluated in the rm statement. sentence in the description is thus incorrect.
The problem is that the result of the expansion is passed to rm as a single argument. This causes rm to look for an file that doesn't exist if this argument contains several items, e.g. /path/to/nydus/lib/dir/FOO /path/to/nydus/lib/dir/BAR.
Also, bash -c "" doesn't buy anything here, you could just make it :
| bash -c "rm -rf ${NYDUS_LIB_DIR}/*" | |
| rm -rf ${NYDUS_LIB_DIR}/* |
There was a problem hiding this comment.
Even this command wasn't working during my testing. It had to do with the fact that the script is required to run as sudo.
I found this link, which I think helps explain the issue:
https://superuser.com/questions/980472/remove-files-from-a-root-owned-directory-using-sudo-with-wildcards
There was a problem hiding this comment.
The link above would explain the issue if you needed to do something like :
sudo bash -c "rm -rf ${NYDUS_LIB_DIR}/*"
as the top level shell running the script might not be able to access the content of the ${NYDUS_LIB_DIR}/ directory.
If the script itself is run with sudo then it will be able to access the directory content and honor the pathname expansion.
There was a problem hiding this comment.
Think I understand now. Fixed it.
Cleanup never happening for these directories. Regex needed to be passed outside of quotes. Quoted variable references only. Signed-Off-By: Ryan Savino <ryan.savino@amd.com>
When no containerd-nydus-grpc pid is found, script will exit on this line. Signed-Off-By: Ryan Savino <ryan.savino@amd.com>
Signed-Off-By: Ryan Savino <ryan.savino@amd.com>
ryansavino
force-pushed
the
cleanup-rm-bug
branch
from
bed97a2
to
7faa12c
Compare
Cleanup never happening for these directories.
Regex not evaluated in the rm statement.