Enable provisioning from Volumes and Volume Snapshots via a proxy type #104

mansam · 2017-09-28T14:58:13Z

This pull request implements proxies for Volumes and Volume Snapshots as subclasses of MiqTemplate, allowing them to be selected during the ManageIQ provisioning workflow. Openstack will automatically create a volume of the appropriate size if a Snapshot is chosen as the boot source.

See also:
ManageIQ/manageiq#16066
ManageIQ/manageiq-ui-classic#2253

gmcculloug · 2017-10-04T14:36:48Z

@agrare Can you review or assign someone. Thanks

agrare · 2017-10-04T18:25:46Z

app/models/manageiq/providers/openstack/cloud_manager/volume_snapshot_template.rb

+
+  belongs_to :cloud_tenant
+
+  def volume_snapshot_template?


Where is this used? Better to just use .kind_of?(klass) I think

These are in line with image? and snapshot? which are present on the parent class and used for differentiating type in the UI.

👍 thanks

agrare · 2017-10-04T18:25:52Z

app/models/manageiq/providers/openstack/cloud_manager/volume_template.rb

+
+  belongs_to :cloud_tenant
+
+  def volume_template?


agrare · 2017-10-04T18:33:48Z

So I'm not a huge fan of creating a dummy miq_template record in refresh so that we can provision from it instead of changing the provision workflow to be able to use the volume as the source, @Fryguy thoughts?

Fryguy · 2017-10-09T16:28:28Z

spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb

@@ -296,9 +309,17 @@ def assert_ems
      expect(@ems.key_pairs.size).to          eq compute_data.key_pairs.count
      security_groups_count = @ems.security_groups.count { |x| x.name != 'default' }
      expect(security_groups_count).to        eq security_groups_count
-      expect(@ems.vms_and_templates.size).to  eq vms_count + images_count
+      if ::Settings.ems.ems_openstack.try(:refresh).try(:inventory_object_refresh) || ::Settings.ems.ems_refresh.try(:openstack).try(:inventory_object_refresh)


Alternately, fetch_path is usable on Settings as well, so this could be

Settings.ems.fetch_path(:ems_openstack, :refresh, :inventory_object_refresh) || Settings.ems.fetch_path(:ems_refresh, :openstack, :inventory_object_refresh)

On that note, why are there even 2 ways to configure things? Pick whichever one you want to be the "official" one (I suggest the former, so you can own the setting in your repo), and create a data migration to convert the other one.

Good to know fetch_path is available on Settings. There are two ways to configure things because the latter (incorrect) option was put in first and merged into a release. I put together a PR to support the correct way and the old way to avoid breaking anyone's deployment since I wasn't aware I could use a migration to change things over. Since the PR that implements the new openstack-owned approach wasn't merged yet, I should be able to implement a migration and get this all cleaned up. :)

Fryguy · 2017-10-09T16:30:47Z

app/models/manageiq/providers/openstack/inventory/collector/cloud_manager.rb

+  end
+
+  def volumes_by_id
+    @volumes_by_id ||= Hash[volume_templates.collect { |f| [f.id, f] }]


Alternately, you can use the index_by method.

@volumes_by_id ||= volume_templates.index_by(&:id)

Thanks for that shortcut!

Fryguy · 2017-10-09T16:34:04Z

@agrare I don't see it creating dummy data, unless I'm mistaken about how the collector works?

agrare · 2017-10-09T17:14:21Z

@Fryguy there already are CloudVolume and CloudVolumeSnapshot models, this also creates volumes as a MiqTemplate type instead of a CloudVolume type. So a duplicate record of another type is probably more correct than a dummy record.

mansam · 2017-10-16T17:39:28Z

@agrare @Fryguy other than the minor changes regarding volume_snapshot_template? and volume_template?, did you have any other thoughts? This feature is a requirement for 4.6 so I'd like to know what else needs to be done to get it in.

miq-bot · 2017-10-17T17:24:13Z

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

agrare · 2017-10-23T13:50:22Z

Given the requirements for provisioning I'm good with this, ideally we would be able to provision from things other than a template but I understand that would be a large undertaking.

@mansam if this is ready can you take it out of [WIP] and rebase to fix up the conflicts?

mansam · 2017-10-23T13:51:24Z

@agrare rebase in progress :)

Refresh will now populate VolumeTemplates, which are a subclass of MiqTemplate that allows for provisioning from Openstack volumes until a more general purpose refactoring of the provisioning workflow can be implemented.

miq-bot · 2017-10-23T16:24:39Z

Some comments on commits mansam/manageiq-providers-openstack@4220abd~...4f82edd

spec/models/manageiq/providers/openstack/cloud_manager/stubbed_refresher_spec.rb

⚠️ - 137 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
⚠️ - 138 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

miq-bot · 2017-10-23T16:24:41Z

Checked commits mansam/manageiq-providers-openstack@4220abd~...4f82edd with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
11 files checked, 3 offenses detected

spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb

❗ - Line 311, Col 39 - Style/ExtraSpacing - Unnecessary spacing detected.
❗ - Line 317, Col 31 - Style/ExtraSpacing - Unnecessary spacing detected.
❗ - Line 323, Col 42 - Style/ExtraSpacing - Unnecessary spacing detected.

mansam force-pushed the volume-snapshot-template-enable-provisioning-from-volume-snapshots branch from b214242 to 7d8cdb3 Compare September 28, 2017 15:00

mansam changed the title ~~Enable provisioning from Volumes and Volume Snapshots via a proxy type~~ [WIP] Enable provisioning from Volumes and Volume Snapshots via a proxy type Sep 28, 2017

mansam mentioned this pull request Sep 28, 2017

Allow Openstack provider Volumes and Snapshots to be eligible for provisioning. ManageIQ/manageiq#16066

Merged

miq-bot added the wip label Sep 28, 2017

mansam mentioned this pull request Sep 28, 2017

Update pre_prov view to support Volumes and Snapshots ManageIQ/manageiq-ui-classic#2253

Merged

agrare reviewed Oct 4, 2017

View reviewed changes

agrare requested a review from Fryguy October 4, 2017 18:33

agrare self-assigned this Oct 4, 2017

Fryguy reviewed Oct 9, 2017

View reviewed changes

miq-bot added the unmergeable label Oct 17, 2017

mansam added 8 commits October 23, 2017 10:38

Enable provisioning from volumes

4220abd

Refresh will now populate VolumeTemplates, which are a subclass of MiqTemplate that allows for provisioning from Openstack volumes until a more general purpose refactoring of the provisioning workflow can be implemented.

Fix tests

a1e1613

Support provisioning from volume snapshots

765cb03

Filter to get only snapshots of bootable volumes

f502b40

Disregard unbootable snapshots from specs

6332d9e

Fix lint

455465c

Clean up tests, remove reference to old configuration

2a5deeb

Update VCRs

4f82edd

mansam force-pushed the volume-snapshot-template-enable-provisioning-from-volume-snapshots branch from 07f7164 to 4f82edd Compare October 23, 2017 16:23

miq-bot removed the unmergeable label Oct 23, 2017

mansam changed the title ~~[WIP] Enable provisioning from Volumes and Volume Snapshots via a proxy type~~ Enable provisioning from Volumes and Volume Snapshots via a proxy type Oct 23, 2017

miq-bot removed the wip label Oct 23, 2017

agrare approved these changes Oct 23, 2017

View reviewed changes

agrare merged commit a356c02 into ManageIQ:master Oct 23, 2017

agrare added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 23, 2017

agrare added the enhancement label Oct 23, 2017

This was referenced Dec 4, 2017

[WIP] Fix Provisioning of disconnected VolumeTemplate ManageIQ/manageiq#16586

Closed

Fix Provisioning of disconnected VolumeTemplate #173

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable provisioning from Volumes and Volume Snapshots via a proxy type #104

Enable provisioning from Volumes and Volume Snapshots via a proxy type #104

mansam commented Sep 28, 2017 •

edited

Loading

gmcculloug commented Oct 4, 2017

agrare Oct 4, 2017

mansam Oct 23, 2017

agrare Oct 23, 2017

agrare Oct 4, 2017 •

edited

Loading

agrare commented Oct 4, 2017

Fryguy Oct 9, 2017 •

edited

Loading

mansam Oct 9, 2017

Fryguy Oct 9, 2017

mansam Oct 9, 2017

Fryguy commented Oct 9, 2017

agrare commented Oct 9, 2017

mansam commented Oct 16, 2017

miq-bot commented Oct 17, 2017

agrare commented Oct 23, 2017

mansam commented Oct 23, 2017

miq-bot commented Oct 23, 2017

miq-bot commented Oct 23, 2017

Enable provisioning from Volumes and Volume Snapshots via a proxy type #104

Enable provisioning from Volumes and Volume Snapshots via a proxy type #104

Conversation

mansam commented Sep 28, 2017 • edited Loading

gmcculloug commented Oct 4, 2017

agrare Oct 4, 2017

Choose a reason for hiding this comment

mansam Oct 23, 2017

Choose a reason for hiding this comment

agrare Oct 23, 2017

Choose a reason for hiding this comment

agrare Oct 4, 2017 • edited Loading

Choose a reason for hiding this comment

agrare commented Oct 4, 2017

Fryguy Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

mansam Oct 9, 2017

Choose a reason for hiding this comment

Fryguy Oct 9, 2017

Choose a reason for hiding this comment

mansam Oct 9, 2017

Choose a reason for hiding this comment

Fryguy commented Oct 9, 2017

agrare commented Oct 9, 2017

mansam commented Oct 16, 2017

miq-bot commented Oct 17, 2017

agrare commented Oct 23, 2017

mansam commented Oct 23, 2017

miq-bot commented Oct 23, 2017

miq-bot commented Oct 23, 2017

mansam commented Sep 28, 2017 •

edited

Loading

agrare Oct 4, 2017 •

edited

Loading

Fryguy Oct 9, 2017 •

edited

Loading