Improve client benchmark identification

jenkins/run-server-func-tests

@@ -142,7 +142,9 @@ else
 fi
 
 if [[ ${exit_status} -ne 0 ]]; then
-    dump_journal
+    if (( ${cleanup_flag} )); then
+        dump_journal
+    fi
     printf -- "\nFunctional tests exited with code %s\n" ${exit_status} >&2
 fi
 

lib/pbench/client/__init__.py

@@ -494,14 +494,19 @@ def set_metadata(self, dataset_id: str, metadata: JSONOBJECT) -> JSONOBJECT:
         ).json()
 
     def update(
-        self, dataset_id: str, access: Optional[str] = None, owner: Optional[str] = None
-    ) -> JSONOBJECT:
+        self,
+        dataset_id: str,
+        access: Optional[str] = None,
+        owner: Optional[str] = None,
+        **kwargs,
+    ) -> requests.Response:
         """Update the dataset access or owner
 
         Args:
             dataset_id: the resource ID of the targeted dataset
             access: set the access mode of the dataset (private, public)
             owner: set the owning username of the dataset (requires ADMIN)
+            kwargs: additional client options
 
         Returns:
             A JSON document containing the response
@@ -515,7 +520,8 @@ def update(
             api=API.DATASETS,
             uri_params={"dataset": dataset_id},
             params=params,
-        ).json()
+            **kwargs,
+        )
 
     def get_settings(self, key: str = "") -> JSONOBJECT:
         """Return requested server setting.

lib/pbench/server/api/resources/datasets_compare.py

@@ -70,7 +70,7 @@ def _get(
         datasets = params.query.get("datasets")
         benchmark_choice = None
         for dataset in datasets:
-            benchmark = Metadata.getvalue(dataset, "dataset.metalog.pbench.script")
+            benchmark = Metadata.getvalue(dataset, Metadata.SERVER_BENCHMARK)
             # Validate if all the selected datasets is of same benchmark
             if not benchmark_choice:
                 benchmark_choice = benchmark
@@ -116,5 +116,5 @@ def _get(
             raise APIInternalError(
                 f"Quisby processing failure. Exception: {quisby_response['exception']}"
             )
-        quisby_response["benchmark"] = benchmark.lower()
+        quisby_response["benchmark"] = benchmark
         return jsonify(quisby_response)

lib/pbench/server/api/resources/datasets_visualize.py

@@ -60,9 +60,8 @@ def _get(
 
         dataset = params.uri["dataset"]
 
-        metadata = Metadata.getvalue(dataset, "dataset.metalog.pbench.script")
-        benchmark = metadata.upper()
-        benchmark_type = BenchmarkName.__members__.get(benchmark)
+        benchmark = Metadata.getvalue(dataset, Metadata.SERVER_BENCHMARK)
+        benchmark_type = BenchmarkName.__members__.get(benchmark.upper())
         if not benchmark_type:
             raise APIAbort(
                 HTTPStatus.BAD_REQUEST, f"Unsupported Benchmark: {benchmark}"
@@ -84,5 +83,5 @@ def _get(
             raise APIInternalError(
                 f"Quisby processing failure. Exception: {quisby_response['exception']}"
             )
-        quisby_response["benchmark"] = benchmark.lower()
+        quisby_response["benchmark"] = benchmark
         return jsonify(quisby_response)

lib/pbench/server/api/resources/endpoint_configure.py

@@ -2,6 +2,7 @@
 from urllib.parse import urljoin
 
 from flask import current_app, jsonify, Request, Response
+from pquisby.lib.post_processing import BenchmarkName
 
 from pbench.server import OperationCode, PbenchServerConfig
 from pbench.server.api.resources import (
@@ -122,6 +123,9 @@ def _get(self, args: ApiParams, request: Request, context: ApiContext) -> Respon
         endpoints = {
             "identification": f"Pbench server {self.server_config.COMMIT_ID}",
             "uri": templates,
+            "visualization": {
+                "benchmarks": [m.lower() for m in BenchmarkName.__members__.keys()]
+            },
         }
 
         client = self.server_config.get("openid", "client")

lib/pbench/server/api/resources/intake_base.py

@@ -419,15 +419,22 @@ def _intake(
             # Add the processed tarball metadata.log file contents, if any.
             #
             # If we were unable to find or parse the tarball's metadata.log
-            # file, construct a minimal metadata context identifying the
-            # dataset as a "foreign" benchmark script, and disable indexing,
-            # which requires the metadata.
+            # file we construct a minimal metadata context identifying the
+            # dataset as an unknown benchmark script, and we disable indexing,
+            # which requires result metadata.
             try:
                 metalog = tarball.metadata
                 if not metalog:
-                    metalog = {"pbench": {"script": "Foreign"}}
+                    benchmark = Metadata.SERVER_BENCHMARK_UNKNOWN
+                    metalog = {"pbench": {"name": dataset.name, "script": benchmark}}
                     metadata[Metadata.SERVER_ARCHIVE] = True
                     attributes["missing_metadata"] = True
+                else:
+                    p = metalog.get("pbench")
+                    if p:
+                        benchmark = p.get("script", Metadata.SERVER_BENCHMARK_UNKNOWN)
+                    else:
+                        benchmark = Metadata.SERVER_BENCHMARK_UNKNOWN
                 Metadata.create(dataset=dataset, key=Metadata.METALOG, value=metalog)
             except Exception as exc:
                 raise APIInternalError(
@@ -447,17 +454,21 @@ def _intake(
             try:
                 retention = datetime.timedelta(days=retention_days)
                 deletion = dataset.uploaded + retention
-                Metadata.setvalue(
-                    dataset=dataset,
-                    key=Metadata.TARBALL_PATH,
-                    value=str(tarball.tarball_path),
-                )
-                Metadata.setvalue(
-                    dataset=dataset,
-                    key=Metadata.SERVER_DELETION,
-                    value=UtcTimeHelper(deletion).to_iso_string(),
+
+                # Make a shallow copy so we can add keys without affecting the
+                # original (which will be recorded in the audit log)
+                meta = metadata.copy()
+                meta.update(
+                    {
+                        Metadata.TARBALL_PATH: str(tarball.tarball_path),
+                        Metadata.SERVER_DELETION: UtcTimeHelper(
+                            deletion
+                        ).to_iso_string(),
+                        Metadata.SERVER_BENCHMARK: benchmark,
+                    }
                 )
-                f = self._set_dataset_metadata(dataset, metadata)
+                current_app.logger.info("Metadata for {}: {}", dataset.name, meta)
+                f = self._set_dataset_metadata(dataset, meta)
                 if f:
                     attributes["failures"] = f
             except Exception as e:

lib/pbench/server/api/resources/query_apis/datasets/__init__.py

@@ -1,9 +1,11 @@
+from http import HTTPStatus
 from typing import AnyStr, List, NoReturn, Union
 
 from flask import current_app
 
 from pbench.server import JSON, PbenchServerConfig
 from pbench.server.api.resources import (
+    APIAbort,
     ApiAuthorizationType,
     ApiContext,
     APIInternalError,
@@ -119,11 +121,13 @@ def get_index(self, dataset: Dataset, root_index_name: AnyStr) -> AnyStr:
         try:
             index_map = Metadata.getvalue(dataset=dataset, key=Metadata.INDEX_MAP)
         except MetadataError as exc:
-            current_app.logger.error("{}", exc)
-            raise APIInternalError(f"Required metadata {Metadata.INDEX_MAP} missing")
+            if Metadata.getvalue(dataset, Metadata.SERVER_ARCHIVE):
+                raise APIAbort(HTTPStatus.CONFLICT, "Dataset is not indexed")
+            raise APIInternalError(
+                f"Required metadata {Metadata.INDEX_MAP} missing"
+            ) from exc
 
         if index_map is None:
-            current_app.logger.error("Index map metadata has no value")
             raise APIInternalError(
                 f"Required metadata {Metadata.INDEX_MAP} has no value"
             )

lib/pbench/server/database/models/datasets.py

@@ -612,6 +612,13 @@ class Metadata(Database.Base):
     METALOG = "metalog"
     NATIVE_KEYS = [GLOBAL, METALOG, SERVER, USER]
 
+    # BENCHMARK provides a simple identification of the native benchmark where
+    # one can be identified. At the simplest for Pbench Agent tarballs this is
+    # just "dataset.metalog.pbench.script" but will be expanded in the future
+    # for example to identify a "pbench-user-benchmark -- fio" as "fio".
+    SERVER_BENCHMARK = "server.benchmark"
+    SERVER_BENCHMARK_UNKNOWN = "unknown"
+
     # DELETION timestamp for dataset based on user settings and system
     # settings when the dataset is created.
     #

lib/pbench/test/functional/server/test_connect.py

@@ -14,6 +14,7 @@ def test_connect(self, server_client: PbenchServerClient):
         assert endpoints
         assert "identification" in endpoints
         assert "uri" in endpoints
+        assert endpoints["visualization"]["benchmarks"] == ["uperf"]
 
         # Verify that all expected endpoints are reported
         for a in endpoints["uri"].keys():

lib/pbench/test/functional/server/test_datasets.py

@@ -79,7 +79,13 @@ def test_upload_all(self, server_client: PbenchServerClient, login_user):
             print(f"\t... uploaded {name}: {a}")
 
         datasets = server_client.get_list(
-            metadata=["dataset.access", "server.tarball-path", "dataset.operations"]
+            metadata=[
+                "dataset.access",
+                "dataset.metalog.pbench.script",
+                "server.benchmark",
+                "server.tarball-path",
+                "dataset.operations",
+            ]
         )
         found = frozenset({d.name for d in datasets})
         expected = frozenset(tarballs.keys())
@@ -91,6 +97,10 @@ def test_upload_all(self, server_client: PbenchServerClient, login_user):
                 t = tarballs[dataset.name]
                 assert dataset.name in dataset.metadata["server.tarball-path"]
                 assert dataset.metadata["dataset.operations"]["UPLOAD"]["state"] == "OK"
+                assert (
+                    dataset.metadata["dataset.metalog.pbench.script"]
+                    == dataset.metadata["server.benchmark"]
+                )
                 assert t.access == dataset.metadata["dataset.access"]
         except HTTPError as exc:
             pytest.fail(
@@ -169,9 +179,17 @@ def test_archive_only(server_client: PbenchServerClient, login_user):
             API.DATASETS_INVENTORY, {"dataset": md5, "target": ""}
         )
         metadata = server_client.get_metadata(
-            md5, ["dataset.operations", "server.archiveonly"]
+            md5,
+            [
+                "dataset.metalog.pbench.script",
+                "dataset.operations",
+                "server.archiveonly",
+                "server.benchmark",
+            ],
         )
+        assert metadata["dataset.metalog.pbench.script"] == "fio"
         assert metadata["server.archiveonly"] is True
+        assert metadata["server.benchmark"] == "fio"
 
         # NOTE: we could wait here; however, the UNPACK operation is only
         # enabled by upload, and INDEX is only enabled by UNPACK: so if they're
@@ -209,9 +227,18 @@ def test_no_metadata(server_client: PbenchServerClient, login_user):
             API.DATASETS_INVENTORY, {"dataset": md5, "target": ""}
         )
         metadata = server_client.get_metadata(
-            md5, ["dataset.operations", "dataset.metalog", "server.archiveonly"]
+            md5,
+            [
+                "dataset.operations",
+                "dataset.metalog",
+                "server.archiveonly",
+                "server.benchmark",
+            ],
         )
-        assert metadata["dataset.metalog"] == {"pbench": {"script": "Foreign"}}
+        assert metadata["dataset.metalog"] == {
+            "pbench": {"name": name, "script": "unknown"}
+        }
+        assert metadata["server.benchmark"] == "unknown"
         assert metadata["server.archiveonly"] is True
 
         # NOTE: we could wait here; however, the UNPACK operation is only
@@ -361,17 +388,21 @@ def test_details(self, server_client: PbenchServerClient, login_user):
         )
         for d in datasets:
             print(f"\t... checking run index for {d.name}")
+            indexed = not d.metadata["server.archiveonly"]
             response = server_client.get(
-                API.DATASETS_DETAIL, {"dataset": d.resource_id}
+                API.DATASETS_DETAIL, {"dataset": d.resource_id}, raise_error=False
             )
             detail = response.json()
-            indexed = not d.metadata["server.archiveonly"]
             if indexed:
+                assert (
+                    response.ok
+                ), f"DETAILS for {d.name} failed with {detail['message']}"
                 assert (
                     d.metadata["dataset.metalog.pbench"]["script"]
                     == detail["runMetadata"]["script"]
                 )
             else:
+                assert response.status_code == HTTPStatus.CONFLICT
                 print(f"\t\t... {d.name} is archiveonly")
 
 
@@ -554,8 +585,15 @@ def test_contents(self, server_client: PbenchServerClient, login_user):
 
         for dataset in datasets:
             response = server_client.get(
-                API.DATASETS_CONTENTS, {"dataset": dataset.resource_id, "target": ""}
+                API.DATASETS_CONTENTS,
+                {"dataset": dataset.resource_id, "target": ""},
+                raise_error=False,
             )
+            archive = dataset.metadata["server.archiveonly"]
+            if archive:
+                assert response.status_code == HTTPStatus.CONFLICT
+                return
+
             assert (
                 response.ok
             ), f"CONTENTS {dataset.name} failed {response.status_code}:{response.json()['message']}"
@@ -565,15 +603,14 @@ def test_contents(self, server_client: PbenchServerClient, login_user):
             # directory is technically possible, but not legal unless it's a
             # trivial "archiveonly" dataset. NOTE: this will also fail if
             # either the "directories" or "files" JSON keys are missing.
-            archive = dataset.metadata["server.archiveonly"]
-            assert json["directories"] or json["files"] or archive
+            assert json["directories"] or json["files"]
 
             # Even if they're empty, both values must be lists
             assert isinstance(json["directories"], list)
             assert isinstance(json["files"], list)
 
-            # Unless archiveonly, we need at least a metadata.log
-            assert archive or "metadata.log" in (f["name"] for f in json["files"])
+            # We need at least a metadata.log
+            assert "metadata.log" in (f["name"] for f in json["files"])
 
             for d in json["directories"]:
                 uri = server_client._uri(
@@ -598,7 +635,7 @@ def test_visualize(self, server_client: PbenchServerClient, login_user):
         """
         datasets = server_client.get_list(
             owner="tester",
-            filter=["dataset.metalog.pbench.script:uperf"],
+            filter=["server.benchmark:uperf"],
         )
 
         for dataset in datasets:
@@ -624,7 +661,7 @@ def test_compare(self, server_client: PbenchServerClient, login_user):
         """
         datasets = server_client.get_list(
             owner="tester",
-            filter=["dataset.metalog.pbench.script:uperf"],
+            filter=["server.benchmark:uperf"],
         )
 
         candidates = [dataset.resource_id for dataset in datasets]
@@ -686,15 +723,25 @@ class TestUpdate:
     @pytest.mark.parametrize("access", ("public", "private"))
     def test_publish(self, server_client: PbenchServerClient, login_user, access):
         expected = "public" if access == "private" else "private"
-        datasets = server_client.get_list(access=access, mine="true")
+        datasets = server_client.get_list(
+            access=access, mine="true", metadata=["server.archiveonly"]
+        )
         print(f" ... updating {access} datasets to {expected} ...")
         for dataset in datasets:
-            server_client.update(dataset.resource_id, access=expected)
-            print(f"\t ... updating {dataset.name} to {access!r}")
-            meta = server_client.get_metadata(
-                dataset.resource_id, metadata="dataset.access"
+            response = server_client.update(
+                dataset.resource_id, access=expected, raise_error=False
             )
-            assert meta["dataset.access"] == expected
+            print(f"\t ... updating {dataset.name} to {access!r}")
+            if response.ok:
+                assert not dataset.metadata["server.archiveonly"]
+                meta = server_client.get_metadata(
+                    dataset.resource_id, metadata="dataset.access"
+                )
+                assert meta["dataset.access"] == expected
+            else:
+                assert dataset.metadata[
+                    "server.archiveonly"
+                ], f"Indexed dataset {dataset.name} failed to update with {response.json()['message']}"
 
 
 class TestDelete:

lib/pbench/test/unit/server/test_datasets_visualize.py

@@ -130,5 +130,5 @@ def mock_get_metadata(_d: Dataset, _k: str, _u: Optional[User] = None) -> JSON:
         monkeypatch.setattr(Metadata, "getvalue", mock_get_metadata)
         monkeypatch.setattr(QuisbyProcessing, "extract_data", mock_extract_data)
         response = query_get_as("uperf_1", "test", HTTPStatus.BAD_REQUEST)
-        assert response.json["message"] == "Unsupported Benchmark: HAMMERDB"
+        assert response.json["message"] == "Unsupported Benchmark: hammerDB"
         assert extract_not_called

lib/pbench/test/unit/server/test_endpoint_configure.py

@@ -54,6 +54,7 @@ def check_config(self, client, server_config, host, my_headers={}):
         uri = urljoin(host, uri_prefix)
         expected_results = {
             "identification": f"Pbench server {server_config.COMMIT_ID}",
+            "visualization": {"benchmarks": ["uperf"]},
             "uri": {
                 "datasets": {
                     "template": f"{uri}/datasets/{{dataset}}",