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

Move and fix GetContainerRuntime check from bpfd proc #1996

Merged
merged 4 commits into from
Mar 23, 2022

Conversation

MyIgel
Copy link
Contributor

@MyIgel MyIgel commented Mar 20, 2022

Fixes #1592 (again)

Description
This adds a slimmed down version of https://github.com/genuinetools/bpfd/blob/a4bfa5e3e9d1bfdbc56268a36a0714911ae9b6bf/proc/proc.go as the original package is unmaintained (~3y no updates of its 0.0.1 release) and kaniko only needs the container runtime detection, see #1686 (comment) . To fix the problem, it has #1686 already applied to it.

The alternative to this change would be to rebuild this functionality, but I'm not totally sure if I could do that by my own due to a lack of go experience but I could try if it would be better to have a "kaniko implementation" of the functionality.

Submitter Checklist

  • Includes unit tests (at least the original ones)
  • Adds integration tests if needed. (nope)

Reviewer Notes

  • The code flow looks good.
  • Unit tests and/or integration tests added.

@google-cla
Copy link

google-cla bot commented Mar 20, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@imjasonh
Copy link
Collaborator

Boilerplate missing in:
././pkg/util/proc_test.go
././pkg/util/proc.go

Please add the boilerplate text to the top of these new files.

Also, could we maybe move them into pkg/util/proc? Having a single huge util package with so much different functionality in it is kind of a smell. I'd like to clean that up in general in the future, and this would be a good step.

@MyIgel
Copy link
Contributor Author

MyIgel commented Mar 21, 2022

I can definitely move it but wouldn't adding the boilerplate mean that it gets relicensed? I mean I actually don't think that this would be an issue as its MIT to Apache 2 and not an substantial part of the code with a relevantly complex software part but whats your opinion here @imjasonh?

@MyIgel
Copy link
Contributor Author

MyIgel commented Mar 22, 2022

I added the boilerplate and did some more cleanup, that should be sufficient i guess :D

@imjasonh imjasonh merged commit 7b16110 into GoogleContainerTools:main Mar 23, 2022
@MyIgel MyIgel deleted the bpfd-proc branch March 27, 2022 09:59
@imjasonh imjasonh mentioned this pull request Apr 1, 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.

Container detection fails on cgroup v2 devices
3 participants