Added parent hook and some renderer #1

Merged
PeterSurda merged 7 commits from Ss_singh/buildbot_multibuild:hooks into master 2021-12-26 09:05:49 +01:00
2 changed files with 39 additions and 39 deletions
Showing only changes of commit bea032dc78 - Show all commits

View File

@ -1,38 +1,24 @@
from buildbot.plugins import util
import re
def _is_github(props):
if re.search(r'[/@]github.com', props.getProperty('repository')):
return True
return False
@util.renderer
def is_github(props):
return _is_github(props)
@util.renderer
def isnt_github(props):
return not _is_github(props)
def _is_gitea(props):
if re.search(r'[/@]git.bitmessage.org', props.getProperty('repository'), re.I):
return True
return False
@util.renderer
def is_gitea(props):
return _is_gitea(props)
@util.renderer
def isnt_gitea(props):
return not _is_gitea(props)
@util.renderer
def is_build_script_available(props):
# Actual check will got here
return props.getProperty('jobname', default='') != ''
return props.getProperty("jobname", default="") != ""

should check for the correct property

should check for the correct property
@util.renderer
def isnt_build_script_available(props):
return not is_build_script_available(props)
@util.renderer
def is_test_script_available(props):
# Actual check will got here
return props.getProperty('jobname', default='') != ''
return props.getProperty("jobname", default="") != ""

should also check the correct property

should also check the correct property
@util.renderer
def isnt_test_script_available(props):
return not is_test_script_available(props)

View File

@ -16,6 +16,8 @@ import requests
import re
from buildbot.plugins import steps, util
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`.
from .lib.renderers import *
ty = "/change_hook/base"
request_headers = {
"Content-Type": "application/x-www-form-urlencoded",
Ss_singh marked this conversation as resolved Outdated

this should be extracted from the current properties.

this should be extracted from the current properties.

You mean it should come from util.Property("repository")? Do I need to save this repo url in some property?
Like, property="repo_url" in buildFactory step and use that later when executing the multibuild.py script?

You mean it should come from `util.Property("repository")`? Do I need to save this repo url in some property? Like, `property="repo_url"` in buildFactory step and use that later when executing the multibuild.py script?

You mean it should come from util.Property("repository")?

Yes.

Do I need to save this repo url in some property?

You should pass it as it is. Similar with "branch". I don't think others need passing.

Like, property="repo_url" in buildFactory step and use that later when executing the multibuild.py script?

If I understand you correctly, yes. These two properties you're basically just proxying from the parent to the child. But you're also adding more properties, mainly "jobname" and "dockerfile".

> You mean it should come from util.Property("repository")? Yes. > Do I need to save this repo url in some property? You should pass it as it is. Similar with "branch". I don't think others need passing. > Like, property="repo_url" in buildFactory step and use that later when executing the multibuild.py script? If I understand you correctly, yes. These two properties you're basically just proxying from the parent to the child. But you're also adding more properties, mainly "jobname" and "dockerfile".
@ -33,11 +35,15 @@ 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
# 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

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" ```
RUN echo 'buildbot ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
USER buildbot
ENTRYPOINT /entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS"
ENTRYPOINT /usr/local/bin/buildbot_entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS"
"""
@ -46,7 +52,7 @@ 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
python3-pip dumb-init curl openssh-client wget
# 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
@ -93,10 +99,12 @@ def get_dockerfile_contents(path, os_codename):
with open(path, "r") as file:
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.
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)
re.sub(r"(?m)^(CMD|ENTRYFILE).*$", "", contents)
return contents + eval("dockerfile_extra_contents_" + os_codename)
return contents + {
"focal": dockerfile_extra_contents_focal,
"bionic": dockerfile_extra_contents_bionic,
}[os_codename]
def trigger_child_hooks(buildbotUrl: str, os_codename: str, directory=".buildbot"):
@ -115,11 +123,13 @@ def trigger_child_hooks(buildbotUrl: str, os_codename: str, directory=".buildbot
test_script_exists = True
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.
# make a post request
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)
request_data["properties"] = {
"dockerfile": get_dockerfile_contents(
join(directory, job, "Dockerfile"), os_codename
),
"build_script_available": is_build_script_available(build_script_exists),
"test_script_available": is_test_script_available(test_script_exists),
}
Review

I think repository and branch are a level higher.

I think `repository` and `branch` are a level higher.
requests.post(request_url, headers=request_headers, data=request_data)
@ -158,7 +168,9 @@ def add_child_build_sh_step(build_factory, job, directory=".buildbot"):
build_factory.addStep(
steps.ShellCommand(
name="build_" + job,
command=["bash", "-c", "cd " + join(directory, job) + " && ./build.sh"],
command=["bash", "-c", join(directory, job, "build.sh")],
Ss_singh marked this conversation as resolved Outdated

analogous changes as with build.sh

analogous changes as with `build.sh`
doStepIf=is_build_script_available,
hideStepIf=isnt_build_script_available,
)
)
@ -170,7 +182,9 @@ def add_child_test_sh_step(build_factory, job, directory=".buildbot"):
build_factory.addStep(
steps.ShellCommand(
name="test_" + job,
command=["bash", "-c", "cd " + join(directory, job) + " && ./test.sh"],
command=["bash", "-c", join(directory, job, "test.sh")],
doStepIf=is_test_script_available,
hideStepIf=isnt_test_script_available,
)
)