From 8f6dcebe519221aeaa6f6c2bb2a8db78af8891cc Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Thu, 11 Nov 2021 18:18:01 +0000 Subject: [PATCH 1/4] bisector: use ArtifactorialService for both upload and download Refactor ArtifactorialService for both upload and download services. This creates the building blocks for adding other services that require specific methods for both uploading artifacts and downloading them. Signed-off-by: Ionela Voinescu --- tools/bisector/bisector/bisector.py | 114 +++++++++++++++++----------- 1 file changed, 70 insertions(+), 44 deletions(-) diff --git a/tools/bisector/bisector/bisector.py b/tools/bisector/bisector/bisector.py index ab1987100..6bb96f367 100755 --- a/tools/bisector/bisector/bisector.py +++ b/tools/bisector/bisector/bisector.py @@ -1958,7 +1958,7 @@ class LISATestStep(ShellStep): upload_service = service_hub.upload if upload_service: try: - artifact_path = upload_service.upload(artifact_path) + artifact_path = upload_service.upload(path=artifact_path) except Exception as e: error('Could not upload exekall artifact, will not delete the folder: ' + str(e)) # Avoid deleting the artifact if something went wrong, so @@ -2187,7 +2187,7 @@ class LISATestStep(ShellStep): upload_service = service_hub.upload if upload_service: try: - url = upload_service.upload(step_res.results_path) + url = upload_service.upload(path=step_res.results_path) step_res.results_path = url except Exception as e: error('Could not upload LISA results: ' + str(e)) @@ -2219,13 +2219,18 @@ class LISATestStep(ShellStep): archive_path=archive_path, archive_dst=archive_dst, )) - try: - urlretrieve(archive_path, archive_dst) - except requests.exceptions.RequestException as e: - error('Could not retrieve {archive_path}: {e}'.format( - archive_path=archive_path, - e=e - )) + download_service = service_hub.download + if download_service: + try: + download_service.download(url=archive_path, + path=archive_dst) + except Exception as e: + error('Could not retrieve {archive_path}: {e}'.format( + archive_path=archive_path, + e=e + )) + else: + error('No download service available.') # Otherwise, assume it is a file and symlink it alongside # the logs. @@ -2473,13 +2478,6 @@ class StepNotifService: self.slave_manager.signal.StepNotif = (step.name, msg, display_time) -class UploadService(abc.ABC): - @abc.abstractmethod - def upload(self, path): - """Upload a file""" - pass - - def urlretrieve(url, path): response = requests.get(url) # Raise an exception is the request failed @@ -2493,39 +2491,47 @@ def urlretrieve(url, path): f.write(response.content) -class ArtifactorialService(UploadService): - """Upload files to Artifactorial.""" +class ArtifactsService(abc.ABC): + @abc.abstractmethod + def upload(self, path, url): + """Upload a file""" + pass + @abc.abstractmethod + def download(self, url, path): + """Download a file""" + pass - def __init__(self, url=None, token=None): + +class ArtifactorialService(ArtifactsService): + """Upload/download files to/from Artifactorial.""" + + def __init__(self, token=None): """ - :param url: URL of the Artifactorial folder to upload to, - or env var ARTIFACTORIAL_FOLDER - :param token: Token granting the write access to the folder, - or env var ARTIFACTORIAL_TOKEN + :param token: Token granting the read & write access to the url + It is not mandatory for the download service. """ - self.url = url or os.getenv('ARTIFACTORIAL_FOLDER') self.token = token or os.getenv('ARTIFACTORIAL_TOKEN') - if not self.token: - raise ValueError("An Artifactorial token must be provided through ARTIFACTORIAL_TOKEN env var") - - if not self.url: - raise ValueError("An Artifactorial folder URL must be provided through ARTIFACTORIAL_FOLDER env var") - - def upload(self, path): + def upload(self, path, url=None): """ Upload a file to Artifactorial. - :param path: path to the file to Uploading + :param path: path to the file to upload + :param url: URL of the Artifactorial folder to upload to, + or env var ARTIFACTORIAL_FOLDER """ - url = self.url token = self.token + if not token: + raise ValueError("An Artifactorial token must be provided through ARTIFACTORIAL_TOKEN env var") + + dest = url or os.getenv('ARTIFACTORIAL_FOLDER') + if not dest: + raise ValueError("An Artifactorial folder URL must be provided through ARTIFACTORIAL_FOLDER env var") - dest = url if not os.path.exists(path): - info(f'Cannot upload {path} as it does not exists.') + warn(f'Cannot upload {path} as it does not exists.') return path info(f'Uploading {path} to {dest} ...') @@ -2558,6 +2564,15 @@ class ArtifactorialService(UploadService): return path + def download(self, url, path): + """ + Download a file from Artifactorial: anonymous access. + + :param url: URL of the Artifactorial file to download + :param path: path to the downloaded file + """ + urlretrieve(url, path) + def update_json(path, mapping): """ @@ -3822,7 +3837,7 @@ class Report(Serializable): url = None if upload_service: try: - url = upload_service.upload(self.path) + url = upload_service.upload(path=self.path) info('Uploaded report ({path}) to {url}'.format( path=self.path, url=url @@ -3932,7 +3947,7 @@ class Report(Serializable): return report @classmethod - def load(cls, path, steps_path=None, use_cache=False): + def load(cls, path, steps_path=None, use_cache=False, service_hub=None): """ Restore a serialized :class:`Report` from the given file. @@ -3956,9 +3971,16 @@ class Report(Serializable): # original name suffix = os.path.basename(url.path) with tempfile.NamedTemporaryFile(suffix=suffix) as temp_report: - path = temp_report.name - urlretrieve(url.geturl(), path) - return cls._load(path, steps_path, use_cache=False) + path=temp_report.name + download_service = service_hub.download + if download_service: + try: + download_service.download(url=url.geturl(), path=path) + return cls._load(path, steps_path, use_cache=False) + except Exception as e: + error('Could not download report: ' + str(e)) + else: + error('No download service available.') else: return cls._load(path, steps_path, use_cache) @@ -5138,10 +5160,13 @@ command line""") SHOW_TRACEBACK = args.debug service_hub = ServiceHub() + try: - service_hub.register_service('upload', ArtifactorialService()) + artifactorial = ArtifactorialService() + service_hub.register_service('upload', artifactorial) + service_hub.register_service('download', artifactorial) except Exception as e: - info('Artifactorial upload will not be available: ' + str(e)) + info('Artifactorial Service is not available: ' + str(e)) # required=True cannot be used in add_argument since it breaks # YAMLCLIOptionsAction, so we just handle that manually @@ -5160,7 +5185,7 @@ command line""") step_options = parse_step_options(args.option) if args.subcommand == 'edit': - report = Report.load(report_path, steps_path) + report = Report.load(report_path, steps_path, service_hub=service_hub) # Partially reinitialize the steps, with the updated command line options report.result.step.reinit(step_options=step_options) report.save() @@ -5318,7 +5343,8 @@ command line""") export_path = args.export use_cache = args.cache - report = Report.load(report_path, steps_path, use_cache=use_cache) + report = Report.load(report_path, steps_path, use_cache=use_cache, + service_hub=service_hub) out, bisect_ret = report.show( service_hub=service_hub, -- GitLab From a48afbcc51408485d7b5ca9fe05f172e29ee6ac6 Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Wed, 17 Nov 2021 11:35:12 +0000 Subject: [PATCH 2/4] bisector: create new ArtifactoryService ArtifactoryService is similar to ArtifactorialService, with the exception that it requires a token (API key) for both upload and download, which it provides through a header included in the request sent to the server. The upload folder and token will be retrived through ARTIFACTORY_FOLDER and ARTIFACTORY_TOKEN env vars. Artifactory also allows to specify properties for the uploaded files, which can be provided as part of ARTIFACTORY_FOLDER. Signed-off-by: Ionela Voinescu --- tools/bisector/bisector/bisector.py | 110 ++++++++++++++++++++++++++-- 1 file changed, 105 insertions(+), 5 deletions(-) diff --git a/tools/bisector/bisector/bisector.py b/tools/bisector/bisector/bisector.py index 6bb96f367..7a930176e 100755 --- a/tools/bisector/bisector/bisector.py +++ b/tools/bisector/bisector/bisector.py @@ -2574,6 +2574,96 @@ class ArtifactorialService(ArtifactsService): urlretrieve(url, path) +class ArtifactoryService(ArtifactsService): + """Upload/download files to/from Artifactory. """ + + def __init__(self, token=None): + """ + :param token: Token granting the read & write access to the url, + or env var ARTIFACTORY_TOKEN + """ + + self.token = token or os.getenv('ARTIFACTORY_TOKEN') + + if not self.token: + raise ValueError("An Artifactory token must be provided through ARTIFACTORY_TOKEN env var") + + def upload(self, path, url=None): + """ + Upload a file to Artifactory. + + :param path: path to the file to upload + :param url: URL of the Artifactory folder to upload to, + or env var ARTIFACTORY_FOLDER + """ + + token = self.token + + dest = url or os.getenv('ARTIFACTORY_FOLDER') + if not dest: + raise ValueError("An Artifactory folder URL must be provided through ARTIFACTORY_FOLDER env var") + + dest = dest + "/" + os.path.basename(path) + + if not os.path.exists(path): + warn(f'Cannot upload {path} as it does not exists.') + return path + + info(f'Uploading {path} to {dest} ...') + + with open(path, 'rb') as f: + content = f.read() + md5sum = hashlib.md5(content).hexdigest(); + + headers = { + "X-Checksum-Md5": md5sum, + "X-JFrog-Art-Api": token + } + + response = requests.put(dest, headers=headers, data=content) + + if response.ok: + url = response.json().get("downloadUri") + info('{path} uploaded to {url} ...'.format( + path=path, + url=url + )) + path = url + # Return the path unmodified if it failed + else: + warn('Failed to upload {path} to {dest} (HTTP {error}/{reason}): {msg}'.format( + path=path, + dest=dest, + error=response.status_code, + reason=response.reason, + msg=response.text, + )) + + return path + + def download(self, url, path): + """ + Download a file from an Artifacts service. + + :param path: path to the file to download + """ + + headers = { + "X-JFrog-Art-Api": self.token + } + + response = requests.get(url, headers=headers) + # Raise an exception is the request failed + response.raise_for_status() + + # If that is a broken symlink, get rid of it + if not os.path.exists(path) and os.path.islink(path): + os.unlink(path) + + with open(path, 'wb') as f: + f.write(response.content) + + def update_json(path, mapping): """ Update a JSON file with the given mapping. @@ -5161,12 +5251,22 @@ command line""") SHOW_TRACEBACK = args.debug service_hub = ServiceHub() + artifacts_service = None try: - artifactorial = ArtifactorialService() - service_hub.register_service('upload', artifactorial) - service_hub.register_service('download', artifactorial) - except Exception as e: - info('Artifactorial Service is not available: ' + str(e)) + artifacts_service = ArtifactoryService() + except Exception: + try: + artifacts_service = ArtifactorialService() + except Exception: + error('No artifacts service could be initialised.') + else: + info('Artifactorial Service was initialized.') + else: + info('Artifactory Service was initialized.') + + if artifacts_service: + service_hub.register_service('upload', artifacts_service) + service_hub.register_service('download', artifacts_service) # required=True cannot be used in add_argument since it breaks # YAMLCLIOptionsAction, so we just handle that manually -- GitLab From a65053739d343a48e9871d5e6f851c8de93d157f Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Wed, 17 Nov 2021 12:25:53 +0000 Subject: [PATCH 3/4] bisector,doc: add documentation for Artifactory After the addition of the ArtifactoryService, modify existing documentation to include mentions of this new service and publish additional documentation related to automated testing. Signed-off-by: Ionela Voinescu --- doc/man1/bisector.1 | 22 +++++++------ doc/workflows/automated_testing.rst | 50 +++++++++++++++++++++-------- tools/bisector/bisector/bisector.py | 13 ++++---- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/doc/man1/bisector.1 b/doc/man1/bisector.1 index 479937525..88103ff2d 100644 --- a/doc/man1/bisector.1 +++ b/doc/man1/bisector.1 @@ -188,12 +188,13 @@ optional arguments: \-\-early\-bailout Restart a new iteration if a step marked the commit as bad or non\-testable. Bisect abortion will still take place even without this option. - \-\-upload\-report Continuously upload the report to Artifactorial after - every iteration. This relies on the following - environment variables: ARTIFACTORIAL_FOLDER set to the - folder\(aqs URL and ARTIFACTORIAL_TOKEN set to the token. - Remember to use the right step option to upload the - results as they are computed if desired. + \-\-upload\-report Continuously upload the report to an artifacts service + after every iteration. This relies on the following + environment variables: ARTIFACTORY_FOLDER or + ARTIFACTORIAL_FOLDER set to the folder\(aqs URL and + ARTIFACTORY_TOKEN or ARTIFACTORIAL_TOKEN set to the + token. Remember to use the right step option to upload + the results as they are computed if desired. \-\-dbus Try enable DBus API if the necessary dependencies are installed. \-\-no\-dbus Disable DBus even when pydbus module is available. @@ -397,10 +398,11 @@ Inherits from: shell name \-o upload\-artifact= (bool) - upload the artifact directory to Artifactorial and update the in\- - memory report. Following env var are needed: ARTIFACTORIAL_FOLDER - set to the folder URL and ARTIFACTORIAL_TOKEN. Note: \-\-export should - be used to save the report with updated paths + upload the artifact directory to an artifacts service and update the + in\-memory report. Following env var are needed: ARTIFACTORY_FOLDER + or ARTIFACTORIAL_FOLDER set to the folder URL, and ARTIFACTORY_TOKEN + or ARTIFACTORIAL_TOKEN. Note: \-\-export should be used to save the + report with updated paths \-o verbose= (bool) increase verbosity diff --git a/doc/workflows/automated_testing.rst b/doc/workflows/automated_testing.rst index ab5ced094..2b57bb09d 100644 --- a/doc/workflows/automated_testing.rst +++ b/doc/workflows/automated_testing.rst @@ -562,26 +562,48 @@ Integration in a CI loop ++++++++++++++++++++++++ ``bisector run`` has the ability of uploading reports on the fly to -*Artifactorial* [#]_. *Artifactorial* is convenient since it allows pushing -large quantities of data to a server, that are automatically cleaned up after a -period of time. The ``LISA-test`` step can upload compressed exekall -artifact archives using ``-oupload-artifact`` run option. It will record the -new HTTP location of the artifacts in the report. In a way, the report becomes -an index that contains enough information to make a decision on what artifact -archive to download for further analysis (usually to look at ``trace-cmd`` -traces). +either Artifactorial or Artifactory. -.. code-block:: sh - - export ARTIFACTORIAL_TOKEN='ONE_TOKEN_TO_RULE_THEM_ALL' - export ARTIFACTORIAL_FOLDER='http://instance.of.artifactorial/artifacts/myfolder' - bisector run --steps steps.yml --report myreport.yml.gz -oupload-artifact --upload-report +The ``LISA-test`` step can upload compressed exekall artifact archives using +``-oupload-artifact`` run option. It will record the new HTTP location of the +artifacts in the report. In a way, the report becomes an index that contains +enough information to make a decision on what artifact archive to download +for further analysis (usually to look at ``trace-cmd`` traces). -.. tip:: ``bisector report`` accept both local files and HTTP URLs +.. tip:: ``bisector report`` accepts both local files and HTTP URLs If the worker is unstable, the latest report can still be used and will contain all the steps information collected so far. When using the ``exekall-LISA-step``, ``-oexport-logs`` will by default download artifact archives accessible over HTTP. That can be changed using ``-odownload=false``. +Artifactorial +------------- + +*Artifactorial* [#]_ is convenient since it allows pushing large quantities +of data to a server, that are automatically cleaned up after a period of time. + +.. code-block:: sh + + export ARTIFACTORIAL_TOKEN='ONE_TOKEN_TO_RULE_THEM_ALL' + export ARTIFACTORIAL_FOLDER='http://instance.of.artifactorial/artifacts/myfolder' + bisector run --steps steps.yml --report myreport.yml.gz -oupload-artifact --upload-report + .. [#] https://github.com/ivoire/Artifactorial + +Artifactory +----------- + +*Artifactory* [#]_ has more complex features and it allows pushing large +quantities of data to a server, while giving you control over the policy used +for data cleaning. The pushed data can also be described through properties +which can be used to drive the cleaning policy and to select the data fetched +from the server at a later point in time. + +.. code-block:: sh + + export ARTIFACTORY_TOKEN='API_KEY' + export ARTIFACTORY_FOLDER='http://instance.of.artifactory/mynamespace.myrepo;prop=val' + bisector run --steps steps.yml --report myreport.yml.gz -oupload-artifact --upload-report + +.. [#] https://jfrog.com/artifactory/ diff --git a/tools/bisector/bisector/bisector.py b/tools/bisector/bisector/bisector.py index 7a930176e..d509085d5 100755 --- a/tools/bisector/bisector/bisector.py +++ b/tools/bisector/bisector/bisector.py @@ -1819,7 +1819,7 @@ class LISATestStep(ShellStep): export_db=BoolOrStrParam('export a merged exekall ValueDB, merging it with existing ValueDB if the file exists', allow_empty=False), export_logs=BoolOrStrParam('export the logs and artifact directory symlink to the given directory'), download=BoolParam('Download the exekall artifact archives if necessary'), - upload_artifact=BoolParam('upload the artifact directory to Artifactorial and update the in-memory report. Following env var are needed: ARTIFACTORIAL_FOLDER set to the folder URL and ARTIFACTORIAL_TOKEN. Note: --export should be used to save the report with updated paths'), + upload_artifact=BoolParam('upload the artifact directory to an artifacts service and update the in-memory report. Following env var are needed: ARTIFACTORY_FOLDER or ARTIFACTORIAL_FOLDER set to the folder URL, and ARTIFACTORY_TOKEN or ARTIFACTORIAL_TOKEN. Note: --export should be used to save the report with updated paths'), ) ) @@ -5139,11 +5139,12 @@ command line""") option.""") run_parser.add_argument('--upload-report', action='store_true', - help="""Continuously upload the report to Artifactorial after every - iteration. This relies on the following environment variables: - ARTIFACTORIAL_FOLDER set to the folder's URL and ARTIFACTORIAL_TOKEN - set to the token. Remember to use the right step option to upload the - results as they are computed if desired. + help="""Continuously upload the report to an artifacts service after + every iteration. This relies on the following environment variables: + ARTIFACTORY_FOLDER or ARTIFACTORIAL_FOLDER set to the folder's URL + and ARTIFACTORY_TOKEN or ARTIFACTORIAL_TOKEN set to the token. + Remember to use the right step option to upload the results as they + are computed if desired. """) dbus_group = run_parser.add_mutually_exclusive_group() -- GitLab From 359450a810c6462ba516a98dcd3d31f1785565c5 Mon Sep 17 00:00:00 2001 From: Ionela Voinescu Date: Wed, 17 Nov 2021 12:39:24 +0000 Subject: [PATCH 4/4] doc: automated_testing: fix unused reference Signed-off-by: Ionela Voinescu --- doc/workflows/automated_testing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/workflows/automated_testing.rst b/doc/workflows/automated_testing.rst index 2b57bb09d..2695be013 100644 --- a/doc/workflows/automated_testing.rst +++ b/doc/workflows/automated_testing.rst @@ -253,7 +253,7 @@ bisector ======== ``bisector`` allows setting up the steps of a test iteration, repeating -them an infinite number of times (by default). +them an infinite number of times (by default), similarly to [#]_. .. seealso:: :ref:`bisector main documentation` -- GitLab