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 built in policy to prevent transformed VM from starting. #17389

Merged
merged 2 commits into from
Jun 12, 2018

Conversation

lfu
Copy link
Member

@lfu lfu commented May 7, 2018

@lfu
Copy link
Member Author

lfu commented May 31, 2018

cc @agrare

@gmcculloug
Copy link
Member

gmcculloug commented May 31, 2018

@lfu My concern here is that as a built-in method there is no option to work around this if the VM is migrated. The intention is to stop someone for accidentally starting a migrated VM which this PR does achieves, but there may be reasons to start the source VM. For example after a successfully migration you may find a need to delete the newly created VM and restart the old one, or you may want to restart the old VM as part of testing.

This needs to discuss further before we can merge this change. It may have to change to be a manually applied policy or changed to use another check like a tag to override this logic.

cc @bthurber

app/models/vm.rb Outdated
@@ -4,6 +4,7 @@ class Vm < VmOrTemplate
has_one :container_deployment_node

virtual_has_one :supported_consoles, :class_name => "Hash"
virtual_column :transformed, :type => :boolean
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to expose this method? Can't the policy be written to check for the tag directly?

@@ -1070,3 +1070,20 @@
:parent_id: 0
:default: true
:single_value: "0"
- :description: V2V - Transformation Status
Copy link
Member

Choose a reason for hiding this comment

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

Drop the V2V from the naming. The point of using Transformation was to make it generic so it can be reused for other operations. P2V for example.

The existing "v2v" tags are specifically for v2v operations and will eventually be replaced with backend operations once we can properly support them with RHV inventory.

@lfu lfu force-pushed the migrated_policy_1565199 branch from 4e3336b to 7a46dd2 Compare June 6, 2018 17:46
@miq-bot
Copy link
Member

miq-bot commented Jun 6, 2018

Checked commits lfu/manageiq@75b03cb~...7a46dd2 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@gmcculloug gmcculloug merged commit 2252542 into ManageIQ:master Jun 12, 2018
simaishi pushed a commit that referenced this pull request Jun 12, 2018
Add built in policy to prevent transformed VM from starting.
(cherry picked from commit 2252542)

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

Gaprindashvili backport details:

$ git log -1
commit 0fc4d711ddbb90cdf31e6304cc597230ac9fbb69
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Tue Jun 12 08:57:52 2018 -0400

    Merge pull request #17389 from lfu/migrated_policy_1565199
    
    Add built in policy to prevent transformed VM from starting.
    (cherry picked from commit 2252542b171a135ab370977b7e0379abde4503c6)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1590430

@agrare agrare added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 15, 2018
@lfu lfu deleted the migrated_policy_1565199 branch September 29, 2018 14:30
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.

6 participants