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 snapshot support #296

Merged
merged 5 commits into from
Mar 2, 2017
Merged

Conversation

Sharpie
Copy link
Contributor

@Sharpie Sharpie commented Aug 1, 2016

This patchset adds provider actions required by the vagrant snapshot command introduced in Vagrant 1.8.0. The semantics of each action are modeled after the Virtualbox implementation:

https://github.com/mitchellh/vagrant/blob/v1.8.0/plugins/providers/virtualbox/action.rb#L230-L281

The actions make use of Vagrant UI strings introduced in 1.8.0 and are thus surrounded by guard statements to prevent them from being loaded by older Vagrant versions.

@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage decreased (-3.9%) to 87.685% when pulling 6339020 on Sharpie:add-snapshot-support into eb06809 on ggiamarchi:master.

@coveralls
Copy link

coveralls commented Aug 21, 2016

Coverage Status

Coverage decreased (-4.3%) to 87.362% when pulling 83dee17 on Sharpie:add-snapshot-support into eb06809 on ggiamarchi:master.

@coveralls
Copy link

coveralls commented Aug 22, 2016

Coverage Status

Coverage decreased (-3.8%) to 87.788% when pulling 599cf79 on Sharpie:add-snapshot-support into 569a7b8 on ggiamarchi:master.

@npwalker
Copy link
Contributor

npwalker commented Nov 2, 2016

This is great, I'd love to see this merged!

@ggiamarchi ggiamarchi added this to the 0.8 milestone Nov 14, 2016
Copy link
Owner

@ggiamarchi ggiamarchi left a comment

Choose a reason for hiding this comment

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

Expect the bug on the boolean value in metadata, everything looks good.

"#{@session.endpoints[:compute]}/servers/#{server_id}/action",
{ createImage: {
name: snapshot_name,
metadata: { vagrant_snapshot: true }
Copy link
Owner

Choose a reason for hiding this comment

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

It fails on vagrant snapshot save. The API respond a 400 error because of the boolean true. Replacing it by the string "true" fix it.

==> default: Snapshotting the machine as 'test-snap-01'...
2016-11-15 20:00 | DEBUG | block in create_snapshot - start
2016-11-15 20:00 | DEBUG | request  => method  : POST
2016-11-15 20:00 | DEBUG | request  => url     : http://openstack:8774/v2/f977fe8845974e64a643c9079bd40c3f/servers/86a4d73f-81e9-4ea4-bdf5-e359b1b16855/action
2016-11-15 20:00 | DEBUG | request  => headers : {"X-Auth-Token"=>"13ef9c8931974c0184c0bf62cc5f014b", :accept=>:json, :content_type=>:json}
2016-11-15 20:00 | DEBUG | request  => body    : {"createImage":{"name":"test-snap-01","metadata":{"vagrant_snapshot":true}}}
2016-11-15 20:00 | DEBUG | response => code    : 400
2016-11-15 20:00 | DEBUG | response => headers : {:content_length=>"137", :content_type=>"application/json; charset=UTF-8", :x_compute_request_id=>"req-a1a74fe0-fafe-486b-bc60-76984af9a0c2", :date=>"Tue, 15 Nov 2016 19:00:44 GMT"}
2016-11-15 20:00 | DEBUG | response => body    : {"badRequest": {"message": "Invalid input for field/attribute vagrant_snapshot. Value: True. True is not of type 'string'", "code": 400}}

@Sharpie Does it work in your tests ? Maybe an implementation difference on nova across different OpenStack versions. I ran my tests on an Canonical distribution based on Liberty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been running a patched provider that includes these changes against Kilo for a few months now. I don't have another OpenStack installation available to test against, but from the error message it seems like the API schema for metadata may have changed to allow only strings --- I'll see if I can track that down.

@Sharpie
Copy link
Contributor Author

Sharpie commented Dec 7, 2016

@ggiamarchi I've updated this patchset to correct the metadata issue and have been running it successfully against OpenStack Mitaka for a couple weeks now.

@ggiamarchi ggiamarchi removed this from the 0.9 milestone Jan 30, 2017
@coveralls
Copy link

coveralls commented Feb 14, 2017

Coverage Status

Coverage decreased (-3.6%) to 87.6% when pulling 12ab906 on Sharpie:add-snapshot-support into 876ad73 on ggiamarchi:master.

@npwalker
Copy link
Contributor

@ggiamarchi I created my own gem with @Sharpie's changes and I'm using it successfully. Is there anything preventing a merge?

@ggiamarchi ggiamarchi merged commit d91b1cb into ggiamarchi:master Mar 2, 2017
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.

4 participants