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

fix: the ebpf program in the tcpretrans plugin failed to load #322

Merged
merged 5 commits into from
May 15, 2024

Conversation

wenhuwang
Copy link
Contributor

@wenhuwang wenhuwang commented Apr 28, 2024

Description

Fix the issues of abnormal status of retina-agent caused by opening tcpretrans plugin. The retina-agent nil pointer is because the Tracer.socketEnricherMap field of the tcpretrans plugin is nil.

Related Issue

#311

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@wenhuwang wenhuwang requested a review from a team as a code owner April 28, 2024 09:14
@rbtr rbtr added lang/go The Go Programming Language type/fix Fixes something area/plugins priority/1 P1 scope/S Change is Small labels Apr 28, 2024
@rbtr
Copy link
Collaborator

rbtr commented May 1, 2024

@wenhuwang this doesn't seem to fully resolve the problem. Instead of a nil pointer, now there is an error instantiating the socketEnricher:

failed to reconcile plugin tcpretrans: failed to new socketEnricher: read BPF iterator: checking if current pid namespace is host pid namespace: host.Init() must be called before calling isHostNamespace()

rbtr

This comment was marked as duplicate.

Copy link
Collaborator

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

needs to be tested

@rbtr rbtr added priority/0 P0 and removed priority/1 P1 labels May 2, 2024
@wenhuwang
Copy link
Contributor Author

wenhuwang commented May 4, 2024

@wenhuwang this doesn't seem to fully resolve the problem. Instead of a nil pointer, now there is an error instantiating the socketEnricher:

failed to reconcile plugin tcpretrans: failed to new socketEnricher: read BPF iterator: checking if current pid namespace is host pid namespace: host.Init() must be called before calling isHostNamespace()

@rbtr Sorry for the late reply, I'm on vacation recently. I have tested it locally before. I will verify this problem in a few days.

@wenhuwang
Copy link
Contributor Author

wenhuwang commented May 8, 2024

I have not reproduced this problem and retina-agent status is normal in local environment .

failed to reconcile plugin tcpretrans: failed to new socketEnricher: read BPF iterator: checking if current pid namespace is host pid namespace: host.Init() must be called before calling isHostNamespace()

platform:
OS: Ubuntu 18.04.5 LTS
Kubernetes Version: 1.22.2
Host: self-host
Kernel Version: 5.10.87-051087-generic
Retina Version: v0.0.8

Do you have any specific steps to reproduce the problem? @rbtr

Copy link
Contributor

@anubhabMajumdar anubhabMajumdar 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 change. Can you add details as to how you verified this change?

pkg/plugin/tcpretrans/tcpretrans_linux.go Show resolved Hide resolved
@wenhuwang
Copy link
Contributor Author

wenhuwang commented May 11, 2024

Thanks for the change. Can you add details as to how you verified this change?

  1. Before adding this change, the retina-agent would panic because of the nil pointer.
  2. After adding this change, retina-agent runs normally and can get tcpretrans plugin metrics.
  3. When troubleshooting the following problem through the logs, I can only see the "Initialized tcpretrans plugin" log but not the "failed to new socketEnricher" related logs.

failed to reconcile plugin tcpretrans: failed to new socketEnricher: read BPF iterator: checking if current pid namespace is host pid namespace: host.Init() must be called before calling isHostNamespace()

auto-merge was automatically disabled May 12, 2024 08:57

Head branch was pushed to by a user without write access

@wenhuwang wenhuwang requested a review from rbtr May 13, 2024 04:42
@rbtr rbtr assigned rbtr and unassigned timraymond May 14, 2024
Copy link
Collaborator

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

enabling tcpretrans plugin lgtm now, thanks @wenhuwang

@rbtr rbtr added this pull request to the merge queue May 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 15, 2024
@rbtr rbtr added this pull request to the merge queue May 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 15, 2024
@rbtr rbtr enabled auto-merge May 15, 2024 18:04
@rbtr rbtr added this pull request to the merge queue May 15, 2024
Merged via the queue into microsoft:main with commit 50d87e2 May 15, 2024
21 checks passed
matmerr pushed a commit to matmerr/retina that referenced this pull request Jul 3, 2024
…oft#322)

# Description
Fix the issues of abnormal status of retina-agent caused by opening
tcpretrans plugin. The retina-agent nil pointer is because the
[Tracer.socketEnricherMap](https://github.com/inspektor-gadget/inspektor-gadget/blob/main/pkg/gadgets/trace/tcpretrans/tracer/tracer.go#L41)
field of the tcpretrans plugin is nil.


## Related Issue

microsoft#311 

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [ ] I have followed the project's style guidelines.
- [ ] I have updated the documentation, if necessary.
- [ ] I have added tests, if applicable.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

---------

Signed-off-by: wenhuwang <[email protected]>
Co-authored-by: Evan Baker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/plugins lang/go The Go Programming Language priority/0 P0 scope/S Change is Small type/fix Fixes something
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants