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

sosreport for rhel-atomic #3486

Closed

Conversation

Projects
None yet
3 participants
@mvollmer
Copy link
Member

commented Jan 15, 2016

No description provided.

@mvollmer mvollmer added the blocked label Jan 15, 2016

@mvollmer

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2016

Image needs to be uploaded.

@mvollmer mvollmer added the needswork label Jan 15, 2016

@mvollmer

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2016

Image needs to be uploaded.

Done.

@dperpeet dperpeet removed the blocked label Jan 15, 2016

@@ -35,11 +34,7 @@ class TestSOS(MachineCase):
b.click('[data-target="#sos"]')
b.wait_visible("#sos")

# HACK - https://bugzilla.redhat.com/show_bug.cgi?id=1278807

This comment has been minimized.

Copy link
@dperpeet

dperpeet Jan 15, 2016

Member

Is this fixed in f23? the test timed out and there has been no final answer in the bug tracker so far.

This comment has been minimized.

Copy link
@stefwalter

stefwalter Jan 16, 2016

Contributor

Because of the pty above, this is no longer necessary.

# HACK - https://bugzilla.redhat.com/show_bug.cgi?id=1278807
# Sosreport waits 5 minutes for a hung ausearch, so we wait 10 minutes
# for sosreport.
with b.wait_timeout(600):

This comment has been minimized.

Copy link
@dperpeet

dperpeet Jan 15, 2016

Member

In any case, isn't the default (I believe 60 seconds) very low here? I would think something like 2 minutes should always be allowed, especially on possibly slower test machines.

This comment has been minimized.

Copy link
@mvollmer

mvollmer Jan 18, 2016

Author Member

I think so too.

@dperpeet

This comment has been minimized.

Copy link
Member

commented Jan 15, 2016

the updated spec file was merged into master, so this needs a rebase

@@ -0,0 +1,7 @@
#! /bin/sh

This comment has been minimized.

Copy link
@dperpeet

dperpeet Jan 15, 2016

Member

I think we should add a license header to new source files.

This comment has been minimized.

Copy link
@stefwalter

stefwalter Jan 16, 2016

Contributor

This is a script that traverses the network each time and doesn't get minified. I don't think it needs a license.

@@ -25,7 +25,6 @@ import subprocess
import unittest

@unittest.skipIf("debian" in os.environ.get("TEST_OS", ""), "No sosreport on Debian.")

This comment has been minimized.

Copy link
@dperpeet

dperpeet Jan 15, 2016

Member

My debian 8 (jessie) has the package sosreport. Is that outdated or not compatible with what we do? Do we just need to build a new debian image?

This comment has been minimized.

Copy link
@mvollmer

mvollmer Jan 18, 2016

Author Member

You are right, I must have missed that somehow at the time. I'll make a separate PR for this.

@@ -0,0 +1,7 @@
#! /bin/sh

if grep -q 'CPE_NAME=.*atomic-host' /etc/os-release; then

This comment has been minimized.

Copy link
@stefwalter

stefwalter Jan 16, 2016

Contributor

Can we just take this branch if the atomic executable is available?

This comment has been minimized.

Copy link
@mvollmer

mvollmer Jan 18, 2016

Author Member

Fedora also has the atomic command, but we want to run sosreport directly there. So we could check atomic && !sosreport. But then we might use the wrong image... so I thought it is best to just detect rhel atomic directly.... What do you think?

This comment has been minimized.

Copy link
@stefwalter

stefwalter Jan 18, 2016

Contributor

Good point about the container. No way to predict that. However the CPE_NAME thingy above doesn't help with using the right image.

@stefwalter

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2016

Needs a rebase onto master.

@mvollmer mvollmer force-pushed the mvollmer:sosreport-rhel-atomic branch 5 times, most recently from cec9799 to ee21c6b Jan 18, 2016

mvollmer added some commits Jan 15, 2016

sosreport: Run sosreport with a pty
"atomic run" seems to require it, and it works better that way in
general (https://bugzilla.redhat.com/show_bug.cgi?id=1278807).

@mvollmer mvollmer force-pushed the mvollmer:sosreport-rhel-atomic branch from ee21c6b to f690d02 Jan 19, 2016

@mvollmer mvollmer removed the needswork label Jan 19, 2016

stefwalter added a commit that referenced this pull request Jan 19, 2016

sosreport: Do the right thing on rhel-atomic
Closes #3486
Reviewed-by: Stef Walter <stefw@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.