Closed tonyhutter closed 3 years ago
I maintain the buildbot package in Fedora, perhaps we could retool this around that to simplify the deployment?
@Conan-Kudo by "retooling", are you talking about updating the README.md instructions (which are currently Ubuntu only) to include Fedora?
Sure. I'd also been planning to have Buildbot added to EPEL8 for OpenZFS infrastructure as well (I'd told @behlendorf about this early last year, I think?).
I think we'd want to wait on updating the instructions until after Fedora is running 3.2.x. It looks like Fedora is on 3.0.0 right now (https://src.fedoraproject.org/rpms/buildbot). buildbot 3.2.0 contains a needed fix (https://github.com/buildbot/buildbot/pull/5401)
I'm updating it now: https://src.fedoraproject.org/rpms/buildbot/c/ef40513be80c20298670211e9e969a108c9bbd69
(I was waiting for your PR before I did it)
Update to Fedora 34 submitted: https://bodhi.fedoraproject.org/updates/FEDORA-2021-affbf0cf1f
I can also start working on bringing this into EPEL to run in OpenZFS infrastructure on RHEL 8 (or similar).
@freqlabs sorry for the late reply, I just got back from vacation.
The AMIs change weekly and the purpose of the current code is to query the latest AMI to use at the time the worker instance is being launched, rather than when the buildbot config is loaded.
Thanks for the info, I totally missed your 2263ee93dfd670f06e416430cdb6dbfe498d3e97 commit. Let me see if I can wrap my brain around it and fix it.
@freqlabs my latest push may or may not have fixed what was in 2263ee9. It doesn't include any changes to buildbot master like you had in your patch, which makes me nervous that it's not going to work, but we'll see. I have this buildbot PR running on an internal server with some log() statements added to print out what AMIs get selected for FreeBSD. I'm hoping to see the AMI hashes change over time, which would prove that the AMIs are being looked up dynamically when workers are spawned, rather than at config load time.
@Conan-Kudo I included instructions in README.md for setting up zfs-buildbot on Fedora as well.
diff --git a/master/buildbot/worker/ec2.py b/master/buildbot/worker/ec2.py
index ab60d5b95..382e4dc00 100644
--- a/master/buildbot/worker/ec2.py
+++ b/master/buildbot/worker/ec2.py
@@ -448,6 +448,7 @@ class EC2LatentWorker(AbstractLatentWorker):
log.msg('%s %s requesting spot instance with price %0.4f' %
(self.__class__.__name__, self.workername, bid_price))
+ image = self.get_image()
reservations = self.ec2.meta.client.request_spot_instances(
SpotPrice=str(bid_price),
LaunchSpecification=self._remove_none_opts(
@@ -473,7 +474,6 @@ class EC2LatentWorker(AbstractLatentWorker):
raise LatentWorkerFailedToSubstantiate()
instance_id = request['InstanceId']
self.instance = self.ec2.Instance(instance_id)
- image = self.get_image()
instance_id, start_time = self._wait_for_instance()
return instance_id, image.id, start_time
I think this patch will still be needed. It moves the call to self.get_image()
so it occurs before we reserve a spot instance, because for FreeBSD we update self.ami
in get_image()
, and self.ami
is part of the spot instance request.
@freqlabs thanks, I'll include your updated patch in my next push.
I've submitted Buildbot 3.3.0 to EPEL 8: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-9c56cda373
@freqlabs my latest push includes your updated patch. Please take another look when you get a chance.
buildbot
has now reached EPEL 8 stable! 🎉
@tonyhutter I've submitted a pull request to buildbot upstream for the one remaining patch: https://github.com/buildbot/buildbot/pull/6200
I've backported the remaining patch into the Fedora EPEL 8 package for buildbot
: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-ceff7556c0
This PR Updates zfs-buildbot from buildbot 0.8.x to 3.2.0. buildbot 3.2.0 is the current version that is being installed with
pip3
on ubuntu 18/20. Some specific changes:'NoneType' object has no attribute 'get_image'
"worker name '<worker name>' is not an identifier
'nextWorker now takes a 3rd argument'
"builtins.TypeError: __init__() takes at least 3 arguments"
Note: This PR is a major revamp of an older PR (#200).