Added parent hook and some renderer #1

Merged
PeterSurda merged 7 commits from Ss_singh/buildbot_multibuild:hooks into master 2021-12-26 08:05:49 +00:00
2 changed files with 59 additions and 6 deletions
Showing only changes of commit 1432e510a1 - Show all commits

View File

@ -13,6 +13,7 @@ Requires docker
from os import walk
from os.path import exists, isfile, join, listdir
import requests
import re
Ss_singh marked this conversation as resolved Outdated

The hostname part of the request_url should be dynamic. However since it doesn't depend on a specific job, it doesn't need to use interpolate.

The hostname part of the `request_url` should be dynamic. However since it doesn't depend on a specific job, it doesn't need to use interpolate.

What's the hostname part again?

What's the hostname part again?
https://git.bitmessage.org/Bitmessage/buildbot-config/src/branch/master/master.cfg#L1435

You said it should be dynamic, where should this come from then?

You said it should be dynamic, where should this come from then?

Something like

c['buildbotURL'] + '/change_hook/base'

the buildbotURL could be passed as an argument to trigger_child_hooks.

Something like ``` c['buildbotURL'] + '/change_hook/base' ``` the `buildbotURL` could be passed as an argument to `trigger_child_hooks`.
ty = '/change_hook/base'
request_headers = {
@ -24,6 +25,39 @@ request_data = {
'comments': 'testcomment',
}
dockerfile_extra_contents_focal = '''
Ss_singh marked this conversation as resolved Outdated

this is fixed, derived from the base hook class.

this is fixed, derived from the base hook class.

Since we don't need type, I'd just remove it

Since we don't need `type`, I'd just remove it

We still need to construct the full URL, which consists of the URL of the buildbot server and the path for the base webhook. The latter part is statoc, I don't even think it's configurable in buildbot, it's derived from the hook class.

We still need to construct the full URL, which consists of the URL of the buildbot server and the path for the base webhook. The latter part is statoc, I don't even think it's configurable in buildbot, it's derived from the hook class.

So what this 'base webhook' should actually be?

So what this 'base webhook' should actually be?

I think it's fixed at /change_hook/base

I think it's fixed at `/change_hook/base`
# Buildbot
Ss_singh marked this conversation as resolved Outdated

this can be extracted from global variables

this can be extracted from global variables

Similarly we don't need node here so it can be removed

Similarly we don't need `node` here so it can be removed
RUN apt-get install -yq --no-install-suggests --no-install-recommends \
buildbot-worker git subversion python3-dev libffi-dev python3-setuptools \
python3-pip dumb-init curl openssh-client wget
RUN echo 'buildbot ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
USER buildbot
ENTRYPOINT /entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS"

should be the same as for bionic (different path, missing wget)

Also I have a pending PR for adding multistage builds in buildbot: https://github.com/buildbot/buildbot/pull/6314

This way we can have all of this in just one Dockerfile stub, just in different stagesand use build arguments to pass the distro docker in realtime.

It would then look something like this:

FROM stage1 AS buildbot-ubuntu-bionic

RUN apt-get install -yq --no-install-suggests --no-install-recommends \
    buildbot-slave git subversion python3-dev libffi-dev python3-setuptools \
    python3-pip dumb-init curl openssh-client

FROM stage1 AS buildbot-ubuntu-focal

RUN apt-get install -yq --no-install-suggests --no-install-recommends \
    buildbot-worker git subversion python3-dev libffi-dev python3-setuptools \
    python3-pip dumb-init curl openssh-client wget

FROM buildbot-$DISTRO-$VARIANT AS buildbot-run

RUN wget -O /usr/local/bin/buildbot_entrypoint.sh https://git.bitmessage.org/Bitmessage/buildbot-scripts/raw/branch/master/docker/bionic/entrypoint.sh
RUN chmod +x /usr/local/bin/buildbot_entrypoint.sh

RUN echo 'buildbot ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers

USER buildbot

ENTRYPOINT /usr/local/bin/buildbot_entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS"
should be the same as for `bionic` (different path, missing wget) Also I have a pending PR for adding multistage builds in buildbot: https://github.com/buildbot/buildbot/pull/6314 This way we can have all of this in just one Dockerfile stub, just in different stagesand use build arguments to pass the distro docker in realtime. It would then look something like this: ``` FROM stage1 AS buildbot-ubuntu-bionic RUN apt-get install -yq --no-install-suggests --no-install-recommends \ buildbot-slave git subversion python3-dev libffi-dev python3-setuptools \ python3-pip dumb-init curl openssh-client FROM stage1 AS buildbot-ubuntu-focal RUN apt-get install -yq --no-install-suggests --no-install-recommends \ buildbot-worker git subversion python3-dev libffi-dev python3-setuptools \ python3-pip dumb-init curl openssh-client wget FROM buildbot-$DISTRO-$VARIANT AS buildbot-run RUN wget -O /usr/local/bin/buildbot_entrypoint.sh https://git.bitmessage.org/Bitmessage/buildbot-scripts/raw/branch/master/docker/bionic/entrypoint.sh RUN chmod +x /usr/local/bin/buildbot_entrypoint.sh RUN echo 'buildbot ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers USER buildbot ENTRYPOINT /usr/local/bin/buildbot_entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS" ```
'''
dockerfile_extra_contents_bionic = '''
# Buildbot
RUN apt-get install -yq --no-install-suggests --no-install-recommends \
buildbot-slave git subversion python3-dev libffi-dev python3-setuptools \
python3-pip dumb-init curl openssh-client
# buildbot entrypoint
RUN wget -O /usr/local/bin/buildbot_entrypoint.sh https://git.bitmessage.org/Bitmessage/buildbot-scripts/raw/branch/master/docker/bionic/entrypoint.sh
RUN chmod +x /usr/local/bin/buildbot_entrypoint.sh
RUN echo 'buildbot ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
USER buildbot
ENTRYPOINT /usr/local/bin/buildbot_entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS"
'''
def list_jobs(directory=".buildbot"):
"""
@ -49,7 +83,19 @@ def find_artifacts(directory="out"):
continue
return join(directory, _)
def trigger_child_hooks(buildbotUrl:str, directory=".buildbot"):
def get_dockerfile_contents(path, os_codename):
'''
Read contents of a Dockerfile and add extra contents for the given os_codename
'''
with open(path, 'r') as file:
contents = file.read()
# remove any line containing CMD or ENTRYFILE keywords
contents = re.sub(r'(?m)^CMD.*$', '', contents)
contents = re.sub(r'(?m)^ENTRYFILE.*$', '', contents)
return contents + eval("dockerfile_extra_contents_" + os_codename)
Ss_singh marked this conversation as resolved
Review

How about just

contents = re.sub(r"(?m)^(CMD|ENTRYFILE).*$", "", contents)
How about just ``` contents = re.sub(r"(?m)^(CMD|ENTRYFILE).*$", "", contents) ```
def trigger_child_hooks(buildbotUrl:str, os_codename:str, directory=".buildbot"):
request_url = buildbotUrl + ty
Ss_singh marked this conversation as resolved Outdated

No eval, it's evil. Maybe use a dict, or just try the multistage.

No `eval`, it's evil. Maybe use a dict, or just try the multistage.
# List all jobs in the directory
@ -65,25 +111,26 @@ def trigger_child_hooks(buildbotUrl:str, directory=".buildbot"):
test_script_exists = True
# make a post request
request_data['dockerfile'] = open(join(directory, job, "Dockerfile"), "r").read()
request_data['dockerfile'] = get_dockerfile_contents(join(directory, job, "Dockerfile"), os_codename)
request_data['build_script_available'] = str(build_script_exists)
request_data['test_script_available'] = str(test_script_exists)
requests.post(request_url, headers=request_headers, data=request_data)
Ss_singh marked this conversation as resolved Outdated

I think it should be

request_data['properties']['dockerfile'] = ...
I think it should be ``` request_data['properties']['dockerfile'] = ... ```
if __name__ == "__main__":
# expect jobname, repository, branch from command line args
# expect jobname, repository, branch, buildbotUrl, os_codename from command line args
import sys
if len(sys.argv) == 5:
if len(sys.argv) == 6:
jobname = sys.argv[1]
Ss_singh marked this conversation as resolved
Review

proxied properties are missing

proxied properties are missing
Review

I think they are still missing. Like repository and branch.

I think they are still missing. Like `repository` and `branch`.
Review

Well, they were already being added before the hook function was getting triggered but anyways, now I've them placed altogether.

Well, they were already being added before the hook function was getting triggered but anyways, now I've them placed altogether.
repository = sys.argv[2]
branch = sys.argv[3]
buildbotUrl = sys.argv[4]
os_codename = sys.argv[5]
# add these into the request_data
request_data['jobname'] = jobname
request_data['repository'] = repository
request_data['branch'] = branch
Review

I think repository and branch are a level higher.

I think `repository` and `branch` are a level higher.
trigger_child_hooks(buildbotUrl)
trigger_child_hooks(buildbotUrl, os_codename)
else:
print("Usage: python3 multibuild.py <jobname> <repository> <branch> <buildbotUrl>")
print("Usage: python3 multibuild.py <jobname> <repository> <branch> <buildbotUrl> <os_codename>")

View File

@ -61,6 +61,11 @@ travis_bash.addStep(
)
)
travis_bash.addStep(steps.SetPropertyFromCommand(

this will be a separate BuildFactory. There will be a child one (what you have above), and a parent one (what you have below)

this will be a separate `BuildFactory`. There will be a child one (what you have above), and a parent one (what you have below)
command="grep ^VERSION_CODENAME= /etc/os-release | cut -d= -f2",
property="os_codename"
))
# execute multibuild.py
travis_bash.addStep(
steps.ShellCommand(
@ -72,6 +77,7 @@ travis_bash.addStep(
util.Property("repository"),
util.Property("branch"),
"https://buildbot.bitmessage.org",
util.Interpolate("%(prop:os_codename)s"),
],
)
)