feat: add syncset controller, feat: syncset readiness #3030

acpana · 2023-10-03T20:59:05Z

in support of the sync set proposal

Adds a syncset_controller and necessary changes for tracking readiness from syncsets and config resources
Introduces a reconciler for data expectations
Implements "TryCancel" for data expectations
Improves the CacheManager to distinguish the watch errors for the gvks it's given
Adds the SyncSet CRD to the manifest.

Signed-off-by: Alex Pana <[email protected]>

maxsmythe

Added comments, have not reviewed tests.

pkg/cachemanager/cachemanager.go

pkg/controller/config/config_controller.go

pkg/controller/syncset/syncset_controller.go

pkg/readiness/object_tracker.go

pkg/readiness/ready_tracker.go

Signed-off-by: Alex Pana <[email protected]>

plus naming, style, etc Signed-off-by: Alex Pana <[email protected]>

Signed-off-by: Alex Pana <[email protected]>

codecov-commenter · 2023-10-06T07:49:18Z

Codecov Report

Attention: 126 lines in your changes are missing coverage. Please review.

Comparison is base (aed792f) 53.21% compared to head (430deea) 53.78%.

Files	Patch %	Lines
pkg/controller/syncset/syncset_controller.go	46.47%	31 Missing and 7 partials ⚠️
pkg/readiness/ready_tracker.go	74.46%	30 Missing and 6 partials ⚠️
pkg/cachemanager/cachemanager.go	76.62%	15 Missing and 3 partials ⚠️
pkg/readiness/pruner/pruner.go	65.51%	9 Missing and 1 partial ⚠️
pkg/cachemanager/aggregator/aggregator.go	50.00%	6 Missing and 1 partial ⚠️
pkg/watch/errorlist.go	81.57%	7 Missing ⚠️
pkg/watch/set.go	0.00%	5 Missing ⚠️
pkg/watch/manager.go	25.00%	2 Missing and 1 partial ⚠️
pkg/controller/config/config_controller.go	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3030      +/-   ##
==========================================
+ Coverage   53.21%   53.78%   +0.56%     
==========================================
  Files         134      136       +2     
  Lines       11917    12198     +281     
==========================================
+ Hits         6342     6561     +219     
- Misses       5080     5136      +56     
- Partials      495      501       +6

Flag	Coverage Δ
unittests	`53.78% <70.42%> (+0.56%)`	⬆️

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.

Signed-off-by: Alex Pana <[email protected]>

- rework interpretErr to return failing gvks - implement TryCancel for trackerMap Signed-off-by: Alex Pana <[email protected]>

Signed-off-by: Alex Pana <[email protected]>

maxsmythe

Getting closer!

Have not reviewed tests.

pkg/cachemanager/cachemanager.go

main.go

pkg/cachemanager/cachemanager.go

pkg/controller/config/config_controller.go

pkg/controller/syncset/syncset_controller.go

Signed-off-by: Alex Pana <[email protected]>

ritazh · 2023-10-11T16:25:00Z

@acpana can you please update #2393 to include what this PR covers and what still remains in order to close that issue out?

pkg/readiness/pruner/pruner.go

pkg/cachemanager/cachemanager.go

pkg/readiness/ready_tracker.go

pkg/readiness/tracker_map.go

pkg/watch/errorlist.go

test/testutils/controller.go

Signed-off-by: Alex Pana <[email protected]>

pkg/watch/errorlist.go

Signed-off-by: Alex Pana <[email protected]>

pkg/cachemanager/cachemanager.go

Signed-off-by: Alex Pana <[email protected]>

pkg/cachemanager/cachemanager.go

Signed-off-by: Alex Pana <[email protected]>

julianKatz

Great work!! Way to stick with it.

passing the baton to Max to close things out on Google approval

pkg/cachemanager/aggregator/aggregator.go

pkg/cachemanager/cachemanager.go

pkg/cachemanager/cachemanager_test.go

pkg/controller/syncset/syncset_controller.go

pkg/controller/syncset/syncset_controller_test.go

pkg/watch/errorlist_test.go

Signed-off-by: Alex Pana <[email protected]>

anlandu · 2023-11-11T00:51:43Z

Looks great!!

maxsmythe

Once the bug in cache manager is addressed and the aggregator test has assertions, LGTM

pkg/cachemanager/aggregator/aggregator_test.go

pkg/cachemanager/cachemanager.go

pkg/readiness/pruner/pruner_test.go

pkg/readiness/ready_tracker_unit_test.go

Signed-off-by: Alex Pana <[email protected]>

maxsmythe

Two minor fixes, then LGTM

pkg/cachemanager/aggregator/aggregator_test.go

pkg/cachemanager/cachemanager.go

Signed-off-by: Alex Pana <[email protected]>

maxsmythe

LGTM

ritazh

minor comments

pkg/readiness/pruner/pruner.go

pkg/cachemanager/cachemanager.go

pkg/cachemanager/aggregator/aggregator.go

pkg/readiness/ready_tracker.go

Co-authored-by: Rita Zhang <[email protected]> Signed-off-by: alex <[email protected]>

Signed-off-by: Alex Pana <[email protected]>

ritazh

LGTM

Thanks for getting this feature cross the finishing line! 🎉

acpana added 4 commits October 3, 2023 20:21

add syncset controller

e34eed0

Signed-off-by: Alex Pana <[email protected]>

add syncset readiness, observe config

3d43814

Signed-off-by: Alex Pana <[email protected]>

reconcile data expectations

0dc69bd

Signed-off-by: Alex Pana <[email protected]>

enable syncset crd

46dfde2

Signed-off-by: Alex Pana <[email protected]>

maxsmythe requested changes Oct 5, 2023

View reviewed changes

acpana added 3 commits October 5, 2023 01:23

interpret registrar err

686af5d

Signed-off-by: Alex Pana <[email protected]>

review: move EM, rename to EP

7e94cdd

plus naming, style, etc Signed-off-by: Alex Pana <[email protected]>

review: remove waitgroup

31979ac

Signed-off-by: Alex Pana <[email protected]>

acpana requested a review from maxsmythe October 6, 2023 18:08

smoll fix for test

b290867

Signed-off-by: Alex Pana <[email protected]>

acpana mentioned this pull request Oct 7, 2023

refactor: remove finalizer code #3043

Closed

acpana added 3 commits October 9, 2023 15:43

Merge branch 'master' into acpana/syncsets-rt-r

49d652e

use logging constant

2192d65

Signed-off-by: Alex Pana <[email protected]>

add TryCancelData

592e243

- rework interpretErr to return failing gvks - implement TryCancel for trackerMap Signed-off-by: Alex Pana <[email protected]>

acpana mentioned this pull request Oct 9, 2023

refactor: more debug logs in RT #3056

Closed

fix lock

8da842c

Signed-off-by: Alex Pana <[email protected]>

acpana marked this pull request as ready for review October 9, 2023 23:46

maxsmythe reviewed Oct 10, 2023

View reviewed changes

acpana added 3 commits October 11, 2023 02:41

review: rm comments, mgr starts pruner

50f7abd

Signed-off-by: Alex Pana <[email protected]>

review: move interface, IsUniversal

7296196

Signed-off-by: Alex Pana <[email protected]>

review: explicitly remove source

6a260a3

Signed-off-by: Alex Pana <[email protected]>

acpana requested a review from maxsmythe October 11, 2023 06:32

ritazh added this to the v3.14.0 milestone Oct 11, 2023

acpana mentioned this pull request Oct 12, 2023

Add support for metadata.gatekeeper.sh/requires-sync-data #2393

Open

Merge branch 'master' into acpana/syncsets-rt-r

a1e153f

julianKatz reviewed Oct 12, 2023

View reviewed changes

acpana added 2 commits October 13, 2023 20:59

limit name to 63 char

689b252

Signed-off-by: Alex Pana <[email protected]>

review: comments, naming, HasBalidationOperation

4708b4d

Signed-off-by: Alex Pana <[email protected]>

maxsmythe reviewed Nov 6, 2023

View reviewed changes

pkg/watch/errorlist.go Outdated Show resolved Hide resolved

pkg/watch/errorlist.go Outdated Show resolved Hide resolved

acpana added 2 commits November 7, 2023 02:06

check err nullability, naming

eeba2bf

Signed-off-by: Alex Pana <[email protected]>

use remove set instead of forceWipe

034e826

Signed-off-by: Alex Pana <[email protected]>

acpana requested a review from maxsmythe November 7, 2023 22:10

maxsmythe reviewed Nov 8, 2023

View reviewed changes

pkg/cachemanager/cachemanager.go Outdated Show resolved Hide resolved

set instead of book for dangling watches

854306e

Signed-off-by: Alex Pana <[email protected]>

acpana requested a review from maxsmythe November 9, 2023 19:15

maxsmythe reviewed Nov 9, 2023

View reviewed changes

pkg/cachemanager/cachemanager.go Outdated Show resolved Hide resolved

general error watch dangling

d7bb9b5

Signed-off-by: Alex Pana <[email protected]>

acpana requested a review from maxsmythe November 10, 2023 00:19

julianKatz approved these changes Nov 10, 2023

View reviewed changes

review feedback

7faecbc

Signed-off-by: Alex Pana <[email protected]>

maxsmythe requested changes Nov 14, 2023

View reviewed changes

pkg/cachemanager/aggregator/aggregator_test.go Outdated Show resolved Hide resolved

pkg/cachemanager/cachemanager.go Outdated Show resolved Hide resolved

pkg/readiness/pruner/pruner_test.go Show resolved Hide resolved

pkg/readiness/ready_tracker_unit_test.go Outdated Show resolved Hide resolved

acpana added 2 commits November 15, 2023 23:53

refactor: use tt

03565fa

Signed-off-by: Alex Pana <[email protected]>

fix: general err dangling watches

3766d66

Signed-off-by: Alex Pana <[email protected]>

acpana requested a review from maxsmythe November 16, 2023 00:01

maxsmythe requested changes Nov 16, 2023

View reviewed changes

pkg/cachemanager/aggregator/aggregator_test.go Show resolved Hide resolved

pkg/cachemanager/cachemanager.go Outdated Show resolved Hide resolved

rf: add set directly

352ae72

Signed-off-by: Alex Pana <[email protected]>

maxsmythe approved these changes Nov 16, 2023

View reviewed changes

Merge branch 'master' into acpana/syncsets-rt-r

a216503

acpana requested review from ritazh and sozercan November 17, 2023 00:30

ritazh reviewed Nov 22, 2023

View reviewed changes

acpana and others added 3 commits November 22, 2023 11:59

Apply suggestions from code review

a280fb7

Co-authored-by: Rita Zhang <[email protected]> Signed-off-by: alex <[email protected]>

review suggestions

c87e934

Signed-off-by: Alex Pana <[email protected]>

Merge branch 'master' into acpana/syncsets-rt-r

430deea

acpana requested a review from ritazh November 22, 2023 20:11

ritazh approved these changes Nov 22, 2023

View reviewed changes

acpana merged commit 6276cb2 into open-policy-agent:master Nov 23, 2023
16 of 17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add syncset controller, feat: syncset readiness #3030

feat: add syncset controller, feat: syncset readiness #3030

acpana commented Oct 3, 2023 •

edited

Loading

maxsmythe left a comment

codecov-commenter commented Oct 6, 2023 •

edited

Loading

maxsmythe left a comment

ritazh commented Oct 11, 2023

julianKatz left a comment

anlandu commented Nov 11, 2023

maxsmythe left a comment

maxsmythe left a comment

maxsmythe left a comment

ritazh left a comment

ritazh left a comment

feat: add syncset controller, feat: syncset readiness #3030

feat: add syncset controller, feat: syncset readiness #3030

Conversation

acpana commented Oct 3, 2023 • edited Loading

maxsmythe left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 6, 2023 • edited Loading

Codecov Report

maxsmythe left a comment

Choose a reason for hiding this comment

ritazh commented Oct 11, 2023

julianKatz left a comment

Choose a reason for hiding this comment

anlandu commented Nov 11, 2023

maxsmythe left a comment

Choose a reason for hiding this comment

maxsmythe left a comment

Choose a reason for hiding this comment

maxsmythe left a comment

Choose a reason for hiding this comment

ritazh left a comment

Choose a reason for hiding this comment

ritazh left a comment

Choose a reason for hiding this comment

acpana commented Oct 3, 2023 •

edited

Loading

codecov-commenter commented Oct 6, 2023 •

edited

Loading