Add supports_conversion_host to Host #315

djberg96 · 2018-12-03T15:21:00Z

This sets supports :conversion_host to the Host model.

The main purpose for this is for the ManageIQ REST API, where support for conversion_host will be checked before attempting to enable or disable the resource.

Part of the V2V effort.

~~https://bugzilla.redhat.com/show_bug.cgi?id=1622728~~
https://bugzilla.redhat.com/show_bug.cgi?id=1695810

agrare · 2018-12-04T21:47:42Z

@djberg96 @fdupont-redhat @borod108 are there any requirements on a RHV host to be able to be a conversion host? I thought there was a specific version that it was added in.

ghost · 2018-12-04T23:25:16Z

@agrare Yes, the minimum version of RHV is 4.2.4. We check it in the enablement playbook. It would be good to check it here too, but I don't know how to check the hypervisor version.

agrare · 2018-12-04T23:26:59Z

@fdupont-redhat it should be ext_management_system.api_version

ghost · 2018-12-05T06:11:08Z

@agrare not necessarily. It is the API version of the manager, but the host may use an older version. I don't know how common it is, though. Today, the wrapper checks the version of vdsm and the minimum version is 4.20.31.

djberg96 · 2018-12-05T20:40:56Z

@agrare @fdupont-redhat Ok, I've updated it so that it must meet a minimum ems.api_version. For now that's set to 4.2.4, but can update to 4.2.31 if you prefer.

borod108

So won't going for the ems version create a bug when the host is of lower version?

djberg96 · 2018-12-06T04:36:37Z

@borod108 I can restore that code if you feel more comfortable with it. For our types of version strings it shouldn't be an issue, but I suppose it's best to avoid any risk.

djberg96 · 2018-12-06T04:43:24Z

@borod108 Well, it's potentially a bug either way I would think, so I'm open to suggestions.

ghost · 2018-12-12T06:22:13Z

LGTM

miq-bot · 2018-12-12T20:26:31Z

This pull request is not mergeable. Please rebase and repush.

djberg96 · 2018-12-18T15:57:35Z

@miq-bot remove_label unmergeable

Update supports_conversion_host to require a minimum EMS api_version.

miq-bot · 2018-12-18T17:52:12Z

Checked commit https://github.com/djberg96/manageiq-providers-ovirt/commit/c3aa70dd41451f7b973db7850dfb3229ec932a45 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. 🍰

agrare · 2018-12-18T18:18:10Z

@borod108 is there anyway to get the API version of the host from the rhevm api?

agrare · 2018-12-18T18:19:25Z

app/models/manageiq/providers/redhat/infra_manager/host.rb

@@ -1,4 +1,6 @@
 class ManageIQ::Providers::Redhat::InfraManager::Host < ::Host
+  MINIMUM_CONVERSION_HOST_VERSION = '4.2.4'.freeze


A constant just used in one place seems like overkill, think its fine to just use "4.2.4" below with a comment about this being the minimum supported version (thought the unsupported_reason_add is pretty self explanatory even)

Another option would be to put this in settings.yml, but I'm fine with whatever other folks prefer.

Even more overkill, don't think we'd ever want to allow that to be changed right? I was only added to rhev once.

I was thinking this would let people switch to 4.2.31 if desired, but if that's a bad idea, then I'll just hard code it.

Was it added in 4.2.31 or 4.2.4?

@agrare I was just trying to provide maximum flexibility as per @fdupont-redhat's comment at #315 (comment)

agrare · 2019-02-01T14:06:54Z

hey @djberg96 @borod108 what's the status of this?

djberg96 · 2019-02-04T20:52:20Z

@agrare I've removed the constant and just hard coded it as per your request. If there's a way to get it dynamically, I never discovered it. I would ask that we go with this for now, and update later if/when needed.

djberg96 · 2019-03-14T18:13:11Z

@miq-bot add_label hammer/yes

Add supports_conversion_host to Host (cherry picked from commit 37e9e07) https://bugzilla.redhat.com/show_bug.cgi?id=1694229

simaishi · 2019-03-29T20:36:13Z

Hammer backport details:

$ git log -1
commit 618cefad991ad7b3219a35a55bd0081fdef862e2
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Feb 5 13:15:21 2019 -0500

    Merge pull request #315 from djberg96/supports_conversion_host
    
    Add supports_conversion_host to Host
    
    (cherry picked from commit 37e9e07055d309134ca6d02cea305ea3fdfb3229)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1694229

This reverts commit 618cefa.

simaishi · 2019-04-01T15:28:24Z

Reverted the backport:

commit fbc2a4b7aff1a092736066899076d25099154c64
Author: Satoe Imaishi <simaishi@redhat.com>
Date:   Mon Apr 1 11:27:41 2019 -0400

    Revert "Merge pull request #315 from djberg96/supports_conversion_host"
    
    This reverts commit 618cefad991ad7b3219a35a55bd0081fdef862e2.

Add supports_conversion_host to Host (cherry picked from commit 37e9e07) https://bugzilla.redhat.com/show_bug.cgi?id=1702019

simaishi · 2019-04-22T18:26:15Z

Hammer backport details:

$ git log -1
commit 1ef3de0858048ebd02bc649c067db190cfd55803
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Feb 5 13:15:21 2019 -0500

    Merge pull request #315 from djberg96/supports_conversion_host
    
    Add supports_conversion_host to Host
    
    (cherry picked from commit 37e9e07055d309134ca6d02cea305ea3fdfb3229)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1702019

borod108 reviewed Dec 6, 2018

View reviewed changes

miq-bot added the unmergeable label Dec 12, 2018

miq-bot removed the unmergeable label Dec 18, 2018

Add supports_conversion_host to Host.

c3aa70d

Update supports_conversion_host to require a minimum EMS api_version.

agrare reviewed Dec 18, 2018

View reviewed changes

Remove the minimum version constant.

19d892c

agrare self-assigned this Feb 5, 2019

agrare approved these changes Feb 5, 2019

View reviewed changes

agrare merged commit 37e9e07 into ManageIQ:master Feb 5, 2019

djberg96 mentioned this pull request Feb 6, 2019

Change ConversionHost resource_type validation ManageIQ/manageiq#18434

Merged

miq-bot added the hammer/yes label Mar 14, 2019

simaishi pushed a commit that referenced this pull request Mar 29, 2019

Merge pull request #315 from djberg96/supports_conversion_host

618cefa

Add supports_conversion_host to Host (cherry picked from commit 37e9e07) https://bugzilla.redhat.com/show_bug.cgi?id=1694229

simaishi added hammer/backported and removed hammer/yes labels Mar 29, 2019

simaishi added a commit that referenced this pull request Apr 1, 2019

Revert "Merge pull request #315 from djberg96/supports_conversion_host"

fbc2a4b

This reverts commit 618cefa.

simaishi added hammer/yes and removed hammer/backported labels Apr 1, 2019

mturley mentioned this pull request Apr 5, 2019

Enable (un-hide) the Conversion Hosts tab of the Settings page ManageIQ/manageiq-v2v#922

Merged

simaishi mentioned this pull request Apr 22, 2019

[Changelog] Update for Hammer-5 release #358

Merged

simaishi pushed a commit that referenced this pull request Apr 22, 2019

Merge pull request #315 from djberg96/supports_conversion_host

1ef3de0

Add supports_conversion_host to Host (cherry picked from commit 37e9e07) https://bugzilla.redhat.com/show_bug.cgi?id=1702019

simaishi added hammer/backported and removed hammer/yes labels Apr 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add supports_conversion_host to Host #315

Add supports_conversion_host to Host #315

djberg96 commented Dec 3, 2018 •

edited

Loading

agrare commented Dec 4, 2018

ghost commented Dec 4, 2018

agrare commented Dec 4, 2018

ghost commented Dec 5, 2018

djberg96 commented Dec 5, 2018

borod108 left a comment

djberg96 commented Dec 6, 2018

djberg96 commented Dec 6, 2018

ghost commented Dec 12, 2018

miq-bot commented Dec 12, 2018

djberg96 commented Dec 18, 2018

miq-bot commented Dec 18, 2018

agrare commented Dec 18, 2018

agrare Dec 18, 2018

djberg96 Dec 18, 2018

agrare Dec 18, 2018

djberg96 Dec 18, 2018

agrare Dec 18, 2018

djberg96 Dec 18, 2018

agrare commented Feb 1, 2019

djberg96 commented Feb 4, 2019

djberg96 commented Mar 14, 2019

simaishi commented Mar 29, 2019

simaishi commented Apr 1, 2019

simaishi commented Apr 22, 2019

		@@ -1,4 +1,6 @@
		class ManageIQ::Providers::Redhat::InfraManager::Host < ::Host
		MINIMUM_CONVERSION_HOST_VERSION = '4.2.4'.freeze

Add supports_conversion_host to Host #315

Add supports_conversion_host to Host #315

Conversation

djberg96 commented Dec 3, 2018 • edited Loading

agrare commented Dec 4, 2018

ghost commented Dec 4, 2018

agrare commented Dec 4, 2018

ghost commented Dec 5, 2018

djberg96 commented Dec 5, 2018

borod108 left a comment

Choose a reason for hiding this comment

djberg96 commented Dec 6, 2018

djberg96 commented Dec 6, 2018

ghost commented Dec 12, 2018

miq-bot commented Dec 12, 2018

djberg96 commented Dec 18, 2018

miq-bot commented Dec 18, 2018

agrare commented Dec 18, 2018

agrare Dec 18, 2018

Choose a reason for hiding this comment

djberg96 Dec 18, 2018

Choose a reason for hiding this comment

agrare Dec 18, 2018

Choose a reason for hiding this comment

djberg96 Dec 18, 2018

Choose a reason for hiding this comment

agrare Dec 18, 2018

Choose a reason for hiding this comment

djberg96 Dec 18, 2018

Choose a reason for hiding this comment

agrare commented Feb 1, 2019

djberg96 commented Feb 4, 2019

djberg96 commented Mar 14, 2019

simaishi commented Mar 29, 2019

simaishi commented Apr 1, 2019

simaishi commented Apr 22, 2019

djberg96 commented Dec 3, 2018 •

edited

Loading