Merge "xenapi: make the xenapi agent optional per image"

7 files changed, 110 insertions, 18 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index eccf13da6..939b64778 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -108,6 +108,8 @@ MAX_USERDATA_SIZE = 65535
 QUOTAS = quota.QUOTAS
 RO_SECURITY_GROUPS = ['default']
 
+SM_IMAGE_PROP_PREFIX = "image_"
+
 
 def check_instance_state(vm_state=None, task_state=(None,)):
     """Decorator to check VM and/or task state before entry to API functions.
@@ -928,9 +930,10 @@ class API(base.Base):
         # Store image properties so we can use them later
         # (for notifications, etc).  Only store what we can.
         instance.setdefault('system_metadata', {})
+        prefix_format = SM_IMAGE_PROP_PREFIX + '%s'
         for key, value in image_properties.iteritems():
             new_value = str(value)[:255]
-            instance['system_metadata']['image_%s' % key] = new_value
+            instance['system_metadata'][prefix_format % key] = new_value
 
         # Keep a record of the original base image that this
         # image's instance is derived from:
@@ -1626,11 +1629,10 @@ class API(base.Base):
         properties.update(extra_properties or {})
 
         # Now inherit image properties from the base image
-        prefix = 'image_'
         for key, value in system_meta.items():
             # Trim off the image_ prefix
-            if key.startswith(prefix):
-                key = key[len(prefix):]
+            if key.startswith(SM_IMAGE_PROP_PREFIX):
+                key = key[len(SM_IMAGE_PROP_PREFIX):]
 
             # Skip properties that are non-inheritable
             if key in CONF.non_inheritable_image_properties:
@@ -1829,12 +1831,12 @@ class API(base.Base):
             orig_sys_metadata = dict(sys_metadata)
             # Remove the old keys
             for key in sys_metadata.keys():
-                if key.startswith('image_'):
+                if key.startswith(SM_IMAGE_PROP_PREFIX):
                     del sys_metadata[key]
             # Add the new ones
             for key, value in image.get('properties', {}).iteritems():
                 new_value = str(value)[:255]
-                sys_metadata['image_%s' % key] = new_value
+                sys_metadata[(SM_IMAGE_PROP_PREFIX + '%s') % key] = new_value
             self.db.instance_system_metadata_update(context,
                     instance['uuid'], sys_metadata, True)
             return orig_sys_metadata
diff --git a/nova/tests/virt/xenapi/imageupload/test_glance.py b/nova/tests/virt/xenapi/imageupload/test_glance.py
index 34e95ab80..7b6fbd43d 100644
--- a/nova/tests/virt/xenapi/imageupload/test_glance.py
+++ b/nova/tests/virt/xenapi/imageupload/test_glance.py
@@ -48,10 +48,12 @@ class TestGlanceStore(test.TestCase):
         properties = {
             'auto_disk_config': True,
             'os_type': 'default',
+            'xenapi_use_agent': 'true',
         }
         image_id = 'fake_image_uuid'
         vdi_uuids = ['fake_vdi_uuid']
-        instance = {'uuid': 'blah'}
+        instance = {'uuid': 'blah',
+                    'system_metadata': {'image_xenapi_use_agent': 'true'}}
         instance.update(properties)
 
         params = {'vdi_uuids': vdi_uuids,
diff --git a/nova/tests/virt/xenapi/test_agent.py b/nova/tests/virt/xenapi/test_agent.py
new file mode 100644
index 000000000..9a4d7c345
--- /dev/null
+++ b/nova/tests/virt/xenapi/test_agent.py
@@ -0,0 +1,51 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 OpenStack Foundation
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+from nova import test
+from nova.virt.xenapi import agent
+
+
+class AgentEnabledCase(test.TestCase):
+    def test_agent_is_present(self):
+        self.flags(xenapi_use_agent_default=False)
+        instance = {"system_metadata":
+            {"image_xenapi_use_agent": "true"}}
+        self.assertTrue(agent.should_use_agent(instance))
+
+    def test_agent_is_disabled(self):
+        self.flags(xenapi_use_agent_default=True)
+        instance = {"system_metadata":
+            {"image_xenapi_use_agent": "false"}}
+        self.assertFalse(agent.should_use_agent(instance))
+
+    def test_agent_uses_deafault_when_prop_invalid(self):
+        self.flags(xenapi_use_agent_default=True)
+        instance = {"system_metadata":
+            {"image_xenapi_use_agent": "bob"},
+            "uuid": "uuid"}
+        self.assertTrue(agent.should_use_agent(instance))
+
+    def test_agent_default_not_present(self):
+        self.flags(xenapi_use_agent_default=False)
+        instance = {"system_metadata": {}}
+        self.assertFalse(agent.should_use_agent(instance))
+
+    def test_agent_default_present(self):
+        self.flags(xenapi_use_agent_default=True)
+        instance = {"system_metadata": {}}
+        self.assertTrue(agent.should_use_agent(instance))
diff --git a/nova/tests/virt/xenapi/test_xenapi.py b/nova/tests/virt/xenapi/test_xenapi.py
index af2d97f67..538752cf2 100644
--- a/nova/tests/virt/xenapi/test_xenapi.py
+++ b/nova/tests/virt/xenapi/test_xenapi.py
@@ -972,6 +972,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase):
     def test_spawn_ssh_key_injection(self):
         # Test spawning with key_data on an instance.  Should use
         # agent file injection.
+        self.flags(xenapi_use_agent_default=True)
         actual_injected_files = []
 
         def fake_inject_file(self, method, args):
@@ -999,6 +1000,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase):
 
     def test_spawn_injected_files(self):
         # Test spawning with injected_files.
+        self.flags(xenapi_use_agent_default=True)
         actual_injected_files = []
 
         def fake_inject_file(self, method, args):
diff --git a/nova/virt/xenapi/agent.py b/nova/virt/xenapi/agent.py
index ea454ea3d..ecba96beb 100644
--- a/nova/virt/xenapi/agent.py
+++ b/nova/virt/xenapi/agent.py
@@ -24,13 +24,18 @@ import uuid
 from oslo.config import cfg
 
 from nova.api.metadata import password
+from nova.compute import api as compute_api
 from nova import context
 from nova import crypto
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
+from nova.openstack.common import strutils
 from nova import utils
 
 
+USE_AGENT_KEY = "xenapi_use_agent"
+USE_AGENT_SM_KEY = compute_api.SM_IMAGE_PROP_PREFIX + USE_AGENT_KEY
+
 LOG = logging.getLogger(__name__)
 
 xenapi_agent_opts = [
@@ -54,9 +59,17 @@ xenapi_agent_opts = [
                     ' flat_injected=True'),
     cfg.BoolOpt('xenapi_disable_agent',
                default=False,
-               help='Disable XenAPI agent. Reduces the amount of time '
-                    'it takes nova to detect that a VM has started, when '
-                    'that VM does not have the agent installed'),
+               help='Disables the use of the XenAPI agent in any image '
+                    'regardless of what image properties are present. '),
+    cfg.BoolOpt('xenapi_use_agent_default',
+               default=False,
+               help='Determines if the xenapi agent should be used when '
+                    'the image used does not contain a hint to declare if '
+                    'the agent is present or not. '
+                    'The hint is a glance property "' + USE_AGENT_KEY + '" '
+                    'that has the value "true" or "false". '
+                    'Note that waiting for the agent when it is not present '
+                    'will significantly increase server boot times.'),
 ]
 
 CONF = cfg.CONF
@@ -310,6 +323,21 @@ def find_guest_agent(base_dir):
     return False
 
 
+def should_use_agent(instance):
+    sys_meta = instance["system_metadata"]
+    if USE_AGENT_SM_KEY not in sys_meta:
+        return CONF.xenapi_use_agent_default
+    else:
+        use_agent_raw = sys_meta[USE_AGENT_SM_KEY]
+        try:
+            return strutils.bool_from_string(use_agent_raw, strict=True)
+        except ValueError:
+            LOG.warn(_("Invalid 'agent_present' value. "
+                       "Falling back to the default."),
+                       instance=instance)
+            return CONF.xenapi_use_agent_default
+
+
 class SimpleDH(object):
     """
     This class wraps all the functionality needed to implement
diff --git a/nova/virt/xenapi/imageupload/glance.py b/nova/virt/xenapi/imageupload/glance.py
index d306e06b0..b9b96d9a8 100644
--- a/nova/virt/xenapi/imageupload/glance.py
+++ b/nova/virt/xenapi/imageupload/glance.py
@@ -20,6 +20,7 @@ from oslo.config import cfg
 from nova import exception
 from nova.image import glance
 import nova.openstack.common.log as logging
+from nova.virt.xenapi import agent
 from nova.virt.xenapi import vm_utils
 
 LOG = logging.getLogger(__name__)
@@ -44,6 +45,10 @@ class GlanceStore(object):
             'os_type': instance['os_type'] or CONF.default_os_type,
         }
 
+        if agent.USE_AGENT_SM_KEY in instance["system_metadata"]:
+            properties[agent.USE_AGENT_KEY] = \
+                instance["system_metadata"][agent.USE_AGENT_SM_KEY]
+
         for attempt_num in xrange(1, max_attempts + 1):
 
             (glance_host,
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index 034556809..29e075655 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -176,12 +176,14 @@ class VMOps(object):
         self.image_upload_handler = importutils.import_object(
                                 CONF.xenapi_image_upload_handler)
 
-    @property
-    def agent_enabled(self):
-        return not CONF.xenapi_disable_agent
+    def agent_enabled(self, instance):
+        if CONF.xenapi_disable_agent:
+            return False
+
+        return xapi_agent.should_use_agent(instance)
 
     def _get_agent(self, instance, vm_ref):
-        if self.agent_enabled:
+        if self.agent_enabled(instance):
             return xapi_agent.XenAPIBasedAgent(self._session, self._virtapi,
                                                instance, vm_ref)
         raise exception.NovaException(_("Error: Agent is disabled"))
@@ -649,7 +651,7 @@ class VMOps(object):
 
             greenthread.sleep(0.5)
 
-        if self.agent_enabled:
+        if self.agent_enabled(instance):
             agent_build = self._virtapi.agent_build_get_by_triple(
                 ctx, 'xen', instance['os_type'], instance['architecture'])
             if agent_build:
@@ -1053,7 +1055,7 @@ class VMOps(object):
 
     def set_admin_password(self, instance, new_pass):
         """Set the root/admin password on the VM instance."""
-        if self.agent_enabled:
+        if self.agent_enabled(instance):
             vm_ref = self._get_vm_opaque_ref(instance)
             agent = self._get_agent(instance, vm_ref)
             agent.set_admin_password(new_pass)
@@ -1062,7 +1064,7 @@ class VMOps(object):
 
     def inject_file(self, instance, path, contents):
         """Write a file to the VM instance."""
-        if self.agent_enabled:
+        if self.agent_enabled(instance):
             vm_ref = self._get_vm_opaque_ref(instance)
             agent = self._get_agent(instance, vm_ref)
             agent.inject_file(path, contents)
@@ -1561,7 +1563,7 @@ class VMOps(object):
 
     def reset_network(self, instance):
         """Calls resetnetwork method in agent."""
-        if self.agent_enabled:
+        if self.agent_enabled(instance):
             vm_ref = self._get_vm_opaque_ref(instance)
             agent = self._get_agent(instance, vm_ref)
             agent.resetnetwork()