operator-controller icon indicating copy to clipboard operation
operator-controller copied to clipboard

🐛 Fix reconciliation blocked on improper permissions for establishing watches on managed content

Open everettraven opened this issue 1 year ago • 2 comments

Description

This PR completely refactors the content management logic to get lower-level and utilize client-go to create informers and give us more flexibility to:

  • Stop the informer and requeue the ClusterExtension if an informer gets an error during a watch after a previously successful sync
  • Stop the informer on a failed sync to prevent reconciliation from being blocked
  • Capture the sync error an informer encountered and put it on the ClusterExtension status instead of only logging it

Additionally, this PR:

  • Fixes a bug where the resources managed by the ClusterExtension were missing the expected labels

  • Optimizes the caching layer by:

    • Identifying informers that need to be stopped+removed and which need to be started+added to the cache. Previously we blew everything away and recreated it
    • When sync errors are encountered, any informers that were able to successfully sync are added to the cache so they don't have to be recreated the next time around
  • Adds a finalizer so that when a ClusterExtension is deleted, the managed content cache associated with it is stopped and deleted

  • Adds a new Managed condition that is set to False when there are issues setting up informers to react to changes to managed content and True when all informers are successfully configured and reacting to events for all managed resources

  • Fixes #1025

  • Fixes #1109

Reviewer Checklist

  • [ ] API Go Documentation
  • [ ] Tests: Unit Tests (and E2E Tests, if appropriate)
  • [ ] Comprehensive Commit Messages
  • [ ] Links to related GitHub Issue(s)

everettraven avatar Aug 13 '24 19:08 everettraven

Deploy Preview for olmv1 ready!

Name Link
Latest commit 71bccb9ec1872b062343dd2504360e7309db474e
Latest deploy log https://app.netlify.com/sites/olmv1/deploys/66d9b9fbb284890008225d48
Deploy Preview https://deploy-preview-1119--olmv1.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Aug 13 '24 19:08 netlify[bot]

Codecov Report

Attention: Patch coverage is 72.04819% with 116 lines in your changes missing coverage. Please review.

Project coverage is 76.59%. Comparing base (c53453b) to head (71bccb9). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...nal/contentmanager/source/internal/eventhandler.go 53.62% 25 Missing and 7 partials :warning:
internal/contentmanager/cache/cache.go 79.20% 19 Missing and 7 partials :warning:
internal/contentmanager/source/dynamicsource.go 67.56% 21 Missing and 3 partials :warning:
internal/contentmanager/contentmanager.go 66.66% 12 Missing and 3 partials :warning:
internal/applier/helm.go 68.96% 6 Missing and 3 partials :warning:
internal/contentmanager/sourcerer.go 80.00% 4 Missing and 3 partials :warning:
cmd/manager/main.go 70.00% 2 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1119      +/-   ##
==========================================
- Coverage   77.12%   76.59%   -0.53%     
==========================================
  Files          36       40       +4     
  Lines        2020     2329     +309     
==========================================
+ Hits         1558     1784     +226     
- Misses        327      389      +62     
- Partials      135      156      +21     
Flag Coverage Δ
e2e 57.70% <57.83%> (+0.08%) :arrow_up:
unit 52.42% <41.68%> (-2.98%) :arrow_down:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 15 '24 21:08 codecov[bot]