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

util: correct errors and suppress stderr for common cases #5959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jcpunk
Copy link
Contributor

@jcpunk jcpunk commented Oct 23, 2024

There were some errors uncovered in the log-capture script as part of the RHEL backport process (#5808).

This corrects the errors and should let the tool run in the default case without output to stderr.

@github-actions github-actions bot added the f42 Fedora 42 label Oct 23, 2024
scripts/log-capture Fixed Show resolved Hide resolved
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

You need to keep the changes consistent between branches. Considering master as the source of truth for backports, let's review this one, and block the rhel-10 and rhel-9 Prs on this getting merged.

scripts/log-capture Fixed Show resolved Hide resolved
scripts/log-capture Fixed Show resolved Hide resolved
scripts/log-capture Outdated Show resolved Hide resolved
scripts/log-capture Outdated Show resolved Hide resolved
scripts/log-capture Outdated Show resolved Hide resolved
scripts/log-capture Outdated Show resolved Hide resolved
@jcpunk jcpunk force-pushed the fix-log-capture branch 2 times, most recently from 54be6c7 to 14fb276 Compare November 1, 2024 14:44
scripts/log-capture Outdated Show resolved Hide resolved

if [[ -e /tmp/pre-anaconda-logs/ ]];then
cp -r /tmp/pre-anaconda-logs ${OUTDIR}
fi

tar cfa ${ARCHIVE} ${OUTDIR}
tar cfa ${ARCHIVE} -C / ./${OUTDIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why we need to change to root? Wasn't it working before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the change to root and addition of ./, tar no longer emits:

tar: Removing leading `/' from member names

@KKoukiou
Copy link
Contributor

/kickstart-tests --testtype logs

scripts/log-capture Fixed Show resolved Hide resolved
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

Please test stuff before submitting for review.

_to_log ls -l /sys/firmware/efi/efivars
fi

if test -e /tmp/*.log; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I think this does not work:
You need to use: compgen -G ""

Copy link
Contributor

Choose a reason for hiding this comment

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

Or something like this:
find /tmp -type f -name '*.log' -exec cp '{}' ${OUTDIR} ';'

scripts/log-capture Fixed Show resolved Hide resolved
@@ -29,24 +31,26 @@
_to_log dmidecode
_to_log lspci -vvnn

for DEV_NAME in $(list-harddrives | cut -d " " -f 1)
for DEV_NAME in /sys/block/*; do

Check failure

Code scanning / shellcheck

SC1073 Error

Couldn't parse this for loop. Fix to allow more checks.
Copy link
Contributor

@rvykydal rvykydal 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f42 Fedora 42
Development

Successfully merging this pull request may close these issues.

5 participants