From 9321ffebec9e354db37d50b2f023d0a653bcd7fa Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Fri, 7 Apr 2023 20:35:31 -0400 Subject: [PATCH 1/8] avoid using getLastHostUpdate --- cli/koji_cli/commands.py | 56 ++++++++++++++++++++++++++-------------- www/kojiweb/index.py | 14 +++++++--- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index e4806a18..ff65f6b3 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3155,24 +3155,14 @@ def anon_handle_list_hosts(goptions, session, args): else: return 'N' - try: - first = session.getLastHostUpdate(hosts[0]['id'], ts=True) - opts = {'ts': True} - except koji.ParameterError: - # Hubs prior to v1.25.0 do not have a "ts" parameter for getLastHostUpdate - first = session.getLastHostUpdate(hosts[0]['id']) - opts = {} + if 'update_ts' not in hosts[0]: + _get_host_update_oldhub(session, hosts) - # pull in the last update using multicall to speed it up a bit - with session.multicall() as m: - result = [m.getLastHostUpdate(host['id'], **opts) for host in hosts[1:]] - updateList = [first] + [x.result for x in result] - - for host, update in zip(hosts, updateList): - if update is None: + for host in hosts: + if host['update_ts'] is None: host['update'] = '-' else: - host['update'] = koji.formatTimeLong(update) + host['update'] = koji.formatTimeLong(host['update_ts']) host['enabled'] = yesno(host['enabled']) host['ready'] = yesno(host['ready']) host['arches'] = ','.join(host['arches'].split()) @@ -3222,6 +3212,33 @@ def anon_handle_list_hosts(goptions, session, args): print(mask % host) +def _get_host_update_oldhub(session, hosts): + """Fetch host update times from older hubs""" + + # figure out if hub supports ts parameter + try: + first = session.getLastHostUpdate(hosts[0]['id'], ts=True) + opts = {'ts': True} + except koji.ParameterError: + # Hubs prior to v1.25.0 do not have a "ts" parameter for getLastHostUpdate + first = session.getLastHostUpdate(hosts[0]['id']) + opts = {} + + with session.multicall() as m: + result = [m.getLastHostUpdate(host['id'], **opts) for host in hosts[1:]] + + updateList = [first] + [x.result for x in result] + + for host, update in zip(hosts, updateList): + if 'ts' in opts: + host['update_ts'] = update + elif update is None: + host['update_ts'] = None + else: + dt = dateutil.parser.parse(update) + host['update_ts'] = time.mktime(dt.timetuple()) + + def anon_handle_list_pkgs(goptions, session, args): "[info] Print the package listing for tag or for owner" usage = "usage: %prog list-pkgs [options]" @@ -3662,11 +3679,10 @@ def anon_handle_hostinfo(goptions, session, args): print("Comment:") print("Enabled: %s" % (info['enabled'] and 'yes' or 'no')) print("Ready: %s" % (info['ready'] and 'yes' or 'no')) - try: - update = session.getLastHostUpdate(info['id'], ts=True) - except koji.ParameterError: - # Hubs prior to v1.25.0 do not have a "ts" parameter for getLastHostUpdate - update = session.getLastHostUpdate(info['id']) + + if 'update_ts' not in info: + _get_host_update_oldhub(session, [info]) + update = info['update_ts'] if update is None: update = "never" else: diff --git a/www/kojiweb/index.py b/www/kojiweb/index.py index f565041b..731fe878 100644 --- a/www/kojiweb/index.py +++ b/www/kojiweb/index.py @@ -1713,11 +1713,17 @@ def hosts(environ, state='enabled', start=None, order='name', ready='all', chann values['channels'] = server.listChannels() - with server.multicall() as m: - updates = [m.getLastHostUpdate(host['id'], ts=True) for host in hosts] + if hosts and 'update_ts' not in hosts[0]: + # be nice with older hub + # TODO remove this compat workaround after a release + with server.multicall() as m: + updates = [m.getLastHostUpdate(host['id'], ts=True) for host in hosts] - for host, lastUpdate in zip(hosts, updates): - host['last_update'] = lastUpdate.result + for host, lastUpdate in zip(hosts, updates): + host['last_update'] = lastUpdate.result + else: + for host in hosts: + host['last_update'] = koji.formatTimeLong(host['update_ts']) # Paginate after retrieving last update info so we can sort on it kojiweb.util.paginateList(values, hosts, start, 'hosts', 'host', order) From d82edd9757842caa51a8c9e9c26b6d3cd7a5378f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Fri, 7 Apr 2023 19:50:15 -0400 Subject: [PATCH 2/8] track update time in host table --- docs/schema.sql | 1 + kojihub/kojihub.py | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/schema.sql b/docs/schema.sql index aef10465..505f0b8d 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -160,6 +160,7 @@ CREATE TABLE host ( id SERIAL NOT NULL PRIMARY KEY, user_id INTEGER NOT NULL REFERENCES users (id), name VARCHAR(128) UNIQUE NOT NULL, + update_time TIMESTAMPTZ, task_load FLOAT CHECK (NOT task_load < 0) NOT NULL DEFAULT 0.0, ready BOOLEAN NOT NULL DEFAULT 'false' ) WITHOUT OIDS; diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 5aba0b8d..a883f813 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -5501,6 +5501,7 @@ def get_host(hostInfo, strict=False, event=None): ('host.id', 'id'), ('host.user_id', 'user_id'), ('host.name', 'name'), + ("date_part('epoch', host.update_time)", 'update_ts'), ('host.ready', 'ready'), ('host.task_load', 'task_load'), ('host_config.arches', 'arches'), @@ -13288,6 +13289,7 @@ class RootExports(object): ('host.id', 'id'), ('host.user_id', 'user_id'), ('host.name', 'name'), + ("date_part('epoch', host.update_time)", 'update_ts'), ('host.ready', 'ready'), ('host.task_load', 'task_load'), ('host_config.arches', 'arches'), @@ -14261,11 +14263,14 @@ class Host(object): def updateHost(self, task_load, ready): host_data = get_host(self.id) task_load = float(task_load) - if task_load != host_data['task_load'] or ready != host_data['ready']: - update = UpdateProcessor('host', clauses=['id=%(id)i'], values={'id': self.id}, - data={'task_load': task_load, 'ready': ready}) - update.execute() - context.commit_pending = True + update = UpdateProcessor( + 'host', + data={'task_load': task_load, 'ready': ready}, + rawdata={'update_time': 'NOW()'}, + clauses=['id=%(id)i'], + values={'id': self.id}, + ) + update.execute() def getLoadData(self): """Get load balancing data From 0b935caf857299f0ea4c3b64bad1fdf58604477a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Wed, 19 Apr 2023 21:32:22 -0400 Subject: [PATCH 3/8] fix import --- cli/koji_cli/commands.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index ff65f6b3..fd2517c5 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, division import ast +import dateutil.parser import fnmatch import itertools import json From 48b8deef6ec897a30328b49d7e4ace5b7e2ee228 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Wed, 19 Apr 2023 22:53:25 -0400 Subject: [PATCH 4/8] fix unit tests --- tests/test_cli/test_hostinfo.py | 5 ++++- tests/test_cli/test_list_hosts.py | 4 +++- tests/test_hub/test_get_host.py | 10 ++++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/test_cli/test_hostinfo.py b/tests/test_cli/test_hostinfo.py index 7f324a3a..72cd5600 100644 --- a/tests/test_cli/test_hostinfo.py +++ b/tests/test_cli/test_hostinfo.py @@ -34,6 +34,7 @@ class TestHostinfo(utils.CliTestCase): 'comment': 'test-comment', 'description': 'test-description'} self.last_update = 1615875554.862938 + self.last_update_str = '2021-03-16 06:19:14.862938-00:00' self.list_channels = [{'id': 1, 'name': 'default'}, {'id': 2, 'name': 'createrepo'}] self.error_format = """Usage: %s hostinfo [options] [ ...] (Specify the --help global option for a list of other help options) @@ -142,6 +143,7 @@ None @mock.patch('sys.stdout', new_callable=StringIO) def test_hostinfo_valid_param_error(self, stdout): + '''Test the fallback code for getting timestamps from old hubs''' expected = """Name: kojibuilder ID: 1 Arches: x86_64 @@ -157,7 +159,8 @@ Active Buildroots: None """ self.session.getHost.return_value = self.hostinfo - self.session.getLastHostUpdate.side_effect = [koji.ParameterError, self.last_update] + # simulate an older hub that doesn't support the ts option for getLastHostUpdate + self.session.getLastHostUpdate.side_effect = [koji.ParameterError, self.last_update_str] self.session.listChannels.return_value = self.list_channels rv = anon_handle_hostinfo(self.options, self.session, [self.hostinfo['name']]) self.assertEqual(rv, None) diff --git a/tests/test_cli/test_list_hosts.py b/tests/test_cli/test_list_hosts.py index a6c0aefa..e3d486c5 100644 --- a/tests/test_cli/test_list_hosts.py +++ b/tests/test_cli/test_list_hosts.py @@ -179,10 +179,12 @@ kojibuilder N Y 0.0/2.0 x86_64 Tue, 16 Mar 2021 06:19:14 UTC @mock.patch('sys.stdout', new_callable=StringIO) def test_list_hosts_param_error_get_last_host_update(self, stdout): - host_update = 1615875554.862938 + # host_update = 1615875554.862938 + host_update = '2021-03-16 06:19:14.862938-00:00' expected = "kojibuilder N Y 0.0/2.0 x86_64 " \ "Tue, 16 Mar 2021 06:19:14 UTC \n" + # simulate an older hub that doesn't support the ts option for getLastHostUpdate self.session.getLastHostUpdate.side_effect = [koji.ParameterError, host_update] self.session.multiCall.return_value = [[host_update]] self.session.listHosts.return_value = self.list_hosts diff --git a/tests/test_hub/test_get_host.py b/tests/test_hub/test_get_host.py index 29c89662..66bfaac7 100644 --- a/tests/test_hub/test_get_host.py +++ b/tests/test_hub/test_get_host.py @@ -33,12 +33,13 @@ class TestSetHostEnabled(unittest.TestCase): self.assertEqual(len(self.queries), 1) query = self.queries[0] - columns = ['host.id', 'host.user_id', 'host.name', 'host.ready', + columns = ['host.id', 'host.user_id', 'host.name', + "date_part('epoch', host.update_time)", 'host.ready', 'host.task_load', 'host_config.arches', 'host_config.capacity', 'host_config.description', 'host_config.comment', 'host_config.enabled'] joins = ['host ON host.id = host_config.host_id'] - aliases = ['id', 'user_id', 'name', 'ready', 'task_load', + aliases = ['id', 'user_id', 'name', 'update_ts', 'ready', 'task_load', 'arches', 'capacity', 'description', 'comment', 'enabled'] clauses = ['(host_config.active = TRUE)', '(host.name = %(host_name)s)'] values = {'host_name': 'hostname'} @@ -54,12 +55,13 @@ class TestSetHostEnabled(unittest.TestCase): self.assertEqual(len(self.queries), 1) query = self.queries[0] - columns = ['host.id', 'host.user_id', 'host.name', 'host.ready', + columns = ['host.id', 'host.user_id', 'host.name', + "date_part('epoch', host.update_time)", 'host.ready', 'host.task_load', 'host_config.arches', 'host_config.capacity', 'host_config.description', 'host_config.comment', 'host_config.enabled'] joins = ['host ON host.id = host_config.host_id'] - aliases = ['id', 'user_id', 'name', 'ready', 'task_load', + aliases = ['id', 'user_id', 'name', 'update_ts', 'ready', 'task_load', 'arches', 'capacity', 'description', 'comment', 'enabled'] clauses = ['(host_config.create_event <= 345 AND ( host_config.revoke_event IS NULL ' 'OR 345 < host_config.revoke_event ))', From 506e4d14298a5eb1fd67d901f4f1e3dd2e2b9053 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 20 Apr 2023 16:24:35 -0400 Subject: [PATCH 5/8] avoid ambiguous columns refs --- kojihub/kojihub.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index a883f813..b3cb0c36 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -2551,7 +2551,7 @@ def get_ready_hosts(): 'expired IS FALSE', 'master IS NULL', 'active IS TRUE', - "update_time > NOW() - '5 minutes'::interval" + "sessions.update_time > NOW() - '5 minutes'::interval" ], joins=[ 'sessions USING (user_id)', @@ -5486,6 +5486,7 @@ def get_host(hostInfo, strict=False, event=None): - id - user_id - name + - update_ts - arches - task_load - capacity @@ -13309,9 +13310,14 @@ class RootExports(object): The timestamp represents the last time the host with the given ID contacted the hub. Returns None if the host has never contacted - the hub.""" + the hub. + + The timestamp returned here may be different than the newer + update_ts field now returned by the getHost and listHosts calls. + """ opts = {'order': '-update_time', 'limit': 1} - query = QueryProcessor(tables=['sessions'], columns=['update_time'], + query = QueryProcessor(tables=['sessions'], columns=['sessions.update_time'], + aliases=['update_time'], joins=['host ON sessions.user_id = host.user_id'], clauses=['host.id = %(hostID)i'], values={'hostID': hostID}, opts=opts) From 8dce3fc9b1e0d5b4f71132168c730d6284898a06 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Tue, 16 May 2023 13:10:26 +0200 Subject: [PATCH 6/8] upgrade schema --- docs/schema-upgrade-1.32-1.33.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/schema-upgrade-1.32-1.33.sql b/docs/schema-upgrade-1.32-1.33.sql index e4705ce1..c1b42c85 100644 --- a/docs/schema-upgrade-1.32-1.33.sql +++ b/docs/schema-upgrade-1.32-1.33.sql @@ -7,4 +7,5 @@ BEGIN; INSERT INTO archivetypes (name, description, extensions) VALUES ('changes', 'Kiwi changes file', 'changes.xz changes') ON CONFLICT DO NOTHING; INSERT INTO archivetypes (name, description, extensions) VALUES ('packages', 'Kiwi packages listing', 'packages') ON CONFLICT DO NOTHING; INSERT INTO archivetypes (name, description, extensions) VALUES ('verified', 'Kiwi verified package list', 'verified') ON CONFLICT DO NOTHING; + ALTER TABLE host ADD COLUMN update_time TIMESTAMPTZ; COMMIT; From 7f7df0165c8181fa77365f3560afa270750de3cb Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Tue, 16 May 2023 13:10:30 +0200 Subject: [PATCH 7/8] fix tests Related: https://pagure.io/koji/issue/3789 --- tests/test_hub/test_get_last_host_update.py | 2 +- tests/test_hub/test_get_ready_hosts.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_hub/test_get_last_host_update.py b/tests/test_hub/test_get_last_host_update.py index b304f5da..4ace4c4f 100644 --- a/tests/test_hub/test_get_last_host_update.py +++ b/tests/test_hub/test_get_last_host_update.py @@ -28,7 +28,7 @@ class TestGetLastHostUpdate(DBQueryTestCase): self.assertEqual(query.joins, ['host ON sessions.user_id = host.user_id']) self.assertEqual(query.clauses, ['host.id = %(hostID)i']) self.assertEqual(query.values, {'hostID': 1}) - self.assertEqual(query.columns, ['update_time']) + self.assertEqual(query.columns, ['sessions.update_time']) def test_valid_datetime(self): if sys.version_info[1] <= 6: diff --git a/tests/test_hub/test_get_ready_hosts.py b/tests/test_hub/test_get_ready_hosts.py index 8c1447ae..7ae8843f 100644 --- a/tests/test_hub/test_get_ready_hosts.py +++ b/tests/test_hub/test_get_ready_hosts.py @@ -45,7 +45,7 @@ class TestGetReadyHosts(unittest.TestCase): 'host_config ON host.id = host_config.host_id']) self.assertEqual(query.clauses, ['active IS TRUE', 'enabled IS TRUE', 'expired IS FALSE', 'master IS NULL', 'ready IS TRUE', - "update_time > NOW() - '5 minutes'::interval"]) + "sessions.update_time > NOW() - '5 minutes'::interval"]) self.assertEqual(query.values, {}) self.assertEqual(query.columns, ['arches', 'capacity', 'host.id', 'name', 'task_load']) From d3c561ebcd4ffd3f5024ed2cdc38bb3ec3428348 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Wed, 17 May 2023 10:22:09 +0200 Subject: [PATCH 8/8] fix flake8 --- kojihub/kojihub.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index b3cb0c36..3ca593d2 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -14267,14 +14267,13 @@ class Host(object): return tasks def updateHost(self, task_load, ready): - host_data = get_host(self.id) task_load = float(task_load) update = UpdateProcessor( - 'host', - data={'task_load': task_load, 'ready': ready}, - rawdata={'update_time': 'NOW()'}, - clauses=['id=%(id)i'], - values={'id': self.id}, + 'host', + data={'task_load': task_load, 'ready': ready}, + rawdata={'update_time': 'NOW()'}, + clauses=['id=%(id)i'], + values={'id': self.id}, ) update.execute()