Skip to content

Conversation

@kbrock
Copy link
Member

@kbrock kbrock commented Sep 29, 2025

Trying out various methods of not including every model in the Vm.all or MiqTemplate.all queries.

Dependes upon:

Before

vmdb(dev)> puts Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."type" IN ('Vm', 'VmServer',
  'ManageIQ::Providers::PhysicalInfraManager::Vm', 'ManageIQ::Providers::InfraManager::Vm',
  'ManageIQ::Providers::CloudManager::Vm', 'ManageIQ::Providers::CiscoIntersight::PhysicalInfraManager::Vm',
  'ManageIQ::Providers::Vmware::InfraManager::Vm', 'ManageIQ::Providers::Ovirt::InfraManager::Vm',
  'ManageIQ::Providers::Nutanix::InfraManager::Vm', 'ManageIQ::Providers::Kubevirt::InfraManager::Vm',
  'ManageIQ::Providers::IbmPowerHmc::InfraManager::Vm', 'ManageIQ::Providers::Redhat::InfraManager::Vm',
  'ManageIQ::Providers::Openshift::InfraManager::Vm', 'ManageIQ::Providers::IbmPowerHmc::InfraManager::Vios',
  'ManageIQ::Providers::IbmPowerHmc::InfraManager::Lpar', 'ManageIQ::Providers::Vmware::CloudManager::Vm',
  'ManageIQ::Providers::OracleCloud::CloudManager::Vm', 'ManageIQ::Providers::Openstack::CloudManager::Vm',
  'ManageIQ::Providers::IbmCloud::VPC::CloudManager::Vm', 'ManageIQ::Providers::IbmCloud::PowerVirtualServers::CloudManager::Vm',
  'ManageIQ::Providers::Google::CloudManager::Vm', 'ManageIQ::Providers::AzureStack::CloudManager::Vm',
  'ManageIQ::Providers::Azure::CloudManager::Vm', 'ManageIQ::Providers::Amazon::CloudManager::Vm',
  'ManageIQ::Providers::IbmPowerVc::CloudManager::Vm', 'ManageIQ::Providers::IbmCic::CloudManager::Vm')
  AND "vms"."template" = FALSE
=> nil
vmdb(dev)> puts ManageIQ::Providers::Vmware::InfraManager::Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."type" = 'ManageIQ::Providers::Vmware::InfraManager::Vm'
  AND "vms"."template" = FALSE

After

vmdb(dev)> puts Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."template" = FALSE
=> nil
vmdb(dev)> puts ManageIQ::Providers::Vmware::InfraManager::Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."type" = 'ManageIQ::Providers::Vmware::InfraManager::Vm'
  AND "vms"."template" = FALSE

Side Effect

Saving a Vm does not set Vm#type so it comes back as a generic VmOrTemplate.

@kbrock
Copy link
Member Author

kbrock commented Sep 29, 2025

[DELETED COMMENT] - I removed this noise and merged into the code
(You can see the text in comment history if you want to see the noise, but viewing the next compare is more useful)

@kbrock kbrock force-pushed the shorter_queries branch 3 times, most recently from 58cf199 to c058255 Compare September 29, 2025 16:55
@kbrock
Copy link
Member Author

kbrock commented Sep 29, 2025

update:

  • added default scope to the experiment
  • changed to using finder_needs_type_condition vs inheritance_column hack.

@kbrock
Copy link
Member Author

kbrock commented Sep 29, 2025

update:

  • removed template fixes (it is trivial and not really necessary. Worrying about the type column first)

@kbrock
Copy link
Member Author

kbrock commented Oct 2, 2025

FYI: I tried

class Vm
  self.abstract_class = true
end

type = Vm.first.type

type == "Vm"                                             # expected value
type == "ManageIQ::Providers::Vmware::InfraManager::Vm"` # actual   value

This may be due to the factory issue, but guessing not

@kbrock
Copy link
Member Author

kbrock commented Oct 3, 2025

update:

  • removed the changes to tests around generic factories.

@kbrock
Copy link
Member Author

kbrock commented Oct 3, 2025

WIP: It is having trouble instantiating a class of type Vm.

Think this needs to depend upon us changing the factories to not try and instantiate the theoretically abstract classes (technically, Vm is not abstract)

@kbrock kbrock changed the title [WIP] [POC] Don't include all types in Vm children [WIP] Don't include all types in Vm children Oct 3, 2025
@kbrock
Copy link
Member Author

kbrock commented Oct 3, 2025

update:

  • incorporated Jason's suggestion to cut this down significantly and remove the inheritance hack

update:

update:

Now that this code has simplified and we know why the tests were failing, this has changed from a pipe dream to me and is now something I would like.

WIP: only a wip to wait for commit 1 / factories to stop instantiating abstract classes.

Don't create entries in the database with parent abstract classes
Avoid Infra or Vm -- instead do a vendor specific vm
(would have preferred to use the dummy provider)
The parent class includes all classes (that are VMs)
No reason to include a `type` query.

Before
======

```
vmdb(dev)> puts Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."type" IN ('Vm', 'VmServer',
  'ManageIQ::Providers::PhysicalInfraManager::Vm', 'ManageIQ::Providers::InfraManager::Vm',
  'ManageIQ::Providers::CloudManager::Vm', 'ManageIQ::Providers::CiscoIntersight::PhysicalInfraManager::Vm',
  'ManageIQ::Providers::Vmware::InfraManager::Vm', 'ManageIQ::Providers::Ovirt::InfraManager::Vm',
  'ManageIQ::Providers::Nutanix::InfraManager::Vm', 'ManageIQ::Providers::Kubevirt::InfraManager::Vm',
  'ManageIQ::Providers::IbmPowerHmc::InfraManager::Vm', 'ManageIQ::Providers::Redhat::InfraManager::Vm',
  'ManageIQ::Providers::Openshift::InfraManager::Vm', 'ManageIQ::Providers::IbmPowerHmc::InfraManager::Vios',
  'ManageIQ::Providers::IbmPowerHmc::InfraManager::Lpar', 'ManageIQ::Providers::Vmware::CloudManager::Vm',
  'ManageIQ::Providers::OracleCloud::CloudManager::Vm', 'ManageIQ::Providers::Openstack::CloudManager::Vm',
  'ManageIQ::Providers::IbmCloud::VPC::CloudManager::Vm', 'ManageIQ::Providers::IbmCloud::PowerVirtualServers::CloudManager::Vm',
  'ManageIQ::Providers::Google::CloudManager::Vm', 'ManageIQ::Providers::AzureStack::CloudManager::Vm',
  'ManageIQ::Providers::Azure::CloudManager::Vm', 'ManageIQ::Providers::Amazon::CloudManager::Vm',
  'ManageIQ::Providers::IbmPowerVc::CloudManager::Vm', 'ManageIQ::Providers::IbmCic::CloudManager::Vm')
  AND "vms"."template" = FALSE
=> nil
vmdb(dev)> puts ManageIQ::Providers::Vmware::InfraManager::Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."type" = 'ManageIQ::Providers::Vmware::InfraManager::Vm'
  AND "vms"."template" = FALSE
```

After
=====

```
vmdb(dev)> puts Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."template" = FALSE
=> nil
vmdb(dev)> puts ManageIQ::Providers::Vmware::InfraManager::Vm.all.to_sql
SELECT "vms".* FROM "vms"
WHERE "vms"."type" = 'ManageIQ::Providers::Vmware::InfraManager::Vm'
  AND "vms"."template" = FALSE
```
@kbrock
Copy link
Member Author

kbrock commented Oct 7, 2025

update:

  • fixed miq_template (mimic vm logic using < instead of !=

update:

  • fixed vm (rubocop)

@Fryguy
Copy link
Member

Fryguy commented Oct 7, 2025

I'm curious why this depends on #23608. We're not actually making Vm and MiqTemplate abstract here.

@kbrock
Copy link
Member Author

kbrock commented Oct 7, 2025

@Fryguy yea. I thought it was a tangent, pulled it out and the tests started failing.

This change freaks out when you create a vm of type="Vm".

Think it was failing HERE
But I can remove the dependency and retry again

self notes

rspec ./spec/models/manageiq/providers/network_manager_spec.rb:8 
rspec ./spec/models/miq_report/generator_spec.rb:71 
rspec ./spec/models/mixins/ci_feature_mixin_spec.rb:4
rspec ./spec/models/mixins/relationship_mixin_spec.rb:437
rspec ./spec/models/mixins/relationship_mixin_spec.rb:446
rspec ./spec/models/mixins/relationship_mixin_spec.rb:408
rspec ./spec/models/mixins/relationship_mixin_spec.rb:419
rspec ./spec/models/vim_performance_state_spec.rb:139

@kbrock
Copy link
Member Author

kbrock commented Oct 7, 2025

@Fryguy ok, for a Vm, we stated that it doesn't need STI, so it does not set the type.

@Fryguy
Copy link
Member

Fryguy commented Oct 7, 2025

I see, so queries to the base VmOrTemplate fail?

@kbrock
Copy link
Member Author

kbrock commented Oct 10, 2025

Well, a query on VmOrTemplate will not tack on anything type or template related, so it will work.
But if your test creates a Vm or other, then it will not populate type - so those will fail tests. In production, I'm assuming we do not create Vm or VmInfra or ..providers::VmOrTemplate

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants