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 +30 -16
3 changed files with 30 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f38fd1c826 - Show all commits

0
lib/wine.py → lib/renderers.py
View File

34
multibuild.py
View File

@ -14,22 +14,16 @@ from os import walk
from os.path import exists, isfile, join, listdir from os.path import exists, isfile, join, listdir
import requests import requests
request_url = "https://buildbot.sysdeploy.org/change_hook/base" ty = '/change_hook/base'
Ss_singh marked this conversation as resolved Outdated
PeterSurda commented 2021-12-23 09:25:48 +01:00
Outdated
Review
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
Outdated
Review
Copy Link

What's the hostname part again?

What's the hostname part again?
PeterSurda commented 2021-12-23 11:18:03 +01:00
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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
request_headers = { request_headers = {
'Content-Type': 'application/x-www-form-urlencoded', 'Content-Type': 'application/x-www-form-urlencoded',
'Accept': 'text/plain' 'Accept': 'text/plain'
} }
request_data = { request_data = {
'repository': 'git@git.bitmessage.org:Sysdeploy/buildbot-scripts.git',
'project': 'testproject', 'project': 'testproject',
Ss_singh marked this conversation as resolved Outdated
PeterSurda commented 2021-12-23 09:26:10 +01:00
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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
'author': 'cloud-init',
'comments': 'testcomment', 'comments': 'testcomment',
} }
ty = 'child_hook'
hn = 'buildbot.sysdeploy.org' #hostname here(required?)
addn = ',"build_script_available":"{build_script_available}","test_script_available":"{test_script_available}"'
def list_jobs(directory=".buildbot"): def list_jobs(directory=".buildbot"):
""" """
Ss_singh marked this conversation as resolved Outdated
PeterSurda commented 2021-12-23 09:26:52 +01:00
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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
Outdated
Review
Copy Link

I think it's fixed at /change_hook/base

I think it's fixed at `/change_hook/base`
@ -55,7 +49,8 @@ def find_artifacts(directory="out"):
continue continue
return join(directory, _) return join(directory, _)
def trigger_child_hooks(directory=".buildbot"): def trigger_child_hooks(buildbotUrl:str, directory=".buildbot"):
request_url = buildbotUrl + ty
# List all jobs in the directory # List all jobs in the directory
jobs = list_jobs(directory) jobs = list_jobs(directory)
@ -69,11 +64,26 @@ def trigger_child_hooks(directory=".buildbot"):
if exists(join(directory, job, "test.sh")): if exists(join(directory, job, "test.sh")):
test_script_exists = True test_script_exists = True
addn = addn.format(build_script_available=build_script_exists, test_script_available=test_script_exists)
# make a post request # make a post request
request_data['properties'] = '{"node":{hn},"type":{ty}{addn}"}'.format(hn=hn, ty=ty, addn=addn) request_data['dockerfile'] = open(join(directory, job, "Dockerfile"), "r").read()
Ss_singh marked this conversation as resolved Outdated
PeterSurda commented 2021-12-23 13:24:29 +01:00
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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
Outdated
Review
Copy Link

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

I've pushed an update resolving this. Kindly have a look
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) requests.post(request_url, headers=request_headers, data=request_data)
if __name__ == "__main__": if __name__ == "__main__":
PeterSurda commented 2021-12-23 13:30:26 +01:00
Outdated
Review
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.
trigger_child_hooks() # expect jobname, repository, branch from command line args
import sys
PeterSurda commented 2021-12-23 09:29:09 +01:00
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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.
if len(sys.argv) == 5:
PeterSurda commented 2021-12-23 09:29:39 +01:00
Outdated
Review
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
Outdated
Review
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
Outdated
Review
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.
jobname = sys.argv[1]
repository = sys.argv[2]
branch = sys.argv[3]
buildbotUrl = sys.argv[4]
# add these into the request_data
request_data['jobname'] = jobname
request_data['repository'] = repository
request_data['branch'] = branch
trigger_child_hooks(buildbotUrl)
else:
print("Usage: python3 multibuild.py <jobname> <repository> <branch> <buildbotUrl>")

12
parent_hook
View File

@ -2,7 +2,7 @@
""" parent webhook """ """ parent webhook """
from buildbot.plugins import steps, util, secrets from buildbot.plugins import steps, util, secrets
from .lib.wine import * from .lib.renderers import *
c = BuildmasterConfig = {} c = BuildmasterConfig = {}
@ -44,7 +44,7 @@ travis_bash.addStep(
name="Execute build script", name="Execute build script",
command=[ command=[
"bash", "bash",
util.Interpolate(".buildbot/$(prop:jobname)/build.sh"), util.Interpolate(".buildbot/%(prop:jobname)s/build.sh"),
Ss_singh marked this conversation as resolved Outdated
PeterSurda commented 2021-12-23 09:30:26 +01:00
Outdated
Review
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
Outdated
Review
Copy Link

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

Got it. It should be `%(prop:<propname>)s`
👍 1
], ],
doStepIf=is_build_script_available, doStepIf=is_build_script_available,
) )
@ -55,7 +55,7 @@ travis_bash.addStep(
name="Execute test script", name="Execute test script",
command=[ command=[
"bash", "bash",
util.Interpolate(".buildbot/$(prop:jobname)/test.sh"), util.Interpolate(".buildbot/%(prop:jobname)s/test.sh"),
], ],
doStepIf=is_test_script_available, doStepIf=is_test_script_available,
) )
@ -67,7 +67,11 @@ travis_bash.addStep(
name="Execute multibuild script", name="Execute multibuild script",
command=[ command=[
"python", "python",
util.Interpolate("multibuild.py"), "multibuild.py",
util.Interpolate("%(prop:jobname)s"),
util.Property("repository"),
util.Property("branch"),
"https://buildbot.bitmessage.org",
], ],
) )
) )