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

tetragon: Remove extra Program and Map structs after split #100

Merged
merged 1 commit into from
May 29, 2022

Conversation

olsajiri
Copy link
Contributor

We ended up with 2 Program and Map structs (and related functions)
after the pkg/sensors/program split [1], removing them.

I might be missing some point why we have this, but it builds and tests
pass ;-) I only noticed this because my other change added new field
to wrong Program struct.

[1] f227832 tetragon: Create a pkg/sensors/program to split up sensors
Signed-off-by: Jiri Olsa [email protected]

@olsajiri olsajiri requested a review from a team as a code owner May 28, 2022 21:30
@olsajiri olsajiri self-assigned this May 28, 2022
@tixxdz
Copy link
Member

tixxdz commented May 28, 2022

Hi Jiri, seems so! good catch I was also adding stuff to the wrong struct... so maybe this one too ?
https://github.com/cilium/tetragon/blob/main/pkg/sensors/program/state.go#L5
https://github.com/cilium/tetragon/blob/main/pkg/sensors/bpf.go#L79 ? remove bpf.go completely ? maybe there are others...

@olsajiri
Copy link
Contributor Author

olsajiri commented May 28, 2022

@tixxdz aah right, I did not check on State ;-) yea, looks like it could be removed.. I'll try that, thanks

We ended up with 2 Program, Map and State structs (and related functions)
after the pkg/sensors/program split [1], removing them.

[1] f227832 tetragon: Create a pkg/sensors/program to split up sensors
Signed-off-by: Jiri Olsa <[email protected]>
@olsajiri olsajiri force-pushed the program_split_fix branch from 1873709 to 8c48ca7 Compare May 28, 2022 22:07
@jrfastab
Copy link
Contributor

Yep LGTM that was my refactoring gone slightly off thanks.

@jrfastab jrfastab self-requested a review May 29, 2022 05:11
@jrfastab jrfastab merged commit 5c3fd60 into cilium:main May 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants