[Benchmark] test update_commits (#9689)

* add delete_file * test * wip * wip * test billing project * wip * wip * wip * clean * fix comment * changes * test_benchmark in build.yaml * clean * fix response * forgot await * forgot await again * remove userdata arg * add case for testing * changes * changes * change formatted commits from list to dict * wip * wip * wip * wip * wip * wip * comment * wip * wip * wip * remove unused import * refactor * refactor * add benchmark-tests to tls/config.yaml * add wait to deploy_benchmark * clean * clean * changes * changes * changes * clean * clean * wip * wip * wip * aaaa * wip * wip * fixed main weirdness * missing line for benchmark-gsa-key * hope this doesnt ruin everything * fix * changes * changes * change timeout from 300s to 1200s
hail-is · Nov 23, 2020 · a940880 · a940880
1 parent be98c56
commit a940880
Show file tree

Hide file tree

Showing 6 changed files with 201 additions and 34 deletions.
diff --git a/benchmark-service/Dockerfile.test b/benchmark-service/Dockerfile.test
@@ -0,0 +1,6 @@
+FROM {{ service_base_image.image }}
+
+COPY benchmark-service/test/ /test/
+RUN python3 -m pip install --no-cache-dir \
+  pytest-instafail==0.4.1 \
+  pytest-asyncio==0.10.0
diff --git a/benchmark-service/benchmark/benchmark.py b/benchmark-service/benchmark/benchmark.py
@@ -34,7 +34,9 @@
 
 BENCHMARK_ROOT = os.path.dirname(os.path.abspath(__file__))
 
-benchmark_data = None
+benchmark_data = {
+    'commits': {}
+}
 
 with open(os.environ.get('HAIL_CI_OAUTH_TOKEN', 'oauth-token/oauth-token'), 'r') as f:
     oauth_token = f.read().strip()
@@ -223,53 +225,109 @@ async def compare(request, userdata):  # pylint: disable=unused-argument
 async def update_commits(app):
     global benchmark_data
     github_client = app['github_client']
-    batch_client = app['batch_client']
-    gs_reader = app['gs_reader']
 
     request_string = f'/repos/hail-is/hail/commits?since={START_POINT}'
     log.info(f'start point is {START_POINT}')
     gh_data = await github_client.getitem(request_string)
     log.info(f'gh_data length is {len(gh_data)}')
-    formatted_new_commits = []
 
     for gh_commit in gh_data:
         sha = gh_commit.get('sha')
         log.info(f'for commit {sha}')
-        file_path = f'{BENCHMARK_RESULTS_PATH}/{sha}.json'
-        has_results_file = gs_reader.file_exists(file_path)
+        await update_commit(app, sha)
 
-        batch_statuses = [b._last_known_status async for b in batch_client.list_batches(q=f'sha={sha}')]
+    log.info('got new commits')
 
-        complete_batch_statuses = [bs for bs in batch_statuses if bs['complete']]
-        running_batch_statuses = [bs for bs in batch_statuses if not bs['complete']]
 
-        if has_results_file:
-            assert complete_batch_statuses, batch_statuses
-            log.info(f'commit {sha} has a results file')
-            status = complete_batch_statuses[-1]
-        elif running_batch_statuses:
-            status = running_batch_statuses[-1]
-            log.info(f'batch already exists for commit {sha}')
-        else:
-            batch_id = await submit_test_batch(batch_client, sha)
-            batch = await batch_client.get_batch(batch_id)
-            status = batch._last_known_status
-            log.info(f'submitted a batch {batch_id} for commit {sha}')
-
-        commit = {
-            'sha': sha,
-            'title': gh_commit['commit']['message'],
-            'author': gh_commit['commit']['author']['name'],
-            'date': gh_commit['commit']['author']['date'],
-            'status': status
-        }
-        formatted_new_commits.append(commit)
+async def get_commit(app, sha):  # pylint: disable=unused-argument
+    github_client = app['github_client']
+    batch_client = app['batch_client']
+    gs_reader = app['gs_reader']
 
-    log.info('got new commits')
+    file_path = f'{BENCHMARK_RESULTS_PATH}/{sha}.json'
+    request_string = f'/repos/hail-is/hail/commits/{sha}'
 
-    benchmark_data = {
-        'commits': formatted_new_commits
+    gh_commit = await github_client.getitem(request_string)
+
+    has_results_file = gs_reader.file_exists(file_path)
+    batch_statuses = [b._last_known_status async for b in batch_client.list_batches(q=f'sha={sha}')]
+    complete_batch_statuses = [bs for bs in batch_statuses if bs['complete']]
+    running_batch_statuses = [bs for bs in batch_statuses if not bs['complete']]
+
+    if has_results_file:
+        assert complete_batch_statuses, batch_statuses
+        log.info(f'commit {sha} has a results file')
+        status = complete_batch_statuses[-1]
+    elif running_batch_statuses:
+        status = running_batch_statuses[-1]
+        log.info(f'batch already exists for commit {sha}')
+    else:
+        status = None
+        log.info(f'no batches or results file exists for {sha}')
+
+    commit = {
+        'sha': sha,
+        'title': gh_commit['commit']['message'],
+        'author': gh_commit['commit']['author']['name'],
+        'date': gh_commit['commit']['author']['date'],
+        'status': status
     }
+    return commit
+
+
+async def update_commit(app, sha):  # pylint: disable=unused-argument
+    commit = await get_commit(app, sha)
+    if commit['status'] is not None:
+        return commit
+
+    batch_client = app['batch_client']
+    batch_id = await submit_test_batch(batch_client, sha)
+    batch = await batch_client.get_batch(batch_id)
+    commit['status'] = batch._last_known_status
+    log.info(f'submitted a batch {batch_id} for commit {sha}')
+
+    return commit
+
+
+@router.get('/api/v1alpha/benchmark/commit/{sha}')
+async def get_status(request):  # pylint: disable=unused-argument
+    sha = str(request.match_info['sha'])
+    app = request.app
+    commit = await get_commit(app, sha)
+    return web.json_response(commit)
+
+
+@router.delete('/api/v1alpha/benchmark/commit/{sha}')
+async def delete_commit(request):  # pylint: disable=unused-argument
+    global benchmark_data
+    app = request.app
+    gs_reader = app['gs_reader']
+    batch_client = app['batch_client']
+    sha = str(request.match_info['sha'])
+    file_path = f'{BENCHMARK_RESULTS_PATH}/{sha}.json'
+
+    if gs_reader.file_exists(file_path):
+        gs_reader.delete_file(file_path)
+        log.info(f'deleted file for sha {sha}')
+
+    async for b in batch_client.list_batches(q=f'sha={sha}'):
+        await b.delete()
+        log.info(f'deleted batch for sha {sha}')
+
+    if benchmark_data['commits'].get(sha):
+        del benchmark_data['commits'][sha]
+        log.info(f'deleted commit {sha} from commit list')
+
+    return web.Response()
+
+
+@router.post('/api/v1alpha/benchmark/commit/{sha}')
+async def call_update_commit(request):  # pylint: disable=unused-argument
+    body = await request.json()
+    sha = body['sha']
+    log.info('call_update_commit')
+    commit = await update_commit(request.app, sha)
+    return web.json_response(commit)
 
 
 async def github_polling_loop(app):

diff --git a/benchmark-service/benchmark/utils.py b/benchmark-service/benchmark/utils.py
@@ -90,5 +90,12 @@ def file_exists(self, file_path):
         bucket = self.storage_client.bucket(bucket_name)
         path = file_info['path']
         exists = storage.Blob(bucket=bucket, name=path).exists()
-        log.info(f'file {path} in bucket {bucket_name} exists? {exists}')
+        log.info(f'file {bucket_name}/{path} in bucket {bucket_name} exists? {exists}')
         return exists
+
+    def delete_file(self, file_path):
+        file_info = parse_file_path(FILE_PATH_REGEX, file_path)
+        bucket_name = file_info['bucket']
+        bucket = self.storage_client.bucket(bucket_name)
+        path = file_info['path']
+        storage.Blob(bucket=bucket, name=path).delete()
diff --git a/benchmark-service/test/test_update_commits.py b/benchmark-service/test/test_update_commits.py
@@ -0,0 +1,54 @@
+import json
+import logging
+import asyncio
+import pytest
+import aiohttp
+
+from hailtop.config import get_deploy_config
+from hailtop.auth import service_auth_headers
+from hailtop.tls import in_cluster_ssl_client_session, get_context_specific_ssl_client_session
+import hailtop.utils as utils
+
+pytestmark = pytest.mark.asyncio
+
+logging.basicConfig(level=logging.INFO)
+log = logging.getLogger(__name__)
+
+sha = 'd626f793ad700c45a878d192652a0378818bbd8b'
+
+
+async def test_update_commits():
+    deploy_config = get_deploy_config()
+    headers = service_auth_headers(deploy_config, 'benchmark')
+    commit_benchmark_url = deploy_config.url('benchmark', f'/api/v1alpha/benchmark/commit/{sha}')
+
+    async with get_context_specific_ssl_client_session(
+            raise_for_status=True,
+            timeout=aiohttp.ClientTimeout(total=60)) as session:
+
+        commit = None
+
+        await utils.request_retry_transient_errors(
+            session, 'DELETE', f'{commit_benchmark_url}', headers=headers, json={'sha': sha})
+
+        resp_status = await utils.request_retry_transient_errors(
+            session, 'GET', f'{commit_benchmark_url}', headers=headers, json={'sha': sha})
+        commit = await resp_status.json()
+        assert commit['status'] is None, commit
+
+        resp_commit = await utils.request_retry_transient_errors(
+            session, 'POST', f'{commit_benchmark_url}', headers=headers, json={'sha': sha})
+        commit = await resp_commit.json()
+
+        async def wait_forever():
+            nonlocal commit
+            while commit is None or not commit['status']['complete']:
+                resp = await utils.request_retry_transient_errors(
+                    session, 'GET', f'{commit_benchmark_url}', headers=headers, json={'sha': sha})
+                commit = await resp.json()
+                await asyncio.sleep(5)
+                print(commit['status'])
+            return commit
+
+        commit = await wait_forever()
+        assert commit['status']['complete'] == True, commit
diff --git a/build.yaml b/build.yaml
@@ -532,6 +532,13 @@ steps:
    publishAs: test-monitoring
    dependsOn:
      - service_base_image
+ - kind: buildImage
+   name: test_benchmark_image
+   dockerFile: benchmark-service/Dockerfile.test
+   contextPath: .
+   publishAs: test-benchmark
+   dependsOn:
+     - service_base_image
  - kind: buildImage
    name: image_fetcher_image
    dockerFile: image-fetcher/Dockerfile
@@ -2056,6 +2063,10 @@ steps:
    namespace:
      valueFrom: default_ns.name
    config: benchmark-service/deployment.yaml
+   wait:
+     - kind: Service
+       name: benchmark
+       for: alive
    dependsOn:
     - default_ns
     - benchmark_image
@@ -2065,6 +2076,34 @@ steps:
     - create_accounts
     - create_billing_projects
     - deploy_batch
+ - kind: runImage
+   name: test_benchmark
+   image:
+     valueFrom: test_benchmark_image.image
+   script: |
+     set -ex
+     python3 -m pytest --log-cli-level=INFO -s -vv --instafail --durations=50 /test/
+   secrets:
+     - name: gce-deploy-config
+       namespace:
+         valueFrom: default_ns.name
+       mountPath: /deploy-config
+     - name: test-dev-tokens
+       namespace:
+         valueFrom: default_ns.name
+       mountPath: /user-tokens
+     - name: ssl-config-benchmark-tests
+       namespace:
+         valueFrom: default_ns.name
+       mountPath: /ssl-config
+   timeout: 1200
+   dependsOn:
+     - test_benchmark_image
+     - create_deploy_config
+     - create_accounts
+     - default_ns
+     - create_certs
+     - deploy_benchmark
  - kind: deploy
    name: deploy_query
    namespace:

diff --git a/tls/config.yaml b/tls/config.yaml
@@ -77,6 +77,9 @@ principals:
 - name: benchmark
   domain: benchmark
   kind: json
+- name: benchmark-tests
+  domain: benchmark-tests
+  kind: json
 - name: address
   domain: address
   kind: json