Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reload stale source_tenant after destroying self (rails 5.2) #19367

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Oct 4, 2019

In rails 5.2, this stale source_tenant has a "source" CloudManager
during this "after_destroy" callback. This causes the
source_tenant.destroy to fail, thinking the "source" CloudManager still
exists.

Fixes:

  5) ManageIQ::Providers::CloudManager OpenStack CloudTenant Mapping #sync_cloud_tenants_with_tenants creation of tenant tree  cleans up created tenant tree when the ems is destroyed
     Failure/Error: expect(Tenant.count).to eq(1)

       expected: 1
            got: 2

       (compared using ==)
     # ./spec/models/manageiq/providers/cloud_manager_spec.rb:205:in `block (5 levels) in <top (required)>'

In rails 5.2, this stale source_tenant has a "source" CloudManager
during this "after_destroy" callback. This causes the
source_tenant.destroy to fail, thinking the "source" CloudManager still
exists.

Fixes:
  5) ManageIQ::Providers::CloudManager OpenStack CloudTenant Mapping #sync_cloud_tenants_with_tenants creation of tenant tree  cleans up created tenant tree when the ems is destroyed
     Failure/Error: expect(Tenant.count).to eq(1)

       expected: 1
            got: 2

       (compared using ==)
     # ./spec/models/manageiq/providers/cloud_manager_spec.rb:205:in `block (5 levels) in <top (required)>'
@miq-bot
Copy link
Member

miq-bot commented Oct 4, 2019

Checked commit jrafanie@c07a434 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -159,6 +159,8 @@ def self.display_name(number = 1)

def destroy_mapped_tenants
if source_tenant
# We just destroyed ourself, reload the source_tenant association
source_tenant.reload
source_tenant.all_subtenants.destroy_all
source_tenant.all_subprojects.destroy_all
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

darn - feels like this should all be destroyed in tenant itself

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agreed

@kbrock kbrock merged commit 953f27c into ManageIQ:master Oct 4, 2019
@kbrock kbrock added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 4, 2019
@jrafanie jrafanie deleted the reload_stale_association_after_destroying_self branch October 4, 2019 19:11
@jrafanie jrafanie restored the reload_stale_association_after_destroying_self branch October 4, 2019 19:53
@jrafanie jrafanie deleted the reload_stale_association_after_destroying_self branch October 4, 2019 19:54
@kbrock kbrock self-assigned this Nov 11, 2019
@chessbyte chessbyte mentioned this pull request Mar 31, 2020
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants