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

Fixes #36938 - Ignore output of some commands in Global Registration #9914

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

stejskalleos
Copy link
Contributor

No description provided.

@stejskalleos
Copy link
Contributor Author

@nofaralfasi review please

Copy link
Contributor

@nofaralfasi nofaralfasi left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -175,7 +175,7 @@ chmod 644 $KATELLO_SERVER_CA_CERT
if ! [ -x "$(command -v subscription-manager)" ] ; then
$PKG_MANAGER_INSTALL subscription-manager
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need to silence this command too?

There are also subscription-manager config commands later in this script, do you know if they need to be silenced too?

Copy link
Contributor

Choose a reason for hiding this comment

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

I reproduced the reported issue and tested it with the changes in this PR. For me, the current modifications cleared all the unnecessary messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$PKG_MANAGER_INSTALL subscription-manager this one no, because if installation of subman fails, users need to know about it.

Copy link
Member

Choose a reason for hiding this comment

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

OK, on a second thought if sub-man is not installed on the system, package manager should not complain about subscriptions, since the plugin that does that, would not be installed at all.

Copy link
Member

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

Didn't test it, but LGTM

@stejskalleos stejskalleos merged commit b404794 into theforeman:develop Nov 29, 2023
9 checks passed
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