Bug 1113398

Summary: Missing organization_id foreign key on a number of tables
Product: Red Hat Satellite Reporter: Partha Aji <paji>
Component: Content ManagementAssignee: Partha Aji <paji>
Status: CLOSED CURRENTRELEASE QA Contact: Katello QA List <katello-qa-list>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 6.0.3CC: bkearney, cwelton, jmontleo, sthirugn
Target Milestone: Unspecified   
Target Release: Unused   
Hardware: Unspecified   
OS: Unspecified   
URL: http://projects.theforeman.org/issues/4870
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-09-11 12:20:54 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Partha Aji 2014-06-26 06:12:21 UTC
There are no foreign keys for any organization_id columns in our database. This includes activation keys, environments, etc.

Comment 1 Partha Aji 2014-06-26 06:12:23 UTC
Created from redmine issue http://projects.theforeman.org/issues/4870

Comment 2 Partha Aji 2014-06-26 06:12:33 UTC
Upstream bug assigned to paji

Comment 4 sthirugn@redhat.com 2014-06-27 18:46:39 UTC
Last year we had a very hard time with foreign key changes in sam.  Is this really slotted for GA? or for future?

Comment 5 Partha Aji 2014-06-27 19:43:07 UTC
Suresh,
I did think about the pain we had last year having written a few of those nightmarish upgrade migration scripts and many of foreign key complaints were on upgrades. And most of those FK issues were because the code would remove one association and forget to remove the child associations. For example if I removed an environment and forgot to remove every place environments were FK'ed we would get a breakage. It was probably harder and more annoying for SAM because many of those tables in the DB were Katello specific and never used by SAM. But atleast we know that there was no dirty data in the DB related to this.

Now for the other side of this issue, if this bug had been merged earlier we would have not enncountered https://bugzilla.redhat.com/show_bug.cgi?id=1113684 where the satellite had repositories that basically pointed to non existent products (due to botched bulk product delete code). The FK'ed code would ve failed earlier and said "bulk delete of product failed"  and left data at a consistent state and not affected a totally different page.

Also we need to consider that this update adds only like 15 foreign keys. There are already like over 100 foreign keys being used in the present product. 

My vote is in favour of including it in the GA,

Comment 6 sthirugn@redhat.com 2014-06-27 19:52:04 UTC
(In reply to Partha Aji from comment #5)
> Suresh,
> I did think about the pain we had last year having written a few of those
> nightmarish upgrade migration scripts and many of foreign key complaints
> were on upgrades. And most of those FK issues were because the code would
> remove one association and forget to remove the child associations. For
> example if I removed an environment and forgot to remove every place
> environments were FK'ed we would get a breakage. It was probably harder and
> more annoying for SAM because many of those tables in the DB were Katello
> specific and never used by SAM. But atleast we know that there was no dirty
> data in the DB related to this.
> 
> Now for the other side of this issue, if this bug had been merged earlier we
> would have not enncountered
> https://bugzilla.redhat.com/show_bug.cgi?id=1113684 where the satellite had
> repositories that basically pointed to non existent products (due to botched
> bulk product delete code). The FK'ed code would ve failed earlier and said
> "bulk delete of product failed"  and left data at a consistent state and not
> affected a totally different page.
> 
> Also we need to consider that this update adds only like 15 foreign keys.
> There are already like over 100 foreign keys being used in the present
> product. 
> 
> My vote is in favour of including it in the GA,

@Partha: Ok, it makes sense to include this in GA.  Thank you for the explanation.

Comment 7 Bryan Kearney 2014-06-27 20:05:23 UTC
Moving to POST since upstream bug http://projects.theforeman.org/issues/4870 has been closed

Comment 9 Bryan Kearney 2014-09-11 12:20:54 UTC
This was delivered with Satellite 6.0 which was released on 10 September 2014.