feat: fix manager memory usage #965

ashnamehrotra · 2024-01-25T00:46:38Z

What this PR does / why we need it:
Utilizes cache.BuilderWithOptions in the manager to free up space from watching unnecessary resources and changes ImageJob controller to use nodeNames to retrieve each node rather than storing all nodes in a list.

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #798

Special notes for your reviewer:
Eraser load test results for cluster with 500 vuln images, 50 nodes, cpu request unset, and high usage daemonset deployed

Before changes:
MAX CPU Manager: 973.889567 m
MAX MEM Manager: 351.07 Mi

After changes:
MAX CPU Manager: 441.082336 m
MAX MEM Manager: 32.94 Mi

sozercan · 2024-01-25T01:29:01Z

@ashnamehrotra this is awesome! looks like e2e has a conflict due to artifact names and please sign the DCO

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

…ev#899) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Fabian Gonzalez <fabiangonz98@gmail.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

…ocs (eraser-dev#923) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Co-authored-by: ashnamehrotra <ashnamehrotra@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

…ser-dev#945) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Co-authored-by: ashnamehrotra <ashnamehrotra@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

…-dev#919) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Xander Grzywinski <xandergr@microsoft.com> Co-authored-by: ashnamehrotra <ashnamehrotra@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

…ser-dev#921) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Xander Grzywinski <xandergr@microsoft.com> Co-authored-by: ashnamehrotra <ashnamehrotra@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

enj · 2024-01-25T18:53:43Z

main.go

+			SelectorsByObject: cache.SelectorsByObject{
+				// to watch eraser pods
+				&corev1.Pod{}: {
+					Field: fields.OneTermEqualSelector("metadata.namespace", utils.GetNamespace()),


I think this is still making a cluster scoped watch and having API server filter them down. You could confirm that one way or the other by reducing the RBAC of the eraser components to use role bindings to properly scope the access.

The RBAC is already scoped to the namespace for the manager role, but removing the field to filter by namespace increases the memory again. I think that could be because for the manager we have ClusterRole instead of Role, but we can't change that since we need to watch ImageJobs/ImageLists in default and eraser pods in eraser-system.

It was the same problem testing with MultiNamespacedCacheBuilder and keeping only the eraser-system and default namespaces, it didn't have an affect on memory consumption.

You should create separate cluster roles/roles/cluster roles bindings/role bindings to express the exact permissions of each component. There is no requirement that everything be done via a single cluster role/cluster role binding.

https://youtu.be/Nw1ymxcLIDI is a good resource to understand RBAC fundamentals.

thank you this was really helpful! I changed the role to create a separate Role and ClusterRole

controllers/imagejob/imagejob_controller.go

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

enj

Minor comments, RBAC LGTM.

enj · 2024-01-31T17:49:48Z

manifest_staging/charts/eraser/templates/eraser-manager-role-role.yaml

The cluster role in manifest_staging/charts/eraser/templates/eraser-imagejob-pods-cluster-role-clusterrole.yaml is empty, should and the associated binding it be deleted?

yup we should only need the service account

enj · 2024-01-31T17:50:54Z

manifest_staging/charts/eraser/templates/eraser-manager-role-clusterrole.yaml

-  - list
-  - patch
-  - update
-  - watch
 - apiGroups:
  - eraser.sh


For the eraser resources, does your controller need access to create and similar permissions on the resource itself (i.e. not /status)?

It does for ImageJob, but not for ImageList. Removed create/delete/update/patch permissions for ImageList. Removed update/patch permissions for ImageJob.

enj · 2024-01-31T17:52:48Z

config/rbac/role_binding.yaml

 metadata:
  name: manager-rolebinding
+  namespace: system


Should this be eraser-system?

by default the roles are in "system" and during deployment it changes to "eraser-system" since the namespace is configurable

kubebuilder/kustomize automatically prefixes these

eraser/config/default/kustomization.yaml

Line 9 in e6c15ca

namePrefix: eraser-

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

sozercan · 2024-01-31T19:35:30Z

controllers/imagejob/imagejob_controller.go

@@ -189,9 +189,9 @@ func checkNodeFitness(pod *corev1.Pod, node *corev1.Node) bool {
 }

 //+kubebuilder:rbac:groups=eraser.sh,resources=imagejobs,verbs=get;list;watch;create;update;patch;delete
-//+kubebuilder:rbac:groups="",resources=podtemplates,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups="",namespace="system",resources=podtemplates,verbs=get;list;watch;create;update;patch;delete


please make sure system doesn't break since kubebuilder adds that prefix in generation but not sure if it applies here

this only generates the files under rbac/ which all use system. Then in manifest staging/ it is replaced to eraser-system through the templates.

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

sozercan

Great job! LGTM

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: Fabian Gonzalez <fabiangonz98@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Co-authored-by: Fabian Gonzalez <fabiangonz98@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Xander Grzywinski <xandergr@microsoft.com>

ashnamehrotra requested review from pmengelbert and sozercan as code owners January 25, 2024 00:46

ashnamehrotra and others added 27 commits January 25, 2024 09:43

use node names instead of NodeList

ffe36b6

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

remove space

40f4448

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

enabling for testing

c75eaea

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: bump @babel/traverse from 7.18.11 to 7.23.2 in /docs (eraser-d…

53d826b

…ev#899) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

bump golang/x/net to 0.17 (eraser-dev#900)

2bde086

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: upgrade google.golang.org/grpc to v1.58.3 (eraser-dev#913)

ef674db

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: upgrade k8s.io/kubernetes to v1.26.8 (eraser-dev#914)

436b492

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: update k8s to v1.26.11 (eraser-dev#917)

38c4785

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

docs: update issues template (eraser-dev#895)

92504f4

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Feat: Support Trivy status filtering (eraser-dev#844)

7c6c22c

Signed-off-by: Fabian Gonzalez <fabiangonz98@gmail.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: bump actions/checkout from 4.1.0 to 4.1.1 (eraser-dev#901)

b6746aa

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

feat: add v1alpha3 api with RuntimeSpec configuration (eraser-dev#922)

b6b0ea5

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: add codeowners (eraser-dev#943)

e538a9f

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

feat: use custom socket configurations from v1alpha3 (eraser-dev#930)

a5422dd

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: update to go 1.21 and trivy 0.48.3 (eraser-dev#941)

f46cf8f

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: Prepare v1.4.0-beta.0 release (eraser-dev#949)

42f152d

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Co-authored-by: ashnamehrotra <ashnamehrotra@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

ci: create pr auto signoff (eraser-dev#950)

6492b36

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: group dependabot PRs (eraser-dev#948)

8c2db30

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

limit cache with MultiNamespacedCacheBuilder

c376dd8

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

remove watch role for nodes

3650cab

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

lint fix

e8fab01

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

docs: fix v1.3.0 docs (eraser-dev#951)

a023627

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

chore: bump codegen version (eraser-dev#946)

b6804bd

Signed-off-by: Sertac Ozercan <sozercan@gmail.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

dependabot bot and others added 12 commits January 25, 2024 09:50

chore: bump the all group with 8 updates (eraser-dev#962)

bede0d2

Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

add node watch back in ClusterRole

dd6bf90

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

try with SelectorsByObject

8c30004

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

remove node watch

d39d3fe

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

add other versions

46b284c

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

change hardcodings

6b15cc4

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

change to job-owner var

de86978

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

remove v1alpha1 crds

d7a1618

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

watch any value

a00c16c

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

remove fields completely

3e721d6

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

revert testing

9ef25a1

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Merge branch 'main' of github.com:Azure/eraser into manager-memory-fix

fbc4c27

ashnamehrotra force-pushed the manager-memory-fix branch from 8d8b525 to fbc4c27 Compare January 25, 2024 17:53

enj reviewed Jan 25, 2024

View reviewed changes

controllers/imagejob/imagejob_controller.go Outdated Show resolved Hide resolved

ashnamehrotra added 2 commits January 26, 2024 10:07

revert node list changes

a817cc2

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

Merge branch 'main' of github.com:Azure/eraser into manager-memory-fix

aff6d40

sozercan requested a review from enj January 26, 2024 18:26

rbac changes

1d3f27d

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

enj reviewed Jan 31, 2024

View reviewed changes

delete empty ClusterRole/Binding for imagejob pods

6ce80bf

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

sozercan reviewed Jan 31, 2024

View reviewed changes

ashnamehrotra added 2 commits January 31, 2024 11:52

remove unneeded imagelist roles

519e942

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

remove update and patch from imagejob

d841589

Signed-off-by: ashnamehrotra <ashnamehrotra@gmail.com>

ashnamehrotra force-pushed the manager-memory-fix branch from 5ef1e9d to d841589 Compare January 31, 2024 20:55

Merge branch 'main' of github.com:Azure/eraser into manager-memory-fix

7ec2156

sozercan approved these changes Feb 1, 2024

View reviewed changes

ashnamehrotra merged commit 1e4401b into eraser-dev:main Feb 1, 2024
178 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: fix manager memory usage #965

feat: fix manager memory usage #965

ashnamehrotra commented Jan 25, 2024

sozercan commented Jan 25, 2024

enj Jan 25, 2024

ashnamehrotra Jan 26, 2024

enj Jan 26, 2024

enj Jan 26, 2024

ashnamehrotra Jan 31, 2024 •

edited

Loading

enj left a comment

enj Jan 31, 2024

ashnamehrotra Jan 31, 2024

enj Jan 31, 2024

ashnamehrotra Jan 31, 2024 •

edited

Loading

enj Jan 31, 2024

ashnamehrotra Jan 31, 2024

sozercan Jan 31, 2024 •

edited

Loading

sozercan Jan 31, 2024

ashnamehrotra Jan 31, 2024

sozercan left a comment

feat: fix manager memory usage #965

feat: fix manager memory usage #965

Conversation

ashnamehrotra commented Jan 25, 2024

sozercan commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashnamehrotra Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

enj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashnamehrotra Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sozercan Jan 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sozercan left a comment

Choose a reason for hiding this comment

ashnamehrotra Jan 31, 2024 •

edited

Loading

ashnamehrotra Jan 31, 2024 •

edited

Loading

sozercan Jan 31, 2024 •

edited

Loading