From 9ee72d884ad014f8417c422f1b4ce0f2953ec7ce Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:42 +0000 Subject: [PATCH 1/9] add history to edit_host Hosts now have history. host table was split to host (containing ephemereal and non-editable data (load, activity, name, user_id)) and host_config containing data changeable by admins (archs, capacity, ...). This table is versioned and searchable via queryHistory. Fixes: https://pagure.io/koji/issue/638 --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 808f26c..fd2707c 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -2772,9 +2772,9 @@ def anon_handle_list_hosts(goptions, session, args): host['arches'] = ','.join(host['arches'].split()) if not options.quiet: - print("Hostname Enb Rdy Load/Cap Arches Last Update") + print("Hostname Enb Rdy Load/Cap Arches Last Update") for host in hosts: - print("%(name)-28s %(enabled)-3s %(ready)-3s %(task_load)4.1f/%(capacity)-3.1f %(arches)-16s %(update)s" % host) + print("%(name)-28s %(enabled)-3s %(ready)-3s %(task_load)4.1f/%(capacity)-4.1f %(arches)-16s %(update)s" % host) def anon_handle_list_pkgs(goptions, session, args): @@ -4027,6 +4027,13 @@ def _print_histline(entry, **kwargs): fmt = "added tag option %(key)s for tag %(tag.name)s" else: fmt = "tag option %(key)s removed for %(tag.name)s" + elif table == 'host_config': + if edit: + fmt = "host configuration for %(host.name)s altered" + elif create: + fmt = "new host: %(host.name)s" + else: + fmt = "host deleted: %(host.name)s" elif table == 'build_target_config': if edit: fmt = "build target configuration for %(build_target.name)s updated" @@ -4141,6 +4148,7 @@ _table_keys = { 'tag_extra' : ['tag_id', 'key'], 'build_target_config' : ['build_target_id'], 'external_repo_config' : ['external_repo_id'], + 'host_config': ['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 new file mode 100644 index 0000000..e4fa6e2 --- /dev/null +++ b/docs/schema-upgrade-1.15-1.16.sql @@ -0,0 +1,47 @@ +-- upgrade script to migrate the Koji database schema +-- from version 1.13 to 1.14 + + +BEGIN; + +-- create host_config table +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', +-- 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) +) WITHOUT OIDS; + +-- copy starting data +-- CREATE FUNCTION pg_temp.user() returns INTEGER as $$ select id from users where name='nobody' $$ language SQL; +CREATE FUNCTION pg_temp.user() returns INTEGER as $$ select 1 $$ language SQL; +-- If you would like to use an existing user instead, then: +-- 1. edit the temporary function to look for the alternate user name + +SELECT 'Copying data from host to host_config'; +INSERT INTO host_config (host_id, arches, capacity, description, comment, enabled, creator_id) + SELECT id, arches, capacity, description, comment, enabled, pg_temp.user() FROM host; + +-- alter original table +SELECT 'Dropping moved columns'; +ALTER TABLE host DROP COLUMN arches; +ALTER TABLE host DROP COLUMN capacity; +ALTER TABLE host DROP COLUMN description; +ALTER TABLE host DROP COLUMN comment; +ALTER TABLE host DROP COLUMN enabled; + +COMMIT; diff --git a/hub/kojihub.py b/hub/kojihub.py index cd46fda..095ab39 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2032,11 +2032,18 @@ def readTagGroups(tag, event=None, inherit=True, incl_pkgs=True, incl_reqs=True) def set_host_enabled(hostname, enabled=True): context.session.assertPerm('admin') - if not get_host(hostname): + host = get_host(hostname) + if not host: raise koji.GenericError('host does not exist: %s' % hostname) - c = context.cnx.cursor() - c.execute("""UPDATE host SET enabled = %(enabled)s WHERE name = %(hostname)s""", locals()) - context.commit_pending = True + + update = UpdateProcessor('host_config', values=host, clauses=['host_id = %(id)i']) + update.make_revoke() + update.execute() + + insert = InsertProcessor('host_config', data=dslice(host, ('user_id', 'name', 'arches', 'capacity', 'description', 'comment', 'enabled'))) + insert.set(host_id=host['id'], enabled=enabled) + insert.make_create() + insert.execute() def add_host_to_channel(hostname, channel_name, create=False): """Add the host to the specified channel @@ -4483,7 +4490,7 @@ def _dml(operation, values): context.commit_pending = True return ret -def get_host(hostInfo, strict=False): +def get_host(hostInfo, strict=False, event=None): """Get information about the given host. hostInfo may be either a string (hostname) or int (host id). A map will be returned containing the following data: @@ -4499,18 +4506,39 @@ def get_host(hostInfo, strict=False): - ready - enabled """ - fields = ('id', 'user_id', 'name', 'arches', 'task_load', - 'capacity', 'description', 'comment', 'ready', 'enabled') - query = """SELECT %s FROM host - WHERE """ % ', '.join(fields) + tables = ['host_config'] + joins = ['host ON host.id = host_config.host_id'] + + fields = {'host.id': 'id', + 'host.user_id': 'user_id', + 'host.name': 'name', + 'host.ready': 'ready', + 'host.task_load': 'task_load', + 'host_config.arches': 'arches', + 'host_config.capacity': 'capacity', + 'host_config.description': 'description', + 'host_config.comment': 'comment', + 'host_config.enabled': 'enabled', + } + clauses = [eventCondition(event, table='host_config')] + if isinstance(hostInfo, int) or isinstance(hostInfo, long): - query += """id = %(hostInfo)i""" + clauses.append("id = %(hostInfo)i") elif isinstance(hostInfo, str): - query += """name = %(hostInfo)s""" + clauses.append("name = %(hostInfo)s") else: raise koji.GenericError('invalid type for hostInfo: %s' % type(hostInfo)) - return _singleRow(query, locals(), fields, strict) + data = {'hostInfo': hostInfo} + fields, aliases = zip(*fields.items()) + query = QueryProcessor(columns=fields, aliases=aliases, tables=tables, + joins=joins, clauses=clauses, values=data) + result = query.executeOne() + if not result: + if strict: + raise koji.GenericError('Invalid hostInfo: %s' % hostInfo) + return None + return result def edit_host(hostInfo, **kw): """Edit information for an existing host. @@ -4532,19 +4560,22 @@ def edit_host(hostInfo, **kw): changes = [] for field in fields: if field in kw and kw[field] != host[field]: - if field == 'capacity': - # capacity is a float, so set the substitution format appropriately - changes.append('%s = %%(%s)f' % (field, field)) - else: - changes.append('%s = %%(%s)s' % (field, field)) + changes.append(field) if not changes: return False - update = 'UPDATE host set ' + ', '.join(changes) + ' where id = %(id)i' - data = kw.copy() - data['id'] = host['id'] - _dml(update, data) + update = UpdateProcessor('host_config', values=host, clauses=['host_id = %(id)i']) + update.make_revoke() + update.execute() + + insert = InsertProcessor('host_config', data=dslice(host, ('arches', 'capacity', 'description', 'comment', 'enabled'))) + insert.set(host_id=host['id']) + for change in changes: + insert.set(**{change: kw[change]}) + insert.make_create() + insert.execute() + return True def get_channel(channelInfo, strict=False): @@ -6521,6 +6552,7 @@ def query_history(tables=None, **kwargs): 'tag_extra': ['tag_id', 'key', 'value'], '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'], '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'], @@ -6537,6 +6569,7 @@ def query_history(tables=None, **kwargs): 'cg_id': ['content_generator'], #group_id is overloaded (special case below) 'tag_id': ['tag'], + 'host_id': ['host'], 'parent_id': ['tag', 'parent'], 'build_target_id': ['build_target'], 'build_tag': ['tag', 'build_tag'], @@ -10542,10 +10575,14 @@ class RootExports(object): krb_principal=krb_principal) #host entry hostID = _singleValue("SELECT nextval('host_id_seq')", strict=True) - arches = " ".join(arches) - insert = """INSERT INTO host (id, user_id, name, arches) - VALUES (%(hostID)i, %(userID)i, %(hostname)s, %(arches)s)""" + insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, %(hostname)s" _dml(insert, locals()) + + insert = InsertProcessor('host_config') + insert.set(host_id=hostID, arches=" ".join(arches)) + insert.make_create() + insert.execute() + #host_channels entry insert = """INSERT INTO host_channels (host_id, channel_id) VALUES (%(hostID)i, %(default_channel)i)""" @@ -10573,11 +10610,8 @@ class RootExports(object): host appears in the list, it will be included in the results. If "ready" and "enabled" are specified, only hosts with the given value for the respective field will be included.""" - fields = ('id', 'user_id', 'name', 'arches', 'task_load', - 'capacity', 'description', 'comment', 'ready', 'enabled') - - clauses = [] - joins = [] + clauses = ['active IS TRUE'] + joins = ['host ON host.id = host_config.host_id'] if arches is not None: if not arches: raise koji.GenericError('arches option cannot be empty') @@ -10589,25 +10623,37 @@ class RootExports(object): clauses.append('(' + ' OR '.join(archClause) + ')') if channelID is not None: channelID = get_channel_id(channelID, strict=True) - joins.append('host_channels on host.id = host_channels.host_id') + joins.append('host_channels ON host.id = host_channels.host_id') clauses.append('host_channels.channel_id = %(channelID)i') if ready is not None: if ready: - clauses.append('ready is true') + clauses.append('ready IS TRUE') else: - clauses.append('ready is false') + clauses.append('ready IS FALSE') if enabled is not None: if enabled: - clauses.append('enabled is true') + clauses.append('enabled IS TRUE') else: - clauses.append('enabled is false') + clauses.append('enabled IS FALSE') if userID is not None: userID = get_user(userID, strict=True)['id'] clauses.append('user_id = %(userID)i') - query = QueryProcessor(columns=fields, tables=['host'], - joins=joins, clauses=clauses, - values=locals(), opts=queryOpts) + fields = {'host.id': 'id', + 'host.user_id': 'user_id', + 'host.name': 'name', + 'host.ready': 'ready', + 'host.task_load': 'task_load', + 'host_config.arches': 'arches', + 'host_config.capacity': 'capacity', + 'host_config.description': 'description', + 'host_config.comment': 'comment', + 'host_config.enabled': 'enabled', + } + tables = ['host_config'] + fields, aliases = zip(*fields.items()) + query = QueryProcessor(columns=fields, aliases=aliases, + tables=tables, joins=joins, clauses=clauses, values=locals()) return query.execute() def getLastHostUpdate(self, hostID): diff --git a/tests/test_hub/test_list_hosts.py b/tests/test_hub/test_list_hosts.py index 62f2002..ba80fcd 100644 --- a/tests/test_hub/test_list_hosts.py +++ b/tests/test_hub/test_list_hosts.py @@ -29,9 +29,9 @@ class TestListHosts(unittest.TestCase): self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, []) + 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',]) @mock.patch('kojihub.get_user') def test_list_hosts_user_id(self, get_user): @@ -40,9 +40,9 @@ class TestListHosts(unittest.TestCase): self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, ['user_id = %(userID)i']) + 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']) @mock.patch('kojihub.get_channel_id') def test_list_hosts_channel_id(self, get_channel_id): @@ -51,27 +51,28 @@ class TestListHosts(unittest.TestCase): self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, ['host_channels on host.id = host_channels.host_id']) - self.assertEqual(query.clauses, ['host_channels.channel_id = %(channelID)i']) + 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']) def test_list_hosts_single_arch(self): self.exports.listHosts(arches='x86_64') self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, [r"""(arches ~ E'\\mx86_64\\M')"""]) + 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')"""]) def test_list_hosts_multi_arch(self): self.exports.listHosts(arches=['x86_64', 's390']) self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, [r"""(arches ~ E'\\mx86_64\\M' OR arches ~ E'\\ms390\\M')"""]) + 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')"""]) def test_list_hosts_bad_arch(self): with self.assertRaises(koji.GenericError): @@ -82,33 +83,33 @@ class TestListHosts(unittest.TestCase): self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, ['ready is true']) + 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']) def test_list_hosts_nonready(self): self.exports.listHosts(ready=0) self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, ['ready is false']) + 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']) def test_list_hosts_enabled(self): self.exports.listHosts(enabled=1) self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, ['enabled is true']) + 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']) def test_list_hosts_disabled(self): self.exports.listHosts(enabled=0) self.assertEqual(len(self.queries), 1) query = self.queries[0] - self.assertEqual(query.tables, ['host']) - self.assertEqual(query.joins, []) - self.assertEqual(query.clauses, ['enabled is false']) + 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']) From 98aa60fb4cfe7cb9b65f87ae62b0b7d0cc7b912c Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 2/9] update schema.sql for host_config --- diff --git a/docs/schema-upgrade-1.15-1.16.sql b/docs/schema-upgrade-1.15-1.16.sql index e4fa6e2..d278289 100644 --- a/docs/schema-upgrade-1.15-1.16.sql +++ b/docs/schema-upgrade-1.15-1.16.sql @@ -23,7 +23,7 @@ CREATE TABLE host_config ( (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) + UNIQUE (host_id, active) ) WITHOUT OIDS; -- copy starting data diff --git a/docs/schema.sql b/docs/schema.sql index 85a0c63..6be88c6 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -145,15 +145,29 @@ CREATE TABLE host ( id SERIAL NOT NULL PRIMARY KEY, user_id INTEGER NOT NULL REFERENCES users (id), name VARCHAR(128) UNIQUE NOT NULL, - arches TEXT, task_load FLOAT CHECK (NOT task_load < 0) NOT NULL DEFAULT 0.0, + ready BOOLEAN NOT NULL DEFAULT 'false', +) WITHOUT OIDS; + +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, - ready BOOLEAN NOT NULL DEFAULT 'false', - enabled BOOLEAN NOT NULL DEFAULT 'true' + 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) ) WITHOUT OIDS; -CREATE INDEX HOST_IS_READY_AND_ENABLED ON host(enabled, ready) WHERE (enabled IS TRUE AND ready IS TRUE); CREATE TABLE host_channels ( host_id INTEGER NOT NULL REFERENCES host(id), From c11030327037c495d20035c439c1d0fb713057a3 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 3/9] JOIN fixes for host_config --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 095ab39..67dfc1c 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2137,10 +2137,12 @@ def get_ready_hosts(): q = """ SELECT %s FROM host JOIN sessions USING (user_id) + JOIN host_config ON host.id = host_config.host_id WHERE enabled = TRUE AND ready = TRUE AND expired = FALSE AND master IS NULL AND update_time > NOW() - '5 minutes'::interval + AND active IS TRUE """ % ','.join(fields) # XXX - magic number in query c.execute(q) @@ -2154,7 +2156,7 @@ def get_ready_hosts(): def get_all_arches(): """Return a list of all (canonical) arches available from hosts""" ret = {} - for (arches,) in _fetchMulti('SELECT arches FROM host', {}): + for (arches,) in _fetchMulti('SELECT arches FROM host_config WHERE active IS TRUE', {}): if arches is None: continue for arch in arches.split(): @@ -11510,7 +11512,7 @@ class Host(object): id = self.id #get arch and channel info for host q = """ - SELECT arches FROM host WHERE id = %(id)s + SELECT arches FROM host_config WHERE id = %(id)s AND active IS TRUE """ c.execute(q, locals()) arches = c.fetchone()[0].split() @@ -11552,7 +11554,7 @@ class Host(object): def isEnabled(self): """Return whether this host is enabled or not.""" - query = """SELECT enabled FROM host WHERE id = %(id)i""" + query = """SELECT enabled FROM host_config WHERE id = %(id)i AND active IS TRUE""" return _singleValue(query, {'id': self.id}, strict=True) class HostExports(object): From af03651ef03632dedece86deeec2027196168529 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 4/9] add index --- diff --git a/docs/schema-upgrade-1.15-1.16.sql b/docs/schema-upgrade-1.15-1.16.sql index d278289..d5a6b08 100644 --- a/docs/schema-upgrade-1.15-1.16.sql +++ b/docs/schema-upgrade-1.15-1.16.sql @@ -25,6 +25,7 @@ CREATE TABLE host_config ( 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) -- copy starting data -- CREATE FUNCTION pg_temp.user() returns INTEGER as $$ select id from users where name='nobody' $$ language SQL; diff --git a/docs/schema.sql b/docs/schema.sql index 6be88c6..83dfc0a 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -168,6 +168,7 @@ CREATE TABLE host_config ( 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) CREATE TABLE host_channels ( host_id INTEGER NOT NULL REFERENCES host(id), From 313a12686975b36220abdc07ecb68d0ac43b08d3 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 5/9] update version in comment --- diff --git a/docs/schema-upgrade-1.15-1.16.sql b/docs/schema-upgrade-1.15-1.16.sql index d5a6b08..66574a1 100644 --- a/docs/schema-upgrade-1.15-1.16.sql +++ b/docs/schema-upgrade-1.15-1.16.sql @@ -1,5 +1,5 @@ -- upgrade script to migrate the Koji database schema --- from version 1.13 to 1.14 +-- from version 1.14 to 1.16 BEGIN; From 7323784e4747835fb8d5d34a23d403193ddad547 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 6/9] host history tests --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 67dfc1c..a61c8a6 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4524,7 +4524,7 @@ def get_host(hostInfo, strict=False, event=None): } clauses = [eventCondition(event, table='host_config')] - if isinstance(hostInfo, int) or isinstance(hostInfo, long): + if isinstance(hostInfo, (int, long)): clauses.append("id = %(hostInfo)i") elif isinstance(hostInfo, str): clauses.append("name = %(hostInfo)s") @@ -10578,7 +10578,7 @@ class RootExports(object): #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" - _dml(insert, locals()) + _dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname'))) insert = InsertProcessor('host_config') insert.set(host_id=hostID, arches=" ".join(arches)) @@ -10588,7 +10588,7 @@ class RootExports(object): #host_channels entry insert = """INSERT INTO host_channels (host_id, channel_id) VALUES (%(hostID)i, %(default_channel)i)""" - _dml(insert, locals()) + _dml(insert, dslice(locals(), ('hostID', 'default_channel'))) return hostID def enableHost(self, hostname): diff --git a/tests/test_hub/test_add_host.py b/tests/test_hub/test_add_host.py new file mode 100644 index 0000000..455a53f --- /dev/null +++ b/tests/test_hub/test_add_host.py @@ -0,0 +1,79 @@ +import unittest +import mock + +import koji +import kojihub + +UP = kojihub.UpdateProcessor +IP = kojihub.InsertProcessor + + +class TestAddHost(unittest.TestCase): + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(update) + return update + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertLogin = mock.MagicMock() + self.context.session.assertPerm = mock.MagicMock() + self.context.opts = {'HostPrincipalFormat': '-%s-'} + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + @mock.patch('kojihub._dml') + @mock.patch('kojihub.get_host') + @mock.patch('kojihub._singleValue') + def test_add_host_exists(self, _singleValue, get_host, _dml): + get_host.return_value = {'id': 123} + with self.assertRaises(koji.GenericError): + self.exports.addHost('hostname', ['i386', 'x86_64']) + _dml.assert_not_called() + get_host.assert_called_once_with('hostname') + _singleValue.assert_not_called() + + @mock.patch('kojihub._dml') + @mock.patch('kojihub.get_host') + @mock.patch('kojihub._singleValue') + def test_add_host_valid(self, _singleValue, get_host, _dml): + get_host.return_value = {} + _singleValue.side_effect = [333, 12] + self.context.session.createUser.return_value = 456 + + r = self.exports.addHost('hostname', ['i386', 'x86_64']) + self.assertEqual(r, 12) + + self.context.session.assertPerm.assert_called_once_with('admin') + kojihub.get_host.assert_called_once_with('hostname') + self.context.session.createUser.assert_called_once_with('hostname', + usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-') + self.assertEqual(_singleValue.call_count, 2) + _singleValue.assert_has_calls([ + 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}) + ]) diff --git a/tests/test_hub/test_edit_host.py b/tests/test_hub/test_edit_host.py new file mode 100644 index 0000000..6759a5a --- /dev/null +++ b/tests/test_hub/test_edit_host.py @@ -0,0 +1,127 @@ +import unittest +import mock + +import koji +import kojihub + +UP = kojihub.UpdateProcessor +IP = kojihub.InsertProcessor + + +class TestSetHostEnabled(unittest.TestCase): + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(update) + return update + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertLogin = mock.MagicMock() + self.context.session.assertPerm = mock.MagicMock() + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + def test_edit_host_missing(self): + kojihub.get_host = mock.MagicMock() + kojihub.get_host.side_effect = koji.GenericError + with self.assertRaises(koji.GenericError): + self.exports.editHost('hostname') + kojihub.get_host.assert_called_once_with('hostname', strict=True) + self.assertEqual(self.inserts, []) + self.assertEqual(self.updates, []) + + def test_edit_host_valid(self): + kojihub.get_host = mock.MagicMock() + kojihub.get_host.return_value = { + 'id': 123, + 'user_id': 234, + 'name': 'hostname', + 'arches': ['x86_64'], + 'capacity': 100.0, + 'description': 'description', + 'comment': 'comment', + 'enabled': False, + } + self.context.event_id = 42 + self.context.session.user_id = 23 + + r = self.exports.editHost('hostname', arches=['x86_64', 'i386'], + capacity=12.0, comment='comment_new', non_existing_kw='bogus') + + self.assertTrue(r) + kojihub.get_host.assert_called_once_with('hostname', strict=True) + + # revoke + self.assertEqual(len(self.updates), 1) + values = kojihub.get_host.return_value + clauses = ['host_id = %(id)i', 'active = TRUE'] + revoke_data = { + 'revoke_event': 42, + 'revoker_id': 23 + } + revoke_rawdata = {'active': 'NULL'} + update = self.updates[0] + self.assertEqual(update.table, 'host_config') + self.assertEqual(update.values, values) + self.assertEqual(update.clauses, clauses) + self.assertEqual(update.data, revoke_data) + self.assertEqual(update.rawdata, revoke_rawdata) + + # insert + self.assertEqual(len(self.inserts), 1) + insert = self.inserts[0] + #data = kojihub.get_host.return_value + data = { + 'create_event': 42, + 'creator_id': 23, + 'host_id': 123, + 'arches': ['x86_64', 'i386'], + 'capacity': 12.0, + 'comment': 'comment_new', + 'description': 'description', + 'enabled': False, + } + rawdata = {} + self.assertEqual(insert.table, 'host_config') + self.assertEqual(insert.data, data) + self.assertEqual(insert.rawdata, rawdata) + + def test_edit_host_no_change(self): + kojihub.get_host = mock.MagicMock() + kojihub.get_host.return_value = { + 'id': 123, + 'user_id': 234, + 'name': 'hostname', + 'arches': ['x86_64'], + 'capacity': 100.0, + 'description': 'description', + 'comment': 'comment', + 'enabled': False, + } + self.context.event_id = 42 + self.context.session.user_id = 23 + + r = self.exports.editHost('hostname') + + self.assertFalse(r) + kojihub.get_host.assert_called_once_with('hostname', strict=True) + + self.assertEqual(len(self.updates), 0) + self.assertEqual(len(self.inserts), 0) diff --git a/tests/test_hub/test_get_host.py b/tests/test_hub/test_get_host.py new file mode 100644 index 0000000..b3e0b8a --- /dev/null +++ b/tests/test_hub/test_get_host.py @@ -0,0 +1,95 @@ +import unittest +import mock + +import koji +import kojihub + +QP = kojihub.QueryProcessor + + +class TestSetHostEnabled(unittest.TestCase): + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + self.queries.append(query) + return query + + def setUp(self): + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + def test_get_host_by_name(self): + self.exports.getHost('hostname') + + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + columns = ['host.id', 'host.user_id', 'host.name', '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', + 'arches', 'capacity', 'description', 'comment', 'enabled'] + clauses = ['(host_config.active = TRUE)', 'name = %(hostInfo)s'] + values = {'hostInfo': 'hostname'} + self.assertEqual(query.tables, ['host_config']) + self.assertEqual(query.joins, joins) + self.assertEqual(set(query.columns), set(columns)) + self.assertEqual(set(query.aliases), set(aliases)) + self.assertEqual(query.clauses, clauses) + self.assertEqual(query.values, values) + + def test_get_host_by_id_event(self): + self.exports.getHost(123, event=345) + + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + columns = ['host.id', 'host.user_id', 'host.name', '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', + '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'] + values = {'hostInfo': 123} + self.assertEqual(query.tables, ['host_config']) + self.assertEqual(query.joins, joins) + self.assertEqual(set(query.columns), set(columns)) + self.assertEqual(set(query.aliases), set(aliases)) + self.assertEqual(query.clauses, clauses) + self.assertEqual(query.values, values) + + def getQueryMissing(self, *args, **kwargs): + q = self.getQuery(*args, **kwargs) + q.executeOne.return_value = [] + return q + + def test_get_host_missing(self): + self.QueryProcessor.side_effect = self.getQueryMissing + + r = self.exports.getHost(123) + self.assertEqual(r, None) + + with self.assertRaises(koji.GenericError): + self.exports.getHost(123, strict=True) + + self.assertEqual(len(self.queries), 2) + + self.QueryProcessor.side_effect = self.getQuery + + def test_get_host_invalid_hostinfo(self): + with self.assertRaises(koji.GenericError): + self.exports.getHost({'host_id': 567}) + + self.assertEqual(len(self.queries), 0) diff --git a/tests/test_hub/test_set_host_enabled.py b/tests/test_hub/test_set_host_enabled.py new file mode 100644 index 0000000..5307d01 --- /dev/null +++ b/tests/test_hub/test_set_host_enabled.py @@ -0,0 +1,147 @@ +import unittest +import mock + +import koji +import kojihub + +UP = kojihub.UpdateProcessor +IP = kojihub.InsertProcessor + + +class TestSetHostEnabled(unittest.TestCase): + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(update) + return update + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertLogin = mock.MagicMock() + self.context.session.assertPerm = mock.MagicMock() + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + def test_enableHost_missing(self): + # non-existing hostname + kojihub.get_host = mock.MagicMock() + kojihub.get_host.return_value = {} + with self.assertRaises(koji.GenericError): + self.exports.enableHost('hostname') + self.assertEqual(self.updates, []) + self.assertEqual(self.inserts, []) + kojihub.get_host.assert_called_once_with('hostname') + + def test_enableHost_valid(self): + kojihub.get_host = mock.MagicMock() + kojihub.get_host.return_value = { + 'id': 123, + 'user_id': 234, + 'name': 'hostname', + 'arches': ['x86_64'], + 'capacity': 100.0, + 'description': 'description', + 'comment': 'comment', + 'enabled': False, + } + self.context.event_id = 42 + self.context.session.user_id = 23 + + self.exports.enableHost('hostname') + + kojihub.get_host.assert_called_once_with('hostname') + # revoke + self.assertEqual(len(self.updates), 1) + values = kojihub.get_host.return_value + clauses = ['host_id = %(id)i', 'active = TRUE'] + revoke_data = { + 'revoke_event': 42, + 'revoker_id': 23 + } + revoke_rawdata = {'active': 'NULL'} + update = self.updates[0] + self.assertEqual(update.table, 'host_config') + self.assertEqual(update.values, values) + self.assertEqual(update.clauses, clauses) + self.assertEqual(update.data, revoke_data) + self.assertEqual(update.rawdata, revoke_rawdata) + + # insert + insert = self.inserts[0] + data = kojihub.get_host.return_value + data['create_event'] = 42 + data['creator_id'] = 23 + data['enabled'] = True + data['host_id'] = data['id'] + del data['id'] + rawdata = {} + self.assertEqual(insert.table, 'host_config') + self.assertEqual(insert.data, data) + self.assertEqual(insert.rawdata, rawdata) + + self.assertEqual(len(self.inserts), 1) + + def test_disableHost_valid(self): + kojihub.get_host = mock.MagicMock() + kojihub.get_host.return_value = { + 'id': 123, + 'user_id': 234, + 'name': 'hostname', + 'arches': ['x86_64'], + 'capacity': 100.0, + 'description': 'description', + 'comment': 'comment', + 'enabled': True, + } + self.context.event_id = 42 + self.context.session.user_id = 23 + + self.exports.disableHost('hostname') + + kojihub.get_host.assert_called_once_with('hostname') + # revoke + self.assertEqual(len(self.updates), 1) + values = kojihub.get_host.return_value + clauses = ['host_id = %(id)i', 'active = TRUE'] + revoke_data = { + 'revoke_event': 42, + 'revoker_id': 23 + } + revoke_rawdata = {'active': 'NULL'} + update = self.updates[0] + self.assertEqual(update.table, 'host_config') + self.assertEqual(update.values, values) + self.assertEqual(update.clauses, clauses) + self.assertEqual(update.data, revoke_data) + self.assertEqual(update.rawdata, revoke_rawdata) + + # insert + insert = self.inserts[0] + data = kojihub.get_host.return_value + data['create_event'] = 42 + data['creator_id'] = 23 + data['enabled'] = False + data['host_id'] = data['id'] + del data['id'] + rawdata = {} + self.assertEqual(insert.table, 'host_config') + self.assertEqual(insert.data, data) + self.assertEqual(insert.rawdata, rawdata) + + self.assertEqual(len(self.inserts), 1) From 4d1ece0b2e92a40ecc332c3775dcb49f4bf3599d Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 7/9] Fix for host_config --- diff --git a/hub/kojihub.py b/hub/kojihub.py index a61c8a6..61b8642 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -4525,9 +4525,9 @@ def get_host(hostInfo, strict=False, event=None): clauses = [eventCondition(event, table='host_config')] if isinstance(hostInfo, (int, long)): - clauses.append("id = %(hostInfo)i") + clauses.append("host.id = %(hostInfo)i") elif isinstance(hostInfo, str): - clauses.append("name = %(hostInfo)s") + clauses.append("host.name = %(hostInfo)s") else: raise koji.GenericError('invalid type for hostInfo: %s' % type(hostInfo)) @@ -10612,7 +10612,7 @@ class RootExports(object): host appears in the list, it will be included in the results. If "ready" and "enabled" are specified, only hosts with the given value for the respective field will be included.""" - clauses = ['active IS TRUE'] + clauses = ['host_config.active IS TRUE'] joins = ['host ON host.id = host_config.host_id'] if arches is not None: if not arches: From a6e4e5efc19987a5be70be9c1344a6f70da31c45 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 8/9] host_channels history --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index fd2707c..943fd5d 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 66574a1..77b5b5f 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 83dfc0a..423cf50 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 61b8642..84768ff 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2064,6 +2064,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): @@ -2083,9 +2084,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""" @@ -2105,6 +2110,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) @@ -2148,7 +2157,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 @@ -4685,16 +4694,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() @@ -6555,6 +6576,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'], @@ -6572,6 +6594,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'], @@ -10577,7 +10600,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') @@ -10586,9 +10609,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): @@ -10627,6 +10652,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') @@ -11517,7 +11543,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 455a53f..ce0cf43 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 b3e0b8a..70d7487 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 ba80fcd..8b09295 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']) From 470ad2c9eaa3813d940b60aaad43febffb5a7e54 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Apr 10 2018 11:53:58 +0000 Subject: [PATCH 9/9] tests for host_channels --- diff --git a/tests/test_hub/test_add_host_to_channel.py b/tests/test_hub/test_add_host_to_channel.py new file mode 100644 index 0000000..7a7792d --- /dev/null +++ b/tests/test_hub/test_add_host_to_channel.py @@ -0,0 +1,135 @@ +import unittest +import mock + +import koji +import kojihub + +IP = kojihub.InsertProcessor + + +class TestAddHostToChannel(unittest.TestCase): + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertLogin = mock.MagicMock() + self.context.session.assertPerm = mock.MagicMock() + self.context.event_id = 42 + self.context.session.user_id = 23 + self.context.opts = {'HostPrincipalFormat': '-%s-'} + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_valid(self, get_host, get_channel_id, list_channels): + name = 'hostname' + cname = 'channel_name' + get_host.return_value = {'id': 123, 'name': name} + get_channel_id.return_value = 456 + list_channels.return_value = [{'id': 1, 'name': 'default'}] + + kojihub.add_host_to_channel(name, cname, create=False) + + get_host.assert_called_once_with(name) + get_channel_id.assert_called_once_with(cname, create=False) + list_channels.assert_called_once_with(123) + + self.assertEqual(len(self.inserts), 1) + insert = self.inserts[0] + data = { + 'host_id': 123, + 'channel_id': 456, + 'creator_id': 23, + 'create_event': 42, + } + self.assertEqual(insert.table, 'host_channels') + self.assertEqual(insert.data, data) + self.assertEqual(insert.rawdata, {}) + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_no_host(self, get_host, get_channel_id, list_channels): + name = 'hostname' + cname = 'channel_name' + get_host.return_value = None + + with self.assertRaises(koji.GenericError): + kojihub.add_host_to_channel(name, cname, create=False) + + get_host.assert_called_once_with(name) + self.assertEqual(len(self.inserts), 0) + + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_no_channel(self, get_host, get_channel_id): + name = 'hostname' + cname = 'channel_name' + get_host.return_value = {'id': 123, 'name': name} + get_channel_id.return_value = None + + with self.assertRaises(koji.GenericError): + kojihub.add_host_to_channel(name, cname, create=False) + + get_host.assert_called_once_with(name) + get_channel_id.assert_called_once_with(cname, create=False) + self.assertEqual(len(self.inserts), 0) + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_no_channel_create(self, get_host, get_channel_id, list_channels): + name = 'hostname' + cname = 'channel_name' + get_host.return_value = {'id': 123, 'name': name} + get_channel_id.return_value = 456 + list_channels.return_value = [{'id': 1, 'name': 'default'}] + + kojihub.add_host_to_channel(name, cname, create=True) + + get_host.assert_called_once_with(name) + get_channel_id.assert_called_once_with(cname, create=True) + list_channels.assert_called_once_with(123) + + self.assertEqual(len(self.inserts), 1) + insert = self.inserts[0] + data = { + 'host_id': 123, + 'channel_id': 456, + 'creator_id': 23, + 'create_event': 42, + } + self.assertEqual(insert.table, 'host_channels') + self.assertEqual(insert.data, data) + self.assertEqual(insert.rawdata, {}) + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_exists(self, get_host, get_channel_id, list_channels): + name = 'hostname' + cname = 'channel_name' + get_host.return_value = {'id': 123, 'name': name} + get_channel_id.return_value = 456 + list_channels.return_value = [{'id': 456, 'name': cname}] + + with self.assertRaises(koji.GenericError): + kojihub.add_host_to_channel(name, cname, create=False) + + get_host.assert_called_once_with(name) + get_channel_id.assert_called_once_with(cname, create=False) + list_channels.assert_called_once_with(123) + self.assertEqual(len(self.inserts), 0) diff --git a/tests/test_hub/test_list_channels.py b/tests/test_hub/test_list_channels.py new file mode 100644 index 0000000..ee4e68f --- /dev/null +++ b/tests/test_hub/test_list_channels.py @@ -0,0 +1,79 @@ +import unittest +import mock + +import koji +import kojihub + +QP = kojihub.QueryProcessor + + +class TestListChannels(unittest.TestCase): + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + self.queries.append(query) + return query + + def setUp(self): + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + + def test_all(self): + kojihub.list_channels() + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['channels']) + self.assertEqual(query.aliases, ('name', 'id')) + self.assertEqual(query.joins, None) + self.assertEqual(query.values, {}) + self.assertEqual(query.columns, ('channels.name', 'channels.id')) + self.assertEqual(query.clauses, None) + + def test_host(self): + kojihub.list_channels(hostID=1234) + + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + joins = ['channels ON channels.id = host_channels.channel_id'] + clauses = [ + '(host_channels.active = TRUE)', + 'host_channels.host_id = %(host_id)s' + ] + self.assertEqual(query.tables, ['host_channels']) + self.assertEqual(query.aliases, ('name', 'id')) + self.assertEqual(query.joins, joins) + self.assertEqual(query.values, {'host_id': 1234}) + self.assertEqual(query.columns, ('channels.name', 'channels.id')) + self.assertEqual(query.clauses, clauses) + + def test_host_and_event(self): + kojihub.list_channels(hostID=1234, event=2345) + + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + joins = ['channels ON channels.id = host_channels.channel_id'] + clauses = [ + '(host_channels.create_event <= 2345 AND ( host_channels.revoke_event IS NULL OR 2345 < host_channels.revoke_event ))', + 'host_channels.host_id = %(host_id)s', + ] + self.assertEqual(query.tables, ['host_channels']) + self.assertEqual(query.aliases, ('name', 'id')) + self.assertEqual(query.joins, joins) + self.assertEqual(query.values, {'host_id': 1234}) + self.assertEqual(query.columns, ('channels.name', 'channels.id')) + self.assertEqual(query.clauses, clauses) + + def test_event_only(self): + with self.assertRaises(koji.GenericError): + kojihub.list_channels(event=1234) + self.assertEqual(len(self.queries), 0) diff --git a/tests/test_hub/test_remove_host_from_channel.py b/tests/test_hub/test_remove_host_from_channel.py new file mode 100644 index 0000000..f2f31af --- /dev/null +++ b/tests/test_hub/test_remove_host_from_channel.py @@ -0,0 +1,103 @@ +import unittest +import mock + +import koji +import kojihub + +UP = kojihub.UpdateProcessor + + +class TestRemoveHostFromChannel(unittest.TestCase): + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(update) + return update + + def setUp(self): + self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertLogin = mock.MagicMock() + self.context.session.assertPerm = mock.MagicMock() + self.context.event_id = 42 + self.context.session.user_id = 23 + self.context.opts = {'HostPrincipalFormat': '-%s-'} + self.exports = kojihub.RootExports() + + def tearDown(self): + mock.patch.stopall() + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_valid(self, get_host, get_channel_id, list_channels): + get_host.return_value = {'id': 123, 'name': 'hostname'} + get_channel_id.return_value = 234 + list_channels.return_value = [{'id': 234, 'name': 'channelname'}] + + kojihub.remove_host_from_channel('hostname', 'channelname') + + get_host.assert_called_once_with('hostname') + get_channel_id.assert_called_once_with('channelname') + list_channels.assert_called_once_with(123) + + self.assertEqual(len(self.updates), 1) + update = self.updates[0] + values = { + 'host_id': 123, + 'channel_id': 234, + } + clauses = [ + 'host_id = %(host_id)i AND channel_id = %(channel_id)i', + 'active = TRUE', + ] + self.assertEqual(update.table, 'host_channels') + self.assertEqual(update.values, values) + self.assertEqual(update.clauses, clauses) + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_wrong_host(self, get_host, get_channel_id, list_channels): + get_host.return_value = None + + with self.assertRaises(koji.GenericError): + kojihub.remove_host_from_channel('hostname', 'channelname') + + get_host.assert_called_once_with('hostname') + self.assertEqual(len(self.updates), 0) + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_wrong_channel(self, get_host, get_channel_id, list_channels): + get_host.return_value = {'id': 123, 'name': 'hostname'} + get_channel_id.return_value = None + list_channels.return_value = [{'id': 234, 'name': 'channelname'}] + + with self.assertRaises(koji.GenericError): + kojihub.remove_host_from_channel('hostname', 'channelname') + + get_host.assert_called_once_with('hostname') + get_channel_id.assert_called_once_with('channelname') + self.assertEqual(len(self.updates), 0) + + @mock.patch('kojihub.list_channels') + @mock.patch('kojihub.get_channel_id') + @mock.patch('kojihub.get_host') + def test_missing_record(self, get_host, get_channel_id, list_channels): + get_host.return_value = {'id': 123, 'name': 'hostname'} + get_channel_id.return_value = 234 + list_channels.return_value = [] + + with self.assertRaises(koji.GenericError): + kojihub.remove_host_from_channel('hostname', 'channelname') + + get_host.assert_called_once_with('hostname') + get_channel_id.assert_called_once_with('channelname') + list_channels.assert_called_once_with(123) + self.assertEqual(len(self.updates), 0)