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

Snapshot Support for Non-Managed Disks #26

Merged
merged 3 commits into from
Sep 11, 2017

Conversation

jerryk55
Copy link
Member

@jerryk55 jerryk55 commented Sep 9, 2017

In the case of non-Managed (Blob) disks, append the snapshot
name (which is in all actuality the date tag returned from
azure-armrest for the snapshot) to the URI to be used to read
the disk.

This is one of several PRs required to fix https://bugzilla.redhat.com/show_bug.cgi?id=1463780.
The PR against ManageIQ/manageiq is ManageIQ/manageiq#15960, and one against ManageIQ/manageiq-providers-azure will be noted here when it is added.

This PR can be merged in any order since it checks for a tag to be passed in and only uses that snapshot tag if it is there.

@roliveri @hsong-rh please review. I'm not sure if we need to back port this to Fine but it wouldn't be a bad idea.

In the case of non-Managed (Blob) disks, append the snapshot
name (which is in all actuality the date tag returned from
azure-armrest for the snapshot) to the URI to be used to read
the disk.
Since the MiqAzureVm now expects a ResourceGroup Object rather than the
string representing its name, the tests need to be fixed to expect the
modified parameter.
#
@uri = os_disk.vhd.uri
@uri << "?snapshot=" << args[:snapshot] if args[:snapshot]
Copy link
Member

Choose a reason for hiding this comment

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

Is

@uri << "?snapshot=#{args[:snapshot]}" if args[:snapshot]

preferred?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure makes sense.

Change string concatenation statement for snapshot URI.
@miq-bot
Copy link
Member

miq-bot commented Sep 11, 2017

Checked commits jerryk55/manageiq-smartstate@eacb131~...b003c9d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 🏆

@jerryk55
Copy link
Member Author

@miq-bot add_label fine/yes

@miq-bot
Copy link
Member

miq-bot commented Sep 12, 2017

@jerryk55 Cannot apply the following label because they are not recognized: fine/yes

@simaishi
Copy link
Contributor

Fine backport (to manageiq-gems-pending repo) details:

$ git log -1
commit 3ff4035f4b8da8cd5798bd598587c46eb0d95484
Author: Richard Oliveri <[email protected]>
Date:   Mon Sep 11 16:05:31 2017 -0400

    Merge pull request #26 from jerryk55/snapshot_nonmanaged_disks
    
    Snapshot Support for Non-Managed Disks
    (cherry picked from commit 0c525af5d9a6f478193b182d3f5932df6c1a59e2)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1491310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants