Don't seed tenant product features for tenant from remote region #18286

lpichler · 2018-12-12T14:36:49Z

let's assume we replicated remote region to a global region,
then tenant records(from remote region) replicated to global region.

then MiqProductFeaure.seed is run on the global region - and it is causing creation of tenant product features also for tenants from remote region.

Note: Highlighted features are created on remote tenants which are hidden in tenant tree.

This PR stops creation of such product tenant features for remote tenants.

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1468795 (this PR is not last for the BZ, there is other bug)
#18100 - learn more about tenant product features
@miq-bot assign @gtanzillo

@miq-bot add_label hammer/yes, gaprindashvili/yes,blocker

lpichler · 2018-12-12T14:48:11Z

@carbonin small question, instances of Tenant(or generally any models) has to have always any region, right ?

carbonin · 2018-12-12T15:25:33Z

Every database object will belong to a region as it's calculated from the id value. Is that what you're asking @lpichler ?

lpichler · 2018-12-12T15:52:41Z

Every database object will belong to a region as it's calculated from the id value. Is that what you're asking @lpichler ?

yes, thanks!

carbonin · 2018-12-12T15:54:25Z

app/models/tenant.rb

@@ -292,6 +292,8 @@ def allowed?
  end

  def create_miq_product_features_for_tenant_nodes
+    return [] if !miq_region || MiqRegion.my_region_number != miq_region.region


I don't understand how this would solve the problem you're seeing. This method is only called by an after_create hook, right? That isn't triggered when rows are replicated from remote regions so I don't see how this method would ever be called on an instance from a remote region in the global.

carbonin · 2018-12-12T15:58:18Z

Are you sure this isn't being done in the MiqProductFeature.seed_tenant_miq_product_features method?

Possibly this line should be Tenant.in_my_region.map ...?

Disclaimer though ... I haven't looked through all of this and I definitely don't fully understand it fully.

carbonin · 2018-12-12T16:04:09Z

Or even better, maybe this should be in_my_region.to_a.index_by(&:identifier)?

gtanzillo · 2018-12-18T14:42:59Z

app/models/tenant.rb

@@ -292,6 +292,8 @@ def allowed?
  end

  def create_miq_product_features_for_tenant_nodes
+    return [] if !miq_region || MiqRegion.my_region_number != miq_region.region
+
    result = MiqProductFeature.with_tenant_feature_root_features.map.each do |tenant_miq_product_feature|


@lpichler I might not be understanding the issue fully but wouldn't the fix be to scope this query to the current region like this?

result = MiqProductFeature.in_my_region.with_tenant_feature_root_features.map.each do |tenant_miq_product_feature|

Then it would never involve product features from other regions

lpichler · 2018-12-19T14:26:07Z

@gtanzillo @carbonin thanks for leading me to correct fix! I had improper assumption about replication. (that is happening under SQL level)

I described it in description.

so let's imagine:
remote region (44):
tenant 1
tenant 2

global region 99:
tenant 1 - replicated id=44..
tenant 2 - replicated id=44..

and then when MiqProductFeature.seed is run it will also create tenant product features for tenant 1 and tenant 2 and these product tenant features are created in global region and they cannot be hidden by
MiqProductFeature.in_my_region because they were created under global region and they are tied to tenants from remote region.

Note:

Or even better, maybe this should be in_my_region.to_a.index_by(&:identifier)?
this is not helping for the issue - issue was about skipping tenants.

miq-bot · 2018-12-19T16:10:58Z

Checked commit lpichler@a4f052d 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. 🏆

carbonin · 2018-12-19T17:56:31Z

👍 Looks like a better approach. MiqProductFeatures are excluded by default so we shouldn't be looping through those from remote regions.

kbrock

You all came up with a great solution. nice teamwork

…atures_remote_tenants Don't seed tenant product features for tenant from remote region (cherry picked from commit 7909bf7) https://bugzilla.redhat.com/show_bug.cgi?id=1468795

simaishi · 2018-12-20T17:56:34Z

Hammer backport details:

$ git log -1
commit 9d4c1af4c2ff4844c621317e76e405acdce289b9
Author: Keenan Brock <[email protected]>
Date:   Thu Dec 20 11:40:02 2018 -0500

    Merge pull request #18286 from lpichler/dont_create_tenant_product_features_remote_tenants
    
    Don't seed tenant product features for tenant from remote region
    
    (cherry picked from commit 7909bf7f11a842f7b98d5e7fc539b6fc3da82de7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1468795

miq-bot assigned gtanzillo Dec 12, 2018

miq-bot added blocker gaprindashvili/yes wip labels Dec 12, 2018

lpichler force-pushed the dont_create_tenant_product_features_remote_tenants branch from 8050f21 to cfdc7b6 Compare December 12, 2018 14:44

lpichler changed the title ~~[WIP] Don't create tenant product feature in remote region~~ Don't create tenant product feature in remote region Dec 12, 2018

miq-bot removed the wip label Dec 12, 2018

lpichler force-pushed the dont_create_tenant_product_features_remote_tenants branch from cfdc7b6 to deeed92 Compare December 12, 2018 14:49

carbonin reviewed Dec 12, 2018

View reviewed changes

lpichler changed the title ~~Don't create tenant product feature in remote region~~ [WIP] Don't create tenant product feature in remote region Dec 13, 2018

miq-bot added the wip label Dec 13, 2018

gtanzillo reviewed Dec 18, 2018

View reviewed changes

lpichler changed the title ~~[WIP] Don't create tenant product feature in remote region~~ [WIP] Don't seed tenant product feature for tenant from remote region Dec 19, 2018

lpichler changed the title ~~[WIP] Don't seed tenant product feature for tenant from remote region~~ [WIP] Don't seed tenant product features for tenant from remote region Dec 19, 2018

lpichler force-pushed the dont_create_tenant_product_features_remote_tenants branch from deeed92 to c27f79a Compare December 19, 2018 14:12

lpichler changed the title ~~[WIP] Don't seed tenant product features for tenant from remote region~~ Don't seed tenant product features for tenant from remote region Dec 19, 2018

miq-bot removed the wip label Dec 19, 2018

lpichler force-pushed the dont_create_tenant_product_features_remote_tenants branch from c27f79a to f7a39d1 Compare December 19, 2018 14:29

Don't create tenant product features for tenants in remote region

a4f052d

lpichler force-pushed the dont_create_tenant_product_features_remote_tenants branch from f7a39d1 to a4f052d Compare December 19, 2018 16:09

kbrock approved these changes Dec 20, 2018

View reviewed changes

kbrock merged commit 7909bf7 into ManageIQ:master Dec 20, 2018

kbrock added this to the Sprint 102 Ending Jan 7, 2019 milestone Dec 20, 2018

kbrock assigned kbrock and unassigned gtanzillo Dec 20, 2018

lpichler deleted the dont_create_tenant_product_features_remote_tenants branch December 20, 2018 16:43

simaishi added hammer/backported and removed hammer/yes labels Dec 20, 2018

lpichler mentioned this pull request Dec 21, 2018

Dynamic product features according to tenants #18100

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't seed tenant product features for tenant from remote region #18286

Don't seed tenant product features for tenant from remote region #18286

lpichler commented Dec 12, 2018 •

edited

Loading

lpichler commented Dec 12, 2018

carbonin commented Dec 12, 2018

lpichler commented Dec 12, 2018

carbonin Dec 12, 2018

carbonin commented Dec 12, 2018 •

edited

Loading

carbonin commented Dec 12, 2018

gtanzillo Dec 18, 2018

lpichler commented Dec 19, 2018 •

edited

Loading

miq-bot commented Dec 19, 2018

carbonin commented Dec 19, 2018

kbrock left a comment

simaishi commented Dec 20, 2018

Don't seed tenant product features for tenant from remote region #18286

Don't seed tenant product features for tenant from remote region #18286

Conversation

lpichler commented Dec 12, 2018 • edited Loading

Links

lpichler commented Dec 12, 2018

carbonin commented Dec 12, 2018

lpichler commented Dec 12, 2018

carbonin Dec 12, 2018

Choose a reason for hiding this comment

carbonin commented Dec 12, 2018 • edited Loading

carbonin commented Dec 12, 2018

gtanzillo Dec 18, 2018

Choose a reason for hiding this comment

lpichler commented Dec 19, 2018 • edited Loading

miq-bot commented Dec 19, 2018

carbonin commented Dec 19, 2018

kbrock left a comment

Choose a reason for hiding this comment

simaishi commented Dec 20, 2018

lpichler commented Dec 12, 2018 •

edited

Loading

carbonin commented Dec 12, 2018 •

edited

Loading

lpichler commented Dec 19, 2018 •

edited

Loading