Skip to content

TestBuildUserNamespaceValidateCapabilitiesAreV2: cleanup daemon storage #42995

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
Merged
cpuguy83 merged 2 commits into from
Nov 10, 2021
Merged

TestBuildUserNamespaceValidateCapabilitiesAreV2: cleanup daemon storage #42995

cpuguy83 merged 2 commits into from
Nov 10, 2021

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Nov 5, 2021 •
edited
Loading

fixes #42974

testutil: daemon.Cleanup(): cleanup more directories

The storage-driver directory caused Jenkins cleanup to fail. While at it, also
removing other directories that we do not include in the "bundles" that are
stored as Jenkins artifacts.

TestBuildUserNamespaceValidateCapabilitiesAreV2: cleanup daemon storage

This should help with Jenkins failing to clean up the Workspace:

  • make sure "cleanup" is also called in the defer for all daemons. keeping
    the daemon's storage around prevented Jenkins from cleaning up.
  • close client connections and some readers (just to be sure)

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah force-pushed the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch from f31faa8 to 0bd0a90 Compare November 8, 2021 12:07
@thaJeztah thaJeztah marked this pull request as draft November 8, 2021 12:08
@thaJeztah thaJeztah force-pushed the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch 10 times, most recently from 00c5cc3 to e9a62ac Compare November 8, 2021 16:06
This was referenced Nov 9, 2021
Merged
Merged
@samuelkarp
Copy link
Member

samuelkarp commented Nov 9, 2021

don't use t.Fatalf(), as defer is not called when using it

https://pkg.go.dev/testing#T.Fatalf

Fatalf is equivalent to Logf followed by FailNow.

https://pkg.go.dev/testing#T.FailNow

FailNow marks the function as having failed and stops its execution by calling runtime.Goexit (which then runs all deferred calls in the current goroutine).

Though maybe we can use t.Cleanup here, since that's actually managed by the test runtime?

@thaJeztah
Copy link
Member Author

thaJeztah commented Nov 9, 2021

Fatalf is equivalent to Logf followed by FailNow.

Oh! Looks like I was mistaken; I guess I followed the code path to the logger interface.

In any case, it didn't solve the issue; currently just trying to disable code to see at what point it starts to fail. But it's a bit of a pain, because once it fails, the Jenkins agent can no longer be used (so I need to delete the agent before trying again😫)

@thaJeztah thaJeztah force-pushed the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch from e9a62ac to ce9819b Compare November 9, 2021 09:29
@thaJeztah thaJeztah mentioned this pull request Nov 9, 2021
Merged
@thaJeztah thaJeztah force-pushed the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch 4 times, most recently from bb0d761 to 63cde01 Compare November 10, 2021 09:45
@thaJeztah
testutil: daemon.Cleanup(): cleanup more directories
1a15a1a 
The storage-driver directory caused Jenkins cleanup to fail. While at it, also
removing other directories that we do not include in the "bundles" that are
stored as Jenkins artifacts.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch from 63cde01 to 9244cc5 Compare November 10, 2021 11:16
@thaJeztah
TestBuildUserNamespaceValidateCapabilitiesAreV2: cleanup daemon storage
eea2758 
This should help with Jenkins failing to clean up the Workspace:

- make sure "cleanup" is also called in the defer for all daemons. keeping
  the daemon's storage around prevented Jenkins from cleaning up.
- close client connections and some readers (just to be sure)

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah changed the title make TestBuildUserNamespaceValidateCapabilitiesAreV2 less flaky TestBuildUserNamespaceValidateCapabilitiesAreV2: cleanup daemon storage Nov 10, 2021
@thaJeztah thaJeztah force-pushed the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch from 9244cc5 to eea2758 Compare November 10, 2021 11:41
@thaJeztah thaJeztah marked this pull request as ready for review November 10, 2021 11:41
@thaJeztah
Copy link
Member Author

thaJeztah commented Nov 10, 2021

OK; looks like this is working now; the culprit was the /var/lib/docker/<storage-driver> directory (/var/lib/docker not "literally", but the test-daemon's root / storage location).

I updated the test-utility's Cleanup() function to remove those files; after this is merged, I also want to have a look at implementing t.Cleanup() for these utilities, so that cleaning up is handled automatically when test completes (we should be able to use that now that we dropped older Go versions)

@thaJeztah thaJeztah mentioned this pull request Nov 10, 2021
Merged
@thaJeztah thaJeztah added this to the 21.xx milestone Nov 10, 2021
tianon
tianon approved these changes Nov 10, 2021
View reviewed changes
Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

👀

@thaJeztah
Copy link
Member Author

thaJeztah commented Nov 10, 2021

This is green, with the exception of TestBuildWCOWSandboxSize on RS5 (which will be addressed by #43009)

cpuguy83
cpuguy83 approved these changes Nov 10, 2021
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83 cpuguy83 merged commit 9850b69 into moby:master Nov 10, 2021
@thaJeztah thaJeztah deleted the fix_TestBuildUserNamespaceValidateCapabilitiesAreV2 branch November 10, 2021 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tianon tianon tianon approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

Assignees

No one assigned

Labels

area/testing kind/bugfix PR's that fix bugs process/cherry-picked status/2-code-review

Projects

None yet

Milestone

23.0.0

Development

Successfully merging this pull request may close these issues.

TestBuildUserNamespaceValidateCapabilitiesAreV2 prevents Jenkins from cleaning up

4 participants

@thaJeztah @samuelkarp @tianon @cpuguy83