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

NETOBSERV-435 - Set default req/limits resources for eBPF agent #155

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented Aug 22, 2022

I set some minimalist resources spec for eBPF agent as already defined for others components.
This works fine on my dev cluster generating some spam on a httpd deployment:

  • memory is around 30MiB
  • max cpu at 84m

Another discussion around what limits should be set for downstream is available in NETOBSERV-380
Requests / limits per cluster size should be documented for upstream in NETOBSERV-493

@Amoghrd
Copy link
Contributor

Amoghrd commented Aug 23, 2022

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 23, 2022
@github-actions
Copy link

New image: ["quay.io/netobserv/network-observability-operator:91a62cb"]. It will expire after two weeks.

Comment on lines 775 to 780
default:
limits:
memory: 100Mi
requests:
cpu: 100m
memory: 50Mi
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these defaults for memory would be too low for eBPF. It might be okay for RSS but I have seen container_working_set_bytes for ebpf pods in the order of around 250-300 Mb. It gets OOMKilled when container_working_set_bytes reaches limits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we can raise that a bit but as said in the scrum today I think we should keep these values as low as possible upstream since we don't know the cluster size.
We need to add documentation to explain expected values per components per expected amount of flows so the user will be able to override this wisely.

Copy link
Contributor

@memodi memodi left a comment

Choose a reason for hiding this comment

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

default memory seems low for ebpf.

@Amoghrd
Copy link
Contributor

Amoghrd commented Aug 26, 2022

Verified, works as expected!

@memodi
Copy link
Contributor

memodi commented Aug 29, 2022

/qe-approved

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 7, 2022
@jotak
Copy link
Member

jotak commented Sep 7, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Sep 7, 2022
@jpinsonneau
Copy link
Contributor Author

/approve

@openshift-ci
Copy link

openshift-ci bot commented Sep 7, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpinsonneau

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

@openshift-ci openshift-ci bot added the approved label Sep 7, 2022
@jpinsonneau jpinsonneau merged commit cc44eeb into netobserv:main Sep 7, 2022
KalmanMeth pushed a commit to KalmanMeth/network-observability-operator that referenced this pull request Feb 13, 2023
…ction

CI: add continue-on-error: true to commmit stage
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.

None yet

5 participants