xenapi: make the xenapi agent optional per image

This adds the ability to decide, per image, if xenapi should use
the agent for servers created from that image.
This opens up the path to using config drive or the metadata
service with cloud-init to perform tasks like file injection

It uses the image properties that are copied into system metadata
to detect if "xenapi_agent_present"="true" on the image the server
was created from.
If the tag is not present, it defaults to the value
of the new conf setting "xenapi_agent_present_default".

Becuase the above setting defaults to False, it means that
the xenapi driver no longer waits for the agent by default.

DocImpact
fixes bug 1178223
part of blueprint xenapi-guest-agent-cloud-init-interop
Change-Id: Ie51a9f54e5b2e85fe4ebebb0aff975db296ba996

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

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,

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))

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):

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

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,

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()