Updated lxc-local template #4368
Conversation
|
This pull request didn't trigger Jenkins as its author isn't in the allow list. An organization member must perform one of the following:
Those commands are simple Github comments of the format: "jenkins: COMMAND" |
templates/lxc-local.in
Outdated
| # shellcheck disable=SC2086 | ||
| tar --anchored ${EXCLUDES} --numeric-owner -xpJf "${LXC_FSTREE}" -C "${LXC_ROOTFS}" | ||
|
|
||
| prepare_rootfs |
There was a problem hiding this comment.
What's this? That function isn't defined at this stage and also isn't part of the code being moved to the new unpack_rootfs function.
There was a problem hiding this comment.
I'll try to order the commits more properly next time. Doing this, I worked using my final product and tried to re-do the changes I made so they were associated with individual commits. In the future I will likely just commit one change at a time, in order.
templates/lxc-local.in
Outdated
| if tar Jxf "${LXC_METADATA}" -C "${LOCAL_TEMP}"; then | ||
| echo "Unpacked metadata file: ${LXC_METADATA}" | ||
| process_excludes | ||
| process_config | ||
| process_fstab | ||
| process_templates | ||
| else | ||
| echo "Unable to unpack metadata file: ${LXC_METADATA}" 2>&1 | ||
| exit 1 |
There was a problem hiding this comment.
Same comment as the other command, if you flip the if statement around, and just call exit 1 in the condition, then you can avoiding needing an else and save a level of indentation for most the function.
templates/lxc-local.in
Outdated
| @@ -302,13 +312,27 @@ prepare_rootfs() { | |||
|
|
|||
| unpack_rootfs() { | |||
| # Unpack the rootfs | |||
| echo "Unpacking the rootfs to: ${LXC_ROOTFS}" | |||
| if [ -n "${LXC_FSTREE}" ]; then | |||
| if [ -f "${LXC_FSTREE}" ]; then | |||
templates/lxc-local.in
Outdated
| @@ -302,13 +312,27 @@ prepare_rootfs() { | |||
|
|
|||
| unpack_rootfs() { | |||
| # Unpack the rootfs | |||
| echo "Unpacking the rootfs to: ${LXC_ROOTFS}" | |||
| if [ -n "${LXC_FSTREE}" ]; then | |||
|
I think I've resolved most of these issues, please let me know if I missed any |
|
jenkins: test this please |
|
I think I've added the missing signoff |
|
Testsuite passed |
templates/lxc-local.in
Outdated
| unpack_metadata() { | ||
| # Unpack file that contains the container metadata | ||
| # If the file does not exist, just warn and continue. | ||
| if [ -n "${LXC_METADATA}" ]; then |
There was a problem hiding this comment.
Both of those if statements should still get the indent fix.
Making the function immediately return if the medata file isn't passed or if it doesn't exist.
|
Looking good, I'll do some history editing, likely squash everything into a single commit with a list of changes and fix a couple more nits I noticed by going through it again, then I'll merge. Thanks for the work! |
…e clear Signed-off-by: Zen <z@pyl.onl>
|
Thanks for the review, I'm happy to contribute. |
Signed-off-by: Zen <z@pyl.onl>
Signed-off-by: Zen <z@pyl.onl>
Signed-off-by: Zen <z@pyl.onl>
|
Ended up rebasing it down to 4 distinct commits that I think represent the changes in this PR pretty well. |
I refactored the script to use functions, added more reasonable checks for existence of the metadata tarball, improved variable naming, added a "--no-dev" option to exclude /dev/ from tarballs (to make creating unprivileged containers easier)