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

[GAPRINDASHVILI] Prevent queueing things for a zone that doesn't exist in the region #18068

Merged
merged 1 commit into from
Oct 10, 2018

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Oct 8, 2018

Prevent queueing things for a zone that doesn't exist in the region
https://bugzilla.redhat.com/show_bug.cgi?id=1635764

(cherry picked from commit 01c28d5)

Based on #17987 (comment)

Prevent queueing things for a zone that doesn't exist in the region
https://bugzilla.redhat.com/show_bug.cgi?id=1635764

(cherry picked from commit 01c28d5)
@jrafanie
Copy link
Member

jrafanie commented Oct 9, 2018

@bdunne I'm assuming this one will cause test failures on the other repos gaprindashvili branch after merge, much the way the original one did on master, right? I'm good with this, but it might require a few PRs on other repos to fix.

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

LGTM, see my comment above though.

@simaishi simaishi changed the title [GAPRINDASHVILI] Merge pull request #17987 from bdunne/ensure_zone_is_valid [GAPRINDASHVILI] Prevent queueing things for a zone that doesn't exist in the region Oct 9, 2018
@simaishi
Copy link
Contributor

simaishi commented Oct 9, 2018

@bdunne
Copy link
Member Author

bdunne commented Oct 10, 2018

@simaishi Yes, I think that's all of them

@simaishi simaishi merged commit de7af62 into ManageIQ:gaprindashvili Oct 10, 2018
@simaishi simaishi added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 10, 2018
@bdunne bdunne deleted the gap_17987 branch October 11, 2018 13:47
@cben
Copy link
Contributor

cben commented Nov 1, 2018

This backport also broke providers-kubernetes & providers-openshift.
(https://travis-ci.org/ManageIQ/manageiq-providers-openshift/builds/442300654
https://travis-ci.org/ManageIQ/manageiq-providers-kubernetes/builds/442299612)
TLDR: all this is already fixed on master, so just need to backport some test fixes.

Took me a long time to diagnose, because chains of rails hooks lead to really disconnected symptoms (like what does null value in column "created_at" violates not-null constraint have to do with this validation !?)
The chain definitely involved here is EMS -> Authentication -> after_save hook -> raise_evm_event;
in the container providers we also have the scary chain EMS -> Endpoint -> after_create/after_destroy hooks -> create/destroy a "child" EMS, suspect but not sure that's involved too...

Took me even longer to debug/fix. The root problem was the following stubbing all over kubernetes & openshift tests:

allow(MiqServer).to receive(:my_zone).and_return("default")

this was affecting the zone with which events were added to MiqQueue to be "default" but no such zone...

So all these stubs can be replaced with the more robust setup (which we already use in some tests):

EvmSpecHelper.local_miq_server(:zone => Zone.seed)

(done in ManageIQ/manageiq-providers-kubernetes@db9bdff but that's not the fix we'll use)

For some reason only then, knowing my_zone is the problem, I found the master PRs that fixed this:
ManageIQ/manageiq-providers-kubernetes#289
ManageIQ/manageiq-providers-kubernetes#297
ManageIQ/manageiq-providers-openshift#114
sending backport PRs now...

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.

4 participants