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.

Sign up for GitHub

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

Jump to bottom

test/extended/cli/mustgather: Separate gather_audit_logs test #24680

Closed

wking wants to merge 1 commit into openshift:master from wking:separate-audit-logs-test-case

Member

wking commented Mar 12, 2020

openshift/must-gather#143 is removing these from the default gather, because they're mostly useful for internal debugging, less useful in end-user bug reports, and can run to hundreds of megabytes. But we still want to ensure that they work as expected when they are explicitly requested. This commit pulls the audit-log checks out of the test-case for the generic invocation. And it adds a new test case with those checks after an explict gather_audit_logs request.

CC @deads2k, @sanchezl, @sferich888, @soltysh

openshift-ci-robot added the size/L label

openshift-ci-robot requested review from deads2k and smarterclayton

March 12, 2020 17:18

wking mentioned this pull request

Gather one week of node logs and drop audit logs (RFE-309) openshift/must-gather#143

Closed

Contributor

deads2k commented Mar 12, 2020

/approve
/lgtm
/hold

holding so you can make it compile. release and retag after.

openshift-ci-robot assigned deads2k

openshift-ci-robot added do-not-merge/hold lgtm approved labels

deads2k mentioned this pull request

relax must-gather e2e test - audit log removed #24678

Closed

wking force-pushed the separate-audit-logs-test-case branch from 6a774d2 to 5b11a80 Compare

March 12, 2020 22:00

openshift-ci-robot removed the lgtm label

wking force-pushed the separate-audit-logs-test-case branch from 5b11a80 to 45a05a1 Compare

March 12, 2020 22:47

Member Author

wking commented Mar 13, 2020

The three e2e failures were:

GCP upgrade: timed out waiting for service "service-test" to have a load balancer.

GCP on the new test:

fail [github.com/openshift/origin/test/extended/cli/mustgather.go:181]: Unexpected error:
    <*errors.errorString | 0xc0000da050>: {s: "EOF"}
    EOF
occurred

And similarly for AWS FIPS.

We probably don't want to land this until have those sorted.

openshift-ci-robot added the needs-rebase label

wking force-pushed the separate-audit-logs-test-case branch from 45a05a1 to 769c5f3 Compare

March 16, 2020 22:04

openshift-ci-robot removed the needs-rebase label

Member Author

wking commented Mar 16, 2020

Rebased around the generated-file conflict (from #24682?) with 45a05a1 -> 769c5f3.

I still have no idea what's going on with the EOF failures, maybe someone who understands must-gather can weigh in?

Member

soltysh commented Mar 17, 2020

I still have no idea what's going on with the EOF failures, maybe someone who understands must-gather can weigh in?

It's very weird, I've just tried with the artifacts gathered after the failure and they work fine with similar gzip code. That EOF suggest the file is empty/corrupted/being written to? Can you add a nasty time.Sleep(10*time.Second), honestly I wonder if this https://github.com/openshift/origin/blob/master/test/extended/cli/mustgather.go#L59 one should not be after the call to must-gather, can you give it a try?

wking force-pushed the separate-audit-logs-test-case branch from 769c5f3 to e86b60e Compare

April 16, 2020 23:34

Member Author

wking commented Apr 16, 2020

Rebased around 705b5e6 (#24719) with 769c5f3 -> e86b60e.

@soltysh, where exactly did you want that new Sleep?

soltysh reviewed

View reviewed changes

test/extended/cli/mustgather.go Show resolved Hide resolved

Contributor

deads2k commented May 20, 2020

/hold cancel

openshift-ci-robot removed the do-not-merge/hold label

Contributor

deads2k commented May 20, 2020

/test all

Member

soltysh commented May 21, 2020

/retest

soltysh approved these changes

View reviewed changes

Member

soltysh left a comment

/lgtm

openshift-ci-robot assigned soltysh

openshift-ci-robot added the lgtm label

Contributor

openshift-bot commented May 21, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

18 similar comments

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 24, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Contributor

openshift-bot commented Jun 25, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

Member

soltysh commented Jun 25, 2020

/hold

openshift-ci-robot added the do-not-merge/hold label

openshift-ci-robot commented Jun 25, 2020

@wking: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp	`6dcbbff`	link	`/test e2e-gcp`
ci/prow/e2e-aws-fips	`6dcbbff`	link	`/test e2e-aws-fips`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

soltysh pushed a commit to soltysh/origin that referenced this pull request


          test/extended/cli/mustgather: Separate gather_audit_logs test

0a79c37

[1] is removing these from the default gather, because they're mostly
useful for internal debugging, less useful in end-user bug reports,
and can run to hundreds of megabytes.  But we still want to ensure
that they work as expected when they are explicitly requested.  This
commit pulls the audit-log checks out of the test-case for the generic
invocation.  And it adds a new test case with those checks after an
explict gather_audit_logs request.

The EOF sleep was recommended by Maciej [2,3], although it's not clear
to me how the oc call could exit success before the output directory
was on disk.

The annotation change was generated with:

  $ hack/verify-generated-bindata.sh

[1]: openshift/must-gather#143
[2]: openshift#24680 (comment)
[3]: openshift#24680 (comment)

soltysh pushed a commit to soltysh/origin that referenced this pull request


          test/extended/cli/mustgather: Separate gather_audit_logs test

f232b84

[1] is removing these from the default gather, because they're mostly
useful for internal debugging, less useful in end-user bug reports,
and can run to hundreds of megabytes.  But we still want to ensure
that they work as expected when they are explicitly requested.  This
commit pulls the audit-log checks out of the test-case for the generic
invocation.  And it adds a new test case with those checks after an
explict gather_audit_logs request.

The EOF sleep was recommended by Maciej [2,3], although it's not clear
to me how the oc call could exit success before the output directory
was on disk.

The annotation change was generated with:

  $ hack/verify-generated-bindata.sh

[1]: openshift/must-gather#143
[2]: openshift#24680 (comment)
[3]: openshift#24680 (comment)

Member

soltysh commented Jul 16, 2020

I've managed to nail down the problems with this PR, updated version for review is in #25283
/close

openshift-ci-robot closed this

openshift-ci-robot commented Jul 16, 2020

@soltysh: Closed this PR.

In response to this:

I've managed to nail down the problems with this PR, updated version for review is in #25283
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

soltysh mentioned this pull request

test/extended/cli/mustgather: Separate gather_audit_logs test #25283

Merged

soltysh pushed a commit to soltysh/origin that referenced this pull request


          test/extended/cli/mustgather: Separate gather_audit_logs test

672dfdf

[1] is removing these from the default gather, because they're mostly
useful for internal debugging, less useful in end-user bug reports,
and can run to hundreds of megabytes.  But we still want to ensure
that they work as expected when they are explicitly requested.  This
commit pulls the audit-log checks out of the test-case for the generic
invocation.  And it adds a new test case with those checks after an
explict gather_audit_logs request.

The EOF sleep was recommended by Maciej [2,3], although it's not clear
to me how the oc call could exit success before the output directory
was on disk.

The annotation change was generated with:

  $ hack/verify-generated-bindata.sh

[1]: openshift/must-gather#143
[2]: openshift#24680 (comment)
[3]: openshift#24680 (comment)

soltysh pushed a commit to soltysh/origin that referenced this pull request


          test/extended/cli/mustgather: Separate gather_audit_logs test

006745a

[1] is removing these from the default gather, because they're mostly
useful for internal debugging, less useful in end-user bug reports,
and can run to hundreds of megabytes.  But we still want to ensure
that they work as expected when they are explicitly requested.  This
commit pulls the audit-log checks out of the test-case for the generic
invocation.  And it adds a new test case with those checks after an
explict gather_audit_logs request.

The EOF sleep was recommended by Maciej [2,3], although it's not clear
to me how the oc call could exit success before the output directory
was on disk.

The annotation change was generated with:

  $ hack/verify-generated-bindata.sh

[1]: openshift/must-gather#143
[2]: openshift#24680 (comment)
[3]: openshift#24680 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment