Skip to content

[SPARK-4000][BUILD] Sends archived unit tests logs to Jenkins master #2845

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.

Sign up for GitHub

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

Closed
liancheng wants to merge 3 commits into from
Closed

[SPARK-4000][BUILD] Sends archived unit tests logs to Jenkins master #2845

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b912f7
Sends archived unit tests logs to Jenkins master
liancheng Oct 19, 2014
68c7010
Logs backup should be implemented in dev/run-tests-jenkins
liancheng Oct 19, 2014
ac8d9d4
Includes build number in messages posted to GitHub
liancheng Oct 20, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 41 additions & 12 deletions dev/run-tests-jenkins
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ function post_message () {
local message=$1
local data="{\"body\": \"$message\"}"
local HTTP_CODE_HEADER="HTTP Response Code: "

echo "Attempting to post to Github..."

local curl_output=$(
curl `#--dump-header -` \
--silent \
Expand All @@ -75,12 +75,12 @@ function post_message () {
echo " > data: ${data}" >&2
# exit $curl_status
fi

local api_response=$(
echo "${curl_output}" \
| grep -v -e "^${HTTP_CODE_HEADER}"
)

local http_code=$(
echo "${curl_output}" \
| grep -e "^${HTTP_CODE_HEADER}" \
Expand All @@ -92,12 +92,39 @@ function post_message () {
echo " > api_response: ${api_response}" >&2
echo " > data: ${data}" >&2
fi

if [ "$curl_status" -eq 0 ] && [ "$http_code" -eq "201" ]; then
echo " > Post successful."
fi
}

function send_archived_logs () {
echo "Archiving unit tests logs..."

local log_files=$(find . -name "unit-tests.log")

if [ -z "$log_files" ]; then
echo "> No log files found." >&2
else
local log_archive="unit-tests-logs.tar.gz"
echo "$log_files" | xargs tar czf ${log_archive}
Copy link
Contributor

@pwendell pwendell Oct 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering, will these appear in the tarfile under the full path (e.g. streaming/target/unit-tests.log)? That's ideal.

Copy link
Contributor Author

@liancheng liancheng Oct 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.


local jenkins_build_dir=${JENKINS_HOME}/jobs/${JOB_NAME}/builds/${BUILD_NUMBER}
Copy link
Contributor

@pwendell pwendell Oct 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add BUILD_NUMBER in the message that we post - that way we can find this more easily when we need to go get logs? Something like this:

Test build #XXX has started/finished for PR 2845 at commit 4b912f7.

Copy link
Contributor

@nchammas nchammas Oct 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could be useful. We may also be able to do away with the "for PR XXXX" part since that's kinda redundant.

Note that you can currently get the build number from the build URL in the existing messages posted to GitHub.

Copy link
Contributor Author

@liancheng liancheng Oct 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build number is directly accessible from environment variables. [1] I think the PR number information can be useful when reading PR comments via mail.

[1] https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21889/injectedEnvVars/

local scp_output=$(scp ${log_archive} amp-jenkins-master:${jenkins_build_dir}/${log_archive})
Copy link
Contributor Author

@liancheng liancheng Oct 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not good to hardcode Jenkins master hostname here. Should inject an extra environment variable $MASTER_NODE_NAME in Jenkins configurations.

Copy link
Contributor

@pwendell pwendell Oct 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused actually - is "amp-jenkins-master" the current hostname of the master machine?

Copy link
Contributor Author

@liancheng liancheng Oct 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hostname is accessible from Jenkins slave nodes.

Copy link
Contributor

@JoshRosen JoshRosen Oct 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liancheng I should be able to modify our pull request builders to inject this variable as JENKINS_MASTER_HOSTNAME.

Copy link
Contributor Author

@liancheng liancheng Oct 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks :)

local scp_status="$?"

if [ "$scp_status" -ne 0 ]; then
echo "Failed to send archived unit tests logs to Jenkins master." >&2
echo "> scp_status: ${scp_status}" >&2
echo "> scp_output: ${scp_output}" >&2
else
echo "> Send successful."
fi

rm -f ${log_archive}
fi
}


# We diff master...$ghprbActualCommit because that gets us changes introduced in the PR
#+ and not anything else added to master since the PR was branched.
Expand All @@ -109,7 +136,7 @@ function post_message () {
else
merge_note=" * This patch merges cleanly."
fi

source_files=$(
git diff master...$ghprbActualCommit --name-only `# diff patch against master from branch point` \
| grep -v -e "\/test" `# ignore files in test directories` \
Expand Down Expand Up @@ -144,12 +171,12 @@ function post_message () {
# post start message
{
start_message="\
[QA tests have started](${BUILD_URL}consoleFull) for \
[Test build ${BUILD_DISPLAY_NAME} has started](${BUILD_URL}consoleFull) for \
PR $ghprbPullId at commit [\`${SHORT_COMMIT_HASH}\`](${COMMIT_URL})."

start_message="${start_message}\n${merge_note}"
# start_message="${start_message}\n${public_classes_note}"

post_message "$start_message"
}

Expand All @@ -159,7 +186,7 @@ function post_message () {
test_result="$?"

if [ "$test_result" -eq "124" ]; then
fail_message="**[Tests timed out](${BUILD_URL}consoleFull)** \
fail_message="**[Test build ${BUILD_DISPLAY_NAME} timed out](${BUILD_URL}consoleFull)** \
for PR $ghprbPullId at commit [\`${SHORT_COMMIT_HASH}\`](${COMMIT_URL}) \
after a configured wait of \`${TESTS_TIMEOUT}\`."

Expand Down Expand Up @@ -187,15 +214,17 @@ function post_message () {
else
failing_test="some tests"
fi

test_result_note=" * This patch **fails $failing_test**."
fi

send_archived_logs
}

# post end message
{
result_message="\
[QA tests have finished](${BUILD_URL}consoleFull) for \
[Test build ${BUILD_DISPLAY_NAME} has finished](${BUILD_URL}consoleFull) for \
PR $ghprbPullId at commit [\`${SHORT_COMMIT_HASH}\`](${COMMIT_URL})."

result_message="${result_message}\n${test_result_note}"
Expand Down