From 180ecc6f8d2ca6938120d3602014d986eade5a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 21 Nov 2021 17:57:11 +0100 Subject: [PATCH] Refactor passing around git commit info --- src/runboat/api.py | 33 +++---- src/runboat/controller.py | 20 ++-- src/runboat/db.py | 16 +++- src/runboat/github.py | 37 +++----- src/runboat/k8s.py | 18 +--- .../kubefiles/kustomization.yaml.jinja | 12 +-- src/runboat/models.py | 93 +++++++++---------- src/runboat/webui-templates/build.html.jinja | 4 +- src/runboat/webui-templates/builds.html.jinja | 6 +- .../webui-templates/runboat-build-element.js | 10 +- tests/test_db.py | 21 +++-- tests/test_render_kubefiles.py | 11 ++- 12 files changed, 131 insertions(+), 150 deletions(-) diff --git a/src/runboat/api.py b/src/runboat/api.py index 609ee3c..542dd53 100644 --- a/src/runboat/api.py +++ b/src/runboat/api.py @@ -43,10 +43,7 @@ class Repo(BaseModel): class Build(BaseModel): name: str - repo: str - target_branch: str - pr: Optional[int] - git_commit: str + commit_info: github.CommitInfo image: str deploy_link: str repo_target_branch_link: str @@ -117,13 +114,8 @@ async def undeploy_builds( ) async def trigger_branch(repo: str, branch: str) -> None: """Trigger build for a branch.""" - branch_info = await github.get_branch_info(repo, branch) - await controller.deploy_or_start( - repo=branch_info.repo, - target_branch=branch_info.name, - pr=None, - git_commit=branch_info.head_sha, - ) + commit_info = await github.get_branch_info(repo, branch) + await controller.deploy_or_start(commit_info) @router.post( @@ -132,13 +124,8 @@ async def trigger_branch(repo: str, branch: str) -> None: ) async def trigger_pull(repo: str, pr: int) -> None: """Trigger build for a pull request.""" - pull_info = await github.get_pull_info(repo, pr) - await controller.deploy_or_start( - repo=pull_info.repo, - target_branch=pull_info.target_branch, - pr=pull_info.number, - git_commit=pull_info.head_sha, - ) + commit_info = await github.get_pull_info(repo, pr) + await controller.deploy_or_start(commit_info) async def _build_by_name(name: str) -> models.Build: @@ -222,13 +209,15 @@ class BuildEventSource: return BuildEvent(event=event, build=Build.from_orm(build)).json() def on_build_event(self, event: models.BuildEvent, build: models.Build) -> None: - if self.repo and build.repo != self.repo: + if self.repo and build.commit_info.repo != self.repo: return - if self.target_branch and build.target_branch != self.target_branch: + if self.target_branch and build.commit_info.target_branch != self.target_branch: return - if self.branch and (build.target_branch != self.branch or build.pr): + if self.branch and ( + build.commit_info.target_branch != self.branch or build.commit_info.pr + ): return - if self.pr and build.pr != self.pr: + if self.pr and build.commit_info.pr != self.pr: return if self.build_name and build.name != self.build_name: return diff --git a/src/runboat/controller.py b/src/runboat/controller.py index d6c5b66..d9b4f17 100644 --- a/src/runboat/controller.py +++ b/src/runboat/controller.py @@ -4,6 +4,7 @@ from typing import Any, Awaitable, Callable from . import k8s from .db import BuildsDb +from .github import CommitInfo from .models import Build, BuildEvent, BuildInitStatus, BuildStatus from .settings import settings @@ -78,24 +79,17 @@ class Controller: def undeploying(self) -> int: return self.db.count_by_status(BuildStatus.undeploying) - async def deploy_or_start( - self, repo: str, target_branch: str, pr: int | None, git_commit: str - ) -> None: + async def deploy_or_start(self, commit_info: CommitInfo) -> None: build = self.db.get_for_commit( - repo=repo, - target_branch=target_branch, - pr=pr, - git_commit=git_commit, + repo=commit_info.repo, + target_branch=commit_info.target_branch, + pr=commit_info.pr, + git_commit=commit_info.git_commit, ) if build is not None: await build.start() return - await Build.deploy( - repo=repo, - target_branch=target_branch, - pr=pr, - git_commit=git_commit, - ) + await Build.deploy(commit_info) def _wakeup(self) -> None: self._wakeup_initializer.set() diff --git a/src/runboat/db.py b/src/runboat/db.py index 4fe0ab3..1de887e 100644 --- a/src/runboat/db.py +++ b/src/runboat/db.py @@ -4,6 +4,7 @@ from enum import Enum from typing import Iterator, Protocol, cast from weakref import WeakSet +from .github import CommitInfo from .models import Build, BuildEvent, BuildInitStatus, BuildStatus, Repo _logger = logging.getLogger(__name__) @@ -40,7 +41,12 @@ class BuildsDb: @classmethod def _build_from_row(cls, row: sqlite3.Row) -> Build: - return Build(**{k: row[k] for k in row.keys()}) + commit_info_fields = {"repo", "target_branch", "pr", "git_commit"} + commit_info = CommitInfo(**{k: row[k] for k in commit_info_fields}) + return Build( + commit_info=commit_info, + **{k: row[k] for k in row.keys() if k not in commit_info_fields} + ) def reset(self) -> None: self._con = sqlite3.connect(":memory:") @@ -123,10 +129,10 @@ class BuildsDb: ( build.name, build.deployment_name, - build.repo, - build.target_branch, - build.pr, - build.git_commit, + build.commit_info.repo, + build.commit_info.target_branch, + build.commit_info.pr, + build.commit_info.git_commit, build.image, build.desired_replicas, build.status, diff --git a/src/runboat/github.py b/src/runboat/github.py index e51803f..96aca42 100644 --- a/src/runboat/github.py +++ b/src/runboat/github.py @@ -1,8 +1,8 @@ -from dataclasses import dataclass from enum import Enum -from typing import Any +from typing import Any, Optional import httpx +from pydantic import BaseModel from .exceptions import NotFoundOnGitHub from .settings import settings @@ -23,37 +23,30 @@ async def _github_request(method: str, url: str, json: Any = None) -> Any: return response.json() -@dataclass -class BranchInfo: +class CommitInfo(BaseModel): repo: str - name: str - head_sha: str + target_branch: str + pr: Optional[int] + git_commit: str -async def get_branch_info(repo: str, branch: str) -> BranchInfo: +async def get_branch_info(repo: str, branch: str) -> CommitInfo: branch_data = await _github_request("GET", f"/repos/{repo}/git/ref/heads/{branch}") - return BranchInfo( + return CommitInfo( repo=repo, - name=branch, - head_sha=branch_data["object"]["sha"], + target_branch=branch, + pr=None, + git_commit=branch_data["object"]["sha"], ) -@dataclass -class PullInfo: - repo: str - number: int - head_sha: str - target_branch: str - - -async def get_pull_info(repo: str, pr: int) -> PullInfo: +async def get_pull_info(repo: str, pr: int) -> CommitInfo: pr_data = await _github_request("GET", f"/repos/{repo}/pulls/{pr}") - return PullInfo( + return CommitInfo( repo=repo, - number=pr, - head_sha=pr_data["head"]["sha"], target_branch=pr_data["base"]["ref"], + pr=pr, + git_commit=pr_data["head"]["sha"], ) diff --git a/src/runboat/k8s.py b/src/runboat/k8s.py index 35f2f13..1ee7d32 100644 --- a/src/runboat/k8s.py +++ b/src/runboat/k8s.py @@ -9,7 +9,7 @@ from contextlib import contextmanager from enum import Enum from importlib import resources from pathlib import Path -from typing import Any, Callable, Generator, Optional, TypedDict, cast +from typing import Any, Callable, Generator, TypedDict, cast import urllib3 from jinja2 import Template @@ -19,6 +19,7 @@ from kubernetes.client.models.v1_deployment import V1Deployment from kubernetes.client.models.v1_job import V1Job from pydantic import BaseModel +from .github import CommitInfo from .settings import settings from .utils import sync_to_async, sync_to_async_iterator @@ -146,10 +147,7 @@ class DeploymentVars(BaseModel): build_name: str build_slug: str build_domain: str - repo: str - target_branch: str - pr: Optional[int] - git_commit: str + commit_info: CommitInfo image_name: str image_tag: str build_env: dict[str, str] @@ -161,10 +159,7 @@ def make_deployment_vars( mode: DeploymentMode, build_name: str, slug: str, - repo: str, - target_branch: str, - pr: int | None, - git_commit: str, + commit_info: CommitInfo, image: str, ) -> DeploymentVars: image_name, image_tag = _split_image_name_tag(image) @@ -174,10 +169,7 @@ def make_deployment_vars( build_name=build_name, build_slug=slug, build_domain=settings.build_domain, - repo=repo, - target_branch=target_branch, - pr=pr, - git_commit=git_commit, + commit_info=commit_info, image_name=image_name, image_tag=image_tag, build_env=settings.build_env or {}, diff --git a/src/runboat/kubefiles/kustomization.yaml.jinja b/src/runboat/kubefiles/kustomization.yaml.jinja index a835eba..c501edd 100644 --- a/src/runboat/kubefiles/kustomization.yaml.jinja +++ b/src/runboat/kubefiles/kustomization.yaml.jinja @@ -19,10 +19,10 @@ commonLabels: runboat/build: "{{ build_name }}" commonAnnotations: - runboat/repo: "{{ repo }}" - runboat/target-branch: "{{ target_branch }}" - runboat/pr: "{{ pr if pr else '' }}" - runboat/git-commit: "{{ git_commit }}" + runboat/repo: "{{ commit_info.repo }}" + runboat/target-branch: "{{ commit_info.target_branch }}" + runboat/pr: "{{ commit_info.pr if commit_info.pr else '' }}" + runboat/git-commit: "{{ commit_info.git_commit }}" images: - name: odoo @@ -41,8 +41,8 @@ configMapGenerator: literals: - PGDATABASE={{ build_name }} - ADDONS_DIR=/mnt/data/odoo-addons-dir - - RUNBOAT_GIT_REPO=https://github.com/{{ repo }} - - RUNBOAT_GIT_REF={{ git_commit }} + - RUNBOAT_GIT_REPO=https://github.com/{{ commit_info.repo }} + - RUNBOAT_GIT_REF={{ commit_info.git_commit }} {%- for key, value in build_env.items() %} - {{ key }}={{ value }} {%- endfor %} diff --git a/src/runboat/models.py b/src/runboat/models.py index 70edf04..51e8c97 100644 --- a/src/runboat/models.py +++ b/src/runboat/models.py @@ -8,7 +8,7 @@ from kubernetes.client.models.v1_deployment import V1Deployment from pydantic import BaseModel from . import github, k8s -from .github import GitHubStatusState +from .github import CommitInfo, GitHubStatusState from .settings import get_build_settings, settings from .utils import slugify @@ -40,10 +40,7 @@ class BuildInitStatus(str, Enum): class Build(BaseModel): name: str deployment_name: str - repo: str - target_branch: str - pr: Optional[int] - git_commit: str + commit_info: CommitInfo image: str status: BuildStatus init_status: BuildInitStatus @@ -83,10 +80,12 @@ class Build(BaseModel): return Build( name=deployment.metadata.labels["runboat/build"], deployment_name=deployment.metadata.name, - repo=deployment.metadata.annotations["runboat/repo"], - target_branch=deployment.metadata.annotations["runboat/target-branch"], - pr=deployment.metadata.annotations.get("runboat/pr") or None, - git_commit=deployment.metadata.annotations["runboat/git-commit"], + commit_info=CommitInfo( + repo=deployment.metadata.annotations["runboat/repo"], + target_branch=deployment.metadata.annotations["runboat/target-branch"], + pr=deployment.metadata.annotations.get("runboat/pr") or None, + git_commit=deployment.metadata.annotations["runboat/git-commit"], + ), image=deployment.spec.template.spec.containers[0].image, init_status=deployment.metadata.annotations["runboat/init-status"], status=cls._status_from_deployment(deployment), @@ -121,17 +120,18 @@ class Build(BaseModel): @classmethod def make_slug( - cls, repo: str, target_branch: str, pr: int | None, git_commit: str + cls, + commit_info: CommitInfo, ) -> str: - slug = f"{slugify(repo)}-{slugify(target_branch)}" - if pr: - slug = f"{slug}-pr{slugify(pr)}" - slug = f"{slug}-{git_commit[:12]}" + slug = f"{slugify(commit_info.repo)}-{slugify(commit_info.target_branch)}" + if commit_info.pr: + slug = f"{slug}-pr{slugify(commit_info.pr)}" + slug = f"{slug}-{commit_info.git_commit[:12]}" return slug @property def slug(self) -> str: - return self.make_slug(self.repo, self.target_branch, self.pr, self.git_commit) + return self.make_slug(self.commit_info) @property def deploy_link(self) -> str: @@ -139,21 +139,27 @@ class Build(BaseModel): @property def repo_target_branch_link(self) -> str: - return f"https://github.com/{self.repo}/tree/{self.target_branch}" + return ( + f"https://github.com/{self.commit_info.repo}" + f"/tree/{self.commit_info.target_branch}" + ) @property def repo_pr_link(self) -> str | None: - if not self.pr: + if not self.commit_info.pr: return None - return f"https://github.com/{self.repo}/pull/{self.pr}" + return f"https://github.com/{self.commit_info.repo}/pull/{self.commit_info.pr}" @property def repo_commit_link(self) -> str: - link = f"https://github.com/{self.repo}" - if self.pr: - return f"{link}/pull/{self.pr}/commits/{self.git_commit}" + link = f"https://github.com/{self.commit_info.repo}" + if self.commit_info.pr: + return ( + f"{link}/pull/{self.commit_info.pr}" + f"/commits/{self.commit_info.git_commit}" + ) else: - return f"{link}/commit/{self.git_commit}" + return f"{link}/commit/{self.commit_info.git_commit}" @property def webui_link(self) -> str: @@ -170,14 +176,12 @@ class Build(BaseModel): return await k8s.log(self.name, job_kind=None) @classmethod - async def deploy( - cls, repo: str, target_branch: str, pr: int | None, git_commit: str - ) -> None: + async def deploy(cls, commit_info: CommitInfo) -> None: """Deploy a build, without starting it.""" name = f"b{uuid.uuid4()}" - slug = cls.make_slug(repo, target_branch, pr, git_commit) + slug = cls.make_slug(commit_info) _logger.info(f"Deploying {slug} ({name}).") - build_settings = get_build_settings(repo, target_branch) + build_settings = get_build_settings(commit_info.repo, commit_info.target_branch) if len(build_settings) > 1: raise NotImplementedError( "Having more than one build per commit is not supported yet." @@ -186,16 +190,13 @@ class Build(BaseModel): k8s.DeploymentMode.deployment, name, slug, - repo.lower(), - target_branch, - pr, - git_commit, + commit_info, build_settings[0].image, ) await k8s.deploy(deployment_vars) await github.notify_status( - repo, - git_commit, + commit_info.repo, + commit_info.git_commit, GitHubStatusState.pending, target_url=None, ) @@ -220,8 +221,8 @@ class Build(BaseModel): await k8s.delete_job(self.name, job_kind=k8s.DeploymentMode.initialize) if await self._patch(init_status=BuildInitStatus.todo, desired_replicas=0): await github.notify_status( - self.repo, - self.git_commit, + self.commit_info.repo, + self.commit_info.git_commit, GitHubStatusState.pending, target_url=self.live_link, ) @@ -253,10 +254,7 @@ class Build(BaseModel): k8s.DeploymentMode.initialize, self.name, self.slug, - self.repo, - self.target_branch, - self.pr, - self.git_commit, + self.commit_info, self.image, ) await k8s.deploy(deployment_vars) @@ -274,10 +272,7 @@ class Build(BaseModel): k8s.DeploymentMode.cleanup, self.name, self.slug, - self.repo, - self.target_branch, - self.pr, - self.git_commit, + self.commit_info, self.image, ) await k8s.deploy(deployment_vars) @@ -288,8 +283,8 @@ class Build(BaseModel): _logger.info(f"Initialization job started for {self}.") if await self._patch(init_status=BuildInitStatus.started, desired_replicas=0): await github.notify_status( - self.repo, - self.git_commit, + self.commit_info.repo, + self.commit_info.git_commit, GitHubStatusState.pending, target_url=self.live_link, ) @@ -302,8 +297,8 @@ class Build(BaseModel): _logger.info(f"Initialization job succeded for {self}, ready to start.") if await self._patch(init_status=BuildInitStatus.succeeded): await github.notify_status( - self.repo, - self.git_commit, + self.commit_info.repo, + self.commit_info.git_commit, GitHubStatusState.success, target_url=self.live_link, ) @@ -316,8 +311,8 @@ class Build(BaseModel): _logger.info(f"Initialization job failed for {self}.") if await self._patch(init_status=BuildInitStatus.failed, desired_replicas=0): await github.notify_status( - self.repo, - self.git_commit, + self.commit_info.repo, + self.commit_info.git_commit, GitHubStatusState.failure, target_url=self.live_link, ) diff --git a/src/runboat/webui-templates/build.html.jinja b/src/runboat/webui-templates/build.html.jinja index 7c2d42a..7fb035d 100644 --- a/src/runboat/webui-templates/build.html.jinja +++ b/src/runboat/webui-templates/build.html.jinja @@ -30,8 +30,8 @@ const build = oEvent.build; buildElement.build = oEvent.event == "upd" ? build : {}; const repolinkElement = document.getElementById("repolink"); - repolinkElement.href = `./builds.html?repo=${build.repo}`; - repolinkElement.text = build.repo; + repolinkElement.href = `./builds.html?repo=${build.commit_info.repo}`; + repolinkElement.text = build.commit_info.repo; } // TODO: evtSource error handling diff --git a/src/runboat/webui-templates/builds.html.jinja b/src/runboat/webui-templates/builds.html.jinja index 1ceb45c..106ae03 100644 --- a/src/runboat/webui-templates/builds.html.jinja +++ b/src/runboat/webui-templates/builds.html.jinja @@ -47,9 +47,9 @@ // update event element buildElement.build = oEvent.build; } else { - var rowId = `branch-${oEvent.build.target_branch}`; - if (oEvent.build.pr) { - rowId += `-pr-${oEvent.build.pr}` + var rowId = `branch-${oEvent.build.commit_info.target_branch}`; + if (oEvent.build.commit_info.pr) { + rowId += `-pr-${oEvent.build.commit_info.pr}` } var rowElement = document.getElementById(rowId); if (!rowElement) { diff --git a/src/runboat/webui-templates/runboat-build-element.js b/src/runboat/webui-templates/runboat-build-element.js index bb9c716..42da2bf 100644 --- a/src/runboat/webui-templates/runboat-build-element.js +++ b/src/runboat/webui-templates/runboat-build-element.js @@ -42,13 +42,13 @@ class RunboatBuildElement extends LitElement {

${this.build.name}

- ${this.build.repo} ${this.build.target_branch} - ${this.build.pr? - html`PR ${this.build.pr}`:"" + ${this.build.commit_info.repo} ${this.build.commit_info.target_branch} + ${this.build.commit_info.pr? + html`PR ${this.build.commit_info.pr}`:"" }
- ${this.build.git_commit? - html`(${this.build.git_commit.substring(0, 8)})`:"" + ${this.build.commit_info.git_commit? + html`(${this.build.commit_info.git_commit.substring(0, 8)})`:"" }

diff --git a/tests/test_db.py b/tests/test_db.py index e96ebbb..0261c38 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -2,6 +2,7 @@ import datetime from unittest.mock import MagicMock from runboat.db import BuildsDb, SortOrder +from runboat.github import CommitInfo from runboat.models import Build, BuildInitStatus, BuildStatus, Repo @@ -18,10 +19,12 @@ def _make_build( return Build( name=name, deployment_name=name + "-odoo", - repo=repo or "oca/mis-builder", - target_branch=target_branch or "15.0", - pr=pr or None, - git_commit="0d35a10f161b410f2baa3d416a338d191b6dabc0", + commit_info=CommitInfo( + repo=repo or "oca/mis-builder", + target_branch=target_branch or "15.0", + pr=pr or None, + git_commit="0d35a10f161b410f2baa3d416a338d191b6dabc0", + ), image="ghcr.io/oca/oca-ci:py3.8-odoo15.0", status=status or BuildStatus.starting, init_status=init_status or BuildInitStatus.todo, @@ -62,13 +65,19 @@ def test_get_for_commit() -> None: db.add(build) assert ( db.get_for_commit( - build.repo, build.target_branch, build.pr, git_commit=build.git_commit + build.commit_info.repo, + build.commit_info.target_branch, + build.commit_info.pr, + git_commit=build.commit_info.git_commit, ) == build ) assert ( db.get_for_commit( - "not-a-build", build.target_branch, build.pr, git_commit=build.git_commit + "not-a-repo", + build.commit_info.target_branch, + build.commit_info.pr, + git_commit=build.commit_info.git_commit, ) is None ) diff --git a/tests/test_render_kubefiles.py b/tests/test_render_kubefiles.py index c94a7de..b6111c9 100644 --- a/tests/test_render_kubefiles.py +++ b/tests/test_render_kubefiles.py @@ -1,3 +1,4 @@ +from runboat.github import CommitInfo from runboat.k8s import DeploymentMode, _render_kubefiles, make_deployment_vars EXPECTED = """\ @@ -70,10 +71,12 @@ def test_render_kubefiles() -> None: mode=DeploymentMode.deployment, build_name="build-name", slug="build-slug", - repo="oca/mis-builder", - target_branch="15.0", - pr=None, - git_commit="abcdef123456789", + commit_info=CommitInfo( + repo="oca/mis-builder", + target_branch="15.0", + pr=None, + git_commit="abcdef123456789", + ), image="ghcr.io/oca/oca-ci:py3.8-odoo15.0", ) with _render_kubefiles(deployment_vars) as tmp_path: