Skip to content

Commit

Permalink
[ci] link to logs for all PRs in CI UI (hail-is#5054)
Browse files Browse the repository at this point in the history
Major Changes:
 - never delete CI jobs, only cancel them
 - Mergeable (success) and Failure build states include the job that
   triggered the build state
 - if a PR's build state has a job, link to that job

Minor Changes:
 - fix location of dk-test instance
 - test that proxy processes are still alive (if proxy creation fails,
   the process usually exits)
 - provide `HAIL_CI_GCS_PATH` for developers to set an alternative
   deploy bucket and path-within-bucket (now that `gs://hail-ci-0-1` is
   protected)
  • Loading branch information
danking authored Jan 2, 2019
1 parent bf5fc7d commit 48dbb61
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 29 deletions.
4 changes: 3 additions & 1 deletion ci/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,14 @@ restart-proxy:
$(shell gcloud compute \
--project "$(PROJECT)" \
ssh \
--zone "us-central1-a" \
--zone "us-east1-b" \
"dk-test" \
--ssh-flag="-R 0.0.0.0:${HAIL_CI_REMOTE_PORT}:127.0.0.1:5000" \
--ssh-flag='-N' \
--ssh-flag='-T' \
--ssh-flag='-v' \
--dry-run) > proxy.log 2>proxy.err & echo $$! > proxy.pid
sleep 2 && kill -0 $$(cat proxy.pid)

restart-batch-proxy:
-kill $(shell cat batch-proxy.pid)
Expand All @@ -50,6 +51,7 @@ restart-batch-proxy:
| sed 's:pods/::' \
| head -n 1))
kubectl port-forward ${BATCH_POD} 8888:5000 > batch-proxy.log 2>batch-proxy.err & echo $$! > batch-proxy.pid
sleep 2 && kill -0 $$(cat batch-proxy.pid)



Expand Down
1 change: 0 additions & 1 deletion ci/ci/batch_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
def try_to_cancel_job(job):
try:
job.cancel()
job.delete()
except requests.exceptions.HTTPError as e:
log.warning(f'could not cancel job {job.id} due to {e}')

Expand Down
32 changes: 21 additions & 11 deletions ci/ci/build_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ def build_state_from_json(d):
if t == 'Merged':
return Merged(d['target_sha'])
elif t == 'Mergeable':
return Mergeable(d['target_sha'])
return Mergeable(d['target_sha'],
batch_client.get_job(d['job_id']))
elif t == 'Failure':
return Failure(d['exit_code'], d['image'], d['target_sha'])
return Failure(d['exit_code'],
batch_client.get_job(d['job_id']),
d['image'],
d['target_sha'])
elif t == 'NoMergeSHA':
return NoMergeSHA(d['exit_code'], d['target_sha'])
elif t == 'Building':
Expand Down Expand Up @@ -83,8 +87,9 @@ def __ne__(self, other):


class Mergeable(object):
def __init__(self, target_sha):
def __init__(self, target_sha, job):
self.target_sha = target_sha
self.job = job

def transition(self, other):
if not isinstance(other, Merged):
Expand All @@ -99,23 +104,26 @@ def __str__(self):
def to_json(self):
return {
'type': 'Mergeable',
'target_sha': self.target_sha
'target_sha': self.target_sha,
'job_id': self.job.id,
}

def gh_state(self):
return 'success'

def __eq__(self, other):
return (isinstance(other, Mergeable) and
self.target_sha == other.target_sha)
self.target_sha == other.target_sha and
self.job.id == other.job.id)

def __ne__(self, other):
return not self == other


class Failure(object):
def __init__(self, exit_code, image, target_sha):
def __init__(self, exit_code, job, image, target_sha):
self.exit_code = exit_code
self.job = job
self.image = image
self.target_sha = target_sha

Expand All @@ -126,12 +134,13 @@ def transition(self, other):
return other

def __str__(self):
return f'failing build {self.exit_code}'
return f'failing build {self.exit_code}, job id {self.job.id}'

def to_json(self):
return {
'type': 'Failure',
'exit_code': self.exit_code,
'job_id': self.job_id,
'image': self.image,
'target_sha': self.target_sha
}
Expand All @@ -142,6 +151,7 @@ def gh_state(self):
def __eq__(self, other):
return (isinstance(other, Failure) and
self.exit_code == other.exit_code and
self.job.id == other.job.id and
self.image == other.image and
self.target_sha == other.target_sha)

Expand Down Expand Up @@ -189,11 +199,11 @@ def __init__(self, job, image, target_sha):
self.image = image
self.target_sha = target_sha

def success(self, merged_sha):
return Mergeable(merged_sha, self.target_sha)
def success(self, merged_sha, job):
return Mergeable(merged_sha, self.target_sha, job)

def failure(self, exit_code):
return Failure(exit_code, self.image, self.target_sha)
def failure(self, exit_code, job):
return Failure(exit_code, job, self.image, self.target_sha)

def no_merge_sha(self, exit_code):
return NoMergeSHA(exit_code, self.target_sha)
Expand Down
11 changes: 6 additions & 5 deletions ci/ci/ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

from .batch_helper import try_to_cancel_job, job_ordering
from .ci_logging import log
from .constants import BUILD_JOB_TYPE, GCS_BUCKET, DEPLOY_JOB_TYPE
from .constants import BUILD_JOB_TYPE, GCS_BUCKET, GCS_BUCKET_PREFIX, \
DEPLOY_JOB_TYPE
from .environment import \
batch_client, \
WATCHED_TARGETS, \
Expand Down Expand Up @@ -334,22 +335,22 @@ def job_log(id):

def receive_ci_job(source, target, job):
upload_public_gs_file_from_string(GCS_BUCKET,
f'ci/{source.sha}/{target.sha}/job.log',
f'{GCS_BUCKET_PREFIX}ci/{source.sha}/{target.sha}/job.log',
job.cached_status()['log'])
upload_public_gs_file_from_filename(
GCS_BUCKET,
f'ci/{source.sha}/{target.sha}/index.html',
f'{GCS_BUCKET_PREFIX}ci/{source.sha}/{target.sha}/index.html',
'index.html')
prs.ci_build_finished(source, target, job)


def receive_deploy_job(target, job):
upload_public_gs_file_from_string(GCS_BUCKET,
f'deploy/{target.sha}/job.log',
f'{GCS_BUCKET_PREFIX}deploy/{target.sha}/job.log',
job.cached_status()['log'])
upload_public_gs_file_from_filename(
GCS_BUCKET,
f'deploy/{target.sha}/index.html',
f'{GCS_BUCKET_PREFIX}deploy/{target.sha}/index.html',
'deploy-index.html')
prs.deploy_build_finished(target, job)

Expand Down
14 changes: 13 additions & 1 deletion ci/ci/constants.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
import os

GITHUB_API_URL = 'https://api.github.com/'
GITHUB_CLONE_URL = 'https://github.com/'
VERSION = '0-1'
BUILD_JOB_PREFIX = 'hail-ci-0-2-1'
BUILD_JOB_TYPE = BUILD_JOB_PREFIX + '-build'
DEPLOY_JOB_TYPE = BUILD_JOB_PREFIX + '-deploy'
GCP_PROJECT = 'broad-ctsa'
GCS_BUCKET = 'hail-ci-' + VERSION
gcs_path = os.environ.get('HAIL_CI_GCS_PATH')
if gcs_path:
path_pieces = gcs_path.split('/')
GCS_BUCKET = path_pieces[0]
if len(path_pieces) > 1:
GCS_BUCKET_PREFIX = '/'.join(path_pieces[1:]) + '/'
else:
GCS_BUCKET_PREFIX = ''
else:
GCS_BUCKET = 'hail-ci-' + VERSION
GCS_BUCKET_PREFIX = ''
SHA_LENGTH = 12
13 changes: 7 additions & 6 deletions ci/ci/pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
Failure, Mergeable, Unknown, NoImage, Building, Buildable, Merged, \
build_state_from_json
from .ci_logging import log
from .constants import BUILD_JOB_TYPE, VERSION, GCS_BUCKET, SHA_LENGTH
from .constants import BUILD_JOB_TYPE, VERSION, GCS_BUCKET, SHA_LENGTH, \
GCS_BUCKET_PREFIX
from .environment import PR_BUILD_SCRIPT, SELF_HOSTNAME, batch_client, CONTEXT
from .git_state import FQSHA, FQRef
from .github import latest_sha_for_ref
Expand Down Expand Up @@ -297,7 +298,7 @@ def notify_github(self, build, status_sha=None):
}
if isinstance(build, Failure) or isinstance(build, Mergeable):
json['target_url'] = \
f'https://storage.googleapis.com/{GCS_BUCKET}/ci/{self.source.sha}/{self.target.sha}/index.html'
f'https://storage.googleapis.com/{GCS_BUCKET}/{GCS_BUCKET_PREFIX}ci/{self.source.sha}/{self.target.sha}/index.html'
try:
post_repo(
self.target.ref.repo.qname,
Expand Down Expand Up @@ -422,7 +423,6 @@ def refresh_from_batch_job(self, job):
elif state == 'Cancelled':
log.error(
f'a job for me was cancelled {short_str_build_job(job)} {self.short_str()}')
job.delete()
return self._new_build(try_new_build(self.source, self.target))
else:
assert state == 'Created', f'{state} {job.id} {job.attributes} {self.short_str()}'
Expand All @@ -434,7 +434,7 @@ def refresh_from_batch_job(self, job):
return self._new_build(Building(job, image, target.sha))
else:
log.info(f'found deploy job {job.id} for wrong target {target}, should be {self.target}')
job.delete()
job.cancel()
return self

def refresh_from_missing_job(self):
Expand Down Expand Up @@ -464,12 +464,13 @@ def update_from_completed_batch_job(self, job):
x = self
elif exit_code == 0:
log.info(f'job finished success {short_str_build_job(job)} {self.short_str()}')
x = self._new_build(Mergeable(self.target.sha))
x = self._new_build(Mergeable(self.target.sha, job))
else:
log.info(f'job finished failure {short_str_build_job(job)} {self.short_str()}')
x = self._new_build(
Failure(exit_code,
job,
job.attributes['image'],
self.target.sha))
job.delete()
job.cancel()
return x
8 changes: 4 additions & 4 deletions ci/ci/prs.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def deploy_build_finished(self, target, job):
else:
log.info(f'deploy job {job.id} succeeded for {target.short_str()}')
self.latest_deployed[target.ref] = target.sha
job.delete()
job.cancel()

def refresh_from_deploy_jobs(self, jobs):
lost_jobs = {target_ref for target_ref in self.deploy_jobs.keys()}
Expand All @@ -366,15 +366,15 @@ def refresh_from_deploy_job(self, target, job):
elif state == 'Cancelled':
log.info(f'refreshing from cancelled deploy job {job.id} {job.attributes}')
del self.deploy_jobs[target.ref]
job.delete()
job.cancel()
else:
assert state == 'Created', f'{state} {job.id} {job.attributes}'
existing_job = self.deploy_jobs[target.ref]
if existing_job is None:
self.deploy_jobs[target.ref] = job
elif existing_job.id != job.id:
log.info(f'found deploy job {job.id} other than mine {existing_job.id}, deleting')
job.delete()
log.info(f'found deploy job {job.id} other than mine {existing_job.id}, canceling')
job.cancel()

def ci_build_finished(self, source, target, job):
assert isinstance(job, Job), job
Expand Down
4 changes: 4 additions & 0 deletions ci/ci/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ <h3>{{ target.short_str() }}</h3>
{{ pr.title }}
</td>
<td align="left">
{% if pr.build.job is defined %}
<a href="job-log/{{ pr.build.job.id }}">{{ pr.build }}</a>
{% else %}
{{ pr.build }}
{% endif %}
</td>
<td align="left">
{{ pr.review }}
Expand Down

0 comments on commit 48dbb61

Please sign in to comment.