fix(dgoss): edgecase where no log is written #997
Merged
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
dklimpel
changed the title
dklimpel
changed the title
aelsabbahy
reviewed
Nov 5, 2024
extras/dgoss/dgoss
Outdated
| @@ -66,6 +68,8 @@ run(){ | |||
| esac | |||
|
|
|||
| $CONTAINER_RUNTIME logs -f "$id" > "$tmp_dir/docker_output.log" 2>&1 & | |||
| # There is a chance that the log will not be written, an empty line is added for mitigation. | |||
| echo "" >> "$tmp_dir/docker_output.log" 2>&1 & | |||
There was a problem hiding this comment.
Perhaps touch makes more sense here?
There was a problem hiding this comment.
I will test and check this.
There was a problem hiding this comment.
I have looked at it. touch does not help. I think I have found an better and more transparent solution.
extras/dgoss/dgoss
Outdated
| @@ -20,6 +20,8 @@ cleanup() { | |||
| set +e | |||
| { kill "$log_pid" && wait "$log_pid"; } 2> /dev/null | |||
| if [ -n "$CONTAINER_LOG_OUTPUT" ]; then | |||
| info "Stopping container and copy log" | |||
| $CONTAINER_RUNTIME stop -t 1 "$id" > /dev/null | |||
There was a problem hiding this comment.
Does the -t flag work consistently with podman or docker?
There was a problem hiding this comment.
aelsabbahy
approved these changes
Nov 21, 2024
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.
Checklist
make test-all(UNIX) passes. CI will also test thisDescription of change
I think this is a bug or a edge case. I used
podmanand always got an empty log file.Partly it is discribed: https://docs.podman.io/en/stable/markdown/podman-logs.1.html#follow-f
I'm not sure if it's related to this or a problem with the flushing of the pipe in the operating system.
If the container is stopped before removal and a second pipe is created, the log is written. The content of the second pipe does not arrive and is therefore presumably swallowed.
If I am set timeout to
0($CONTAINER_RUNTIME stop -t 0 "$id" > /dev/null) then the last log entry appears in the log file in the tmp directory, but is apparently written to late and not copied to the target file.