fix(reporters): restore old behavior for Gerrit reporting #27

Merged
jade merged 3 commits from bring-back-old-gerrit-reporting into forkos 2024-10-18 23:41:11 +00:00
5 changed files with 109 additions and 52 deletions

1
.envrc Normal file
View file

@ -0,0 +1 @@
use flake

View file

@ -4,42 +4,41 @@ import os
import sys
import graphlib
import base64
import random
from collections.abc import Generator
from dataclasses import dataclass, field
from pathlib import Path
from typing import TYPE_CHECKING, Any
import tempfile
from typing import Any
import buildbot
from buildbot.configurators import ConfiguratorBase
from buildbot.plugins import reporters, schedulers, secrets, steps, util, worker
from buildbot.process import buildstep, logobserver, remotecommand
from buildbot.process.log import StreamLog
from buildbot.process.project import Project
from buildbot.process.properties import Properties
from buildbot.process.results import ALL_RESULTS, statusToString
from buildbot.www.auth import AuthBase
from buildbot.www.oauth2 import OAuth2Auth
from buildbot.changes.gerritchangesource import GerritChangeSource
from buildbot.reporters.utils import getURLForBuildrequest
from buildbot.reporters.generators.build import BuildStatusGenerator
from buildbot.reporters.message import MessageFormatterFunction
from buildbot.reporters.utils import getURLForBuildrequest
from buildbot.process.buildstep import EXCEPTION
from buildbot.process.buildstep import SUCCESS
from buildbot.process.buildstep import BuildStepFailed
from buildbot.process.results import worst_status
import requests
if TYPE_CHECKING:
from buildbot.process.log import Log
from twisted.internet import defer
from twisted.logger import Logger
from .binary_cache import S3BinaryCacheConfig
from .message_formatter import ReasonableMessageFormatter, CallbackPayloadBuild, CallbackPayloadBuildSet, CallbackReturn
log = Logger()
# util is a plugin variable, not a module...
BuilderConfig = util.BuilderConfig
MasterLock = util.MasterLock
FLAKE_TARGET_ATTRIBUTE_FOR_JOBS = "hydraJobs"
@dataclass
@ -48,7 +47,7 @@ class EvaluatorSettings:
worker_count: int
max_memory_size: int
gc_roots_dir: str
lock: util.MasterLock
lock: MasterLock
@dataclass
class NixBuilder:
@ -92,6 +91,8 @@ class OAuth2Config:
debug: bool = False
class KeycloakOAuth2Auth(OAuth2Auth):
userinfoUri: str
def __init__(self, *args, debug=False, **kwargs):
super().__init__(*args, **kwargs)
self.debug = debug
@ -255,7 +256,7 @@ class BuildTrigger(steps.BuildStep):
def run(self):
self.running = True
build_props = self.build.getProperties()
logs: Log = yield self.addLog("build info")
logs: StreamLog = yield self.addLog("build info")
builds_to_schedule = list(self.jobs)
build_schedule_order = []
@ -421,7 +422,7 @@ class NixEvalCommand(buildstep.ShellMixin, steps.BuildStep):
# Filter out failed evaluations
succeeded_jobs = [job for job in filtered_jobs if job.get('error') is None]
drv_show_log: Log = yield self.getLog("stdio")
drv_show_log: StreamLog = yield self.getLog("stdio")
all_deps = dict()
def closure_of(key, deps):
@ -515,9 +516,9 @@ class NixConfigure(buildstep.CommandMixin, steps.BuildStep):
@defer.inlineCallbacks
def run(self) -> Generator[Any, object, Any]:
try:
configure_log: Log = yield self.getLog("stdio")
configure_log: StreamLog = yield self.getLog("stdio")
except Exception:
configure_log: Log = yield self.addLog("stdio")
configure_log: StreamLog = yield self.addLog("stdio")
# Takes precedence.
configure_log.addStdout("checking if there's a .ci/buildbot.nix...\n")
@ -565,7 +566,7 @@ class NixBuildCommand(buildstep.ShellMixin, steps.BuildStep):
if error := self.getProperty("error"):
attr = self.getProperty("attr")
# show eval error
error_log: Log = yield self.addLog("nix_error")
error_log: StreamLog = yield self.addLog("nix_error")
error_log.addStderr(f"{attr} failed to evaluate:\n{error}")
return util.FAILURE
@ -589,10 +590,10 @@ def nix_eval_config(
project: GerritProject,
worker_names: list[str],
supported_systems: list[str],
eval_lock: util.MasterLock,
eval_lock: MasterLock,
worker_count: int,
max_memory_size: int,
) -> util.BuilderConfig:
) -> BuilderConfig:
"""
Uses nix-eval-jobs to evaluate the entrypoint of this project.
For each evaluated attribute a new build pipeline is started.
@ -676,7 +677,7 @@ def nix_build_config(
build_stores: list[str],
signing_keyfile: str | None = None,
binary_cache_config: S3BinaryCacheConfig | None = None
) -> util.BuilderConfig:
) -> BuilderConfig:
"""Builds one nix flake attribute."""
factory = util.BuildFactory()
@ -806,11 +807,11 @@ def config_for_project(
nix_supported_systems: list[str],
nix_eval_worker_count: int,
nix_eval_max_memory_size: int,
eval_lock: util.MasterLock,
eval_lock: MasterLock,
nix_builders: list[NixBuilder],
signing_keyfile: str | None = None,
binary_cache_config: S3BinaryCacheConfig | None = None
) -> Project:
) -> None:
config["projects"].append(Project(project.name))
config["schedulers"].extend(
[
@ -885,22 +886,25 @@ class PeriodicWithStartup(schedulers.Periodic):
yield self.setState("last_build", None)
yield super().activate()
def gerritReviewFmt(url, data):
if 'build' not in data:
raise ValueError('`build` is supposed to be present to format a build')
def gerritReviewFmt(url: str, payload: CallbackPayloadBuild | CallbackPayloadBuildSet) -> CallbackReturn:
assert isinstance(payload, CallbackPayloadBuild), "BuildSet are not handled yet!"
build = data['build']
build = payload.build
if 'builder' not in build and 'name' not in build['builder']:
raise ValueError('either `builder` or `builder.name` is not present in the build dictionary, unexpected format request')
builderName = build['builder']['name']
result = build['results']
log.info("Formatting a message for a Gerrit build: {} -- result is {}".format(builderName, result))
if result == util.RETRY:
return dict()
return CallbackReturn()
if builderName != f'{build["properties"].get("event.project")}/nix-eval':
return dict()
expectedBuilderName = f'{build["properties"].get("event.project")[0]}/nix-eval'
if builderName != expectedBuilderName:
log.info("Passing {} builder which is not of the form '{}'".format(builderName, expectedBuilderName))
return CallbackReturn()
failed = build['properties'].get('failed_builds', [[]])[0]
@ -924,7 +928,8 @@ def gerritReviewFmt(url, data):
message += "\nFor more details visit:\n"
message += build['url'] + "\n"
return dict(message=message, labels=labels)
log.info("Message formatted: {}, labels: Verified={}".format(message, labels['Verified']))
return CallbackReturn(body=message, extra_info={'labels': labels})
class GerritNixConfigurator(ConfiguratorBase):
"""Janitor is a configurator which create a Janitor Builder with all needed Janitor steps"""
@ -982,32 +987,32 @@ class GerritNixConfigurator(ConfiguratorBase):
self.auth_method = auth_method
def configure(self, config: dict[str, Any]) -> None:
worker_config = json.loads(read_secret_file(self.nix_workers_secret_name))
def configure(self, config_dict: dict[str, Any]) -> None:
worker_config_dict = json.loads(read_secret_file(self.nix_workers_secret_name))
worker_names = []
if self.manhole is not None:
config["manhole"] = self.manhole
config_dict["manhole"] = self.manhole
config.setdefault("projects", [])
config.setdefault("secretsProviders", [])
config.setdefault("www", {
'allowed_origins': self.allowed_origins
})
config_dict.setdefault("projects", [])
config_dict.setdefault("secretsProviders", [])
for item in worker_config:
print('Default allowed origins for this Buildbot server: {}'.format(', '.join(self.allowed_origins)))
config_dict["www"]["allowed_origins"] = self.allowed_origins
for item in worker_config_dict:
cores = item.get("cores", 0)
for i in range(cores):
for arch in self.nix_supported_systems + ["other"]:
worker_name = f"{item['name']}-{i:03}"
config["workers"].append(worker.Worker(f"{worker_name}-{arch}", item["pass"]))
config_dict["workers"].append(worker.Worker(f"{worker_name}-{arch}", item["pass"]))
worker_names.append(worker_name)
eval_lock = util.MasterLock("nix-eval")
for project in self.projects:
config_for_project(
config,
config_dict,
self.gerrit_config,
GerritProject(name=project, private_sshkey_path=self.gerrit_sshkey_path),
worker_names,
@ -1020,15 +1025,15 @@ class GerritNixConfigurator(ConfiguratorBase):
binary_cache_config=self.binary_cache_config
)
config["change_source"] = self.gerrit_change_source
config["services"].append(
config_dict["change_source"] = self.gerrit_change_source
config_dict["services"].append(
reporters.GerritStatusPush(self.gerrit_server, self.gerrit_user,
port=self.gerrit_port,
identity_file=self.gerrit_sshkey_path,
generators=[
# gerritReviewCB / self.url
BuildStatusGenerator(
message_formatter=MessageFormatterFunction(
mode='all',
jade marked this conversation as resolved
Review

i wonder if this is the cause of the excessive emails: previously we were only being called when all the builds have finished (either failure or success), right?

i wonder if this is the cause of the excessive emails: previously we were only being called when all the builds have finished (either failure or success), right?
Review

hm, no, this doesn't seem to be the cause of that, and there seem to be reasonable numbers of emails so far? let's merge it.

hm, no, this doesn't seem to be the cause of that, and there seem to be reasonable numbers of emails so far? let's merge it.
message_formatter=ReasonableMessageFormatter(
lambda data: gerritReviewFmt(self.url, data),
"plain",
want_properties=True,
@ -1036,11 +1041,10 @@ class GerritNixConfigurator(ConfiguratorBase):
),
),
])
# startCB, summaryCB are too noisy, we won't use them.
)
if self.prometheus_config is not None:
config['services'].append(reporters.Prometheus(port=self.prometheus_config.get('port', 9100), interface=self.prometheus_config.get('address', '')))
config_dict['services'].append(reporters.Prometheus(port=self.prometheus_config.get('port', 9100), interface=self.prometheus_config.get('address', '')))
# Upstream defaults pretend they already do something similar
# but they didn't work, hence the custom function.
@ -1050,7 +1054,7 @@ class GerritNixConfigurator(ConfiguratorBase):
return ref
return ref.rsplit('/', 1)[0]
config["services"].append(
config_dict["services"].append(
util.OldBuildCanceller(
"build_canceller",
filters=[
@ -1073,12 +1077,12 @@ class GerritNixConfigurator(ConfiguratorBase):
systemd_secrets = secrets.SecretInAFile(
dirname=os.environ["CREDENTIALS_DIRECTORY"],
)
config["secretsProviders"].append(systemd_secrets)
config_dict["secretsProviders"].append(systemd_secrets)
config["www"].setdefault("plugins", {})
config_dict["www"].setdefault("plugins", {})
if "authz" not in config["www"]:
config["www"]["authz"] = util.Authz(
if "authz" not in config_dict["www"]:
config_dict["www"]["authz"] = util.Authz(
allowRules=[
util.AnyEndpointMatcher(role="admin", defaultDeny=False),
util.StopBuildEndpointMatcher(role="owner"),
@ -1092,5 +1096,5 @@ class GerritNixConfigurator(ConfiguratorBase):
],
)
if "auth" not in config["www"] and self.auth_method is not None:
config["www"]["auth"] = self.auth_method
if "auth" not in config_dict["www"] and self.auth_method is not None:
config_dict["www"]["auth"] = self.auth_method

View file

@ -0,0 +1,46 @@
from typing import Any, Callable, Literal
from buildbot.reporters.message import MessageFormatterBase
import dataclasses
@dataclasses.dataclass
class CallbackPayloadBuild:
# buddy i have no idea what the fuck is in this
build: dict[str, Any]
@dataclasses.dataclass
class CallbackPayloadBuildSet:
buildset: dict[str, Any]
# i have no idea what the fuck is in this honestly
builds: Any
@dataclasses.dataclass
class CallbackReturn:
body: str | None = None
subject: str | None = None
type: Literal['plain'] | Literal['html'] | Literal['json'] = 'plain'
extra_info: dict[str, Any] | None = None
# FIXME: support other template types, if they actually become necessary
template_type: Literal['plain'] = 'plain'
class ReasonableMessageFormatter(MessageFormatterBase):
"""
Message formatter which uses strongly typed data classes to reduce suffering slightly.
"""
CallbackFunc = Callable[[CallbackPayloadBuild | CallbackPayloadBuildSet], CallbackReturn]
def __init__(self, function: CallbackFunc, template_type: str, **kwargs):
super().__init__(**kwargs)
self.template_type = template_type
self._function = function
def format_message_for_build(self, master, build, **kwargs):
return dataclasses.asdict(self._function(CallbackPayloadBuild(build=build)))
def format_message_for_buildset(self, master, buildset, builds, **kwargs):
return dataclasses.asdict(self._function(CallbackPayloadBuildSet(buildset=buildset, builds=builds)))
# These only exist as callbacks, the only one actually used is render_message_dict
def render_message_body(self, context):
return None
def render_message_subject(self, context):
return None

View file

@ -1,4 +1,5 @@
{ setuptools, buildPythonPackage }:
# TODO: figure out how to make this depend upon buildbot without leaking it further.
buildPythonPackage {
name = "buildbot-nix";
format = "pyproject";

View file

@ -42,6 +42,11 @@
pkgs.ruff
];
};
devShells.default = pkgs.mkShell {
packages = [
self'.packages.buildbot-nix
];
};
packages.buildbot-nix = pkgs.python3.pkgs.callPackage ./default.nix { };
checks =
let