feat: add GVK whitelisting support for status back-reporting#419
Open
michaelawyu (michaelawyu) wants to merge 1 commit into
Open
Conversation
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| flags.DurationVar(&o.ResourceChangesCollectionDuration, "resource-changes-collection-duration", 15*time.Second, | ||
| "The duration for collecting resource changes into one snapshot. The default is 15 seconds, which means that the controller will collect resource changes for 15 seconds before creating a resource snapshot.") | ||
| o.RateLimiterOpts.AddFlags(flags) | ||
| flags.BoolVar(&o.EnableStatusBackReporting, "enable-status-back-reporting", false, "If set, the status back-reporting feature is enabled. This allows users to view object status of placed objects on the member cluster side directly on the hub cluster side.") |
There was a problem hiding this comment.
if we start this as false, we need to change OM charts to enable it
| } | ||
|
|
||
| // Register adds a new GVK into the whitelist. | ||
| func (w *WhitelistedGVKs) Register(apiGroup, apiVersion, kind string) { |
There was a problem hiding this comment.
the current PR gets the GVK list from a flag which makes it static, why do we need all the locks?
| } | ||
|
|
||
| // NewWhitelistedGVKs returns a new WhitelistedGVKs instance. | ||
| func NewWhitelistedGVKs() *WhitelistedGVKs { |
There was a problem hiding this comment.
we already have something similar in checking if a GVK should be propagated, can we not reuse it?
Member
Author
|
Putting this PR on hold per offline discussion -> will switch to a dynamic configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
This PR adds GVK whitelisting support for status back-reporting so that the hub agent will only back-report status information to the original resource for a specific set of GVKs. This helps most in cases where the Fleet hub cluster runs controllers that would reconcile such objects and add their own status, which would lead to contention with the Fleet hub agent.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer
This PR is a prerequisite before we enable the status back-reporting feature in the system.