Skip to content
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

feat(publishReports): add failIfEmpty parameter #853

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions test/groovy/PublishReportsStepTests.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ class PublishReportsStepTests extends BaseTest {
assertTrue(assertMethodCallContainsPattern('withEnv', 'TIMEOUT=60'))
assertTrue(assertMethodCallContainsPattern('withEnv', "FILENAME=${file}"))
assertTrue(assertMethodCallContainsPattern('withEnv', 'UPLOADFLAGS=--content-type="text/html"'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage blob upload --account-name=prodjenkinsreports --container=reports --timeout=${TIMEOUT} --file=${FILENAME} --name=${FILENAME} ${UPLOADFLAGS} --overwrite'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage blob upload --account-name=prodjenkinsreports --container=reports --timeout="${TIMEOUT}" --file="${FILENAME}" --name="${FILENAME}" "${UPLOADFLAGS}" --overwrite'))
// another filename manipulations is in place
assertTrue(assertMethodCallContainsPattern('withEnv', 'SOURCE_DIRNAME=.'))
assertTrue(assertMethodCallContainsPattern('withEnv', 'DESTINATION_PATH=/'))
assertTrue(assertMethodCallContainsPattern('withEnv', 'PATTERN=foo.html'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage file upload-batch --account-name prodjenkinsreports --destination reports --source ${SOURCE_DIRNAME} --destination-path ${DESTINATION_PATH} --pattern ${PATTERN} ${UPLOADFLAGS}'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage file upload-batch --account-name prodjenkinsreports --destination reports --source "${SOURCE_DIRNAME}" --destination-path "${DESTINATION_PATH}" --pattern "${PATTERN}" "${UPLOADFLAGS}"'))
assertJobStatusSuccess()
}

Expand All @@ -76,12 +76,12 @@ class PublishReportsStepTests extends BaseTest {
assertTrue(assertMethodCallContainsPattern('withEnv', 'TIMEOUT=60'))
assertTrue(assertMethodCallContainsPattern('withEnv', "FILENAME=${file}"))
assertTrue(assertMethodCallContainsPattern('withEnv', 'UPLOADFLAGS=--content-type="text/css"'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage blob upload --account-name=prodjenkinsreports --container=reports --timeout=${TIMEOUT} --file=${FILENAME} --name=${FILENAME} ${UPLOADFLAGS} --overwrite'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage blob upload --account-name=prodjenkinsreports --container=reports --timeout="${TIMEOUT}" --file="${FILENAME}" --name="${FILENAME}" "${UPLOADFLAGS}" --overwrite'))
// another filename manipulations is in place
assertTrue(assertMethodCallContainsPattern('withEnv', 'SOURCE_DIRNAME=/bar'))
assertTrue(assertMethodCallContainsPattern('withEnv', 'DESTINATION_PATH=/bar'))
assertTrue(assertMethodCallContainsPattern('withEnv', 'PATTERN=foo.css'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage file upload-batch --account-name prodjenkinsreports --destination reports --source ${SOURCE_DIRNAME} --destination-path ${DESTINATION_PATH} --pattern ${PATTERN} ${UPLOADFLAGS}'))
assertTrue(assertMethodCallContainsPattern('sh', 'az storage file upload-batch --account-name prodjenkinsreports --destination reports --source "${SOURCE_DIRNAME}" --destination-path "${DESTINATION_PATH}" --pattern "${PATTERN}" "${UPLOADFLAGS}"'))
assertJobStatusSuccess()
}
}
21 changes: 17 additions & 4 deletions vars/publishReports.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
/**
* Wrapper for publishing reports to the reports host
*
* @param timeout: timout in minutes for the blob storage upload (default: 60)
* @param failIfEmpty: set to true to fail if the file to upload isn't empty (default: false)
*
* See https://issues.jenkins-ci.org/browse/INFRA-947 for more
*/
def call(List<String> files, Map params = [:]) {
def timeout = params.get('timeout') ?: '60'
def failIfEmpty = params.get('failIfEmpty') ?: false

if (!infra.isTrusted() && !infra.isInfra()) {
error 'Can only call publishReports from within the trusted.ci environment'
Expand Down Expand Up @@ -52,11 +56,20 @@ def call(List<String> files, Map params = [:]) {
"DESTINATION_PATH=${dirname ?: '/'}",
"PATTERN=${ basename ?: '*' }",
]) {
// Blob container can be removed once files are uploaded on the azure file storage
sh 'az storage blob upload --account-name=prodjenkinsreports --container=reports --timeout=${TIMEOUT} --file=${FILENAME} --name=${FILENAME} ${UPLOADFLAGS} --overwrite'
sh '''
# If the "failIsEmpty" flag is active and the file is empty, fail the pipeline
lineCount=$(wc -l "${filename}" | awk '{print $1}')
if [[ "${failIsEmpty}" != false && "${lineCount}" -eq 0 ]]; then
echo "ERROR: ${filename} is empty."
exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Fundamental question: do we want the calling pipeline to fail? At first sight it looks like but since the only known use case today is jenkins-infra/helpdesk#4056 then I'm asking.

Is the condition (and its maintenance burden: it is not even tested since it is implemented with shell code) worth the gain considering PHS reports are now always generated?

Copy link
Member Author

Choose a reason for hiding this comment

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

So... rename this PR and change failIfEmpty by skipIfEmpty?

else
# Blob container can be removed once files are uploaded on the azure file storage
az storage blob upload --account-name=prodjenkinsreports --container=reports --timeout="${TIMEOUT}" --file="${FILENAME}" --name="${FILENAME}" "${UPLOADFLAGS}" --overwrite

// `az storage file upload` doesn't support file uploaded in a remote directory that doesn't exist but upload-batch yes. Unfortunately the cli syntax is a bit different and requires filename and directory name to be set differently.
sh 'az storage file upload-batch --account-name prodjenkinsreports --destination reports --source ${SOURCE_DIRNAME} --destination-path ${DESTINATION_PATH} --pattern ${PATTERN} ${UPLOADFLAGS}'
# `az storage file upload` doesn't support file uploaded in a remote directory that doesn't exist but upload-batch yes. Unfortunately the cli syntax is a bit different and requires filename and directory name to be set differently.
az storage file upload-batch --account-name prodjenkinsreports --destination reports --source "${SOURCE_DIRNAME}" --destination-path "${DESTINATION_PATH}" --pattern "${PATTERN}" "${UPLOADFLAGS}"
fi
'''
}
}
}
Expand Down