ceph
rbd: Add timeout for cryptsetup commands
Describe what this PR does
This PR modifies the execCryptSetupCommand so that the process is killed in an event of lock timeout.
Useful in cases where the volume lock is released but the command is still running.
This looks reasonable to me. Did you manage to test this somehow, and see how everything gets corrected by a different container?
we need to ensure that the CLI command we start is killed and doesn't continue to execute, please test it the changes to ensure that it works as expected and also provide some test results on how this is tested
This looks reasonable to me. Did you manage to test this somehow, and see how everything gets corrected by a different container?
Trying but not able to yet. I am thinking of having a wrapper to cryptsetup that sleeps for some time and then tries to execute the intended command. The key is to kill the process in midst of operations.
I will test it and report.
Trying but not able to yet. I am thinking of having a wrapper to cryptsetup that sleeps for some time and then tries to execute the intended command. The key is to kill the process in midst of operations.
I also think the only way reproduce this, is by adding a wrapper for the command. It helps to make the command do something that can be verified after aborting, maybe something like this (untested):
#!/bin/sh
#
# Assume a 1GB block-device, write unencrypted data at 500MB offset.
#
# run normal crypsetup, it will be quick and exits cleanly
cryptsetup.real $@
# run dd in a loop, small I/O with sync to make it slower
# only runs the loop if /tmp/testing exists
while [ -e /tmp/testing ]
do
# set correct device and verify parameters
dd if=/dev/urandom of=/dev/rbd... bs=1 count=100m offset=500m
done
Make sure to name the script cryptsetup and place is in the location where cryptsetup normally lives. The original cryptsetup executable should be renamed to cryptsetup.real so that it still can be executed.
Hey @black-dragon74 , I'll mark this as a draft for now, otherwise I keep checking it for updates. Please move it back as ready for review once you have tested it. Thanks!
![mergify[bot] avatar](https://avatars.githubusercontent.com/in/10562?v=4 "Published by a pub.dev verified publisher"){kind=small}
queue
✅ The pull request has been merged automatically
The pull request has been merged automatically at 1c02e69ba49f411390a79eb5025084bed85b6833
