Merge lp:~anso/nova/rescue-mode into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

Adds rescue and unrescue commands.

Adds rescue and unrescue commands to api, compute, and the libvirt driver. Rescue mode simply reboots the instance with a rescue image as the first drive and the regular disk image as a second attached drive. Unrescue reboots the image back in normal mode.

A couple minor fixes to the handling of get_info to make it return a standard exception (NotFound) were also made to allow rescuing and rebooting of images that the hypervisor doesn't know about. Reboot was also fixed because it was passing the ec2_id instead of the internal_id to api/cloud.py

[lp:~anso/nova/rescue-mode]

287. By Vish Ishaya on 2010-10-25

logging.warn not raise logging.Warn

[Jay Pipes (jaypipes)]

Very nice work, Vish. Been through it all and can't see any issues at all.

review: Approve

[Devin Carlen (devcamcar)]

approve!

review: Approve

=== modified file 'nova/api/cloud.py'
--- nova/api/cloud.py	2010-10-14 07:30:42 +0000
+++ nova/api/cloud.py	2010-10-25 05:59:40 +0000
@@ -36,3 +36,23 @@
              db.queue_get_for(context, FLAGS.compute_topic, host),
              {"method": "reboot_instance",
               "args": {"instance_id": instance_ref['id']}})
+
+
+def rescue(instance_id, context):
+    """Rescue the given instance."""
+    instance_ref = db.instance_get_by_internal_id(context, instance_id)
+    host = instance_ref['host']
+    rpc.cast(context,
+             db.queue_get_for(context, FLAGS.compute_topic, host),
+             {"method": "rescue_instance",
+              "args": {"instance_id": instance_ref['id']}})
+
+
+def unrescue(instance_id, context):
+    """Unrescue the given instance."""
+    instance_ref = db.instance_get_by_internal_id(context, instance_id)
+    host = instance_ref['host']
+    rpc.cast(context,
+             db.queue_get_for(context, FLAGS.compute_topic, host),
+             {"method": "unrescue_instance",
+              "args": {"instance_id": instance_ref['id']}})

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2010-10-21 22:26:06 +0000
+++ nova/api/ec2/cloud.py	2010-10-25 05:59:40 +0000
@@ -932,8 +932,21 @@
 
     def reboot_instances(self, context, instance_id, **kwargs):
         """instance_id is a list of instance ids"""
-        for id_str in instance_id:
-            cloud.reboot(id_str, context=context)
+        for ec2_id in instance_id:
+            internal_id = ec2_id_to_internal_id(ec2_id)
+            cloud.reboot(internal_id, context=context)
+        return True
+
+    def rescue_instance(self, context, instance_id, **kwargs):
+        """This is an extension to the normal ec2_api"""
+        internal_id = ec2_id_to_internal_id(instance_id)
+        cloud.rescue(internal_id, context=context)
+        return True
+
+    def unrescue_instance(self, context, instance_id, **kwargs):
+        """This is an extension to the normal ec2_api"""
+        internal_id = ec2_id_to_internal_id(instance_id)
+        cloud.unrescue(internal_id, context=context)
         return True
 
     def update_instance(self, context, ec2_id, **kwargs):

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2010-10-14 06:26:58 +0000
+++ nova/compute/manager.py	2010-10-25 05:59:40 +0000
@@ -20,10 +20,8 @@
 Handles all code relating to instances (guest vms)
 """
 
-import base64
 import datetime
 import logging
-import os
 
 from twisted.internet import defer
 
@@ -59,7 +57,11 @@
         """Update the state of an instance from the driver info"""
         # FIXME(ja): include other fields from state?
         instance_ref = self.db.instance_get(context, instance_id)
-        state = self.driver.get_info(instance_ref.name)['state']
+        try:
+            info = self.driver.get_info(instance_ref['name'])
+            state = info['state']
+        except exception.NotFound:
+            state = power_state.NOSTATE
         self.db.instance_set_state(context, instance_id, state)
 
     @defer.inlineCallbacks
@@ -126,16 +128,15 @@
     def reboot_instance(self, context, instance_id):
         """Reboot an instance on this server."""
         context = context.elevated()
+        instance_ref = self.db.instance_get(context, instance_id)
         self._update_state(context, instance_id)
-        instance_ref = self.db.instance_get(context, instance_id)
 
         if instance_ref['state'] != power_state.RUNNING:
-            raise exception.Error(
-                    'trying to reboot a non-running'
-                    'instance: %s (state: %s excepted: %s)' %
-                    (instance_ref['internal_id'],
-                     instance_ref['state'],
-                     power_state.RUNNING))
+            logging.warn('trying to reboot a non-running '
+                         'instance: %s (state: %s excepted: %s)',
+                         instance_ref['internal_id'],
+                         instance_ref['state'],
+                         power_state.RUNNING)
 
         logging.debug('instance %s: rebooting', instance_ref['name'])
         self.db.instance_set_state(context,
@@ -145,6 +146,38 @@
         yield self.driver.reboot(instance_ref)
         self._update_state(context, instance_id)
 
+    @defer.inlineCallbacks
+    @exception.wrap_exception
+    def rescue_instance(self, context, instance_id):
+        """Rescue an instance on this server."""
+        context = context.elevated()
+        instance_ref = self.db.instance_get(context, instance_id)
+
+        logging.debug('instance %s: rescuing',
+                      instance_ref['internal_id'])
+        self.db.instance_set_state(context,
+                                   instance_id,
+                                   power_state.NOSTATE,
+                                   'rescuing')
+        yield self.driver.rescue(instance_ref)
+        self._update_state(context, instance_id)
+
+    @defer.inlineCallbacks
+    @exception.wrap_exception
+    def unrescue_instance(self, context, instance_id):
+        """Rescue an instance on this server."""
+        context = context.elevated()
+        instance_ref = self.db.instance_get(context, instance_id)
+
+        logging.debug('instance %s: unrescuing',
+                      instance_ref['internal_id'])
+        self.db.instance_set_state(context,
+                                   instance_id,
+                                   power_state.NOSTATE,
+                                   'unrescuing')
+        yield self.driver.unrescue(instance_ref)
+        self._update_state(context, instance_id)
+
     @exception.wrap_exception
     def get_console_output(self, context, instance_id):
         """Send the console output for an instance."""

=== modified file 'nova/tests/virt_unittest.py'
--- nova/tests/virt_unittest.py	2010-10-22 07:48:27 +0000
+++ nova/tests/virt_unittest.py	2010-10-25 05:59:40 +0000
@@ -91,7 +91,7 @@
             FLAGS.libvirt_type = libvirt_type
             conn = libvirt_conn.LibvirtConnection(True)
 
-            uri, template = conn.get_uri_and_template()
+            uri, _template, _rescue = conn.get_uri_and_templates()
             self.assertEquals(uri, expected_uri)
 
             xml = conn.to_xml(instance_ref)
@@ -114,7 +114,7 @@
         for (libvirt_type, (expected_uri, checks)) in type_uri_map.iteritems():
             FLAGS.libvirt_type = libvirt_type
             conn = libvirt_conn.LibvirtConnection(True)
-            uri, template = conn.get_uri_and_template()
+            uri, _template, _rescue = conn.get_uri_and_templates()
             self.assertEquals(uri, testuri)
 
     def tearDown(self):

=== modified file 'nova/virt/fake.py'
--- nova/virt/fake.py	2010-10-22 00:15:21 +0000
+++ nova/virt/fake.py	2010-10-25 05:59:40 +0000
@@ -22,10 +22,9 @@
 This module also documents the semantics of real hypervisor connections.
 """
 
-import logging
-
 from twisted.internet import defer
 
+from nova import exception
 from nova.compute import power_state
 
 
@@ -119,6 +118,18 @@
         """
         return defer.succeed(None)
 
+    def rescue(self, instance):
+        """
+        Rescue the specified instance.
+        """
+        return defer.succeed(None)
+
+    def unrescue(self, instance):
+        """
+        Unrescue the specified instance.
+        """
+        return defer.succeed(None)
+
     def destroy(self, instance):
         """
         Destroy (shutdown and delete) the specified instance.
@@ -148,7 +159,12 @@
         current memory the instance has, in KiB, 'num_cpu': The current number
         of virtual CPUs the instance has, 'cpu_time': The total CPU time used
         by the instance, in nanoseconds.
+
+        This method should raise exception.NotFound if the hypervisor has no
+        knowledge of the instance
         """
+        if instance_name not in self.instances:
+            raise exception.NotFound("Instance %s Not Found" % instance_name)
         i = self.instances[instance_name]
         return {'state': i._state,
                 'max_mem': 0,

=== added file 'nova/virt/libvirt.rescue.qemu.xml.template'
--- nova/virt/libvirt.rescue.qemu.xml.template	1970-01-01 00:00:00 +0000
+++ nova/virt/libvirt.rescue.qemu.xml.template	2010-10-25 05:59:40 +0000
@@ -0,0 +1,37 @@
+<domain type='%(type)s'>
+    <name>%(name)s</name>
+    <os>
+        <type>hvm</type>
+        <kernel>%(basepath)s/rescue-kernel</kernel>
+        <initrd>%(basepath)s/rescue-ramdisk</initrd>
+        <cmdline>root=/dev/vda1 console=ttyS0</cmdline>
+    </os>
+    <features>
+        <acpi/>
+    </features>
+    <memory>%(memory_kb)s</memory>
+    <vcpu>%(vcpus)s</vcpu>
+    <devices>
+        <disk type='file'>
+            <source file='%(basepath)s/rescue-disk'/>
+            <target dev='vda' bus='virtio'/>
+        </disk>
+        <disk type='file'>
+            <source file='%(basepath)s/disk'/>
+            <target dev='vdb' bus='virtio'/>
+        </disk>
+        <interface type='bridge'>
+            <source bridge='%(bridge_name)s'/>
+            <mac address='%(mac_address)s'/>
+            <!--   <model type='virtio'/>  CANT RUN virtio network right now -->
+            <filterref filter="nova-instance-%(name)s">
+                <parameter name="IP" value="%(ip_address)s" />
+                <parameter name="DHCPSERVER" value="%(dhcp_server)s" />
+            </filterref>
+        </interface>
+        <serial type="file">
+            <source path='%(basepath)s/console.log'/>
+            <target port='1'/>
+        </serial>
+    </devices>
+</domain>

=== added file 'nova/virt/libvirt.rescue.uml.xml.template'
--- nova/virt/libvirt.rescue.uml.xml.template	1970-01-01 00:00:00 +0000
+++ nova/virt/libvirt.rescue.uml.xml.template	2010-10-25 05:59:40 +0000
@@ -0,0 +1,26 @@
+<domain type='%(type)s'>
+    <name>%(name)s</name>
+    <memory>%(memory_kb)s</memory>
+    <os>
+        <type>%(type)s</type>
+        <kernel>/usr/bin/linux</kernel>
+        <root>/dev/ubda1</root>
+    </os>
+    <devices>
+        <disk type='file'>
+            <source file='%(basepath)s/rescue-disk'/>
+            <target dev='ubd0' bus='uml'/>
+        </disk>
+        <disk type='file'>
+            <source file='%(basepath)s/disk'/>
+            <target dev='ubd1' bus='uml'/>
+        </disk>
+        <interface type='bridge'>
+            <source bridge='%(bridge_name)s'/>
+            <mac address='%(mac_address)s'/>
+        </interface>
+        <console type="file">
+            <source path='%(basepath)s/console.log'/>
+        </console>
+    </devices>
+</domain>

=== added file 'nova/virt/libvirt.rescue.xen.xml.template'
--- nova/virt/libvirt.rescue.xen.xml.template	1970-01-01 00:00:00 +0000
+++ nova/virt/libvirt.rescue.xen.xml.template	2010-10-25 05:59:40 +0000
@@ -0,0 +1,34 @@
+<domain type='%(type)s'>
+    <name>%(name)s</name>
+    <os>
+        <type>linux</type>
+        <kernel>%(basepath)s/kernel</kernel>
+        <initrd>%(basepath)s/ramdisk</initrd>
+        <root>/dev/xvda1</root>
+        <cmdline>ro</cmdline>
+    </os>
+    <features>
+        <acpi/>
+    </features>
+    <memory>%(memory_kb)s</memory>
+    <vcpu>%(vcpus)s</vcpu>
+    <devices>
+        <disk type='file'>
+            <source file='%(basepath)s/rescue-disk'/>
+            <target dev='sda' />
+        </disk>
+        <disk type='file'>
+            <source file='%(basepath)s/disk'/>
+            <target dev='sdb' />
+        </disk>
+        <interface type='bridge'>
+            <source bridge='%(bridge_name)s'/>
+            <mac address='%(mac_address)s'/>
+        </interface>
+        <console type="file">
+            <source path='%(basepath)s/console.log'/>
+            <target port='1'/>
+        </console>
+    </devices>
+</domain>
+

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py	2010-10-22 00:15:21 +0000
+++ nova/virt/libvirt_conn.py	2010-10-25 05:59:40 +0000
@@ -48,6 +48,19 @@
 
 
 FLAGS = flags.FLAGS
+flags.DEFINE_string('libvirt_rescue_xml_template',
+                    utils.abspath('virt/libvirt.rescue.qemu.xml.template'),
+                    'Libvirt RESCUE XML Template for QEmu/KVM')
+flags.DEFINE_string('libvirt_rescue_xen_xml_template',
+                    utils.abspath('virt/libvirt.rescue.xen.xml.template'),
+                    'Libvirt RESCUE XML Template for xen')
+flags.DEFINE_string('libvirt_rescue_uml_xml_template',
+                    utils.abspath('virt/libvirt.rescue.uml.xml.template'),
+                    'Libvirt RESCUE XML Template for user-mode-linux')
+# TODO(vish): These flags should probably go into a shared location
+flags.DEFINE_string('rescue_image_id', 'ami-rescue', 'Rescue ami image')
+flags.DEFINE_string('rescue_kernel_id', 'aki-rescue', 'Rescue aki image')
+flags.DEFINE_string('rescue_ramdisk_id', 'ari-rescue', 'Rescue ari image')
 flags.DEFINE_string('libvirt_xml_template',
                     utils.abspath('virt/libvirt.qemu.xml.template'),
                     'Libvirt XML Template for QEmu/KVM')
@@ -87,9 +100,12 @@
 
 class LibvirtConnection(object):
     def __init__(self, read_only):
-        self.libvirt_uri, template_file = self.get_uri_and_template()
+        (self.libvirt_uri,
+         template_file,
+         rescue_file) = self.get_uri_and_templates()
 
         self.libvirt_xml = open(template_file).read()
+        self.rescue_xml = open(rescue_file).read()
         self._wrapped_conn = None
         self.read_only = read_only
 
@@ -112,17 +128,20 @@
                 return False
             raise
 
-    def get_uri_and_template(self):
+    def get_uri_and_templates(self):
         if FLAGS.libvirt_type == 'uml':
             uri = FLAGS.libvirt_uri or 'uml:///system'
             template_file = FLAGS.libvirt_uml_xml_template
+            rescue_file = FLAGS.libvirt_rescue_uml_xml_template
         elif FLAGS.libvirt_type == 'xen':
             uri = FLAGS.libvirt_uri or 'xen:///'
             template_file = FLAGS.libvirt_xen_xml_template
+            rescue_file = FLAGS.libvirt_rescue_xen_xml_template
         else:
             uri = FLAGS.libvirt_uri or 'qemu:///system'
             template_file = FLAGS.libvirt_xml_template
-        return uri, template_file
+            rescue_file = FLAGS.libvirt_rescue_xml_template
+        return uri, template_file, rescue_file
 
     def _connect(self, uri, read_only):
         auth = [[libvirt.VIR_CRED_AUTHNAME, libvirt.VIR_CRED_NOECHOPROMPT],
@@ -138,7 +157,7 @@
         return [self._conn.lookupByID(x).name()
                 for x in self._conn.listDomainsID()]
 
-    def destroy(self, instance):
+    def destroy(self, instance, cleanup=True):
         try:
             virt_dom = self._conn.lookupByName(instance['name'])
             virt_dom.destroy()
@@ -146,10 +165,11 @@
             pass
             # If the instance is already terminated, we're still happy
         d = defer.Deferred()
-        d.addCallback(lambda _: self._cleanup(instance))
+        if cleanup:
+            d.addCallback(lambda _: self._cleanup(instance))
         # FIXME: What does this comment mean?
         # TODO(termie): short-circuit me for tests
-       # WE'LL save this for when we do shutdown,
+        # WE'LL save this for when we do shutdown,
         # instead of destroy - but destroy returns immediately
         timer = task.LoopingCall(f=None)
 
@@ -199,8 +219,8 @@
     @defer.inlineCallbacks
     @exception.wrap_exception
     def reboot(self, instance):
+        yield self.destroy(instance, False)
         xml = self.to_xml(instance)
-        yield self._conn.lookupByName(instance['name']).destroy()
         yield self._conn.createXML(xml, 0)
 
         d = defer.Deferred()
@@ -229,6 +249,48 @@
 
     @defer.inlineCallbacks
     @exception.wrap_exception
+    def rescue(self, instance):
+        yield self.destroy(instance, False)
+
+        xml = self.to_xml(instance, rescue=True)
+        rescue_images = {'image_id': FLAGS.rescue_image_id,
+                         'kernel_id': FLAGS.rescue_kernel_id,
+                         'ramdisk_id': FLAGS.rescue_ramdisk_id}
+        yield self._create_image(instance, xml, 'rescue-', rescue_images)
+        yield self._conn.createXML(xml, 0)
+
+        d = defer.Deferred()
+        timer = task.LoopingCall(f=None)
+
+        def _wait_for_rescue():
+            try:
+                state = self.get_info(instance['name'])['state']
+                db.instance_set_state(None, instance['id'], state)
+                if state == power_state.RUNNING:
+                    logging.debug('instance %s: rescued', instance['name'])
+                    timer.stop()
+                    d.callback(None)
+            except Exception, exn:
+                logging.error('_wait_for_rescue failed: %s', exn)
+                db.instance_set_state(None,
+                                      instance['id'],
+                                      power_state.SHUTDOWN)
+                timer.stop()
+                d.callback(None)
+
+        timer.f = _wait_for_rescue
+        timer.start(interval=0.5, now=True)
+        yield d
+
+    @defer.inlineCallbacks
+    @exception.wrap_exception
+    def unrescue(self, instance):
+        # NOTE(vish): Because reboot destroys and recreates an instance using
+        #             the normal xml file, we can just call reboot here
+        yield self.reboot(instance)
+
+    @defer.inlineCallbacks
+    @exception.wrap_exception
     def spawn(self, instance):
         xml = self.to_xml(instance)
         db.instance_set_state(context.get_admin_context(),
@@ -239,8 +301,6 @@
               setup_nwfilters_for_instance(instance)
         yield self._create_image(instance, xml)
         yield self._conn.createXML(xml, 0)
-        # TODO(termie): this should actually register
-        # a callback to check for successful boot
         logging.debug("instance %s: is running", instance['name'])
 
         local_d = defer.Deferred()
@@ -311,15 +371,16 @@
         return d
 
     @defer.inlineCallbacks
-    def _create_image(self, inst, libvirt_xml):
+    def _create_image(self, inst, libvirt_xml, prefix='', disk_images=None):
         # syntactic nicety
-        basepath = lambda fname='': os.path.join(FLAGS.instances_path,
+        basepath = lambda fname='', prefix=prefix: os.path.join(
+                                                 FLAGS.instances_path,
                                                  inst['name'],
-                                                 fname)
+                                                 prefix + fname)
 
         # ensure directories exist and are writable
-        yield process.simple_execute('mkdir -p %s' % basepath())
-        yield process.simple_execute('chmod 0777 %s' % basepath())
+        yield process.simple_execute('mkdir -p %s' % basepath(prefix=''))
+        yield process.simple_execute('chmod 0777 %s' % basepath(prefix=''))
 
         # TODO(termie): these are blocking calls, it would be great
         #               if they weren't.
@@ -328,12 +389,17 @@
         f.write(libvirt_xml)
         f.close()
 
-        os.close(os.open(basepath('console.log'), os.O_CREAT | os.O_WRONLY,
-                 0660))
+        # NOTE(vish): No need add the prefix to console.log
+        os.close(os.open(basepath('console.log', ''),
+                         os.O_CREAT | os.O_WRONLY, 0660))
 
         user = manager.AuthManager().get_user(inst['user_id'])
         project = manager.AuthManager().get_project(inst['project_id'])
 
+        if not disk_images:
+            disk_images = {'image_id': inst['image_id'],
+                           'kernel_id': inst['kernel_id'],
+                           'ramdisk_id': inst['ramdisk_id']}
         if not os.path.exists(basepath('disk')):
             yield images.fetch(inst.image_id, basepath('disk-raw'), user,
                                project)
@@ -379,7 +445,9 @@
                                                     ['local_gb']
                                                     * 1024 * 1024 * 1024)
 
-        resize = inst['instance_type'] != 'm1.tiny'
+        resize = True
+        if inst['instance_type'] == 'm1.tiny' or prefix == 'rescue-':
+            resize = False
         yield disk.partition(basepath('disk-raw'), basepath('disk'),
                              local_bytes, resize, execute=execute)
 
@@ -387,7 +455,7 @@
             yield process.simple_execute('sudo chown root %s' %
                                          basepath('disk'))
 
-    def to_xml(self, instance):
+    def to_xml(self, instance, rescue=False):
         # TODO(termie): cache?
         logging.debug('instance %s: starting toXML method', instance['name'])
         network = db.project_get_network(context.get_admin_context(),
@@ -409,13 +477,19 @@
                     'mac_address': instance['mac_address'],
                     'ip_address': ip_address,
                     'dhcp_server': dhcp_server}
-        libvirt_xml = self.libvirt_xml % xml_info
+        if rescue:
+            libvirt_xml = self.rescue_xml % xml_info
+        else:
+            libvirt_xml = self.libvirt_xml % xml_info
         logging.debug('instance %s: finished toXML method', instance['name'])
 
         return libvirt_xml
 
     def get_info(self, instance_name):
-        virt_dom = self._conn.lookupByName(instance_name)
+        try:
+            virt_dom = self._conn.lookupByName(instance_name)
+        except:
+            raise exception.NotFound("Instance %s not found" % instance_name)
         (state, max_mem, mem, num_cpu, cpu_time) = virt_dom.info()
         return {'state': state,
                 'max_mem': max_mem,