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

sysdump: exclude endpoint object files by default #1258

Merged
merged 3 commits into from Dec 14, 2022

Conversation

tbalthazar
Copy link
Contributor

Set the --exclude-object-files flag that was added in cilium/cilium#22370 as a default flag when running sysdump.

@tbalthazar tbalthazar requested a review from a team as a code owner December 2, 2022 15:49
@tbalthazar tbalthazar temporarily deployed to ci December 2, 2022 15:49 Inactive
@tbalthazar
Copy link
Contributor Author

@pchaigno @joamaki the follow-up PR to cilium/cilium#22370

Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Thanks for the PRs!

@christarazi christarazi added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. kind/enhancement This would improve or streamline existing functionality. area/sysdump labels Dec 2, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 2, 2022
@pchaigno pchaigno added dont-merge/bad-bot To prevent MLH from marking ready-to-merge. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Dec 3, 2022
sysdump/sysdump.go Outdated Show resolved Hide resolved
@tklauser tklauser removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. dont-merge/bad-bot To prevent MLH from marking ready-to-merge. labels Dec 5, 2022
@tbalthazar tbalthazar temporarily deployed to ci December 7, 2022 16:57 — with GitHub Actions Inactive
sysdump/sysdump.go Outdated Show resolved Hide resolved
@maintainer-s-little-helper

This comment was marked as resolved.

@tklauser
Copy link
Member

The staticcheck failures on the latest version of the branch look related: https://github.com/cilium/cilium-cli/actions/runs/3683477845

@tbalthazar tbalthazar temporarily deployed to ci December 13, 2022 13:05 — with GitHub Actions Inactive
@tbalthazar
Copy link
Contributor Author

@tklauser lint issue should be fixed now

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Thanks @tbalthazar! I think we're almost there, only two minor remarks left.

sysdump/sysdump.go Outdated Show resolved Hide resolved
k8s/client.go Show resolved Hide resolved
Set the `--exclude-object-files` flag that was added in
cilium/cilium#22370 as a default flag when
running sysdump. (only if cilium version >= 1.13.0).

Signed-off-by: Thomas Balthazar <thomas@balthazar.info>
It will be used in other palces too.

Signed-off-by: Thomas Balthazar <thomas@balthazar.info>
Signed-off-by: Thomas Balthazar <thomas@balthazar.info>
@tbalthazar tbalthazar temporarily deployed to ci December 13, 2022 15:41 — with GitHub Actions Inactive
@tbalthazar
Copy link
Contributor Author

@tklauser updated!

@tklauser tklauser self-requested a review December 13, 2022 15:50
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @tbalthazar!

@tklauser tklauser merged commit 646869c into cilium:master Dec 14, 2022
@tbalthazar tbalthazar deleted the tbalthazar/sysdump branch December 15, 2022 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sysdump kind/enhancement This would improve or streamline existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants