From b50479abb002872689237ac00517d1befcec91a1 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Wed, 21 Feb 2018 13:31:33 +0100 Subject: [PATCH] host_channels history --- cli/koji_cli/commands.py | 6 ++++ docs/schema-upgrade-1.15-1.16.sql | 46 ++++++++++++++++-------- docs/schema.sql | 12 ++++++- hub/kojihub.py | 60 ++++++++++++++++++++++--------- tests/test_hub/test_add_host.py | 10 ++---- tests/test_hub/test_get_host.py | 4 +-- tests/test_hub/test_list_hosts.py | 22 +++++++----- 7 files changed, 109 insertions(+), 51 deletions(-) diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 32c040d4..4c0c18c8 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -4034,6 +4034,11 @@ def _print_histline(entry, **kwargs): fmt = "new host: %(host.name)s" else: fmt = "host deleted: %(host.name)s" + elif table == 'host_channels': + if create: + fmt = "host %(host.name)s added to channel %(channels.name)s" + else: + fmt = "host %(host.name)s removed from channel %(channels.name)s" elif table == 'build_target_config': if edit: fmt = "build target configuration for %(build_target.name)s updated" @@ -4149,6 +4154,7 @@ _table_keys = { 'build_target_config' : ['build_target_id'], 'external_repo_config' : ['external_repo_id'], 'host_config': ['host_id'], + 'host_channels': ['host_id'], 'tag_external_repos' : ['tag_id', 'external_repo_id'], 'tag_listing' : ['build_id', 'tag_id'], 'tag_packages' : ['package_id', 'tag_id'], diff --git a/docs/schema-upgrade-1.15-1.16.sql b/docs/schema-upgrade-1.15-1.16.sql index 66574a15..77b5b5f1 100644 --- a/docs/schema-upgrade-1.15-1.16.sql +++ b/docs/schema-upgrade-1.15-1.16.sql @@ -8,22 +8,22 @@ BEGIN; SELECT 'Creating table host_config'; CREATE TABLE host_config ( host_id INTEGER NOT NULL REFERENCES host(id), - arches TEXT, - capacity FLOAT CHECK (capacity > 1) NOT NULL DEFAULT 2.0, - description TEXT, - comment TEXT, - enabled BOOLEAN NOT NULL DEFAULT 'true', + arches TEXT, + capacity FLOAT CHECK (capacity > 1) NOT NULL DEFAULT 2.0, + description TEXT, + comment TEXT, + enabled BOOLEAN NOT NULL DEFAULT 'true', -- versioned - see desc above - create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(), - revoke_event INTEGER REFERENCES events(id), - creator_id INTEGER NOT NULL REFERENCES users(id), - revoker_id INTEGER REFERENCES users(id), - active BOOLEAN DEFAULT 'true' CHECK (active), - CONSTRAINT active_revoke_sane CHECK ( - (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL) - OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)), - PRIMARY KEY (create_event, host_id), - UNIQUE (host_id, active) + create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(), + revoke_event INTEGER REFERENCES events(id), + creator_id INTEGER NOT NULL REFERENCES users(id), + revoker_id INTEGER REFERENCES users(id), + active BOOLEAN DEFAULT 'true' CHECK (active), + CONSTRAINT active_revoke_sane CHECK ( + (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL) + OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)), + PRIMARY KEY (create_event, host_id), + UNIQUE (host_id, active) ) WITHOUT OIDS; CREATE INDEX host_config_by_active_and_enabled ON host_config(active, enabled) @@ -45,4 +45,20 @@ ALTER TABLE host DROP COLUMN description; ALTER TABLE host DROP COLUMN comment; ALTER TABLE host DROP COLUMN enabled; +-- history for host_channels +SELECT 'Adding versions to host_channels' +ALTER TABLE host_channels ADD COLUMN create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(); +ALTER TABLE host_channels ADD COLUMN revoke_event INTEGER REFERENCES events(id); +-- we need some default for alter table, but drop it after +ALTER TABLE host_channels ADD COLUMN creator_id INTEGER NOT NULL REFERENCES users(id) DEFAULT pg_temp.user(); +ALTER TABLE host_channels ALTER COLUMN creator_id DROP DEFAULT; +ALTER TABLE host_channels ADD COLUMN revoker_id INTEGER REFERENCES users(id); +ALTER TABLE host_channels ADD COLUMN active BOOLEAN DEFAULT 'true' CHECK (active); +ALTER TABLE host_channels ADD CONSTRAINT active_revoke_sane CHECK ( + (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL) + OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)); +ALTER TABLE host_channels ADD PRIMARY KEY (create_event, host_id, channel_id); +ALTER TABLE host_channels ADD UNIQUE (host_id, channel_id, active); +ALTER TABLE host_channels DROP CONSTRAINT host_channels_host_id_channel_id_key; + COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index 83dfc0a0..423cf506 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -173,7 +173,17 @@ CREATE INDEX host_config_by_active_and_enabled ON host_config(active, enabled) CREATE TABLE host_channels ( host_id INTEGER NOT NULL REFERENCES host(id), channel_id INTEGER NOT NULL REFERENCES channels(id), - UNIQUE (host_id,channel_id) +-- versioned - see desc above + create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(), + revoke_event INTEGER REFERENCES events(id), + creator_id INTEGER NOT NULL REFERENCES users(id), + revoker_id INTEGER REFERENCES users(id), + active BOOLEAN DEFAULT 'true' CHECK (active), + CONSTRAINT active_revoke_sane CHECK ( + (active IS NULL AND revoke_event IS NOT NULL AND revoker_id IS NOT NULL) + OR (active IS NOT NULL AND revoke_event IS NULL AND revoker_id IS NULL)), + PRIMARY KEY (create_event, host_id, channel_id), + UNIQUE (host_id, channel_id, active) ) WITHOUT OIDS; diff --git a/hub/kojihub.py b/hub/kojihub.py index c6d8cfc3..731eea4b 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2062,6 +2062,7 @@ def add_host_to_channel(hostname, channel_name, create=False): raise koji.GenericError('host %s is already subscribed to the %s channel' % (hostname, channel_name)) insert = InsertProcessor('host_channels') insert.set(host_id=host_id, channel_id=channel_id) + insert.make_create() insert.execute() def remove_host_from_channel(hostname, channel_name): @@ -2081,9 +2082,13 @@ def remove_host_from_channel(hostname, channel_name): break if not found: raise koji.GenericError('host %s is not subscribed to the %s channel' % (hostname, channel_name)) - c = context.cnx.cursor() - c.execute("""DELETE FROM host_channels WHERE host_id = %(host_id)d and channel_id = %(channel_id)d""", locals()) - context.commit_pending = True + + values = {'host_id': host_id, 'channel_id': channel_id} + clauses = ['host_id = %(host_id)i AND channel_id = %(channel_id)i'] + update = UpdateProcessor('host_channels', values=values, clauses=clauses) + update.make_revoke() + update.execute() + def rename_channel(old, new): """Rename a channel""" @@ -2103,6 +2108,10 @@ def remove_channel(channel_name, force=False): Channel must have no hosts, unless force is set to True If a channel has associated tasks, it cannot be removed + and an exception will be raised. + + Removing channel will remove also remove complete history + for that channel. """ context.session.assertPerm('admin') channel_id = get_channel_id(channel_name, strict=True) @@ -2146,7 +2155,7 @@ def get_ready_hosts(): c.execute(q) hosts = [dict(zip(aliases, row)) for row in c.fetchall()] for host in hosts: - q = """SELECT channel_id FROM host_channels WHERE host_id=%(id)s""" + q = """SELECT channel_id FROM host_channels WHERE host_id=%(id)s AND active IS TRUE""" c.execute(q, host) host['channels'] = [row[0] for row in c.fetchall()] return hosts @@ -4706,16 +4715,28 @@ def get_buildroot(buildrootID, strict=False): raise koji.GenericError("More that one buildroot with id: %i" % buildrootID) return result[0] -def list_channels(hostID=None): +def list_channels(hostID=None, event=None): """List channels. If hostID is specified, only list channels associated with the host with that ID.""" - fields = ('id', 'name') - query = """SELECT %s FROM channels - """ % ', '.join(fields) - if hostID != None: - query += """JOIN host_channels ON channels.id = host_channels.channel_id - WHERE host_channels.host_id = %(hostID)i""" - return _multiRow(query, locals(), fields) + fields = {'channels.id': 'id', 'channels.name': 'name'} + columns, aliases = zip(*fields.items()) + if hostID: + tables = ['host_channels'] + joins = ['channels ON channels.id = host_channels.channel_id'] + clauses = [ + eventCondition(event, table='host_channels'), + 'host_channels.host_id = %(host_id)s'] + values = {'host_id': hostID} + query = QueryProcessor(tables=tables, aliases=aliases, + columns=columns, joins=joins, + clauses=clauses, values=values) + elif event: + raise koji.GenericError('list_channels with event and ' + 'not host is not allowed.') + else: + query = QueryProcessor(tables=['channels'], aliases=aliases, + columns=columns) + return query.execute() def new_package(name, strict=True): c = context.cnx.cursor() @@ -6590,6 +6611,7 @@ def query_history(tables=None, **kwargs): 'build_target_config': ['build_target_id', 'build_tag', 'dest_tag'], 'external_repo_config': ['external_repo_id', 'url'], 'host_config': ['host_id', 'arches', 'capacity', 'description', 'comment', 'enabled'], + 'host_channels': ['host_id', 'channel_id'], 'tag_external_repos': ['tag_id', 'external_repo_id', 'priority'], 'tag_listing': ['build_id', 'tag_id'], 'tag_packages': ['package_id', 'tag_id', 'owner', 'blocked', 'extra_arches'], @@ -6607,6 +6629,7 @@ def query_history(tables=None, **kwargs): #group_id is overloaded (special case below) 'tag_id': ['tag'], 'host_id': ['host'], + 'channel_id': ['channels'], 'parent_id': ['tag', 'parent'], 'build_target_id': ['build_target'], 'build_tag': ['tag', 'build_tag'], @@ -10612,7 +10635,7 @@ class RootExports(object): krb_principal=krb_principal) #host entry hostID = _singleValue("SELECT nextval('host_id_seq')", strict=True) - insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s" + insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s)" _dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname'))) insert = InsertProcessor('host_config') @@ -10621,9 +10644,11 @@ class RootExports(object): insert.execute() #host_channels entry - insert = """INSERT INTO host_channels (host_id, channel_id) - VALUES (%(hostID)i, %(default_channel)i)""" - _dml(insert, dslice(locals(), ('hostID', 'default_channel'))) + insert = InsertProcessor('host_channels') + insert.set(host_id=hostID, channel_id=default_channel) + insert.make_create() + insert.execute() + return hostID def enableHost(self, hostname): @@ -10662,6 +10687,7 @@ class RootExports(object): channelID = get_channel_id(channelID, strict=True) joins.append('host_channels ON host.id = host_channels.host_id') clauses.append('host_channels.channel_id = %(channelID)i') + clauses.append('host_channels.active IS TRUE') if ready is not None: if ready: clauses.append('ready IS TRUE') @@ -11552,7 +11578,7 @@ class Host(object): c.execute(q, locals()) arches = c.fetchone()[0].split() q = """ - SELECT channel_id FROM host_channels WHERE host_id = %(id)s + SELECT channel_id FROM host_channels WHERE host_id = %(id)s AND active is TRUE """ c.execute(q, locals()) channels = [x[0] for x in c.fetchall()] diff --git a/tests/test_hub/test_add_host.py b/tests/test_hub/test_add_host.py index 455a53f1..ce0cf437 100644 --- a/tests/test_hub/test_add_host.py +++ b/tests/test_hub/test_add_host.py @@ -70,10 +70,6 @@ class TestAddHost(unittest.TestCase): mock.call("SELECT id FROM channels WHERE name = 'default'"), mock.call("SELECT nextval('host_id_seq')", strict=True) ]) - self.assertEqual(_dml.call_count, 2) - _dml.assert_has_calls([ - mock.call("INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s", - {'hostID': 12, 'userID': 456, 'hostname': 'hostname'}), - mock.call("""INSERT INTO host_channels (host_id, channel_id)\n VALUES (%(hostID)i, %(default_channel)i)""", - {'hostID': 12, 'default_channel': 333}) - ]) + self.assertEqual(_dml.call_count, 1) + _dml.assert_called_once_with("INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s)", + {'hostID': 12, 'userID': 456, 'hostname': 'hostname'}) diff --git a/tests/test_hub/test_get_host.py b/tests/test_hub/test_get_host.py index b3e0b8ab..70d74878 100644 --- a/tests/test_hub/test_get_host.py +++ b/tests/test_hub/test_get_host.py @@ -39,7 +39,7 @@ class TestSetHostEnabled(unittest.TestCase): joins = ['host ON host.id = host_config.host_id'] aliases = ['id', 'user_id', 'name', 'ready', 'task_load', 'arches', 'capacity', 'description', 'comment', 'enabled'] - clauses = ['(host_config.active = TRUE)', 'name = %(hostInfo)s'] + clauses = ['(host_config.active = TRUE)', 'host.name = %(hostInfo)s'] values = {'hostInfo': 'hostname'} self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, joins) @@ -61,7 +61,7 @@ class TestSetHostEnabled(unittest.TestCase): aliases = ['id', 'user_id', 'name', '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 ))', - 'id = %(hostInfo)i'] + 'host.id = %(hostInfo)i'] values = {'hostInfo': 123} self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, joins) diff --git a/tests/test_hub/test_list_hosts.py b/tests/test_hub/test_list_hosts.py index ba80fcd6..8b092950 100644 --- a/tests/test_hub/test_list_hosts.py +++ b/tests/test_hub/test_list_hosts.py @@ -31,7 +31,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE',]) + self.assertEqual(query.clauses, ['host_config.active IS TRUE',]) @mock.patch('kojihub.get_user') def test_list_hosts_user_id(self, get_user): @@ -42,7 +42,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE', 'user_id = %(userID)i']) + self.assertEqual(query.clauses, ['host_config.active IS TRUE', 'user_id = %(userID)i']) @mock.patch('kojihub.get_channel_id') def test_list_hosts_channel_id(self, get_channel_id): @@ -54,7 +54,11 @@ class TestListHosts(unittest.TestCase): self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id', 'host_channels ON host.id = host_channels.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE','host_channels.channel_id = %(channelID)i']) + self.assertEqual(query.clauses, [ + 'host_config.active IS TRUE', + 'host_channels.channel_id = %(channelID)i', + 'host_channels.active IS TRUE', + ]) def test_list_hosts_single_arch(self): self.exports.listHosts(arches='x86_64') @@ -63,7 +67,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE',r"""(arches ~ E'\\mx86_64\\M')"""]) + self.assertEqual(query.clauses, ['host_config.active IS TRUE',r"""(arches ~ E'\\mx86_64\\M')"""]) def test_list_hosts_multi_arch(self): self.exports.listHosts(arches=['x86_64', 's390']) @@ -72,7 +76,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE',r"""(arches ~ E'\\mx86_64\\M' OR arches ~ E'\\ms390\\M')"""]) + self.assertEqual(query.clauses, ['host_config.active IS TRUE',r"""(arches ~ E'\\mx86_64\\M' OR arches ~ E'\\ms390\\M')"""]) def test_list_hosts_bad_arch(self): with self.assertRaises(koji.GenericError): @@ -85,7 +89,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE','ready IS TRUE']) + self.assertEqual(query.clauses, ['host_config.active IS TRUE','ready IS TRUE']) def test_list_hosts_nonready(self): self.exports.listHosts(ready=0) @@ -94,7 +98,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE','ready IS FALSE']) + self.assertEqual(query.clauses, ['host_config.active IS TRUE','ready IS FALSE']) def test_list_hosts_enabled(self): self.exports.listHosts(enabled=1) @@ -103,7 +107,7 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE','enabled IS TRUE']) + self.assertEqual(query.clauses, ['host_config.active IS TRUE','enabled IS TRUE']) def test_list_hosts_disabled(self): self.exports.listHosts(enabled=0) @@ -112,4 +116,4 @@ class TestListHosts(unittest.TestCase): query = self.queries[0] self.assertEqual(query.tables, ['host_config']) self.assertEqual(query.joins, ['host ON host.id = host_config.host_id']) - self.assertEqual(query.clauses, ['active IS TRUE','enabled IS FALSE']) + self.assertEqual(query.clauses, ['host_config.active IS TRUE','enabled IS FALSE'])