Modify the code to make the libvirt url actually configurable
ClosedPublic
Actions

Authored by lnie on Aug 1 2016, 5:43 AM.

Details

Reviewers

lbrabec
kparal
tflink
jskladan

Maniphest Tasks

T817: finish making the libvirt url configurable

Commits

rTCLOUDdc6e7859dc9a: make libvirt url configurable

Summary

Modify the code to make the libvirt url actually configurable,

as described in https://phab.qadevel.cloud.fedoraproject.org/T817

Test Plan

just run testcloud as normal to see whether it still works well

Diff Detail

Repository

rTCLOUD testcloud

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

lnie retitled this revision from to Modify the code to make the libvirt url actually configurable.Aug 1 2016, 5:43 AM

lnie updated this object.

lnie edited the test plan for this revision. (Show Details)

lnie added reviewers: jskladan, kparal, lbrabec, tflink.

lnie added a task: T769: rpm_utils.get_dist_tag() doesn't understand EPEL dist tag.Aug 1 2016, 5:44 AM

lnie removed a task: T769: rpm_utils.get_dist_tag() doesn't understand EPEL dist tag.

lnie added a task: T817: finish making the libvirt url configurable.Aug 1 2016, 5:47 AM

modify the diff

I'm not all that familiar with testcloud code. @lbrabec, @tflink, does that code look reasonable?

Lili, have you tested it with qemu:///session?

testcloud/cli.py
346	The new parameter should be added to the docstring.
testcloud/instance.py
338	For all these methods in `Instance`, wouldn't it be easier to add the `connection` parameter to `__init__()`, instead of adding it to each instance method?

This revision now requires changes to proceed.Aug 8 2016, 12:28 PM

Lili, have you tested it with qemu:///session?

Nope(until I saw this comment),I'm not familiar with it,but doesn't it has drawbacks compared to qemu:///system?Is it likely that users will use qemu:///session instead of qemu:///system?
In case some users will use qemu:///session,I have tested it,and testcloud works well with it.

testcloud/instance.py
338	yeah,we don't need to add it to each instance method.Actually,I typed it incidentally after I typed "connection":)

The point of that ticket is to make it configurable, so I expect people will use different libvirt connections :) qemu:///session is almost the same qemu:///system, it just doesn't need admin privileges, and its networking is not that good, but should be good enough.

I tried this:

$ testcloud instance --connection qemu:///session create -u 'file:///var/lib/taskotron/images/160729_1122-fedora-23-taskotron_cloud-x86_64.qcow2' foo

Which seemingly works, but the VM is created in the system scope, not in the session scope, so there's still some problem somewhere. Doesn't work as expected, the machine should be created in session scope (you can easily see that in virt-manager, if you connect to both qemu:///session and qemu:///system).

Also, please rebase to the latest dev branch:

$ git checkout dev
$ git update
$ git checkout libvirt-url
$ git rebase dev

In D956#18198, @kparal wrote:

The point of that ticket is to make it configurable, so I expect people will use different libvirt connections :) qemu:///session is almost the same >qemu:///system, it just doesn't need admin privileges, and its networking is not that good, but should be good enough.

Can't agree more,"the point of that ticket is to make it configurable",that's exactly what I thought when I created this diff^^. Making the -c argument "not hardcoded",ie,configurable and that's all.Whether a specific connection works well is likely that kind connection's problem,and beyond the consideration of this diff.Think it's better to open another ticket if we see a problem with a specific connection:)

I tried this:
$ testcloud instance --connection qemu:///session create -u 'file:///var/lib/taskotron/images/160729_1122-fedora-23-taskotron_cloud-x86_64.qcow2' foo
Which seemingly works, but the VM is created in the system scope, not in the session scope, so there's still some problem somewhere. Doesn't work as >expected, the machine should be created in session scope (you can easily see that in virt-manager, if you connect to both qemu:///session and >qemu:///system).

I got the conclusion that this diff works well with qemu:///session by running a similar task but with sudo(haven't checked which scope it's in)

sudo testcloud instance --connection qemu:///session create -u 'file:///home/lnie/Fedora-Cloud-Base-24-1.2.x86_64.qcow2'  foo

Maybe that's the problem?

Also, please rebase to the latest dev branch:
$ git checkout dev
$ git update
$ git checkout libvirt-url
$ git rebase dev

I should do that every time before I try to submit a diff,will keep it on mind: )

In D956#18198, @kparal wrote:
I tried this:
$ testcloud instance --connection qemu:///session create -u 'file:///var/lib/taskotron/images/160729_1122-fedora-23-taskotron_cloud-x86_64.qcow2' foo
Which seemingly works, but the VM is created in the system scope, not in the session scope, so there's still some problem somewhere. Doesn't work as expected, the machine should be created in session scope (you can easily see that in virt-manager, if you connect to both qemu:///session and qemu:///system).

I was wrong about this, I haven't had this patch applied properly. It actually created 'foo' VM in qemu:///session scope, but it fails with this:

DEBUG:Creating domain foo
libvirt: Network Driver error : Network not found: no network with matching name 'default'
Traceback (most recent call last):
  File "/home/kparal/devel/taskotron/env_taskotron/bin/testcloud", line 9, in <module>
    load_entry_point('testcloud==0.1.2', 'console_scripts', 'testcloud')()
  File "/home/kparal/devel/testcloud/testcloud/cli.py", line 328, in main
    args.func(args)
  File "/home/kparal/devel/testcloud/testcloud/cli.py", line 96, in _create_instance
    tc_instance.start(args.connection, args.timeout)
  File "/home/kparal/devel/testcloud/testcloud/instance.py", line 383, in start
    create_status = dom.create()
  File "/usr/lib64/python2.7/site-packages/libvirt.py", line 1035, in create
    if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
libvirt.libvirtError: Network not found: no network with matching name 'default'

The problem seems to be that testcloud configures networking to always use the default network, but that is only available in system scope, not in session scope - in session scope, you need to use user-mode networking.

The other issue I identified is that some calls do not use connection argument, like instance.find_instance and therefore things like testcloud instance --connection qemu:///session remove foo doesn't work properly.

We can tackle the user-mode networking issue in a separate diff, but please fix this patch as suggested (using instance as an attribute of Instance class instead of providing it to each class method) and make sure it is used everywhere (like instance.find_instance).

Whether a specific connection works well is likely that kind connection's problem,and beyond the consideration of this diff.Think it's better to open another ticket if we see a problem with a specific connection:)

I agree with you in general, but the problem here is that you need at least two different libvirt connections to be working in order to test that you actually made the connection configurable and didn't forget about anything. But I have no problem doing in this several smaller steps - let's fix this patch and commit this, then fix networking, and then figure out whether we forgot about anything to be made configurable.

We can tackle the user-mode networking issue in a separate diff, but please fix this patch as suggested (using instance as an attribute of Instance class >instead of providing it to each class method) and make sure it is used everywhere (like instance.find_instance).

Sure thing.

Whether a specific connection works well is likely that kind connection's problem,and beyond the consideration of this diff.Think it's better to open another >>ticket if we see a problem with a specific connection:)

I agree with you in general, but the problem here is that you need at least two different libvirt connections to be working in order to test that you actually >made the connection configurable and didn't forget about anything. But I have no problem doing in this several smaller steps - let's fix this patch and commit >this, then fix networking, and then figure out whether we forgot about anything to be made configurable.

Sure,I'm going to update this patch ,but before that we need identify a problem:

The other issue I identified is that some calls do not use connection argument, like instance.find_instance and therefore things like testcloud instance >--connection qemu:///session remove foo doesn't work properly.

What do you mean by saying: remove foo with qemu:///session doesn't work properly?I have successfully removed the foo instance with qemu:///session connection.

(env_taskotron)[lnie@dhcp-128-50 testcloud]$ sudo testcloud instance -c qemu:///session remove foo
DEBUG:remove instance: foo
DEBUG:removing instance foo from libvirt.
DEBUG:Unregistering domain from libvirt.
DEBUG:removing instance /var/lib/testcloud/instances/foo from disk

In D956#18279, @lnie wrote:
What do you mean by saying: remove foo with qemu:///session doesn't work properly?I have successfully removed the foo instance with qemu:///session connection.
(env_taskotron)[lnie@dhcp-128-50 testcloud]$ sudo testcloud instance -c qemu:///session remove foo
DEBUG:remove instance: foo
DEBUG:removing instance foo from libvirt.
DEBUG:Unregistering domain from libvirt.
DEBUG:removing instance /var/lib/testcloud/instances/foo from disk

You must not use sudo. If you use sudo, you're connecting to the session instance of root, which is... qemu:///system. So the way you run it, you're actually using qemu:///system.

You must not use sudo. If you use sudo, you're connecting to the session instance of root, which is... qemu:///system. So the way you run it, you're actually >using qemu:///system.

I thought you saw a problem using "sudo",and we still have other kinds of problem to handle before I update the diff:)

modify the diff according to great suggestion gotten from the revision

This looks good. I played with it and a few adjustments are needed, and hopefully that should be all.

testcloud/instance.py
87	This also needs to get `connection='qemu:///system'`. All places where `find_instance` is called need to provide the argument.
98	And we pass `connection` in here.
464–466	Please add something like: else: log.warning('Domain "%s" not found in libvirt (%s). Was it removed already? Did you ' 'want to use a different libvirt connection?', self.name, self.connection) If the user runs `testcloud instance remove` with wrong libvirt connection, at least he/she will be notified that the domain was not found (and he/she needs to remove it manually).

update the diff

In D956#18354, @kparal wrote:

This looks good. I played with it and a few adjustments are needed, and hopefully that should be all.

Looks professional now. Thanks for your great patience and time with this:)

Thanks, I'll do some further minor changes and push this.

testcloud/instance.py
395–397	This does not make much sense. The fact that the domain can't be found means the connection is wrong, not that it was started already (in that case, it would have been found). I believe it only makes sense to print this kind of message for `remove()`, because we don't raise exceptions if we try to remove something that doesn't exist. For other cases (`stop()`, `start()`) the user already receives a "can't find" error.

This revision is now accepted and ready to land.Aug 15 2016, 9:31 AM

Closed by commit rTCLOUDdc6e7859dc9a: make libvirt url configurable (authored by lnie, committed by kparal). · Explain WhyAug 15 2016, 9:38 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents

			Path	Packages
M			testcloud/cli.py (22 lines)
M			testcloud/instance.py (63 lines)
M			testcloud/util.py (4 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	2433	25dbb11		Aug 1 2016, 5:43 AM	★	★
Diff 2	2449	25dbb11	modify the diff	Aug 5 2016, 8:50 AM	★	★
Diff 3	2476	c3a7ecc	modify the diff according to great suggestion gotten from the revision	Aug 12 2016, 4:51 AM	★	★
Diff 4	2483	c3a7ecc	update the diff	Aug 15 2016, 3:32 AM	★	★
Diff 5	2487	2728907	rTCLOUDdc6e7859dc9a3e27d96a3f89785d5f2e7f15360b	Aug 15 2016, 9:38 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
431e38c51081	3c388e086a35	c3a7eccc1113	lnie	Modify the code to make the libvirt url actually configurable (Show More…)	Aug 1 2016, 5:16 AM

Diff 2487

View Options

testcloud/cli.py

Show All 35 Lines
36	################################################################################	36		################################################################################
37		37
38	def _list_instance(args):	38		def _list_instance(args):
39	"""Handler for 'list' command. Expects the following elements in args:	39		"""Handler for 'list' command. Expects the following elements in args:
40	* name(str)	40		* name(str)
41		41
42	:param args: args from argparser	42		:param args: args from argparser
43	"""	43		"""
44	instances = instance.list_instances()	44		instances = instance.list_instances(args.connection)
45		45
46	print("{:<16} {:^30} {:<10}".format("Name", "IP", "State"))	46		print("{:<16} {:^30} {:<10}".format("Name", "IP", "State"))
47	print("-"*60)	47		print("-"*60)
48	for inst in instances:	48		for inst in instances:
49	if args.all or inst['state'] == 'running':	49		if args.all or inst['state'] == 'running':
50	print("{:<27} {:^22} {:<10}".format(inst['name'],	50		print("{:<27} {:^22} {:<10}".format(inst['name'],
51	inst['ip'],	51		inst['ip'],
52	inst['state']))	52		inst['state']))
53		53
54	print("")	54		print("")
55		55
56		56
57	def _create_instance(args):	57		def _create_instance(args):
58	"""Handler for 'instance create' command. Expects the following elements in args:	58		"""Handler for 'instance create' command. Expects the following elements in args:
59	* name(str)	59		* name(str)
60		60
61	:param args: args from argparser	61		:param args: args from argparser
62	"""	62		"""
63		63
64	log.debug("create instance")	64		log.debug("create instance")
65		65
66	tc_image = image.Image(args.url)	66		tc_image = image.Image(args.url)
67	tc_image.prepare()	67		tc_image.prepare()
68		68
69	existing_instance = instance.find_instance(args.name, tc_image)	69		existing_instance = instance.find_instance(args.name, image=tc_image,
		70		connection=args.connection)
70		71
71	# can't create existing instances	72		# can't create existing instances
72	if existing_instance is not None:	73		if existing_instance is not None:
73	raise TestcloudCliError("A testcloud instance named {} already "	74		raise TestcloudCliError("A testcloud instance named {} already "
74	"exists at {}. Use 'testcloud instance start "	75		"exists at {}. Use 'testcloud instance start "
75	"{}' to start the instance or remove it before"	76		"{}' to start the instance or remove it before"
76	" re-creating ".format(args.name,	77		" re-creating ".format(args.name,
77	existing_instance.path,	78		existing_instance.path,
78	args.name))	79		args.name))
79		80
80	else:	81		else:
81	tc_instance = instance.Instance(args.name, tc_image)	82		tc_instance = instance.Instance(args.name, image=tc_image, connection=args.connection)
82		83
83	# set ram size	84		# set ram size
84	tc_instance.ram = args.ram	85		tc_instance.ram = args.ram
85		86
86	# set disk size	87		# set disk size
87	tc_instance.disk_size = args.disksize	88		tc_instance.disk_size = args.disksize
88		89
89	# prepare instance	90		# prepare instance
90	tc_instance.prepare()	91		tc_instance.prepare()
91		92
92	# create instance domain	93		# create instance domain
93	tc_instance.spawn_vm()	94		tc_instance.spawn_vm()
94		95
95	# start created domain	96		# start created domain
96	tc_instance.start(args.timeout)	97		tc_instance.start(args.timeout)
97		98
98	# find vm ip	99		# find vm ip
99	vm_ip = find_vm_ip(args.name)	100		vm_ip = find_vm_ip(args.name, args.connection)
100		101
101	# Write ip to file	102		# Write ip to file
102	tc_instance.create_ip_file(vm_ip)	103		tc_instance.create_ip_file(vm_ip)
103	print("The IP of vm {}: {}".format(args.name, vm_ip))	104		print("The IP of vm {}: {}".format(args.name, vm_ip))
104		105
105		106
106	def _start_instance(args):	107		def _start_instance(args):
107	"""Handler for 'instance start' command. Expects the following elements in args:	108		"""Handler for 'instance start' command. Expects the following elements in args:
108	* name(str)	109		* name(str)
109		110
110	:param args: args from argparser	111		:param args: args from argparser
111	"""	112		"""
112	log.debug("start instance: {}".format(args.name))	113		log.debug("start instance: {}".format(args.name))
113		114
114	tc_instance = instance.find_instance(args.name)	115		tc_instance = instance.find_instance(args.name, connection=args.connection)
115		116
116	if tc_instance is None:	117		if tc_instance is None:
117	raise TestcloudCliError("Cannot start instance {} because it does "	118		raise TestcloudCliError("Cannot start instance {} because it does "
118	"not exist".format(args.name))	119		"not exist".format(args.name))
119		120
120	tc_instance.start(args.timeout)	121		tc_instance.start(args.timeout)
121	with open(os.path.join(config_data.DATA_DIR, 'instances', args.name, 'ip'), 'r') as ip_file:	122		with open(os.path.join(config_data.DATA_DIR, 'instances', args.name, 'ip'), 'r') as ip_file:
122	vm_ip = ip_file.read()	123		vm_ip = ip_file.read()
123	print("The IP of vm {}: {}".format(args.name, vm_ip))	124		print("The IP of vm {}: {}".format(args.name, vm_ip))
124		125
125		126
126	def _stop_instance(args):	127		def _stop_instance(args):
127	"""Handler for 'instance stop' command. Expects the following elements in args:	128		"""Handler for 'instance stop' command. Expects the following elements in args:
128	* name(str)	129		* name(str)
129		130
130	:param args: args from argparser	131		:param args: args from argparser
131	"""	132		"""
132	log.debug("stop instance: {}".format(args.name))	133		log.debug("stop instance: {}".format(args.name))
133		134
134	tc_instance = instance.find_instance(args.name)	135		tc_instance = instance.find_instance(args.name, connection=args.connection)
135		136
136	if tc_instance is None:	137		if tc_instance is None:
137	raise TestcloudCliError("Cannot stop instance {} because it does "	138		raise TestcloudCliError("Cannot stop instance {} because it does "
138	"not exist".format(args.name))	139		"not exist".format(args.name))
139		140
140	tc_instance.stop()	141		tc_instance.stop()
141		142
142		143
143	def _remove_instance(args):	144		def _remove_instance(args):
144	"""Handler for 'instance remove' command. Expects the following elements in args:	145		"""Handler for 'instance remove' command. Expects the following elements in args:
145	* name(str)	146		* name(str)
146		147
147	:param args: args from argparser	148		:param args: args from argparser
148	"""	149		"""
149	log.debug("remove instance: {}".format(args.name))	150		log.debug("remove instance: {}".format(args.name))
150		151
151	tc_instance = instance.find_instance(args.name)	152		tc_instance = instance.find_instance(args.name, connection=args.connection)
152		153
153	if tc_instance is None:	154		if tc_instance is None:
154	raise TestcloudCliError("Cannot remove instance {} because it does "	155		raise TestcloudCliError("Cannot remove instance {} because it does "
155	"not exist".format(args.name))	156		"not exist".format(args.name))
156		157
157	tc_instance.remove(autostop=args.force)	158		tc_instance.remove(autostop=args.force)
158		159
159		160
▲ Show 20 Lines • Show All 177 Lines • ▼ Show 20 Line(s)		337	def main():
337	parser = get_argparser()	338		parser = get_argparser()
338	args = parser.parse_args()	339		args = parser.parse_args()
339		340
340	_configure_logging()	341		_configure_logging()
341		342
342	args.func(args)	343		args.func(args)
343		344
344		345
345	def find_vm_ip(name):	346		def find_vm_ip(name, connection='qemu:///system'):
			kparalUnsubmitted Not Done The new parameter should be added to the docstring.
346	"""Finds the ip of a local vm given it's name used by libvirt.	347		"""Finds the ip of a local vm given it's name used by libvirt.
347		348
348	:param name: name of the VM (as used by libvirt)	349		:param str name: name of the VM (as used by libvirt)
		350		:param str connection: name of the libvirt connection uri
349	:returns: ip address of VM	351		:returns: ip address of VM
350	:rtype: str	352		:rtype: str
351	"""	353		"""
352		354
353	for _ in xrange(100):	355		for _ in xrange(100):
354	vm_xml = util.get_vm_xml(name)	356		vm_xml = util.get_vm_xml(name, connection)
355	if vm_xml is not None:	357		if vm_xml is not None:
356	break	358		break
357		359
358	else:	360		else:
359	sleep(.2)	361		sleep(.2)
360	else:	362		else:
361	raise DomainNotFoundError	363		raise DomainNotFoundError
362		364
Show All 17 Lines

View Options

testcloud/instance.py

Show First 20 Lines • Show All 58 Lines • ▼ Show 20 Line(s)		52	for dir in instance_dir:
59	except IOError:	59		except IOError:
60	instance_details['ip'] = None	60		instance_details['ip'] = None
61		61
62	instance_list.append(instance_details)	62		instance_list.append(instance_details)
63		63
64	return instance_list	64		return instance_list
65		65
66		66
67	def _list_system_domains(connection):	67		def _list_domains(connection):
68	"""List known domains for a given hypervisor connection.	68		"""List known domains for a given hypervisor connection.
69		69
70	:param connection: libvirt compatible hypervisor connection	70		:param connection: libvirt compatible hypervisor connection
71	:returns: dictionary mapping of name:state	71		:returns: dictionary mapping of name -> state
72	:rtype: dict	72		:rtype: dict
73	"""	73		"""
74		74
75	domains = {}	75		domains = {}
76	conn = libvirt.openReadOnly(connection)	76		conn = libvirt.openReadOnly(connection)
77	for domain in conn.listAllDomains():	77		for domain in conn.listAllDomains():
78	# the libvirt docs seem to indicate that the second int is for state	78		# the libvirt docs seem to indicate that the second int is for state
79	# details, only used when state is ERROR, so only looking at the first	79		# details, only used when state is ERROR, so only looking at the first
80	# int returned for domain.state()	80		# int returned for domain.state()
81
82	domains[domain.name()] = DOMAIN_STATUS_ENUM[domain.state()[0]]	81		domains[domain.name()] = DOMAIN_STATUS_ENUM[domain.state()[0]]
83		82
84	return domains	83		return domains
85		84
86		85
87	def find_instance(name, image=None):	86		def find_instance(name, image=None, connection='qemu:///system'):
	kparalUnsubmitted Not Done This also needs to get `connection='qemu:///system'`. All places where `find_instance` is called need to provide the argument.
88	"""Find an instance using a given name and image, if it exists.	87		"""Find an instance using a given name and image, if it exists.
89		88
90	:param name: name of instance to find	89		Please note that ``connection`` is not taken into account when searching for the instance, but
91	:param image: :py:class:`testcloud.image.Image`	90		the instance object returned has the specified connection set. It's your responsibility to
92	:returns: :py:class:`Instance` if the instance exists, None if it doesn't	91		make sure the provided connection is valid for this instance.
		92
		93		:param str name: name of instance to find
		94		:param image: instance of :py:class:`testcloud.image.Image`
		95		:param str connection: name of libvirt connection uri
		96		:returns: :py:class:`Instance` if the instance exists, ``None`` if it doesn't
93	"""	97		"""
94		98
95	instances = _list_instances()	99		instances = _list_instances()
96	for inst in instances:	100		for inst in instances:
97	if inst['name'] == name:	101		if inst['name'] == name:
98	return Instance(name, image)	102		return Instance(name, image, connection)
	kparalUnsubmitted Not Done And we pass `connection` in here.
99	return None	103		return None
100		104
101		105
102	def list_instances(connection='qemu:///system'):	106		def list_instances(connection='qemu:///system'):
103	"""List instances known by testcloud and the state of each instance	107		"""List instances known by testcloud and the state of each instance
104		108
105	:param connection: libvirt compatible connection to use when listing domains	109		:param connection: libvirt compatible connection to use when listing domains
106	:returns: dictionary of instance_name to domain_state mapping	110		:returns: dictionary of instance_name to domain_state mapping
107	"""	111		"""
108	system_domains = _list_system_domains(connection)	112		domains = _list_domains(connection)
109	all_instances = _list_instances()	113		all_instances = _list_instances()
110		114
111	instances = []	115		instances = []
112		116
113	for instance in all_instances:	117		for instance in all_instances:
114	if instance['name'] not in system_domains.keys():	118		if instance['name'] not in domains.keys():
115	log.warn('{} is not registered, might want to delete it.'.format(instance['name']))	119		log.warn('{} is not registered, might want to delete it.'.format(instance['name']))
116	instance['state'] = 'de-sync'	120		instance['state'] = 'de-sync'
117		121
118	instances.append(instance)	122		instances.append(instance)
119		123
120	else:	124		else:
121		125
122	# Add the state of the instance	126		# Add the state of the instance
123	instance['state'] = system_domains[instance['name']]	127		instance['state'] = domains[instance['name']]
124		128
125	instances.append(instance)	129		instances.append(instance)
126		130
127	return instances	131		return instances
128		132
129		133
130	class Instance(object):	134		class Instance(object):
131	"""Handles creating, starting, stopping and removing virtual machines	135		"""Handles creating, starting, stopping and removing virtual machines
132	defined on the local system, using an existing :py:class:`Image`.	136		defined on the local system, using an existing :py:class:`Image`.
133	"""	137		"""
134		138
135	def __init__(self, name, image=None, hostname=None):	139		def __init__(self, name, image=None, connection='qemu:///system', hostname=None):
136	self.name = name	140		self.name = name
137	self.image = image	141		self.image = image
		142		self.connection = connection
138	self.path = "{}/instances/{}".format(config_data.DATA_DIR, self.name)	143		self.path = "{}/instances/{}".format(config_data.DATA_DIR, self.name)
139	self.seed_path = "{}/{}-seed.img".format(self.path, self.name)	144		self.seed_path = "{}/{}-seed.img".format(self.path, self.name)
140	self.meta_path = "{}/meta".format(self.path)	145		self.meta_path = "{}/meta".format(self.path)
141	self.local_disk = "{}/{}-local.qcow2".format(self.path, self.name)	146		self.local_disk = "{}/{}-local.qcow2".format(self.path, self.name)
142	self.xml_path = "{}/{}-domain.xml".format(self.path, self.name)	147		self.xml_path = "{}/{}-domain.xml".format(self.path, self.name)
143		148
144	self.ram = config_data.RAM	149		self.ram = config_data.RAM
145	# desired size of disk, in GiB	150		# desired size of disk, in GiB
▲ Show 20 Lines • Show All 136 Lines • ▼ Show 20 Line(s)		280	imgcreate_command = ['qemu-img',
282	]	287		]
283		288
284	# make sure to expand the resultant disk if the size is set	289		# make sure to expand the resultant disk if the size is set
285	if self.disk_size > 0:	290		if self.disk_size > 0:
286	imgcreate_command.append("{}G".format(self.disk_size))	291		imgcreate_command.append("{}G".format(self.disk_size))
287		292
288	subprocess.call(imgcreate_command)	293		subprocess.call(imgcreate_command)
289		294
290	def _get_domain(self, hypervisor="qemu:///system"):	295		def _get_domain(self):
291	"""Create the connection to libvirt to control instance lifecycle.	296		"""Create the connection to libvirt to control instance lifecycle.
292	returns: libvirt domain object"""	297		returns: libvirt domain object"""
293	conn = libvirt.open(hypervisor)	298		conn = libvirt.open(self.connection)
294	return conn.lookupByName(self.name)	299		return conn.lookupByName(self.name)
295		300
296	def create_ip_file(self, ip):	301		def create_ip_file(self, ip):
297	"""Write the ip address found after instance creation to a file	302		"""Write the ip address found after instance creation to a file
298	for easier management later. This is likely going to break	303		for easier management later. This is likely going to break
299	and need a better solution."""	304		and need a better solution."""
300		305
301	with open("{}/instances/{}/ip".format(config_data.DATA_DIR,	306		with open("{}/instances/{}/ip".format(config_data.DATA_DIR,
Show All 23 Lines		325	instance_values = {'domain_name': self.name,
325	'mac_address': util.generate_mac_address()}	330		'mac_address': util.generate_mac_address()}
326		331
327	# Write out the final xml file for the domain	332		# Write out the final xml file for the domain
328	with open(self.xml_path, 'w') as dom_template:	333		with open(self.xml_path, 'w') as dom_template:
329	dom_template.write(xml_template.render(instance_values))	334		dom_template.write(xml_template.render(instance_values))
330		335
331	return	336		return
332		337
333	def spawn_vm(self):	338		def spawn_vm(self):
			kparalUnsubmitted Not Done For all these methods in `Instance`, wouldn't it be easier to add the `connection` parameter to `__init__()`, instead of adding it to each instance method?
			lnieAuthorUnsubmitted Not Done yeah,we don't need to add it to each instance method.Actually,I typed it incidentally after I typed "connection":)
334	"""Create and boot the instance, using prepared data."""	339		"""Create and boot the instance, using prepared data."""
335		340
336	self.write_domain_xml()	341		self.write_domain_xml()
337		342
338	with open(self.xml_path, 'r') as xml_file:	343		with open(self.xml_path, 'r') as xml_file:
339	domain_xml = ''.join([x for x in xml_file.readlines()])	344		domain_xml = ''.join([x for x in xml_file.readlines()])
340		345
341	conn = libvirt.open('qemu:///system')	346		conn = libvirt.open(self.connection)
342	conn.defineXML(domain_xml)	347		conn.defineXML(domain_xml)
343		348
344	def expand_qcow(self, size="+10G"):	349		def expand_qcow(self, size="+10G"):
345	"""Expand the storage for a qcow image. Currently only used for Atomic	350		"""Expand the storage for a qcow image. Currently only used for Atomic
346	Hosts."""	351		Hosts."""
347		352
348	log.info("expanding qcow2 image {}".format(self.image_path))	353		log.info("expanding qcow2 image {}".format(self.image_path))
349	subprocess.call(['qemu-img',	354		subprocess.call(['qemu-img',
Show All 24 Lines		378	:param int timeout: number of seconds to wait before timing out.
374	Setting this to 0 will disable timeout, default	379		Setting this to 0 will disable timeout, default
375	is configured with :py:const:`BOOT_TIMEOUT` config	380		is configured with :py:const:`BOOT_TIMEOUT` config
376	value.	381		value.
377	:raises TestcloudInstanceError: if there is an error while creating the	382		:raises TestcloudInstanceError: if there is an error while creating the
378	instance or if the timeout is reached	383		instance or if the timeout is reached
379	while looking for a network interface	384		while looking for a network interface
380	"""	385		"""
381		386
382	log.debug("Creating domain {}".format(self.name))	387		log.debug("Creating instance {}".format(self.name))
383	dom = self._get_domain()	388		dom = self._get_domain()
384	create_status = dom.create()	389		create_status = dom.create()
385		390
386	# libvirt doesn't directly raise errors on boot failure, check the	391		# libvirt doesn't directly raise errors on boot failure, check the
387	# return code to verify that the boot process was successful from	392		# return code to verify that the boot process was successful from
388	# libvirt's POV	393		# libvirt's POV
389	if create_status != 0:	394		if create_status != 0:
390	raise TestcloudInstanceError("Instance {} did not start "	395		raise TestcloudInstanceError("Instance {} did not start "
391	"successfully, see libvirt logs for "	396		"successfully, see libvirt logs for "
392	"details".format(self.name))	397		"details".format(self.name))
			kparalUnsubmitted Not Done This does not make much sense. The fact that the domain can't be found means the connection is wrong, not that it was started already (in that case, it would have been found). I believe it only makes sense to print this kind of message for `remove()`, because we don't raise exceptions if we try to remove something that doesn't exist. For other cases (`stop()`, `start()`) the user already receives a "can't find" error.
393	log.debug("Polling domain for active network interface")	398		log.debug("Polling instance for active network interface")
394		399
395	poll_tick = 0.5	400		poll_tick = 0.5
396	timeout_ticks = timeout / poll_tick	401		timeout_ticks = timeout / poll_tick
397	count = 0	402		count = 0
398		403
399	# poll libvirt for domain interfaces, returning when an interface is	404		# poll libvirt for domain interfaces, returning when an interface is
400	# found, indicating that the boot process is post-cloud-init	405		# found, indicating that the boot process is post-cloud-init
401	while count <= timeout_ticks:	406		while count <= timeout_ticks:
Show All 13 Lines
415	def stop(self):	420		def stop(self):
416	"""Stop the instance	421		"""Stop the instance
417		422
418	:raises TestcloudInstanceError: if the instance does not exist	423		:raises TestcloudInstanceError: if the instance does not exist
419	"""	424		"""
420		425
421	log.debug("stopping instance {}.".format(self.name))	426		log.debug("stopping instance {}.".format(self.name))
422		427
423	system_domains = _list_system_domains("qemu:///system")	428		domains = _list_domains(self.connection)
424	domain_exists = self.name in system_domains	429		domain_exists = self.name in domains
425		430
426	if not domain_exists:	431		if not domain_exists:
427	raise TestcloudInstanceError(	432		raise TestcloudInstanceError("Instance doesn't exist: {}".format(self.name))
428	"Instance doesn't exist: {}".format(self.name))
429		433
430	if system_domains[self.name] == 'shutoff':	434		if domains[self.name] == 'shutoff':
431	log.debug('Instance already shut off, not stopping: {}'.format(	435		log.debug('Instance already shut off, not stopping: {}'.format(self.name))
432	self.name))
433	return	436		return
434		437
435	# stop (destroy) the vm	438		# stop (destroy) the vm
436	self._get_domain().destroy()	439		self._get_domain().destroy()
437		440
438	def remove(self, autostop=True):	441		def remove(self, autostop=True):
439	"""Remove an already stopped instance	442		"""Remove an already stopped instance
440		443
441	:param bool autostop: if the instance is running, stop it first	444		:param bool autostop: if the instance is running, stop it first
442	:raises TestcloudInstanceError: if the instance does not exist, or is still	445		:raises TestcloudInstanceError: if the instance does not exist, or is still
443	running and ``autostop==False``	446		running and ``autostop==False``
444	"""	447		"""
445		448
446	log.debug("removing instance {} from libvirt.".format(self.name))	449		log.debug("removing instance {} from libvirt.".format(self.name))
447		450
448	# this should be changed if/when we start supporting configurable	451		# this should be changed if/when we start supporting configurable
449	# libvirt connections	452		# libvirt connections
450	system_domains = _list_system_domains("qemu:///system")	453		domains = _list_domains(self.connection)
451
452	# Check that the domain is registered with libvirt	454		# Check that the domain is registered with libvirt
453	domain_exists = self.name in system_domains	455		domain_exists = self.name in domains
454	if domain_exists and system_domains[self.name] == 'running':	456		if domain_exists and domains[self.name] == 'running':
455		457
456	if autostop:	458		if autostop:
457	self.stop()	459		self.stop()
458	else:	460		else:
459	raise TestcloudInstanceError(	461		raise TestcloudInstanceError(
460	"Cannot remove running instance {}. Please stop the "	462		"Cannot remove running instance {}. Please stop the "
461	"instance before removing.".format(self.name))	463		"instance before removing.".format(self.name))
462		464
463	# remove from libvirt, assuming that it's stopped already	465		# remove from libvirt, assuming that it's stopped already
464	if domain_exists:	466		if domain_exists:
465	self._get_domain().undefine()	467		self._get_domain().undefine()
466	log.debug("Unregistering domain from libvirt.")	468		log.debug("Unregistering instance from libvirt.")
	kparalUnsubmitted Not Done Please add something like: else: log.warning('Domain "%s" not found in libvirt (%s). Was it removed already? Did you ' 'want to use a different libvirt connection?', self.name, self.connection) If the user runs `testcloud instance remove` with wrong libvirt connection, at least he/she will be notified that the domain was not found (and he/she needs to remove it manually).
		469		else:
		470		log.warn('Instance "{}" not found in libvirt "{}". Was it removed already? Should '
		471		'you have used a different connection?'.format(self.name, self.connection))
467		472
468	log.debug("removing instance {} from disk".format(self.path))	473		log.debug("removing instance {} from disk".format(self.path))
469		474
470	# remove from disk	475		# remove from disk
471	shutil.rmtree(self.path)	476		shutil.rmtree(self.path)
472		477
473	def destroy(self):	478		def destroy(self):
474	'''A deprecated method. Please call :meth:`remove` instead.'''	479		'''A deprecated method. Please call :meth:`remove` instead.'''
475		480
476	log.debug('DEPRECATED: destroy() method was deprecated. Please use remove()')	481		log.debug('DEPRECATED: destroy() method was deprecated. Please use remove()')
477	self.remove()	482		self.remove()

View Options

testcloud/util.py

Show All 15 Lines
16	import xml.etree.ElementTree as ET	16	import xml.etree.ElementTree as ET
17		17
18	from . import config	18	from . import config
19		19
20	log = logging.getLogger('testcloud.util')	20	log = logging.getLogger('testcloud.util')
21	config_data = config.get_config()	21	config_data = config.get_config()
22		22
23		23
24	def get_vm_xml(instance_name):	24	def get_vm_xml(instance_name, connection='qemu:///system'):
25	"""Query virsh for the xml of an instance by name."""	25	"""Query virsh for the xml of an instance by name."""
26		26
27	con = libvirt.openReadOnly('qemu:///system')	27	con = libvirt.openReadOnly(connection)
28	try:	28	try:
29	domain = con.lookupByName(instance_name)	29	domain = con.lookupByName(instance_name)
30		30
31	except libvirt.libvirtError:	31	except libvirt.libvirtError:
32	return None	32	return None
33		33
34	result = domain.XMLDesc()	34	result = domain.XMLDesc()
35		35
Show All 33 Lines

Modify the code to make the libvirt url actually configurableClosedPublicActions

Details

Diff Detail

Revision Contents

Diff 2487

testcloud/cli.py

testcloud/instance.py

testcloud/util.py

Modify the code to make the libvirt url actually configurable
ClosedPublic
Actions