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.

Sign up for GitHub

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

Jump to bottom

Disconnect storage when disconnecting a VM #472

Merged
merged 1 commit into from
Nov 28, 2018
Merged

Disconnect storage when disconnecting a VM #472

merged 1 commit into from
Nov 28, 2018

Conversation

agrare
Copy link
Member

@agrare agrare commented Nov 14, 2018 •
edited
Loading

Instead of calling disconnect_storage separately from the targeted
refresh, disconnect the storage when disconnect_inv is called from the
targeted refresh.

Depends: ManageIQ/manageiq#18200

https://bugzilla.redhat.com/show_bug.cgi?id=1644770

@agrare
Disconnect storage when disconnecting a VM
0fee98b 
Instead of calling disconnect_storage separately from the targeted
refresh, disconnect the storage when disconnect_inv is called from the
targeted refresh.

https://bugzilla.redhat.com/show_bug.cgi?id=1644770
@agrare agrare mentioned this pull request Nov 14, 2018
Merged
@miq-bot
Copy link
Member

miq-bot commented Nov 14, 2018

Checked commit agrare@0fee98b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@coveralls
Copy link

coveralls commented Nov 14, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 2346

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.273%
Totals Coverage Status
Change from base Build 2341: 0.0%
Covered Lines: 2925
Relevant Lines: 3007
💛 - Coveralls

@gmcculloug gmcculloug self-assigned this Nov 14, 2018
@gmcculloug gmcculloug requested a review from lfu November 14, 2018 18:40
@agrare agrare changed the title Disconnect storage when disconnecting a VM [WIP] Disconnect storage when disconnecting a VM Nov 14, 2018
@miq-bot miq-bot added the wip label Nov 14, 2018
@lfu
Copy link
Member

lfu commented Nov 14, 2018

I like the new approach 👍

@Fryguy
Copy link
Member

Fryguy commented Nov 15, 2018 •
edited
Loading

@agrare Do you also want to remove https://github.com/ManageIQ/manageiq-automation_engine/search?q=src_vm_disconnect_storage&unscoped_q=src_vm_disconnect_storage and https://github.com/ManageIQ/manageiq-automation_engine/search?q=miq_src_vm_disconnect_storage&unscoped_q=miq_src_vm_disconnect_storage

@agrare
Copy link
Member Author

agrare commented Nov 15, 2018

I didn't want to delete anything that could have been used by customer automate scripts but if @lfu and @gmcculloug think they can be deleted I'm all for removing them

@agrare agrare changed the title [WIP] Disconnect storage when disconnecting a VM Disconnect storage when disconnecting a VM Nov 15, 2018
@miq-bot miq-bot removed the wip label Nov 15, 2018
@lfu
Copy link
Member

lfu commented Nov 15, 2018

Those methods can be removed from automate along with https://github.com/ManageIQ/manageiq/search?q=src_vm_disconnect_storage&unscoped_q=src_vm_disconnect_storage.

Should we remove them in a separate PR? @gmcculloug

@gmcculloug
Copy link
Member

The concern is always that a user could have copied these event instances to a custom domain and we would have no control over that.

I would be in favor of removing it on master, but I'm thinking maybe we need to keep the automate entry-point as a no-op for older branches. Thoughts?

@agrare
Copy link
Member Author

agrare commented Nov 15, 2018

works for me

@agrare
Copy link
Member Author

agrare commented Nov 15, 2018

Okay create a PR to automate_engine ManageIQ/manageiq-automation_engine#272 and deleted the method from ManageIQ/manageiq@0a529d7

@gmcculloug
Copy link
Member

@agrare According to your comments on ManageIQ/manageiq#18200 (comment) this PR should be merged after ManageIQ/manageiq-providers-vmware#336. Please let us know when the VMware PR is merged.

@gmcculloug gmcculloug mentioned this pull request Nov 16, 2018
Merged
@JPrause
Copy link
Member

JPrause commented Nov 19, 2018

@miq-bot add_label blocker

@JPrause
Copy link
Member

JPrause commented Nov 26, 2018

@agrare was there anything else needed before this can be merged.

@agrare
Copy link
Member Author

agrare commented Nov 26, 2018

@JPrause ManageIQ/manageiq#18200 needs to be merged

@Fryguy Fryguy merged commit 6032cc1 into ManageIQ:master Nov 28, 2018
@Fryguy Fryguy added this to the Sprint 100 Ending Dec 3, 2018 milestone Nov 28, 2018
@agrare agrare deleted the bz_1644770_disconnect_storage_when_disconnecting_vms branch November 28, 2018 15:48
simaishi pushed a commit that referenced this pull request Nov 28, 2018
@Fryguy @simaishi
Merge pull request #472 from agrare/bz_1644770_disconnect_storage_whe…
bd8fa53 
…n_disconnecting_vms

Disconnect storage when disconnecting a VM

(cherry picked from commit 6032cc1)

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

Gaprindashvili backport details:

$ git log -1
commit bd8fa5379ad1ba877aac5e3ab4f7668d839cd7a2
Author: Jason Frey <[email protected]>
Date:   Wed Nov 28 10:32:54 2018 -0500

    Merge pull request #472 from agrare/bz_1644770_disconnect_storage_when_disconnecting_vms
    
    Disconnect storage when disconnecting a VM
    
    (cherry picked from commit 6032cc1a9861563d800893a22ca7583a2e776076)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1654436

simaishi pushed a commit that referenced this pull request Nov 30, 2018
@Fryguy @simaishi
Merge pull request #472 from agrare/bz_1644770_disconnect_storage_whe…
dd7ae82 
…n_disconnecting_vms

Disconnect storage when disconnecting a VM

(cherry picked from commit 6032cc1)

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

Hammer backport details:

$ git log -1
commit dd7ae8254c258f6e638ba05027304bc24a579fd6
Author: Jason Frey <[email protected]>
Date:   Wed Nov 28 10:32:54 2018 -0500

    Merge pull request #472 from agrare/bz_1644770_disconnect_storage_when_disconnecting_vms
    
    Disconnect storage when disconnecting a VM
    
    (cherry picked from commit 6032cc1a9861563d800893a22ca7583a2e776076)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1644770

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lfu lfu lfu approved these changes

Assignees

@gmcculloug gmcculloug

Labels
blocker bug events gaprindashvili/backported hammer/backported providers/virtual
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@agrare @miq-bot @coveralls @lfu @Fryguy @gmcculloug @JPrause @simaishi