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

Smart state Snapshot Managed Disk Name 80 Char Limit #157

Merged

Conversation

jerryk55
Copy link
Member

@jerryk55 jerryk55 commented Nov 3, 2017

Changes required since there is an 80 character limit on the name
of the snapshot for a managed disk -

  1. Use the VM name instead of the OS DIsk name.
  2. Shorten the suffix for the snapshot from 20 to 13 characters.
  3. If the above is still longer than 80 characters truncate the VM name to 80-13 characters.
  4. Always pass the snapshot name to MiqAzureVm so that it does not have to compute the name
    as well.

This PR goes hand-in-hand with ManageIQ/manageiq-smartstate#44 which receives the name of the snapshot to use when communicating with Azure. Both PRs fix the issue https://bugzilla.redhat.com/show_bug.cgi?id=1508577.
The two PRs can be merged in any order. The GEMfile for this provider must be updated after the smartstate gem is updated.

This PR should be back ported to both FINE and Gaprindashvili.

@roliveri @hsong-rh @bronaghs @djberg96 please review and merge. Thanks.

Changes required since there is an 80 character limit on the name
of the snapshot for a managed disk -
1) Use the VM name instead of the OS DIsk name.
2) Shorten the suffix for the snapshot from 20 to 13 characters.
3) If the above is still longer than 80 characters truncate the VM name to 80-13 characters.
4) Always pass the snapshot name to MiqAzureVm so that it does not have to compute the name
   as well.
@jerryk55
Copy link
Member Author

jerryk55 commented Nov 3, 2017

@miq-bot add_label fine/yes

@jerryk55
Copy link
Member Author

jerryk55 commented Nov 3, 2017

@miq-bot add_label gaprindashvili/yes

@jerryk55
Copy link
Member Author

jerryk55 commented Nov 3, 2017

@miq-bot add_label bug

@jerryk55
Copy link
Member Author

jerryk55 commented Nov 3, 2017

@miq-bot add_label "smart state"

@miq-bot
Copy link
Member

miq-bot commented Nov 3, 2017

@jerryk55 Cannot apply the following label because they are not recognized: "smart state"

@miq-bot miq-bot added the bug label Nov 3, 2017
@miq-bot
Copy link
Member

miq-bot commented Nov 3, 2017

Checked commit jerryk55@9a1b8d3 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@djberg96
Copy link
Collaborator

djberg96 commented Nov 3, 2017

Will we need to migrate existing data?

@jerryk55
Copy link
Member Author

jerryk55 commented Nov 3, 2017

@djberg96 no - the snapshot is removed after the Smartstate scan is finished. So any old names shouldn't exist anymore.

@djberg96 djberg96 added this to the Sprint 73 Ending Nov 13, 2017 milestone Nov 3, 2017
@djberg96
Copy link
Collaborator

djberg96 commented Nov 3, 2017

ok, LGTM. 👍

@djberg96 djberg96 merged commit e6d33b9 into ManageIQ:master Nov 3, 2017
simaishi pushed a commit that referenced this pull request Nov 7, 2017
…60chars

Smart state Snapshot Managed Disk Name 80 Char Limit
(cherry picked from commit e6d33b9)
@simaishi
Copy link
Contributor

simaishi commented Nov 7, 2017

Gaprindashvili backport details:

$ git log -1
commit 0709b6adff5ea2826b52522691c8fbb9ed3f82dc
Author: Daniel Berger <djberg96@gmail.com>
Date:   Fri Nov 3 11:37:15 2017 -0600

    Merge pull request #157 from jerryk55/snapshot_disknames_longer_than_60chars
    
    Smart state Snapshot Managed Disk Name 80 Char Limit
    (cherry picked from commit e6d33b9bec96a69f48e1a356dc2060765069a2a8)

simaishi pushed a commit that referenced this pull request Nov 14, 2017
…60chars

Smart state Snapshot Managed Disk Name 80 Char Limit
(cherry picked from commit e6d33b9)

https://bugzilla.redhat.com/show_bug.cgi?id=1512967
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit dd70691735386c387183f899e15d2ac47037ee3e
Author: Daniel Berger <djberg96@gmail.com>
Date:   Fri Nov 3 11:37:15 2017 -0600

    Merge pull request #157 from jerryk55/snapshot_disknames_longer_than_60chars
    
    Smart state Snapshot Managed Disk Name 80 Char Limit
    (cherry picked from commit e6d33b9bec96a69f48e1a356dc2060765069a2a8)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1512967

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.

None yet

4 participants