-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #14814: Send the run log at the end of the run through HTTP #224
Fixes #14814: Send the run log at the end of the run through HTTP #224
Conversation
PR rebased |
85946ff
to
d96dc2c
Compare
qa-test
Outdated
set -ex | ||
|
||
# Shellcheck | ||
find . -path ./.git -prune -o -type f -exec grep -Eq '^#!(.*/|.*env +)(sh|bash|ksh)' {} \; -print | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion for more performance with big files:
find . -type f -exec bash -c "head -n1 {} | grep -Eq '^#!(.*/|.*env +)(sh|bash|ksh)'" ; -print
share/commands/agent-run
Outdated
COLOR="-Cnever" | ||
|
||
# When debugging, reports will be huge, skip HTTP reporting | ||
DONT_DO_HTTP=0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer a DO_HTTP name, and a true/false value
share/commands/agent-run
Outdated
@@ -27,6 +27,8 @@ | |||
# @man + | |||
# @man *-R*: run the agent in completely unparsed mode, with no return code of 1 in case of error. A little faster. | |||
# @man + | |||
# @man *-N-: do not write log in outputs dir (used when called internally) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-N- ?
PR updated with a new commit |
share/lib/timestamp.sh
Outdated
@@ -0,0 +1,8 @@ | |||
#!/bin/sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an include
share/lib/timestamp.sh
Outdated
# Same as in agent with a T instead of space (iso8601 UTC) | ||
date=$(/bin/date -u "+%Y-%m-%dT%T+00:00") | ||
printf "${date} %s\n" "$line" | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newline
PR updated with a new commit |
share/lib/timestamp.sh
Outdated
# Timestamps all lines | ||
while IFS= read -r line; do | ||
# Same as in agent with a T instead of space (iso8601 UTC) | ||
date=$(/bin/date -u "+%Y-%m-%dT%T+00:00") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
date should be in path, just use date
share/commands/agent-run
Outdated
report_file="${TMP_REPORTS_DIR}/${report_file_name}" | ||
touch "${report_file}" | ||
chmod 600 "${report_file}" | ||
runlog_output='tee ${report_file}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
""
share/commands/agent-run
Outdated
logdir=/var/rudder/cfengine-community/outputs | ||
touch "${logdir}/${logfile}" | ||
chmod 600 "${logdir}/${logfile}" | ||
log_outputs='tee ${logdir}/${logfile}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
""
share/commands/agent-run
Outdated
runlog_output='cat' | ||
fi | ||
|
||
"${RUDDER_VAR}/cfengine-community/bin/cf-agent" ${VERBOSITY} ${COLOR} -K ${BUNDLE} ${CLASS} | eval $log_outputs | eval $timestamp | eval $runlog_output | eval ${PRETTY} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove some evals
return 1 | ||
fi | ||
|
||
# (Very likely) atomic move in ready reports dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes except if they are on a different filesystem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hence the very likely, because /var/rudder/ is very likely on a single FS
PR updated with a new commit |
1 similar comment
PR updated with a new commit |
OK, squash merging this PR |
bc73b86
to
2b3d1fc
Compare
https://issues.rudder.io/issues/14814