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

Run extra CRI queries even when parse_containerd succeeds #32

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

gnosek
Copy link
Contributor

@gnosek gnosek commented Apr 8, 2021

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area driver-kmod

/area driver-ebpf

/area libscap

/area libsinsp

/area tests

What this PR does / why we need it:

We can get two things from containerd-specific data that save
us more effort later:

  • resource limits
  • IP address

Unfortunately, cri-o now passes the parse_containerd checks
as well, which makes us skip the extra CRI queries (e.g. cri-o
1.20 supports the JSON in the info field, but that does not
contain all the data containerd reports).

Make parse_containerd return true only when we successfully get
the resource limit data and use its return value only to control
the cgroup read.

If we already have the IP address after parse_containerd, don't
query CRI again.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Image ids should be reported (again) for recent versions of cri-o.

We can get two things from containerd-specific data that save
us more effort later:
- resource limits
- IP address

Unfortunately, cri-o now passes the parse_containerd checks
as well, which makes us skip the extra CRI queries (e.g. cri-o
1.20 supports the JSON in the info field, but that does not
contain all the data containerd reports).

Make parse_containerd return true only when we successfully get
the resource limit data and use its return value only to control
the cgroup read.

If we already have the IP address after parse_containerd, don't
query CRI again.

Signed-off-by: Grzegorz Nosek <[email protected]>
@poiana
Copy link
Contributor

poiana commented May 28, 2021

LGTM label has been added.

Git tree hash: dff07279c19c8faaaa3b772112dc6ac9d1448350

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Jun 4, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana added the approved label Jun 4, 2021
@poiana poiana merged commit 41b1e96 into falcosecurity:master Jun 4, 2021
@poiana poiana mentioned this pull request Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants