Bug 487316 - Alter Channel Subscriptions fails
Alter Channel Subscriptions fails
Status: CLOSED CURRENTRELEASE
Product: Red Hat Satellite 5
Classification: Red Hat
Component: WebUI (Show other bugs)
520
All Linux
medium Severity high
: ---
: ---
Assigned To: Jay Dobies
Michael Mráka
https://sputnik-prod.brq.redhat.com/
:
Depends On:
Blocks: 456985
  Show dependency treegraph
 
Reported: 2009-02-25 08:50 EST by Jan Sarenik
Modified: 2009-09-10 16:33 EDT (History)
5 users (show)

See Also:
Fixed In Version: sat530
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-10 16:33:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Satellite traceback (6.03 KB, text/plain)
2009-02-25 08:50 EST, Jan Sarenik
no flags Details

  None (edit)
Description Jan Sarenik 2009-02-25 08:50:17 EST
Created attachment 333160 [details]
Satellite traceback

On RHN Satellite 5.2.0 running at https://sputnik-prod.brq.redhat.com

When I want to add a channel subscription for a RHEL4 system
(via Systems -> Overview -> <system>* -> Alter Channel Descriptions),
I get this error message:

-----------------------------------------------------------------------
 error  lookup.default.title

This error may have occurred in one of three ways:

   1. lookup.default.reason1
   2. lookup.default.reason2
   3. You've found an error in our site. Please help us by filling
      out this form with details of how you received this message.
-----------------------------------------------------------------------

 * in my particular case the system was 'jasan-chroot-rhel4'

Traceback (got from mzazrive@redhat.com) included as an attachment.
Comment 1 Miroslav Suchý 2009-02-26 04:10:11 EST
AFAIK it happend when you sync more z-stream channel for given base channel. 
It happend to me too. Magically start working after few days.
Comment 2 Brandon Perkins 2009-02-26 10:53:49 EST
Accepting as we've had multiple reports of this.  Some important parts of the traceback for finding this bug easier:

The following exception occurred while executing this request:
GET /rhn/systems/details/SystemChannels.do

Exception:
com.redhat.rhn.common.hibernate.LookupException: Found multiple default EUS channels for RHEL version.
Comment 3 Jay Dobies 2009-03-05 13:47:38 EST
The issue is in the fact that the query being called is base_eus_channels_by_version_channel_arch.

That query takes into account version (e.g. 4AS, 5Server), channel_arch_id (FK to rhnChannelArch), and product name ('rhel' in all cases; hardcoded). It also only checks for the is_default flag to be set to yes.

On sputnik, which I would guess is indicative of how the channels would normally sync, there are two entries that match the criteria in use:

Product Name:  rhel
Version:       4AS
Channel Arhc: i386

The two rows that match differ in release (7 v. 8) and the associated channel (rhel-i386-as-4.6.z and rhel-i386-as-4.7.z respectively).

Again, the is_default flag is set to yes for both of these entries in rhnReleaseChannelMap and is the cause of the issue (the Java call to the query expects and verifies that only one channel is returned, presumably assuming only one will be flagged as the default). I believe this flag is incorrect for the rhel-i386-as-4.6.z channel, as once the 4.7.z channel came in it should have been identified as the default.

I'm not fully sure if this represents a bug in the sync code or in the data that comes down in the sync.

In any event, I believe we can enhance the code/query to only return the result with the highest release, acting as a safety in case the is_default column mechanism breaks down.
Comment 4 Jay Dobies 2009-03-05 14:31:05 EST
It looks like the data was incorrect on the hosted server at the time of the sync. The is_default flag was set to true for both the 4.6.z and 4.7.z channel. The code/query are correct in their use of the flag.

I updated the DB directly on sputnik and the alter subscription now works for 'jasan-chroot-rhel4'.

The data was verified to be correct on hosted now, so tonight I'll sync the channels myself just to be sure that it works correctly off a setup where the DB wasn't manually munged.
Comment 6 Jan Sarenik 2009-03-05 14:49:45 EST
On Thu, Mar 05, 2009 at 02:31:06PM -0500, Jason Dobies wrote:
> I updated the DB directly on sputnik and the alter subscription now works for
> 'jasan-chroot-rhel4'.

Thank you very much for solving my problem!
Comment 7 Jan Pazdziora 2009-03-09 06:03:18 EDT
(In reply to comment #4)
> It looks like the data was incorrect on the hosted server at the time of the
> sync. The is_default flag was set to true for both the 4.6.z and 4.7.z channel.
> The code/query are correct in their use of the flag.
> 
> I updated the DB directly on sputnik and the alter subscription now works for
> 'jasan-chroot-rhel4'.
> 
> The data was verified to be correct on hosted now, so tonight I'll sync the
> channels myself just to be sure that it works correctly off a setup where the
> DB wasn't manually munged.  

Jason,

if our application code expects something (in this case that there will only be one record with given properties), shouldn't the database schema enforce this expectation, presumably via unique constraint, so that this problem cannot occur again?
Comment 8 Jay Dobies 2009-03-12 16:51:30 EDT
Constraints are a good idea, but there are two cases I'm unsure about:

- If we were to do that in an upgrade, what happens if there is currently data in the table that violates the constraint? The DB upgrade will fail, and while it should be easy enough to tell the customer to make sure to sync their channels first (assuming we keep prod with reasonably clean data) I'm not sure the impact of that.

- I'm not sure what would happen in the sync code if incorrect data came down and violated the constraint. Regardless, it's probably a better idea to catch it at the sync level than in the UI as was seen by the bug.
Comment 10 Jay Dobies 2009-03-19 11:02:30 EDT
Commit: af62cc0126b220ea6d8e7b8afad7528036897494

Added uniqueness constraint on product_name/version/arch/is_default. This basically ensures that the database does not allow multiple eus channels to be considered the most recent (i.e. 4.6.z and 4.7.z both being indicated as the 'default' or latest eus channel).

My only testing concern is what happens when a new EUS channel is introduced, assuming the correct data in the sync. We need to make sure that the is_default flag is cleared on the existing entry (e.g. 4.6.z) before or at the same time the flag is set for the new entry (4.7.z) to make sure we don't trip this constraint for a valid transition from one EUS release to the next.
Comment 11 Jan Pazdziora 2009-03-23 04:31:29 EDT
Jason, is it correct that you created the constraint in the upgrade script but you did not put it to the schema definition in the first place?
Comment 12 Jay Dobies 2009-03-23 09:50:36 EDT
Commit: 2dfde55fc85657de8af6f0479f3f1781e09d24cd

Thanks Jan, I had missed this.
Comment 14 Jay Dobies 2009-04-07 10:16:30 EDT
Testing is going to be a bit tricky since it involves bad data coming from a sync. The database change in this bug is to prevent the bad data from being saved.

Is it possible for webqa to be temporarily changed? If so, then setting multiple EUS channels to be the default for a given base channel should show that the sync reports the error. Set the default flag to true for the two channels mentioned in this BZ (rhel-i386-as-4.6.z and rhel-i386-as-4.7.z) and attempt to sync both. The sync should fail since there should only be one default EUS channel for each combination of product, version, and architecture.

Since this was a database change, it should be checked for both new installs and upgrades.

Another aspect that should be tested is an upgrade on a database that already has incorrect data in it (i.e. two EUS channels that are set to default). The upgrade should fail since the constraint cannot be applied.

Even if webqa is not changed, this should be easy to set up. The two EUS channels can be syncced and the database changed to set both to default before the upgrade is done.

Assuming the two channels mentioned in this comment were used, the SQL below should flag the two EUS channels to both be default.

UPDATE rhnReleaseChannelMap
   SET is_default = 'Y'
 WHERE product = 'rhel'
   AND version = '4AS'
   AND channel_arch_id IN
       (SELECT id FROM rhnChannelArch WHERE name = 'IA-32')

One last sanity check is to ensure that for both new installs and upgrades the RHN_RCM_PVA_DEF_UNIQ constraint is created on the rhnReleaseChannelMap table.
Comment 16 Jay Dobies 2009-06-08 15:07:35 EDT
I'm not sure this is even still a relevant BZ anymore. The is_default flag was removed from the database, so it won't be possible to run into this inconsistency again.
Comment 17 Jan Sarenik 2009-06-09 02:28:09 EDT
As a reporter, I really think you can close it.
Comment 18 Michael Mráka 2009-06-12 09:10:39 EDT
As the underlying code had changed a lot, new testplan was:
* install satellite
* sync base rhel channel
* sync 2 or more Z-stream channels for the base channel above
* sync child channel (e.g. rhn-tools) for the base channel
* register a system to the base channel
* webui: go to system -> alter channel and add the child channel

Verified. Satellite-5.3.0-RHEL4-re20090529.0-x86_64
Comment 19 Preethi Thomas 2009-08-13 09:21:22 EDT
Release Pending
Alter channel works for both eus & non eus channel
Comment 20 Brandon Perkins 2009-09-10 16:33:47 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHEA-2009-1434.html

Note You need to log in before you can comment on or make changes to this bug.