filesystem: Bubble up errors rather than panicing #414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Attempt to overlay a file into a directory in the filesystem which doesn't exist. The expected behavior is to cause an error. This test is currently a manual test case and is not hooked into the CI. Signed-off-by: Christopher Obbard <[email protected]>
If the overlay action fails to copy a file into the target filesystem
(e.g. destination path inside the filesystem doesn't exist), the current
behaviour is to panic, which causes a debos call inside the fakemachine
to panic, which isn't detected by the outer debos call.
This causes the execution of the inner debos call to stop (i.e. the
remaining recipe actions are no longer ran which is expected), but the
postexec commands still run and the overall exection is marked as
successful!
Rework the panics to instead bubble up errors so that any errors when
overlaying files causes the recipe to error out correctly rather than
this unexpected behaviour.
Before this commit is applied, the panic causes the debos call inside the
fakemachine to fail without being trapped by the outer debos call and the
outer debos call to still run the postprocess commands and exit with no
error as if the overlay action was successful:
$ debos tests/overlay-non-existent-destination/overlay-non-existent-destination.yaml
2023/07/05 11:07:24 ==== Overlay file to a non-existent destination ====
2023/07/05 11:07:24 Overlaying tests/overlay-non-existent-destination/overlay-non-existent-destination.yaml on /scratch/root/this/path/does/not/exist
2023/07/05 11:07:24 Failed to copy file tests/overlay-non-existent-destination/overlay-non-existent-destination.yaml: open /scratch/root/this/path/does/not/3277940894: no such file or directory
2023/07/05 12:07:24 ==== run ====
2023/07/05 12:07:24 echo Test | Test
2023/07/05 12:07:24 ==== Recipe done ====
$ echo $?
0
With this commit applied, the execution of the outer debos call stops when
the overlay action fails and the error is correctly bubbled up to the user:
$ debos tests/overlay-non-existent-destination/overlay-non-existent-destination.yaml
2023/07/05 11:08:15 ==== Overlay file to a non-existent destination ====
2023/07/05 11:08:15 Overlaying tests/overlay-non-existent-destination/overlay-non-existent-destination.yaml on /scratch/root/this/path/does/not/exist
2023/07/05 11:08:15 Action `Overlay file to a non-existent destination` failed at stage Run, error: Failed to copy file tests/overlay-non-existent-destination/overlay-non-existent-destination.yaml: open /scratch/root/this/path/does/not/1742738134: no such file or directory
$ echo $?
1
Fixes: #401
Signed-off-by: Christopher Obbard <[email protected]>
obbardc
force-pushed
the
wip/obbardc/actions-overlay-fix-panic
branch
from
310bcfc
to
ba3b209
Compare
sjoerdsimons
approved these changes
Jul 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the overlay action fails to copy a file into the target filesystem
(e.g. destination path inside the filesystem doesn't exist), the current
behaviour is to panic, which causes a debos call inside the fakemachine
to panic, which isn't detected by the outer debos call.
This causes the exection of the inner debos call to stop (i.e. the
remaining recipe actions are no longer ran which is expected), but the
postexec commands still run and the overall exection is marked as
successful!
Rework the panics to instead bubble up errors so that any errors when
overlaying files causes the recipe to error out correctly rather than
this unexpected behaviour.