Use systemd-nspawn instead of chroot #74
No reviewers
Labels
No labels
bkctld
bug
duplicate
enhancement
invalid
question
wontfix
zzz_evobackup
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: evolix/evobackup#74
Loading…
Reference in a new issue
No description provided.
Delete branch "server-nspawn"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -466,2 +492,4 @@
grep --extended-regexp --only-matching "${pattern}" "${file}" | tail -1 | cut -d= -f2
}
get_jail_sshd_pid(){
I'm not sure this will work correctly all the time.
In a random test I got this process list :
and the resulted PID was the one for
/bin/sh start.sh
and notsshd
:(Maybe you missed one of the two pgrep?
I checked this morning, it was okay.
But... yeah. The code isn't that clear, thats not helping :v
I've changed it in
f4319c252d
I've changed the sshd script to exec, so it get replaced by sshd, and removed one pgrep accordingly
Would it be a good time to finaly rename the bkctld commands that are confusing (looking at "rm" command) ?
\cc @jlecour
Okay.... this is it I guess.
While I have others idea in mind, I'll keep those for a next version as this one already introduce lots of changes.
I'm looking forward for a review/comments.
@ -58,0 +54,4 @@
echo -n "${output}" | grep -E "^UNKNOWN"
echo -n "${output}" | grep -E "^CRITICAL"
echo -n "${output}" | grep -E "^WARNING"
echo -n "${output}" | grep -E "^OK"
I'm not a fan of replacing printf with echo in general.
shellcheck wasn't happy with it (the intial form).
An alternative could be something like
printf "%s" "${output}" | grep -E "^UNKNOWN"
that wouldn't trigger it@ -77,0 +73,4 @@
echo -n "${output}" | grep -E "^UNKNOWN"
echo -n "${output}" | grep -E "^CRITICAL"
echo -n "${output}" | grep -E "^WARNING"
echo -n "${output}" | grep -E "^OK"
Same here about using echo instead of printf.
@ -92,0 +86,4 @@
echo -n "${output}" | grep -E "^UNKNOWN"
echo -n "${output}" | grep -E "^CRITICAL"
echo -n "${output}" | grep -E "^WARNING"
echo -n "${output}" | grep -E "^OK"
printf to echo :(
@ -0,0 +10,4 @@
jail_name="${1:?}"
if [ -z "${jail_name}" ]; then
show_help && exit 1
fi
Useless test, if
${1:?}
is unset or null shell will fail by itself and won't execute the following test on${jail_name}
.It's the same pattern in other (unchanged) parts of the code.
Shall we replace it with only
jail_name="${1}"
, ensuring that the test would then be reached to display the error message & help ?@ -0,0 +13,4 @@
fi
jail_path=$(jail_path "${jail_name}")
test -d "${jail_path}" || error "${jail_name}: jail not found" 2
Redefining a function by a variable isn't a good idea. Since it's a single used variable, I suggest to just remove it and modify the test to be:
It's used more than once.
But indeed we could replace all occurrences of the variable with the function call. Would it be better?
@ -19,3 +19,3 @@
port="${2}"
ip="${3}"
debug "Accept \`${ip}:${port}' for jail \`${jail_name}'"
debug "Accept '${ip}:${port}' for jail '${jail_name}'"
I would, I have like a separate PR for such stylistic changes peppered in this PR.
@ -17,2 +16,3 @@
jail_rootfs_path=$(jail_rootfs_path "${jail_name}")
test -d "${jail_path}" || error "${jail_name}: jail not found" 2
test -d "${jail_rootfs_path}" || error "${jail_name}: jail rootfs not found" 2
It would be nice to have the error message printing the path that was tested, it simplifies debugging.
@ -19,2 +19,3 @@
jail_sshd_config="${jail_path}/${SSHD_CONFIG}"
# Ensure we manipulate jails in the current/compatible version format
test "$(get_jail_version "${jail_name}")" -ne "${CURRENT_JAIL_VERSION}" && error "${jail_name}: jail needs to be uptated to version ${CURRENT_JAIL_VERSION} (currently $(get_jail_version "${jail_name}")) - Use bkctld convert-v2 <jail>" 3
This line is too long, it probably should be split.
@ -0,0 +11,4 @@
if [ -z "${jail_name}" ]; then
show_help && exit 1
fi
Dead test, just as the previous one.
@ -0,0 +12,4 @@
if [ -z "${jail_name}" ]; then
show_help && exit 1
fi
jail_path=$(jail_path "${jail_name}")
Redefining the
jail_path
function by a variable :/Oh! Nice one :D
@ -0,0 +14,4 @@
fi
jail_path=$(jail_path "${jail_name}")
test -d "${jail_path}" || error "${jail_name}: jail not found" 2
Would be nice to have the path on the failing test.
@ -0,0 +16,4 @@
test -d "${jail_path}" || error "${jail_name}: jail not found" 2
get_jail_version "${jail_name}"
Overall, this script in context with
get_jail_version
seems to be overly complicated, just to cat aversion
file inget_jail_version
.@ -19,0 +18,4 @@
test -d "${jail_rootfs_path}" || error "${jail_name}: jail rootfs not found" 2
# Ensure we manipulate jails in the current/compatible version format
test "$(get_jail_version "${jail_name}")" -ne "${CURRENT_JAIL_VERSION}" && error "${jail_name}: jail needs to be uptated to version ${CURRENT_JAIL_VERSION} (currently $(get_jail_version "${jail_name}")) - Use bkctld convert-v2 <jail>" 3
Line too long.
@ -0,0 +10,4 @@
LIBDIR="$(dirname $0)" && . "${LIBDIR}/includes"
jail_name="${1:?}"
if [ -z "${jail_name}" ]; then
Dead test, like the others.
@ -0,0 +17,4 @@
test -d "${jail_path}" || error "${jail_name}: jail not found" 2
journalctl --reverse --unit systemd-nspawn@"${jail_name}"
TIL
--reverse
the option I didn't knew I needed.@ -16,6 +16,7 @@ if [ -z "${jail_name}" ] || [ -z "${new_jail_name}" ]; then
fi
jail_path=$(jail_path "${jail_name}")
#jail_rootfs_path=$(jail_rootfs_path "${jail_name}")
This variable assignation is commented, but it's used in the next lines.
No. It's not. It's unused (that's why I commented it)
@ -20,2 +20,2 @@
# Prepare the chroot
mount_jail_fs "${jail_name}"
# Ensure we only start jails in the current/compatible version format
test "$(get_jail_version "${jail_name}")" -ne "${CURRENT_JAIL_VERSION}" && error "${jail_name}: jail needs to be updated to version ${CURRENT_JAIL_VERSION} (currently $(get_jail_version "${jail_name}")) - Use bkctld convert-v2 <jail>" 3
Line too long.
@ -14,3 +14,3 @@
[ -d "${JAILDIR}/${jail_name}" ] || error "${jail_name} : jail is missing.\nUse '$0 status [all]' to get the status of all jails."
incs_policy_file=$(current_jail_incs_policy_file ${jail_name})
test "$(get_jail_version "${jail_name}")" -ne "${CURRENT_JAIL_VERSION}" && error "${jail_name}: jail needs to be updated to version ${CURRENT_JAIL_VERSION} (currently $(get_jail_version "${jail_name}")) - Use bkctld convert-v2 <jail>" 3
Line too long.
@ -16,2 +16,4 @@
test -d "${jail_path}" || error "${jail_name}: jail not found" 2
# Ensure we manipulate jails in the current/compatible version format
test "$(get_jail_version "${jail_name}")" -ne "${CURRENT_JAIL_VERSION}" && error "${jail_name}: jail needs to be updated to version ${CURRENT_JAIL_VERSION} (currently $(get_jail_version "${jail_name}")) - Use bkctld convert-v2 <jail>" 3
Line too long.
@ -20,0 +19,4 @@
[ -n "${NODE}" ] || error "Sync need config for \$NODE variable in /etc/default/bkctld !"
# Ensure the same version of bkctld runs on the other node
node_version=$(ssh "${NODE}" grep "^VERSION=" "${LIBDIR}/includes" | cut -d'=' -f2 | sed 's/"//g')
sed 's/"//g'
would not work if someone would replace"
with'
.Will replace it with
| tr -d "'\""
@ -8,2 +8,3 @@
VERSION="22.11"
VERSION="24.08"
CURRENT_JAIL_VERSION="2"
Can't we just use
VERSION
for that?We could. It felt easier/simpler to have a separate numbering scheme.
Tomorrow, 24.09 without any changes in the jail structure, it would stay in jail v2 mode.
So no need to handle the jail upgrade
WIP: Use systemd-nspawn instead of chrootto Use systemd-nspawn instead of chroot3dcc0bc01a
to7f5fe8effa