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

zero: resource bundle reconciler #4445

Merged
merged 14 commits into from
Aug 17, 2023

Conversation

wasaga
Copy link
Contributor

@wasaga wasaga commented Aug 8, 2023

Summary

Bundle Reconciler watches a group of resource bundles, downloads them and reconciles their contents with the databroker.

Bundles are not expected to have any intersection across record types.

For caching purposes, an entry with etag/last-modified is kept in the databroker, and the upload is conditional. That way, there's no need to keep a cache of downloaded bundles.

Related issues

Implements https://github.com/pomerium/internal/issues/1423

User Explanation

Checklist

  • reference any related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@wasaga wasaga requested a review from a team as a code owner August 8, 2023 02:42
@wasaga wasaga requested review from kenjenkins and removed request for a team August 8, 2023 02:42
@wasaga wasaga marked this pull request as draft August 8, 2023 13:59
@wasaga wasaga changed the base branch from feature/zero to wasaga/zero-retry August 13, 2023 02:47
Base automatically changed from wasaga/zero-retry to feature/zero August 16, 2023 16:45
@wasaga wasaga marked this pull request as ready for review August 17, 2023 01:56
package reconciler

// RecordSetBundle is an index of databroker records by type
type RecordSetBundle[T Record[T]] map[string]RecordSet[T]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'm not sure why this is a generic when it's only ever used for DatabrokerRecord?

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 found it convenient to use generics for a higher level functionality like this one, as it allows writing more concise and easy to read tests compared to using the full data structure as it might be pretty large. See record_tests.go

}

// MarkForSyncLater marks the bundle with the given ID for syncing later (after all other bundles).
func (b *BundleQueue) MarkForSyncLater(id string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this method is O(n). If the goal of using the heap package was to achieve O(log n) performance, shouldn't all the methods be that or better? Otherwise why not just use a loop?

@wasaga wasaga merged commit 3b65049 into feature/zero Aug 17, 2023
9 checks passed
@wasaga wasaga deleted the wasaga/zero-resource-bundles-reconciler branch August 17, 2023 17:19
kenjenkins pushed a commit that referenced this pull request Nov 14, 2023
kenjenkins pushed a commit that referenced this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants