Collect customization_specs #614

agrare · 2020-08-18T19:06:33Z

The collection of customization_specs with streaming refresh was missed when moving from legacy refresh because there weren't any in the VCRs for the spec tests

TODO:

Record updated VCR cassettes
Update provisioning to convert Hash to VimHash CustomizationSpecInfo type

Follow-up:

Schema PR to un-VimHash-ify CustomizationSpec#spec Replace Vim* types in CustomizationSpec#spec manageiq-schema#508
Jansa-backport friendly fix for existing CustomizationSpec#spec with VimHashes CustomizationSpec#spec may contain VimHashes manageiq#20536

Fixes #611

agrare · 2020-08-18T19:11:59Z

app/models/manageiq/providers/vmware/infra_manager/inventory/parser.rb

@@ -377,4 +391,16 @@ def lazy_find_managed_object(managed_object)
    parent_collection = persister.vim_class_to_collection(managed_object)
    parent_collection.lazy_find(managed_object._ref)
  end
+
+  def rbvmomi_to_basic_types(obj)


Ideally we could just do obj.to_h and the RbVmomi classes would support this, going to work on an rbvmomi PR to add this

My change for this actually got merged but hasn't been released yet, I'll update this when a release is cut

agrare · 2020-08-20T15:25:47Z

app/models/manageiq/providers/vmware/infra_manager/provision/customization.rb

-      return cs
+
+      source.with_provider_connection do |vim|
+        vim.getVimCustomizationSpecManager.getCustomizationSpec(cs.name)&.spec


The CustomizationSpec has a number of properties where the VIM class matters (e.g.: CustomizationLinuxOptions vs CustomizationWinOptions) so we can't just use the standard hash stored in the DB

Previously this was a serialized VimHash which included the class names (we'll need a migration to clean these up, didn't realize that CustomizationSpec#spec was the full vim object)

Fetching the spec from the vcenter seemed to be the best option, other option is to add the VIM class name as a hash key when we convert from the native object to a hash in the parser as something like a :_xsiType key but the user would be able to modify those in the provision dialog unless we strip them out.

I'm open to other ideas though

Fryguy · 2020-08-21T21:29:36Z

@agrare Do you think we can get this in in time for jansa-1 release? I'd rather not have this regression in the release if we can avoid it.

cc @chessbyte @gtanzillo

Fryguy · 2020-08-21T21:31:35Z

app/models/manageiq/providers/vmware/infra_manager/inventory/collector.rb

+  def retrieve_customization_spec(spec_manager, cached_props)
+    cached_props[:info].to_a.each do |spec_info|
+      spec_info.props[:spec] = spec_manager.GetCustomizationSpec(:name => spec_info.name)&.spec
+    rescue RbVmomi::Fault => err


If the failure occur in the middle of the loop, does this move on or end the loop?

Also ewww N+1...There's no way to bulk ask this?

It would move on to the next one not skip, and no unfortunately not you can't get multiple customization scripts at once.

This was done here previously, the other option is to get the full spec at provision time which is closer to what the vsphere client does

app/models/manageiq/providers/vmware/infra_manager/inventory/parser.rb

The collection of customization_spces with streaming refresh was missed when moving from legacy refresh

miq-bot · 2020-09-10T14:46:00Z

Checked commits agrare/manageiq-providers-vmware@16d3f94~...f6ecfb0 with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
5 files checked, 0 offenses detected
Everything looks fine. 🏆

simaishi · 2020-09-24T18:55:03Z

@agrare backport conflicts on vcr cassettes as #591 is not in jansa branch. Not sure if you need to re-record for jansa?

agrare · 2020-09-28T12:58:53Z

If possible I'd like to keep the VCRs the same otherwise every PR we backport will conflict (aka OpenStack provider). I'll mark #591 as jansa/yes?

Collect customization_specs (cherry picked from commit 333b9d0)

simaishi · 2020-10-01T19:38:40Z

Jansa backport details:

$ git log -1
commit d4bda6fedda4c7ca62401bc02c2cdac0a6177b61
Author: Jason Frey <[email protected]>
Date:   Thu Sep 17 14:30:30 2020 -0400

    Merge pull request #614 from agrare/collect_customization_specs

    Collect customization_specs

    (cherry picked from commit 333b9d004e7e170398021631f7c9a1dc79a63a32)

Nikkurer

It seems to me that there is a typo. Please check.

Nikkurer · 2020-10-20T07:35:25Z

app/models/manageiq/providers/vmware/infra_manager/inventory/parser.rb

+    props[:info].to_a.each do |spec_info|
+      persister.customization_specs.build(
+        :name             => spec_info[:name],
+        :typ              => spec_info[:type],


I think it should be :type

:type is a reserved column name used by STI, :typ is correct here used to not conflict with STI.

Here is the database table definition: https://github.com/ManageIQ/manageiq-schema/blob/master/db/migrate/20130923182042_collapsed_initial_migration.rb#L303

agrare requested a review from Fryguy as a code owner August 18, 2020 19:06

agrare added bug wip labels Aug 18, 2020

agrare changed the title ~~Collect customization_specs~~ [WIP] Collect customization_specs Aug 18, 2020

agrare force-pushed the collect_customization_specs branch 2 times, most recently from 5e23239 to 8297e5e Compare August 18, 2020 19:10

agrare commented Aug 18, 2020

View reviewed changes

agrare force-pushed the collect_customization_specs branch 6 times, most recently from ffacb3e to 1ecbe28 Compare August 20, 2020 13:49

agrare commented Aug 20, 2020

View reviewed changes

agrare changed the title ~~[WIP] Collect customization_specs~~ Collect customization_specs Aug 20, 2020

miq-bot removed the wip label Aug 20, 2020

agrare assigned Fryguy Aug 20, 2020

Fryguy reviewed Aug 21, 2020

View reviewed changes

app/models/manageiq/providers/vmware/infra_manager/inventory/parser.rb Outdated Show resolved Hide resolved

agrare force-pushed the collect_customization_specs branch 3 times, most recently from 94f71d8 to 8f061a8 Compare August 26, 2020 22:43

Collect customization_specs

16d3f94

The collection of customization_spces with streaming refresh was missed when moving from legacy refresh

agrare force-pushed the collect_customization_specs branch 2 times, most recently from 59f660c to 2487628 Compare September 10, 2020 14:41

Fetch the customization spec when provisioning

f6ecfb0

agrare force-pushed the collect_customization_specs branch from 2487628 to f6ecfb0 Compare September 10, 2020 14:41

agrare requested a review from gtanzillo September 10, 2020 14:56

This was referenced Sep 10, 2020

Replace Vim* types in CustomizationSpec#spec ManageIQ/manageiq-schema#508

Merged

CustomizationSpec#spec may contain VimHashes ManageIQ/manageiq#20536

Merged

Fryguy merged commit 333b9d0 into ManageIQ:master Sep 17, 2020

agrare deleted the collect_customization_specs branch September 17, 2020 18:30

agrare added the jansa/yes? label Sep 17, 2020

gtanzillo added jansa/yes and removed jansa/yes? labels Sep 24, 2020

simaishi added the jansa/conflict label Sep 24, 2020

agrare mentioned this pull request Sep 28, 2020

Collect and parse the guest ipStack dns/route info #591

Merged

simaishi pushed a commit that referenced this pull request Oct 1, 2020

Merge pull request #614 from agrare/collect_customization_specs

d4bda6f

Collect customization_specs (cherry picked from commit 333b9d0)

simaishi added jansa/backported and removed jansa/conflict labels Oct 1, 2020

agrare mentioned this pull request Oct 19, 2020

[Jansa-1] Provisioning Virtual Machines : Custom Specifications not retrieved from provider ManageIQ/manageiq#20704

Closed

Nikkurer reviewed Oct 20, 2020

View reviewed changes

phospi mentioned this pull request Dec 3, 2020

VMware customization specifications not detected in Jansa-2 #674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collect customization_specs #614

Collect customization_specs #614

agrare commented Aug 18, 2020 •

edited

Loading

agrare Aug 18, 2020

agrare Dec 1, 2020

agrare Aug 20, 2020

Fryguy commented Aug 21, 2020

Fryguy Aug 21, 2020

Fryguy Aug 21, 2020

agrare Aug 21, 2020

agrare Aug 21, 2020 •

edited

Loading

miq-bot commented Sep 10, 2020

simaishi commented Sep 24, 2020

agrare commented Sep 28, 2020

simaishi commented Oct 1, 2020

Nikkurer left a comment

Nikkurer Oct 20, 2020

agrare Oct 20, 2020 •

edited

Loading

Collect customization_specs #614

Collect customization_specs #614

Conversation

agrare commented Aug 18, 2020 • edited Loading

agrare Aug 18, 2020

Choose a reason for hiding this comment

agrare Dec 1, 2020

Choose a reason for hiding this comment

agrare Aug 20, 2020

Choose a reason for hiding this comment

Fryguy commented Aug 21, 2020

Fryguy Aug 21, 2020

Choose a reason for hiding this comment

Fryguy Aug 21, 2020

Choose a reason for hiding this comment

agrare Aug 21, 2020

Choose a reason for hiding this comment

agrare Aug 21, 2020 • edited Loading

Choose a reason for hiding this comment

miq-bot commented Sep 10, 2020

simaishi commented Sep 24, 2020

agrare commented Sep 28, 2020

simaishi commented Oct 1, 2020

Nikkurer left a comment

Choose a reason for hiding this comment

Nikkurer Oct 20, 2020

Choose a reason for hiding this comment

agrare Oct 20, 2020 • edited Loading

Choose a reason for hiding this comment

agrare commented Aug 18, 2020 •

edited

Loading

agrare Aug 21, 2020 •

edited

Loading

agrare Oct 20, 2020 •

edited

Loading