Skip to content

Commit

Permalink
Update deploy agent logging (#1258)
Browse files Browse the repository at this point in the history
* wsfix

* Add S flag to curl

From the curl documentation:

    -S, --show-error
           When used with -s, --silent, it makes curl show an error message if it fails.

Add the flag to show the error in case of failures, which an be helpful
for debugging

* Update variable names

Update the variable naming to describe the type in contains

* Update checksum verification warning

When the sha1 reuqest for a build returns non-200, the verification is
skipped.

Since this happens often and is usually expected, switch to
`log.warning`.

Also, update the warning message to clarify the sha1 verification is
skipped

* Update error logging

In case of tarball extraction failures, log the exception and stack
trace. These exceptions could cause the host to become stuck. The stack
trace can help with debugging.
  • Loading branch information
osoriano authored Sep 27, 2023
1 parent b795155 commit 161945e
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 23 deletions.
4 changes: 2 additions & 2 deletions deploy-agent/deployd/common/status_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down
16 changes: 8 additions & 8 deletions deploy-agent/deployd/download/downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand Down Expand Up @@ -72,7 +72,7 @@ def download(self):
if extension == 'gpg':
try:
log.info("gpg decrypting {}.".format(local_full_fn))

# assuming we use the standard GPG toolset, it creates the following format with gpg --encrypt:
# input: file.extension
# output: file.extension.gpg
Expand All @@ -84,14 +84,14 @@ def download(self):

# decrypt gpg archive
status = gpgHelper.decryptFile(local_full_fn, dest_full_fn)

if status != Status.SUCCEEDED:
# die if we hit a decryption or signing error
return status

# remove encrypted gpg archive since it is no longer needed
os.remove(local_full_fn)

# Rebase the extension and file path to the decrypted file
local_full_fn = dest_full_fn
extension = innerExtension
Expand Down Expand Up @@ -121,13 +121,13 @@ def download(self):
log.info("Successfully extracted {} to {}".format(local_full_fn, working_dir))
except tarfile.TarError as e:
status = Status.FAILED
log.error("Failed to extract files: {}".format(e))
log.exception("Failed to extract tar files")
except OSError as e:
status = Status.FAILED
log.error("Failed: {}".format(e))
log.exception("Failed to extract files. OSError:")
except Exception:
status = Status.FAILED
log.error(traceback.format_exc())
log.exception("Failed to extract files")
finally:
return status

Expand Down
24 changes: 11 additions & 13 deletions deploy-agent/deployd/download/http_download_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -27,34 +27,32 @@
class HTTPDownloadHelper(DownloadHelper):

def _download_files(self, local_full_fn):
download_cmd = ['curl', '-o', local_full_fn, '-fks', self._url]
download_cmd = ['curl', '-o', local_full_fn, '-fksS', self._url]
log.info('Running command: {}'.format(' '.join(download_cmd)))
error_code = Status.SUCCEEDED
output, error, status = Caller.call_and_log(download_cmd, cwd=os.getcwd())
output, error, process_return_code = Caller.call_and_log(download_cmd, cwd=os.getcwd())
status_code = Status.FAILED if process_return_code else Status.SUCCEEDED
if output:
log.info(output)
if error:
log.error(error)
if status:
error_code = Status.FAILED
log.info('Finish downloading: {} to {}'.format(self._url, local_full_fn))
return error_code
return status_code

def download(self, local_full_fn):
log.info("Start to download from url {} to {}".format(
self._url, local_full_fn))

error = self._download_files(local_full_fn)
if error != Status.SUCCEEDED:
status_code = self._download_files(local_full_fn)
if status_code != Status.SUCCEEDED:
log.error('Failed to download the tar ball for {}'.format(local_full_fn))
return error
return status_code

try:
sha_url = '{}.sha1'.format(self._url)
sha_r = requests.get(sha_url)
if sha_r.status_code != 200:
log.error('sha1 file does not exist for {}, ignore checksum.'.format(self._url))
return error
log.warning('Skip checksum verification. Invalid response from {}'.format(sha_url))
return status_code

sha_value = sha_r.content
hash_value = self.hash_file(local_full_fn)
Expand Down

0 comments on commit 161945e

Please sign in to comment.