Merge lp://staging/~smoser/nova/lp845155 into lp://staging/~hudson-openstack/nova/trunk

Proposed by Scott Moser
Status: Merged
Approved by: Vish Ishaya
Approved revision: 1543
Merged at revision: 1576
Proposed branch: lp://staging/~smoser/nova/lp845155
Merge into: lp://staging/~hudson-openstack/nova/trunk
Diff against target: 33 lines (+6/-6)
1 file modified
nova/api/ec2/cloud.py (+6/-6)
To merge this branch: bzr merge lp://staging/~smoser/nova/lp845155
Reviewer Review Type Date Requested Status
Vish Ishaya (community) Approve
William Wolf (community) Approve
Review via email: mp+74705@code.staging.launchpad.net

Commit message

if no public-key is given (--key), do not show public-keys in metadata service

To post a comment you must log in.
lp://staging/~smoser/nova/lp845155 updated
1541. By Scott Moser

metadata key is 'public-keys', not 'keys'

1542. By Scott Moser

put key into meta-data, not top level 'data'

Revision history for this message
William Wolf (throughnothing) wrote :

Can a test be written to verify the old behavior in the bug, and prove that this fixes it?

Revision history for this message
William Wolf (throughnothing) :
review: Needs Information
Revision history for this message
Scott Moser (smoser) wrote :

I don't know if a test can be written or not. I've not played with nova's test suite. I can try that.
That said, I've verified this is now functioning correctly.

$ python -c 'import boto.utils, pprint; pprint.pprint(boto.utils.get_instance_metadata());'
{'ami-id': 'ami-0000002d',
 'ami-launch-index': '0',
 'ami-manifest-path': 'FIXME',
 'block-device-mapping': {'ami': 'vda', 'root': '/dev/vda'},
 'hostname': 'server-43',
 'instance-action': 'none',
 'instance-id': 'i-0000002b',
 'instance-type': 'm1.small',
 'local-hostname': 'server-43',
 'local-ipv4': '10.0.0.5',
 'mpi': {'None': '10.0.0.5 slots=1',
         'mykey': ['10.0.0.4 slots=1', '10.0.0.3 slots=1']},
 'placement': {'availability-zone': 'nova'},
 'public-hostname': 'server-43',
 'public-ipv4': '',
 'reservation-id': 'r-ilepn8ro',
 'security-groups': 'default'}

Note, there is no key. When launched with a key you'll still have the 'public-keys' there.

Revision history for this message
William Wolf (throughnothing) wrote :

I believe a test for this should go in nova/tests/test_cloud.py, but it seems that our ec2 api tests are pretty sparse and lacking.

Your fix looks good to me.

review: Approve
Revision history for this message
William Wolf (throughnothing) wrote :

Actually, you do have one pep8 error:

25 + # public-keys should only show up if it is non-empty (if user specified one)

That line needs to be 79 chars or less, so split that up, and then I think you're good.

review: Needs Fixing
lp://staging/~smoser/nova/lp845155 updated
1543. By Scott Moser

shorten comment to < 79 chars

Revision history for this message
Scott Moser (smoser) wrote :

fixed the line length issue.

Revision history for this message
William Wolf (throughnothing) wrote :

Thanks Scott.

review: Approve
Revision history for this message
Vish Ishaya (vishvananda) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.