From: Michael Tremer Date: Sun, 8 Oct 2017 12:48:32 +0000 (+0100) Subject: Refactor builders X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e704b8e2d9453d569f29072c2490e93eaa6278d5;p=pbs.git Refactor builders Signed-off-by: Michael Tremer --- diff --git a/src/buildservice/builders.py b/src/buildservice/builders.py index c9f54950..2ae6d629 100644 --- a/src/buildservice/builders.py +++ b/src/buildservice/builders.py @@ -17,21 +17,49 @@ from .decorators import * from .users import generate_password_hash, check_password_hash, generate_random_string class Builders(base.Object): + def _get_builder(self, query, *args): + res = self.db.get(query, *args) + + if res: + return Builder(self.backend, res.id, data=res) + + def _get_builders(self, query, *args): + res = self.db.query(query, *args) + + for row in res: + yield Builder(self.backend, row.id, data=row) + + def __iter__(self): + builders = self._get_builders("SELECT * FROM builders \ + WHERE deleted IS FALSE ORDER BY name") + + return iter(builders) + + def create(self, name, user=None, log=True): + """ + Creates a new builder. + """ + builder = self._get_builder("INSERT INTO builders(name) \ + VALUES(%s) RETURNING *", name) + + # Generate a new passphrase. + passphrase = builder.regenerate_passphrase() + + # Log what we have done. + if log: + builder.log("created", user=user) + + # The Builder object and the passphrase are returned. + return builder, passphrase + def auth(self, name, passphrase): # If either name or passphrase is None, we don't check at all. if None in (name, passphrase): return # Search for the hostname in the database. - # The builder must not be deleted. - builder = self.db.get("SELECT id FROM builders WHERE name = %s AND \ - NOT status = 'deleted'", name) - - if not builder: - return - - # Get the whole Builder object from the database. - builder = self.get_by_id(builder.id) + builder = self._get_builder("SELECT * FROM builders \ + WHERE name = %s AND deleted IS FALSE", name) # If the builder was not found or the passphrase does not match, # you have bad luck. @@ -41,26 +69,16 @@ class Builders(base.Object): # Otherwise we return the Builder object. return builder - def get_all(self): - builders = self.db.query("SELECT * FROM builders WHERE NOT status = 'deleted' ORDER BY name") - - return [Builder(self.pakfire, b.id, b) for b in builders] - - def get_by_id(self, id): - if not id: - return - - return Builder(self.pakfire, id) + def get_by_id(self, builder_id): + return self._get_builder("SELECT * FROM builders WHERE id = %s", builder_id) def get_by_name(self, name): - builder = self.db.get("SELECT * FROM builders WHERE name = %s LIMIT 1", name) - - if builder: - return Builder(self.pakfire, builder.id, builder) + return self._get_builder("SELECT * FROM builders \ + WHERE name = %s AND deleted IS FALSE", name) def get_load(self): res1 = self.db.get("SELECT SUM(max_jobs) AS max_jobs FROM builders \ - WHERE status = 'enabled'") + WHERE enabled IS TRUE and deleted IS FALSE") res2 = self.db.get("SELECT COUNT(*) AS count FROM jobs \ WHERE state = 'dispatching' OR state = 'running' OR state = 'uploading'") @@ -108,32 +126,13 @@ class Builders(base.Object): class Builder(base.DataObject): table = "builders" - def __cmp__(self, other): - if other is None: - return -1 - - return cmp(self.id, other.id) + def __eq__(self, other): + if isinstance(other, self.__class__): + return self.id == other.id - @classmethod - def create(cls, pakfire, name, user=None, log=True): - """ - Creates a new builder. - """ - builder_id = pakfire.db.execute("INSERT INTO builders(name, time_created) \ - VALUES(%s, NOW())", name) - - # Create Builder object. - builder = cls(pakfire, builder_id) - - # Generate a new passphrase. - passphrase = builder.regenerate_passphrase() - - # Log what we have done. - if log: - builder.log("created", user=user) - - # The Builder object and the passphrase are returned. - return builder, passphrase + def __lt__(self, other): + if isinstance(other, self.__class__): + return self.name < other.name def log(self, action, user=None): user_id = None @@ -150,8 +149,8 @@ class Builder(base.DataObject): The new passphrase is returned to be sent to the user (once). """ - # Generate a random string with 20 chars. - passphrase = generate_random_string(length=20) + # Generate a random string with 40 chars. + passphrase = generate_random_string(length=40) # Create salted hash. passphrase_hash = generate_password_hash(passphrase) @@ -175,10 +174,6 @@ class Builder(base.DataObject): description = property(lambda s: s.data.description or "", set_description) - @property - def status(self): - return self.data.status - @property def keepalive(self): """ @@ -207,49 +202,40 @@ class Builder(base.DataObject): pakfire_version, cpu_model, cpu_count, cpu_arch, cpu_bogomips, host_key, os_name, self.id) - def get_enabled(self): - return self.status == "enabled" - - def set_enabled(self, value): - # XXX deprecated - - if value: - value = "enabled" - else: - value = "disabled" - - self.set_status(value) + def set_enabled(self, enabled): + self._set_attribute("enabled", enabled) - enabled = property(get_enabled, set_enabled) + enabled = property(lambda s: s.data.enabled, set_enabled) @property def disabled(self): return not self.enabled - def set_status(self, status, user=None, log=True): - assert status in ("created", "enabled", "disabled", "deleted") - - if self.status == status: - return + @property + def native_arch(self): + """ + The native architecture of this builder + """ + return self.cpu_arch - self._set_attribute("status", status) + @lazy_property + def supported_arches(self): + # Every builder supports noarch + arches = ["noarch"] - if log: - self.log(status, user=user) + # We can always build our native architeture + if self.native_arch: + arches.append(self.native_arch) - @lazy_property - def arches(self): - if self.cpu_arch: + # Get all compatible architectures res = self.db.query("SELECT build_arch FROM arches_compat \ - WHERE host_arch = %s", self.cpu_arch) - - arches += [r.build_arch for r in res] - if not self.cpu_arch in arches: - arches.append(self.cpu_arch) + WHERE native_arch = %s", self.native_arch) - return arches + for row in res: + if not row.build_arch in arches: + arches.append(row.build_arch) - return [] + return sorted(arches) def get_build_release(self): return self.data.build_release == "Y" @@ -462,7 +448,7 @@ class Builder(base.DataObject): """ Returns a list of jobs that can be built on this host. """ - return self.pakfire.jobs.get_next(arches=self.arches, limit=limit) + return self.pakfire.jobs.get_next(arches=self.buildable_arches, limit=limit) def get_next_job(self): """ diff --git a/src/database.sql b/src/database.sql index ae451299..62c8c0b8 100644 --- a/src/database.sql +++ b/src/database.sql @@ -639,8 +639,9 @@ ALTER TABLE arches OWNER TO pakfire; -- CREATE TABLE arches_compat ( - host_arch text NOT NULL, - build_arch text NOT NULL + native_arch text NOT NULL, + build_arch text NOT NULL, + CONSTRAINT arches_compat_unique CHECK ((native_arch <> build_arch)) ); @@ -655,8 +656,8 @@ CREATE TABLE builders ( name text NOT NULL, passphrase text, description text, - status builders_status DEFAULT 'disabled'::builders_status NOT NULL, - disabled builders_disabled DEFAULT 'Y'::builders_disabled NOT NULL, + enabled boolean DEFAULT false NOT NULL, + deleted boolean DEFAULT false NOT NULL, loadavg text DEFAULT '0'::character varying NOT NULL, arches text, build_release builders_build_release DEFAULT 'N'::builders_build_release NOT NULL, @@ -673,8 +674,7 @@ CREATE TABLE builders ( overload builders_overload DEFAULT 'N'::builders_overload NOT NULL, free_space bigint DEFAULT 0 NOT NULL, host_key_id text, - deleted builders_deleted DEFAULT 'N'::builders_deleted NOT NULL, - time_created timestamp without time zone NOT NULL, + time_created timestamp without time zone DEFAULT now() NOT NULL, time_updated timestamp without time zone, time_keepalive timestamp without time zone, loadavg1 double precision, @@ -698,7 +698,7 @@ CREATE VIEW arches_builders AS SELECT arches_compat.build_arch AS arch, builders.id AS builder_id FROM (arches_compat - LEFT JOIN builders ON ((arches_compat.host_arch = builders.cpu_arch))); + LEFT JOIN builders ON ((arches_compat.native_arch = builders.cpu_arch))); ALTER TABLE arches_builders OWNER TO pakfire; @@ -842,7 +842,7 @@ CREATE VIEW builders_ready AS builders.build_scratch, builders.build_test FROM builders - WHERE (((builders.status = 'enabled'::builders_status) AND (builders.time_keepalive >= (now() - '00:05:00'::interval))) AND (builders.max_jobs > ( SELECT count(*) AS count + WHERE ((((builders.deleted IS FALSE) AND (builders.enabled IS TRUE)) AND (builders.time_keepalive >= (now() - '00:05:00'::interval))) AND (builders.max_jobs > ( SELECT count(*) AS count FROM jobs_active WHERE (jobs_active.builder_id = builders.id)))) ORDER BY ( SELECT count(*) AS count @@ -2509,6 +2509,14 @@ ALTER TABLE ONLY users_emails ALTER COLUMN id SET DEFAULT nextval('users_emails_ ALTER TABLE ONLY users_permissions ALTER COLUMN id SET DEFAULT nextval('users_permissions_id_seq'::regclass); +-- +-- Name: arches_compat_unique; Type: CONSTRAINT; Schema: public; Owner: pakfire; Tablespace: +-- + +ALTER TABLE ONLY arches_compat + ADD CONSTRAINT arches_compat_unique UNIQUE (native_arch, build_arch); + + -- -- Name: arches_name; Type: CONSTRAINT; Schema: public; Owner: pakfire; Tablespace: -- @@ -2814,24 +2822,31 @@ ALTER TABLE ONLY sessions -- --- Name: builds_watchers_build_id; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: +-- Name: arches_compat_native_arch; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: -- -CREATE INDEX builds_watchers_build_id ON builds_watchers USING btree (build_id); +CREATE INDEX arches_compat_native_arch ON arches_compat USING btree (native_arch); -- --- Name: filelists_name; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: +-- Name: builders_name; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: -- -CREATE INDEX filelists_name ON filelists USING btree (name); +CREATE UNIQUE INDEX builders_name ON builders USING btree (name) WHERE (deleted IS FALSE); + + +-- +-- Name: builds_watchers_build_id; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: +-- + +CREATE INDEX builds_watchers_build_id ON builds_watchers USING btree (build_id); -- --- Name: idx_2197949_host_arch; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: +-- Name: filelists_name; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: -- -CREATE INDEX idx_2197949_host_arch ON arches_compat USING btree (host_arch); +CREATE INDEX filelists_name ON filelists USING btree (name); -- @@ -3109,6 +3124,14 @@ ALTER TABLE mirrors_checks CLUSTER ON mirrors_checks_sort; CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON sources FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_sources(); +-- +-- Name: arches_compat_build_arch; Type: FK CONSTRAINT; Schema: public; Owner: pakfire +-- + +ALTER TABLE ONLY arches_compat + ADD CONSTRAINT arches_compat_build_arch FOREIGN KEY (build_arch) REFERENCES arches(name); + + -- -- Name: builders_history_builder_id; Type: FK CONSTRAINT; Schema: public; Owner: pakfire -- diff --git a/src/templates/builders/detail.html b/src/templates/builders/detail.html index 8f2abcce..a140f29e 100644 --- a/src/templates/builders/detail.html +++ b/src/templates/builders/detail.html @@ -90,14 +90,10 @@ {{ _("State") }} - {% if builder.status == "enabled" %} + {% if builder.enabled %} {{ _("Enabled") }} - {% elif builder.status == "disabled" %} - {{ _("Disabled") }} - {% elif builder.status == "deleted" %} - {{ _("Deleted") }} {% else %} - {{ _("Unknown status: %s") % builder.status }} + {{ _("Disabled") }} {% end %} @@ -152,7 +148,7 @@ {{ _("Supported architectures") }} - {{ locale.list(builder.arches) }} + {{ locale.list(builder.supported_arches) }} diff --git a/src/web/handlers_builders.py b/src/web/handlers_builders.py index a94d3d3a..bcaa29d2 100644 --- a/src/web/handlers_builders.py +++ b/src/web/handlers_builders.py @@ -8,9 +8,7 @@ from .handlers_base import * class BuilderListHandler(BaseHandler): def get(self): - builders = self.pakfire.builders.get_all() - - self.render("builders/list.html", builders=builders) + self.render("builders/list.html", builders=self.backend.builders) class BuilderDetailHandler(BaseHandler): @@ -52,7 +50,7 @@ class BuilderNewHandler(BaseHandler): # Create a new builder. builder, passphrase = \ - builders.Builder.create(self.pakfire, name, user=self.current_user) + self.backend.builders.create(name, user=self.current_user) self.render("builders/pass.html", action="new", builder=builder, passphrase=passphrase) @@ -120,7 +118,8 @@ class BuilderDeleteHandler(BaseHandler): confirmed = self.get_argument("confirmed", None) if confirmed: - builder.set_status("deleted", user=self.current_user) + with self.db.transaction(): + builder.deleted = True self.redirect("/builders") return @@ -129,7 +128,7 @@ class BuilderDeleteHandler(BaseHandler): class BuilderStatusChangeHandler(BaseHandler): - new_status = None + enabled = None @tornado.web.authenticated def get(self, hostname): @@ -139,14 +138,15 @@ class BuilderStatusChangeHandler(BaseHandler): # Check for sufficient right to edit things. if self.current_user.has_perm("maintain_builders"): - builder.set_status(self.status, user=self.current_user) + with self.db.transaction(): + builder.enabled = self.enabled self.redirect("/builder/%s" % builder.name) class BuilderEnableHander(BuilderStatusChangeHandler): - status = "enabled" + enabled = True class BuilderDisableHander(BuilderStatusChangeHandler): - status = "disabled" + enabled = False