Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add libcloud support for image guest os features. #825

Closed
wants to merge 2 commits into from

Conversation

illfelder
Copy link
Contributor

@illfelder illfelder commented Jun 25, 2016

Image "guestOsFeature" support during image copy and creation.

Description

We are introducing a new optional property on the image resource known as "guestOsFeature". The feature is available in the alpha API and is needed to optimize virtual machine settings during instance creation. The only value currently supported (if specified) is "VIRTIO_SCSI_MULTIQUEUE".

Status

Done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@tonybaloney
Copy link
Contributor

cc @supertom @erjohnso

@@ -4752,29 +4753,31 @@ def ex_copy_image(self, name, url, description=None, family=None,
:param family: The family of the image
:type family: ``str``

:param guest_os_feature: The features of the guest opertaing system.
:type guest_os_feature: ``str``
:param guest_os_features: The features of the guest opertaing system.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"operating"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@supertom
Copy link
Contributor

LGTM, couple of spelling errors I noted.

if guest_os_features:
image_data['guestOsFeatures'] = []
for feature in guest_os_features:
if feature in possble_features:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that should be for feature in possible_features: and also showing this case needs a test :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, thanks for the note. I missed the typo (and failing test) when I refactored some of the code).

@tonybaloney
Copy link
Contributor

@illfelder 1 typo to correct and please also improve the test scenario to validate the outgoing payload.

Guest OS features are a repeated field in the API.
@illfelder
Copy link
Contributor Author

Added testing to validate the request is getting created properly. I also fixed a comment about the type of guest_os_features when creating an image.

@tonybaloney
Copy link
Contributor

@illfelder think you broke the build with the extra commit :-)

@illfelder
Copy link
Contributor Author

Yup, fixed the linter issue. Should be good now whenever travis next runs.

@tonybaloney
Copy link
Contributor

OK 👍

@asfgit asfgit closed this in 370c749 Jun 29, 2016
asfgit pushed a commit that referenced this pull request Jun 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants