parse containerid for a pid from cgroup as a hex chain

[esara]

address #588

[CLAassistant]

All committers have signed the CLA.

[mariomac]

Thank you for the Pull Requests!!

Now some test cases are failing because any 64-hexadecimal string is recognized as a container ID. The following container ID formats, taken also from real cgroup files, are failing:

0::/docker/8afe480d66074930353da456a1344caca810fe31c1e31f6e08c95a66887235d6/kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods-besteffort.slice/kubelet-kubepods-besteffort-pod44c76ce5_f953_4bd3_bc89_12621681af49.slice/cri-containerd-40c03570b6f4c30bc8d69923d37ee698f5cfcced92c7b7df1c47f6f7887378a9.scope

Also, any other entry in the Cgroup file is recognized as a docker container even if they aren't docker containers. For example:

9:memory:/docker/a2ffe0e97ac22657a2a023ad628e9df837c38a03b1ebc904d3f6d644eb1a1a81

I tink we'd need to analyse one to one the the different container formats and then use different regular expressions for each of them.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0da32eb) 79.85% compared to head (503be23) 44.81%.
Report is 6 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #589       +/-   ##
===========================================
- Coverage   79.85%   44.81%   -35.05%     
===========================================
  Files          70       68        -2     
  Lines        5917     5775      -142     
===========================================
- Hits         4725     2588     -2137     
- Misses        971     3034     +2063     
+ Partials      221      153       -68     

Flag	Coverage Δ
integration-test	?
unittests	44.81% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[esara]

@mariomac I updated the unit tests while also allowing it to work with the previous regex (for docker) and then check for k8s cgroups before bailing out

this is working with both my EKS and GKE clusters, but I would really to make it work for [@krisztianfekete] as well, before considering to merge it

[krisztianfekete]

How about searching for the last hex block (that might also have additional characters after it, which is always (?) scope) in the string?

[esara]

How about searching for the last hex block (that might also have additional characters after it, which is always (?) scope) in the string?

my original change was something like

-var dockerCgroup = regexp.MustCompile(`^\d+:.*:.*/docker.*-([\da-fA-F]+)\.scope`)
+var dockerCgroup = regexp.MustCompile(`([0-9a-f]{64})`)

but that failed a few of the unit tests originally, so we decided to keep it compatible with the previous unit tests examples
even though, I think they are no longer valid if running in k8s, they would all pass now and the current change works with a few real examples I included in the unit tests from various container runtime configurations

[grcevski]

I think they are no longer valid if running in k8s, they would all pass now and the current change works with a few real examples I included in the unit tests from various container runtime configurations

If the tests are not valid, I think we should adjust them :). Do you perhaps have an example of IDs in k8s that you can share please?

[esara]

I think they are no longer valid if running in k8s, they would all pass now and the current change works with a few real examples I included in the unit tests from various container runtime configurations

If the tests are not valid, I think we should adjust them :). Do you perhaps have an example of IDs in k8s that you can share please?

hi, I updated the tests and included the examples both as comments in the code and in the unit tests

[mariomac]

@esara this PR should make it done: esara#1

[grcevski]

LGTM! Thanks so much for working on this @esara !!!