Bitmessage/buildbot_multibuild
6
0
Fork 5
Code Issues 4 Pull Requests Projects Releases Wiki Activity

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
Conversation 2 Commits 7 Files Changed 2 +199 -4
Ss_singh commented 2021-12-23 09:02:55 +01:00
Contributor
Copy Link
No description provided.
Ss_singh added 1 commit 2021-12-23 09:02:55 +01:00
Ss_singh closed this pull request 2021-12-23 09:03:50 +01:00
Ss_singh reopened this pull request 2021-12-23 09:04:01 +01:00
Ss_singh commented 2021-12-23 09:04:36 +01:00
Author
Contributor
Copy Link

Closed it by mistake. Expecting reviews.

Closed it by mistake. Expecting reviews.
PeterSurda requested changes 2021-12-23 09:32:12 +01:00
PeterSurda left a comment
Owner
Copy Link

lib/wine.py should be renamed, maybe something like renderers.py

`lib/wine.py` should be renamed, maybe something like `renderers.py`
multibuild.py Outdated
@ -14,1 +14,4 @@
from os.path import exists, isfile, join, listdir
import requests
request_url = "https://buildbot.sysdeploy.org/change_hook/base"
PeterSurda commented 2021-12-23 09:25:48 +01:00
Owner
Copy Link

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.
Ss_singh commented 2021-12-23 10:43:29 +01:00
Author
Contributor
Copy Link

What's the hostname part again?

What's the hostname part again?
PeterSurda commented 2021-12-23 11:18:03 +01:00
Owner
Copy Link

https://git.bitmessage.org/Bitmessage/buildbot-config/src/branch/master/master.cfg#L1435

https://git.bitmessage.org/Bitmessage/buildbot-config/src/branch/master/master.cfg#L1435
Ss_singh commented 2021-12-23 11:53:31 +01:00
Author
Contributor
Copy Link

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

You said it should be dynamic, where should this come from then?
PeterSurda commented 2021-12-23 12:02:28 +01:00
Owner
Copy Link

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`.
👍 1
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -15,0 +20,4 @@
'Accept': 'text/plain'
}
request_data = {
'repository': 'git@git.bitmessage.org:Sysdeploy/buildbot-scripts.git',
PeterSurda commented 2021-12-23 09:26:10 +01:00
Owner
Copy Link

this should be extracted from the current properties.

this should be extracted from the current properties.
Ss_singh commented 2021-12-23 10:54:06 +01:00
Author
Contributor
Copy Link

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?
PeterSurda commented 2021-12-23 11:22:16 +01:00
Owner
Copy Link

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".
👍 1
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -15,0 +26,4 @@
'comments': 'testcomment',
}
ty = 'child_hook'
PeterSurda commented 2021-12-23 09:26:52 +01:00
Owner
Copy Link

this is fixed, derived from the base hook class.

this is fixed, derived from the base hook class.
Ss_singh commented 2021-12-23 11:21:40 +01:00
Author
Contributor
Copy Link

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

Since we don't need `type`, I'd just remove it
PeterSurda commented 2021-12-23 11:23:38 +01:00
Owner
Copy Link

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.
Ss_singh commented 2021-12-23 11:44:04 +01:00
Author
Contributor
Copy Link

So what this 'base webhook' should actually be?

So what this 'base webhook' should actually be?
PeterSurda commented 2021-12-23 11:52:00 +01:00
Owner
Copy Link

I think it's fixed at /change_hook/base

I think it's fixed at `/change_hook/base`
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -15,0 +27,4 @@
}
ty = 'child_hook'
hn = 'buildbot.sysdeploy.org' #hostname here(required?)
PeterSurda commented 2021-12-23 09:27:17 +01:00
Owner
Copy Link

this can be extracted from global variables

this can be extracted from global variables
Ss_singh commented 2021-12-23 11:22:29 +01:00
Author
Contributor
Copy Link

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

Similarly we don't need `node` here so it can be removed
PeterSurda commented 2021-12-23 11:23:55 +01:00
Owner
Copy Link

yes

yes
👍 1
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -40,0 +72,4 @@
addn = addn.format(build_script_available=build_script_exists, test_script_available=test_script_exists)
# make a post request
request_data['properties'] = '{"node":{hn},"type":{ty}{addn}"}'.format(hn=hn, ty=ty, addn=addn)
PeterSurda commented 2021-12-23 09:29:09 +01:00
Owner
Copy Link

node and type we don't need, that's for sysdeploy. we should put all the properties (build_script_available, test_script_available, dockerfile, ...) into this dict, there shouldn't be a sub-dict.

`node` and `type` we don't need, that's for sysdeploy. we should put all the properties (build_script_available, test_script_available, dockerfile, ...) into this dict, there shouldn't be a sub-dict.
👀 1
PeterSurda commented 2021-12-23 11:27:57 +01:00
Owner
Copy Link

I think it would look something like this (still may need fine tuning):

request_data = {
    'repository': Util.Property('repository'),
    'branch': Util.Property('branch'),
    'properties': {
                  'build_script_available': build_script_exists,
                  'test_script_available': test_script_exists,
                  'dockerfile': dockerfile
                  }
               }

requests.post(request_url, headers=request_headers, data=request_data)
I think it would look something like this (still may need fine tuning): ``` request_data = { 'repository': Util.Property('repository'), 'branch': Util.Property('branch'), 'properties': { 'build_script_available': build_script_exists, 'test_script_available': test_script_exists, 'dockerfile': dockerfile } } requests.post(request_url, headers=request_headers, data=request_data) ```
Ss_singh commented 2021-12-23 11:47:49 +01:00
Author
Contributor
Copy Link

Can I access this Util in multibuid.py as well? I just passed the 'repository' and 'branch' as command line args while executing multibuild.py

Can I access this `Util` in multibuid.py as well? I just passed the 'repository' and 'branch' as command line args while executing multibuild.py
PeterSurda commented 2021-12-23 11:54:54 +01:00
Owner
Copy Link

multibuild.py will have to be loaded from the buildbot config, just like now it's importing lib/wine.py, for example. But it can be made into a separate pip module, for example, so it can be developed and used independently.

`multibuild.py` will have to be loaded from the buildbot config, just like now it's importing `lib/wine.py`, for example. But it can be made into a separate pip module, for example, so it can be developed and used independently.
multibuild.py Outdated
@ -40,0 +73,4 @@
# make a post request
request_data['properties'] = '{"node":{hn},"type":{ty}{addn}"}'.format(hn=hn, ty=ty, addn=addn)
requests.post(request_url, headers=request_headers, data=request_data)
PeterSurda commented 2021-12-23 09:29:39 +01:00
Owner
Copy Link

we also need to add the repo information, i.e. the pre-existing properties.

we also need to add the repo information, i.e. the pre-existing properties.
Ss_singh commented 2021-12-23 11:49:46 +01:00
Author
Contributor
Copy Link

And those are what? 'repository', 'branch', 'author', these things?

And those are what? 'repository', 'branch', 'author', these things?
PeterSurda commented 2021-12-23 11:58:38 +01:00
Owner
Copy Link

yes. I think for an MVP we only need repository and branch, maybe revision. For later, probably owner and owners (the way I have it setup now, it would allow people to re-run jobs for their own PRs manually, without these two only admin can re-run job). I don't think author is needed, that's just something I borrowed from somewhere else.

yes. I think for an MVP we only need `repository` and `branch`, maybe `revision`. For later, probably `owner` and `owners` (the way I have it setup now, it would allow people to re-run jobs for their own PRs manually, without these two only admin can re-run job). I don't think `author` is needed, that's just something I borrowed from somewhere else.
parent_hook Outdated
@ -0,0 +44,4 @@
name="Execute build script",
command=[
"bash",
util.Interpolate(".buildbot/$(prop:jobname)/build.sh"),
PeterSurda commented 2021-12-23 09:30:26 +01:00
Owner
Copy Link

typo (I forgot the correct syntax but at the very least s is missing after ))

typo (I forgot the correct syntax but at the very least `s` is missing after `)`)
Ss_singh commented 2021-12-23 11:48:37 +01:00
Author
Contributor
Copy Link

Got it. It should be %(prop:<propname>)s

Got it. It should be `%(prop:<propname>)s`
👍 1
Ss_singh marked this conversation as resolved
parent_hook Outdated
@ -0,0 +61,4 @@
)
)
# execute multibuild.py
PeterSurda commented 2021-12-23 09:31:18 +01:00
Owner
Copy Link

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)
Ss_singh added 1 commit 2021-12-23 12:27:16 +01:00
Ss_singh commented 2021-12-23 12:30:34 +01:00
Author
Contributor
Copy Link

I've made a few changes

I've made a few changes
PeterSurda requested changes 2021-12-23 13:33:40 +01:00
lib/renderers.py Outdated
@ -0,0 +26,4 @@
@util.renderer
def isnt_gitea(props):
return not _is_gitea(props)
PeterSurda commented 2021-12-23 13:20:34 +01:00
Owner
Copy Link

the "old" renderers (above) I would leave out of here and just keep them in the config repo. They aren't really related to this project.

the ones below can stay.

the "old" renderers (above) I would leave out of here and just keep them in the config repo. They aren't really related to this project. the ones below can stay.
Ss_singh commented 2021-12-24 03:25:20 +01:00
Author
Contributor
Copy Link

But we need to check if its github or gitea, don't we?

But we need to check if its github or gitea, don't we?
PeterSurda commented 2021-12-24 03:29:37 +01:00
Owner
Copy Link

Yes but we don't need to do this in this project. The master.cfg would look something like this:

parent = util.BuildFactory()
parent.addStep(steps.GitHub(
    repourl=util.Property('repository'),
    name='github',
    doStepIf=is_github,
    hideStepIf=isnt_github,
    branch=util.Property('branch'),
    mode='incremental'
))
parent.addStep(steps.Gitea(
    repourl=util.Property('repository'),
    sshPrivateKey=gitea_privkey,
    sshKnownHosts=gitea_known_hosts,
    name='gitea',
    doStepIf=is_gitea,
    hideStepIf=isnt_gitea,
    branch=util.Property('branch'),
    mode='incremental'
))
buildbot_multibuild.add_parent_step(parent)

child = util.BuildFactory()
child.addStep(steps.GitHub(
    repourl=util.Property('repository'),
    name='github',
    doStepIf=is_github,
    hideStepIf=isnt_github,
    branch=util.Property('branch'),
    mode='incremental'
))
child.addStep(steps.Gitea(
    repourl=util.Property('repository'),
    sshPrivateKey=gitea_privkey,
    sshKnownHosts=gitea_known_hosts,
    name='gitea',
    doStepIf=is_gitea,
    hideStepIf=isnt_gitea,
    branch=util.Property('branch'),
    mode='incremental'
))
buildbot_multiboot.add_child_build_sh(child)
buildbot_multiboot.add_child_test_sh(child)
Yes but we don't need to do this in this project. The `master.cfg` would look something like this: ``` parent = util.BuildFactory() parent.addStep(steps.GitHub( repourl=util.Property('repository'), name='github', doStepIf=is_github, hideStepIf=isnt_github, branch=util.Property('branch'), mode='incremental' )) parent.addStep(steps.Gitea( repourl=util.Property('repository'), sshPrivateKey=gitea_privkey, sshKnownHosts=gitea_known_hosts, name='gitea', doStepIf=is_gitea, hideStepIf=isnt_gitea, branch=util.Property('branch'), mode='incremental' )) buildbot_multibuild.add_parent_step(parent) child = util.BuildFactory() child.addStep(steps.GitHub( repourl=util.Property('repository'), name='github', doStepIf=is_github, hideStepIf=isnt_github, branch=util.Property('branch'), mode='incremental' )) child.addStep(steps.Gitea( repourl=util.Property('repository'), sshPrivateKey=gitea_privkey, sshKnownHosts=gitea_known_hosts, name='gitea', doStepIf=is_gitea, hideStepIf=isnt_gitea, branch=util.Property('branch'), mode='incremental' )) buildbot_multiboot.add_child_build_sh(child) buildbot_multiboot.add_child_test_sh(child) ```
Ss_singh commented 2021-12-24 03:54:16 +01:00
Author
Contributor
Copy Link

But see, it still need those functions

But see, it still need those functions
Ss_singh commented 2021-12-24 03:56:09 +01:00
Author
Contributor
Copy Link

Ah wait, which master.cfg you mean here? The one in buildbot-config repo? Or you mean that hook here should be renamed to master.cfg?

Ah wait, which `master.cfg` you mean here? The one in buildbot-config repo? Or you mean that hook here should be renamed to `master.cfg`?
PeterSurda commented 2021-12-24 05:03:50 +01:00
Owner
Copy Link

This repo is a kind of plugin, or module. It's not a full configuration for a buildbot instance. For deployment, you edit the instance configuration and include this module.

This repo is a kind of plugin, or module. It's not a full configuration for a buildbot instance. For deployment, you edit the instance configuration and include this module.
Ss_singh commented 2021-12-25 06:22:56 +01:00
Author
Contributor
Copy Link

Created a PR addressing this in the buildbot-config repo

Created a PR addressing this in the buildbot-config repo
multibuild.py Outdated
@ -40,0 +65,4 @@
test_script_exists = True
# make a post request
request_data['dockerfile'] = open(join(directory, job, "Dockerfile"), "r").read()
PeterSurda commented 2021-12-23 13:24:29 +01:00
Owner
Copy Link

There should be additional code for modifying the dockerfile. At the very least, it needs to install buildbot-worker and its dependencies, setup the default worker and run it in entrypoint. At the moment we can assume "apt", but in the future it should be able to detect the distro and adjust commands based on that.

For security it should also filter the file to remove CMD or ENTRYFILE lines.

There should be additional code for modifying the dockerfile. At the very least, it needs to install buildbot-worker and its dependencies, setup the default worker and run it in entrypoint. At the moment we can assume "apt", but in the future it should be able to detect the distro and adjust commands based on that. For security it should also filter the file to remove CMD or ENTRYFILE lines.
Ss_singh commented 2021-12-24 03:29:03 +01:00
Author
Contributor
Copy Link

Okay it's like take the contents of already available Dockerfile and append it with extra commands based on the distro.

Okay it's like take the contents of already available Dockerfile and append it with extra commands based on the distro.
PeterSurda commented 2021-12-24 03:30:55 +01:00
Owner
Copy Link

Yes something like that, but it should be only the buildbot-specific stuff and not the whole existing Dockerfile

Yes something like that, but it should be only the buildbot-specific stuff and not the whole existing `Dockerfile`
Ss_singh commented 2021-12-24 05:07:26 +01:00
Author
Contributor
Copy Link

I've pushed an update resolving this. Kindly have a look

I've pushed an update resolving this. Kindly have a look
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -40,0 +70,4 @@
request_data['test_script_available'] = str(test_script_exists)
requests.post(request_url, headers=request_headers, data=request_data)
if __name__ == "__main__":
PeterSurda commented 2021-12-23 13:30:26 +01:00
Owner
Copy Link

I realised a problem. The code needs to be split between master and worker parts. Master code doesn't see the directories and files, and worker code doesn't have access to properties.

So my suggestion would be to add a BuildStep that constructs a JSON string (i.e. a renderer) and uploads it into the worker as a file. The next step will instruct the worker to run a script that will fetch the props from this JSON files, combines them with the per-job props and calls the child hooks.

I realised a problem. The code needs to be split between master and worker parts. Master code doesn't see the directories and files, and worker code doesn't have access to properties. So my suggestion would be to add a `BuildStep` that constructs a JSON string (i.e. a renderer) and uploads it into the worker as a file. The next step will instruct the worker to run a script that will fetch the props from this JSON files, combines them with the per-job props and calls the child hooks.
parent_hook Outdated
@ -0,0 +15,4 @@
gitea_privkey = util.Secret('gitea_privkey')
travis_bash = util.BuildFactory()
travis_bash.addStep(
PeterSurda commented 2021-12-23 13:33:21 +01:00
Owner
Copy Link

I think something more like the brew_package_steps from the config repo. One for adding steps for the parent BuildFactory, one for the children.

Then we can just have an example file to show how to use it, instead of a full buildbot config needing to reside in this repo.

I think something more like the `brew_package_steps` from the config repo. One for adding steps for the parent `BuildFactory`, one for the children. Then we can just have an example file to show how to use it, instead of a full buildbot config needing to reside in this repo.
PeterSurda marked this conversation as resolved
Ss_singh added 1 commit 2021-12-24 05:06:52 +01:00
Ss_singh added 1 commit 2021-12-25 05:49:37 +01:00
PeterSurda requested changes 2021-12-25 06:57:14 +01:00
PeterSurda left a comment
Owner
Copy Link

Not bad

Not bad
multibuild.py Outdated
@ -15,0 +37,4 @@
USER buildbot
ENTRYPOINT /entrypoint.sh "$BUILDMASTER" "$WORKERNAME" "$WORKERPASS"
PeterSurda commented 2021-12-25 06:48:32 +01:00
Owner
Copy Link

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" ```
multibuild.py
@ -40,0 +93,4 @@
with open(path, "r") as file:
contents = file.read()
# remove any line containing CMD or ENTRYFILE keywords
contents = re.sub(r"(?m)^CMD.*$", "", contents)
PeterSurda commented 2021-12-25 06:49:38 +01:00
Owner
Copy Link

How about just

contents = re.sub(r"(?m)^(CMD|ENTRYFILE).*$", "", contents)
How about just ``` contents = re.sub(r"(?m)^(CMD|ENTRYFILE).*$", "", contents) ```
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -40,0 +96,4 @@
contents = re.sub(r"(?m)^CMD.*$", "", contents)
contents = re.sub(r"(?m)^ENTRYFILE.*$", "", contents)
return contents + eval("dockerfile_extra_contents_" + os_codename)
PeterSurda commented 2021-12-25 06:50:50 +01:00
Owner
Copy Link

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.
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -40,0 +115,4 @@
test_script_exists = True
# make a post request
request_data["dockerfile"] = get_dockerfile_contents(
PeterSurda commented 2021-12-25 06:52:41 +01:00
Owner
Copy Link

I think it should be

request_data['properties']['dockerfile'] = ...
I think it should be ``` request_data['properties']['dockerfile'] = ... ```
Ss_singh marked this conversation as resolved
multibuild.py
@ -40,0 +120,4 @@
)
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)
PeterSurda commented 2021-12-25 06:53:05 +01:00
Owner
Copy Link

proxied properties are missing

proxied properties are missing
PeterSurda commented 2021-12-25 09:18:04 +01:00
Owner
Copy Link

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

I think they are still missing. Like `repository` and `branch`.
Ss_singh commented 2021-12-25 12:25:56 +01:00
Author
Contributor
Copy Link

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.
Ss_singh marked this conversation as resolved
multibuild.py
@ -40,0 +156,4 @@
Add a step to the build factory
"""
build_factory.addStep(
steps.ShellCommand(
PeterSurda commented 2021-12-25 06:55:06 +01:00
Owner
Copy Link

should also have conditionals for build_script_exists. If it doesn't, this step is skipped and hidden.

should also have conditionals for `build_script_exists`. If it doesn't, this step is skipped and hidden.
Ss_singh marked this conversation as resolved
multibuild.py
@ -40,0 +158,4 @@
build_factory.addStep(
steps.ShellCommand(
name="build_" + job,
command=["bash", "-c", "cd " + join(directory, job) + " && ./build.sh"],
PeterSurda commented 2021-12-25 06:56:16 +01:00
Owner
Copy Link

I wouldn't change the directory, but just run it from it, i.e.

command=["bash", "-c", join(directory, job, "build.sh")],
I wouldn't change the directory, but just run it from it, i.e. ``` command=["bash", "-c", join(directory, job, "build.sh")],
Ss_singh marked this conversation as resolved
multibuild.py Outdated
@ -40,0 +168,4 @@
Add a step to the build factory
"""
build_factory.addStep(
steps.ShellCommand(
PeterSurda commented 2021-12-25 06:56:44 +01:00
Owner
Copy Link

analogous changes as with build.sh

analogous changes as with `build.sh`
Ss_singh marked this conversation as resolved
Ss_singh added 1 commit 2021-12-25 07:58:03 +01:00
PeterSurda requested changes 2021-12-25 09:18:32 +01:00
lib/renderers.py Outdated
@ -0,0 +5,4 @@
@util.renderer
def is_build_script_available(props):
# Actual check will got here
return props.getProperty("jobname", default="") != ""
PeterSurda commented 2021-12-25 09:15:09 +01:00
Owner
Copy Link

should check for the correct property

should check for the correct property
lib/renderers.py Outdated
@ -0,0 +16,4 @@
@util.renderer
def is_test_script_available(props):
# Actual check will got here
return props.getProperty("jobname", default="") != ""
PeterSurda commented 2021-12-25 09:15:20 +01:00
Owner
Copy Link

should also check the correct property

should also check the correct property
Ss_singh added 1 commit 2021-12-25 12:22:50 +01:00
PeterSurda requested changes 2021-12-26 03:09:24 +01:00
PeterSurda left a comment
Owner
Copy Link

We also need some way to construct the worker environment for the parent job
(such as a Dockerfile). Currently multibuild.py won't be on the worker
running the parent job, so it won't run.

We also need some way to construct the worker environment for the parent job (such as a Dockerfile). Currently `multibuild.py` won't be on the worker running the parent job, so it won't run.
multibuild.py
@ -40,0 +129,4 @@
),
"build_script_available": is_build_script_available(build_script_exists),
"test_script_available": is_test_script_available(test_script_exists),
"repository": repository,
PeterSurda commented 2021-12-26 02:59:39 +01:00
Owner
Copy Link

I think repository and branch are a level higher.

I think `repository` and `branch` are a level higher.
multibuild.py
@ -40,0 +159,4 @@
branch,
"https://buildbot.bitmessage.org",
util.Interpolate("%(prop:os_codename)s"),
],
PeterSurda commented 2021-12-26 03:02:19 +01:00
Owner
Copy Link

For now it's probably ok. Over longer term however the os_codename isn't extracted from the parent (parent may be running something else than the child should), but from the child Dockerfile. The Dockerfile will begin with something like FROM ubuntu:bionic AS ..., so that determines the OS.

For now it's probably ok. Over longer term however the `os_codename` isn't extracted from the parent (parent may be running something else than the child should), but from the child `Dockerfile`. The `Dockerfile` will begin with something like `FROM ubuntu:bionic AS ...`, so that determines the OS.
multibuild.py
@ -40,0 +164,4 @@
)
def add_child_build_sh_step(build_factory, job, directory=".buildbot"):
PeterSurda commented 2021-12-26 03:05:59 +01:00
Owner
Copy Link

I'm not 100% sure this will pass properties. A safer solution would be to create a child of ShellCommand and call super with a constructed command.

I'm not 100% sure this will pass properties. A safer solution would be to create a child of `ShellCommand` and call `super` with a constructed `command`.
parent_hook Outdated
@ -0,0 +1,83 @@
#!/usr/bin/env python3
PeterSurda commented 2021-12-26 03:08:03 +01:00
Owner
Copy Link

I think we can remove this file.

I think we can remove this file.
Ss_singh added 1 commit 2021-12-26 08:31:15 +01:00
Removed parent hook
Some checks failed
buildbot/travis_bionic Build done.
Details
6039d3a39b
PeterSurda approved these changes 2021-12-26 09:04:35 +01:00
PeterSurda changed title from WIP: Added parent hook and some renderer to Added parent hook and some renderer 2021-12-26 09:04:40 +01:00
PeterSurda merged commit 6039d3a39b into master 2021-12-26 09:05:49 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
PeterSurda
Labels
Clear labels   
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Bitmessage/buildbot_multibuild#1
No description provided.
Delete Branch "Ss_singh/buildbot_multibuild:hooks"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?