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

Fix CDN button visibility (Backport to RHEL to fix CentOS stream) #3020

Conversation

M4rtinK
Copy link
Contributor

@M4rtinK M4rtinK commented Nov 26, 2020

Only show the Red Hat CDN button if the Subscription
module appears to be running. To achieve that, we do
the same thing as with the HMC button - the CDN
button invisible by default. And enable it only if it
looks like the Subscription module is running.

@M4rtinK
Copy link
Contributor Author

M4rtinK commented Nov 26, 2020

@jcpunk Please create a RHEL 8 bug describing the issues CentOS Stream has with the CDN button & let me know the bug number. I'll handle the rest after that. Thanks! :)

@M4rtinK M4rtinK changed the title Fix CDN button visibility Fix CDN button visibility (Backport to RHEL to fix CentOS stream) Nov 26, 2020
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

I am not sure why we are doing this since CentOS already uses a patch with a fix. I think that the point of #3015 was to make sure that this bug is fixed in upstream.

@jcpunk
Copy link
Contributor

jcpunk commented Dec 1, 2020

@M4rtinK M4rtinK force-pushed the rhel-8-backport_cdn_button_fix_for_centos_stream branch from 4b9c2dc to c29802a Compare December 7, 2020 13:33
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Dec 7, 2020

I've filed https://bugzilla.redhat.com/show_bug.cgi?id=1903178

@jcpunk Thanks! I've updated the PR to reference the bug number with take it from there process wise. :)

I am not sure why we are doing this since CentOS already uses a patch with a fix. I think that the point of #3015 was to make sure that this bug is fixed in upstream.

@poncovka So I think this is the patch CentOS is using at the moment:
https://git.centos.org/rpms/anaconda/blob/c8/f/SOURCES/0002-CentOS-disable-cdn-radiobutton.patch

I guess that works, but I think backporting #2850 is simple enough that it could be better to just include it in our RHEL 8 branch so that CentOS (and possibly other community rebuilds) don't need to carry a patch for this themselves. Also this way the downstream patches can't break if we do some changes in the Subscription spoke.

@jcpunk
Copy link
Contributor

jcpunk commented Dec 7, 2020

When reasonable, I prefer to patches 'upstream'. In theory it prevents folks downstream from rehashing solved problems.

@carlwgeorge
Copy link
Contributor

I just checked anaconda-33.16.5.2-1, and every patch CentOS has been applying has been included except this one. Any chance this can be merged for the next tag?

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@M4rtinK
Copy link
Contributor Author

M4rtinK commented Jun 15, 2021

Still working fine on latest RHEL 8 as far as I can tell. :)

Only show the Red Hat CDN button if the Subscription
module appears to be running. To achieve that, we do
the same thing as with the HMC button - the CDN
button invisible by default. And enable it only if it
looks like the Subscription module is running.

(cherry picked from commit: aca2975)

Resolves: rhbz#1903178
@M4rtinK M4rtinK force-pushed the rhel-8-backport_cdn_button_fix_for_centos_stream branch from c29802a to 81f2437 Compare June 15, 2021 12:27
@M4rtinK M4rtinK added the ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). label Jun 15, 2021
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Jun 15, 2021

I just checked anaconda-33.16.5.2-1, and every patch CentOS has been applying has been included except this one. Any chance this can be merged for the next tag?

I did some more testing & rebased the commit on top of latest RHEL 8 branch and it should be merged shortly. :)

@poncovka
Copy link
Contributor

/kickstart-test --testtype smoke

@rvykydal rvykydal merged commit f14dbee into rhinstaller:rhel-8 Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). rhel8-branch
7 participants