Non Watched Namespace Labeled selectors #515
Open
Conversation
This file contains 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
It will now update all resources if a namespace has been added.
kdorosh
reviewed
Aug 19, 2022
kdorosh
reviewed
Aug 19, 2022
kdorosh
reviewed
Aug 19, 2022
pkg/code-generator/codegen/templates/snapshot_emitter_template.go
Outdated
Show resolved
Hide resolved
with the selectors
… into labeled-selectors
nfuden
reviewed
Aug 29, 2022
pkg/code-generator/codegen/templates/snapshot_emitter_template.go
Outdated
Show resolved
Hide resolved
nfuden
reviewed
Aug 29, 2022
kdorosh
reviewed
Aug 30, 2022
kdorosh
reviewed
Aug 30, 2022
… into labeled-selectors
jackstine
commented
Sep 22, 2022
Comment on lines
+163
to
+167
| rc.namespaceLock.Lock() | ||
| rc.namespaceWhitelist = append(rc.namespaceWhitelist, namespace) | ||
| rc.namespaceLock.Unlock() | ||
| return nil | ||
| } |
There was a problem hiding this comment.
should we consider getting rid of this white list?
jackstine
commented
Sep 22, 2022
Comment on lines
+107
to
+109
| // Note it might be best to use this as a Singleton instead of a Structure | ||
| // if a Singleton suggestion is proposed, then Register will no long be needed, | ||
| // and we can just use Register New Namespace. Also this would clear up some mutexes, that are being used. |
There was a problem hiding this comment.
Do we want to consider this idea, or at least consider registering and starting the resource client factory on NewFunction()?
jackstine
commented
Sep 22, 2022
Comment on lines
+346
to
+348
| for _, ns := range missingNamespaces { | ||
| mockResourceChan <- mockResourceListWithNamespace{list: MockResourceList{}, namespace: ns} | ||
| } |
There was a problem hiding this comment.
If a namespace is deleted, this causes the namespace to be empty. The entry in the map is never deleted. I think this is fine.
Deleting the entry would require a RemoveRegisteredNamespace() for the clients, and a deletion from the Kube-Controller. Since this was never handled originally, I think this is fine. Since clients make upgrades as well, I think this is fine as too.
… of the informers. added some comments, and some other things.
|
adding this closed PR solo-io/gloo#7097 |
added 8 commits
July 21, 2023 19:18
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adding expression label filters to non-watched namespaces.
watchNamespacesis empty andExpressionSelectoris emptywatchNamespacesis set andExpressionSelectoris emptywatchNamespaceswatchNamespacesis empty andExpressionSelectoris not emptyExpressionSelectorExpressionSelectorwatchNamespacesis set andExpressionSelectoris setwatchNamespacesBOT NOTES:
resolves [Migrated] fine-grained selector for watched CRs gloo#5868