metadata injected at nova boot does not arrive in /meta.js

[Matthew Gilliard]

The command:

   nova boot --flavor $FLAV --key_name $KEY --image $IMG --meta foo=bar meta1

should inject a file into `/meta.js` with content `{"foo":"bar"}`. Currently in devstack this doesn't work.

It looks as if the data is arriving to n-cpu as:

    metadata={u'foo': u'bar'}

But n-cpu is expecting:

    metadata = [{"key": "foo", "value": "bar"}]

[Matthew Gilliard (matthew-gilliard-u)]

Devstack version used: 0df99e2d4ba70eb9669df6e18fb97ed37313c582

[Leandro Ignacio Costantino (leandro-i-costantino)]

Are you using config drive?
In that case the data seems to be available by mounting /dev/sr0 on the instance. [ if using iso9660]
(actually lot of data !)

[Loganathan Parthipan (parthipan)]

Can reproduce this from devstack: nova (4e4369151ef8a4423c7e9381a337474e0bd5611c)

2014-02-26 16:31:22.113 DEBUG nova.virt.disk.api [req-a8240f12-b8b2-4b3d-9e9f-6021a7155bc4 demo demo] Inject metadata fs=<nova.virt.disk.vfs.localfs.VFSLocalFS object at 0x50dfb50> metadata={u'parthi': u'yes'} _inject_metadata_into_fs /opt/stack/nova/nova/virt/disk/api.py:473
2014-02-26 16:31:22.113 WARNING nova.virt.disk.api [req-a8240f12-b8b2-4b3d-9e9f-6021a7155bc4 demo demo] Ignoring error injecting metadata into image (string indices must be integers)

[Loganathan Parthipan (parthipan)]

I checked out an older version and it worked. (037e9622fa8610cef9490d27adfa07c404d62f5a)

I'll do a git bisect later to find the offending commit.

[John Garbutt (johngarbutt)]

Seems like an API regression

[Loganathan Parthipan (parthipan)]

# first bad commit: [fbd214e267e493352040e74fa8f8d84b5ceadadb] Convert nova.compute.manager's _spawn to objects

[Loganathan Parthipan (parthipan)]

I think it's the object_compat decorator

[Loganathan Parthipan (parthipan)]

+ @object_compat
     def _spawn(self, context, instance, image_meta, network_info,
                block_device_info, injected_files, admin_password,
                set_access_ip=False):
         """Spawn an instance with error logging and update its power state."""

[Matthew Gilliard (matthew-gilliard-u)]

Parthi's analysis is correct - the @object_compat decorator transforms a dict with:

u'metadata': [{u'instance_uuid': u'686b0697-61a2-4f84-8c91-0c2d200e8339', u'deleted': 0, u'created_at': u'2014-02-27T13:19:28.000000', u'updated_at': None, u'value': u'bar', u'key': u'foo', u'deleted_at': None, u'id': 14}]

into an object with metadata property:

{u'foo': u'bar'}

  I assume that in other cases this transformation is desired (waiting for Nikola Dipanov to comment as I think he was the original author of that part of the decorator) - so I think the best fix might be a flag to @object_compat which tells it to leave the metadata alone.

[Matthew Gilliard (matthew-gilliard-u)]

... and to pass that flag in from the _spawn method only.

[Nikola Đipanov (ndipanov)]

So the first issue is that object_compat is really meant to be used on RPC exposed methods - it can technically be used there.

The way I see it - once you move the code around spawn to use the instance objects - you HAVE to change all the driver's spawn methods to deal with the new data format, even if that means moving for a list of dicts, to a dict.

>I assume that in other cases this transformation is desired (waiting for Nikola Dipanov to comment as I think he was the original author of that part of the decorator) - so I think the best fix might be a flag to @object_compat which tells it to leave the metadata alone.

So in my mind the above is not goof- we need to fix this i the libvirt (and other drviers) that use the medatadata in any way. Metadata should be a dict. I can't seem to find the code that used to changes it to be a list - but it's wrong.

[Matthew Gilliard (matthew-gilliard-u)]

I still don't understand _why_ the shape of the metadata property changes when using objects instead of a dict.

Do you think that a patch fixing this would need to change the spawn method in every driver? Or separate patches for each one? And I think a tempest test would be helpful - each part is unit-tested but as a whole it doesn't work - do you agree?

[Nikola Đipanov (ndipanov)]

IMHO the object is doing the right thing. I was unable to track down what code makes the list yesterday, and will try to dig more today.

As for the number of patches - I'd say it depends on the size, but afaict xen is the only other driver that tries to inject metadata into the root disk, and it will handle both formats (which is precisely what libvirt should try to do as well - see nova.virt.xenapi.vmops.VMOps._inject_instance_metadata).

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/77166

[Loganathan Parthipan (parthipan)]

To me it looks like the conversion that happens in disk:api:_inject_metadata_into_fs should have been removed when object_compat in compute manager was unwrapping the metadata. Xen would also get the metadata after the object_compat converts it, so IMHO it might not break Xen.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/77166
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=aaf5762be5d37cac022dc321b6400b9743a25303
Submitter: Jenkins
Branch: master

commit aaf5762be5d37cac022dc321b6400b9743a25303
Author: Matthew Gilliard <email address hidden>
Date: Fri Feb 28 12:08:20 2014 +0000

    Libvirt: Repair metadata injection into guests

    The change to use Instance objects when spawning instances
    changed the format of the 'metadata' property. But this wasn't
    taken into account in the code which actually injects metadata
    into /meta.js in the guest's fs.

    This patch ensures that the meta.js file injection expects the
    new format.

    Closes-bug: #1284733

    Change-Id: I37049ac01b65d30d588578e351fcddac457f95a7