Skip to content

Commit

Permalink
xenapi: make the xenapi agent optional per image
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JohnGarbutt committed May 27, 2013
1 parent e06ab58 commit 0357b01
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 18 deletions.
14 changes: 8 additions & 6 deletions nova/compute/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@
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.
Expand Down Expand Up @@ -928,9 +930,10 @@ def _populate_instance_for_create(self, base_options, image,
# 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:
Expand Down Expand Up @@ -1626,11 +1629,10 @@ def _create_image(self, context, instance, name, image_type,
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:
Expand Down Expand Up @@ -1829,12 +1831,12 @@ def _reset_image_metadata():
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
Expand Down
4 changes: 3 additions & 1 deletion nova/tests/virt/xenapi/imageupload/test_glance.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,12 @@ def fake_get_sr_path(*_args, **_kwargs):
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,
Expand Down
51 changes: 51 additions & 0 deletions nova/tests/virt/xenapi/test_agent.py
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 2 additions & 0 deletions nova/tests/virt/xenapi/test_xenapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,7 @@ def test_spawn_with_network_qos(self):
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):
Expand Down Expand Up @@ -999,6 +1000,7 @@ def noop(*args, **kwargs):

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):
Expand Down
34 changes: 31 additions & 3 deletions nova/virt/xenapi/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,18 @@
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 = [
Expand All @@ -54,9 +59,17 @@
' 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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions nova/virt/xenapi/imageupload/glance.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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__)
Expand All @@ -44,6 +45,10 @@ def upload_image(self, context, session, instance, vdi_uuids, image_id):
'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,
Expand Down
18 changes: 10 additions & 8 deletions nova/virt/xenapi/vmops.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,14 @@ def __init__(self, session, virtapi):
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"))
Expand Down Expand Up @@ -649,7 +651,7 @@ def _boot_new_instance(self, instance, vm_ref, injected_files,

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:
Expand Down Expand Up @@ -1053,7 +1055,7 @@ def reboot(self, instance, reboot_type, bad_volumes_callback=None):

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)
Expand All @@ -1062,7 +1064,7 @@ def set_admin_password(self, instance, new_pass):

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)
Expand Down Expand Up @@ -1561,7 +1563,7 @@ def unplug_vifs(self, instance, network_info):

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

0 comments on commit 0357b01

Please sign in to comment.