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

CI: enable and collect core dumps in tests #941

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Conversation

ansiwen
Copy link
Collaborator

@ansiwen ansiwen commented Nov 9, 2023

This change enables core dumps. If a core dump is created, its backtrace is logged with gdb and it is put into the artifacts together with the test executable.

To see it in action, see #923, which creates a core dump and is based on this PR.

@ansiwen ansiwen force-pushed the pr/ansiwen/ci-coredump branch 5 times, most recently from b6cb498 to ef94272 Compare November 9, 2023 16:35
@ansiwen
Copy link
Collaborator Author

ansiwen commented Nov 9, 2023

@ansiwen ansiwen force-pushed the pr/ansiwen/ci-coredump branch 6 times, most recently from cff913a to 830610d Compare November 9, 2023 19:01
@ansiwen
Copy link
Collaborator Author

ansiwen commented Nov 9, 2023

This is ready for review

@phlogistonjohn
Copy link
Collaborator

Question: don't core files usually end up outside the container? Is that why you set the core_pattern in the kernel, to make it drop the cores w/in the container?

testing/containers/ceph/Dockerfile Outdated Show resolved Hide resolved
entrypoint.sh Show resolved Hide resolved
entrypoint.sh Show resolved Hide resolved
@ansiwen
Copy link
Collaborator Author

ansiwen commented Nov 10, 2023

@phlogistonjohn

Question: don't core files usually end up outside the container? Is that why you set the core_pattern in the kernel, to make it drop the cores w/in the container?

I guess that's not true nowadays anymore. I set the pattern to have a deterministic name I can look for. Different distributions have different defaults. But without it, the core dump also ended in the container (tmp or run directory).

Edit: maybe it still would end up outside the container and it only works because this is a bind-mount? Anyway, important is, that is working like this. :-)

@ansiwen ansiwen force-pushed the pr/ansiwen/ci-coredump branch from 830610d to fba78e8 Compare November 10, 2023 14:13
@ansiwen ansiwen requested a review from anoopcs9 November 10, 2023 14:18
Copy link
Collaborator

@anoopcs9 anoopcs9 left a comment

Choose a reason for hiding this comment

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

Almost looks good to me.

@anoopcs9 anoopcs9 dismissed their stale review November 12, 2023 08:58

Changes included in the subsequent version.

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

LGTM

@phlogistonjohn
Copy link
Collaborator

@ansiwen not sure if you did it on purpose or not, but there's no label yet so it wont automerge. I'll let you add it in case you did it in order to get >1 review.

@phlogistonjohn phlogistonjohn added the no-API This PR does not include any changes to the public API of a go-ceph package label Nov 16, 2023
@phlogistonjohn
Copy link
Collaborator

The pr has two approvals, so I added a label myself. ;-)

@mergify mergify bot merged commit fa8e6c8 into master Nov 16, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-API This PR does not include any changes to the public API of a go-ceph package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants