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

✨ Cache: Allow defining options that apply to all namespaces that themselves have no explicit config #2528

Merged
merged 1 commit into from Oct 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 46 additions & 0 deletions pkg/cache/cache.go
Expand Up @@ -22,8 +22,10 @@ import (
"net/http"
"time"

"golang.org/x/exp/maps"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -121,6 +123,10 @@ type Informer interface {
HasSynced() bool
}

// AllNamespaces should be used as the map key to deliminate namespace settings
// that apply to all namespaces that themselves do not have explicit settings.
const AllNamespaces = metav1.NamespaceAll

// Options are the optional arguments for creating a new Cache object.
type Options struct {
// HTTPClient is the http client to use for the REST client
Expand Down Expand Up @@ -172,6 +178,11 @@ type Options struct {
// the namespaces in here will be watched and it will by used to default
// ByObject.Namespaces for all objects if that is nil.
//
// It is possible to have specific Config for just some namespaces
// but cache all namespaces by using the AllNamespaces const as the map key.
// This will then include all namespaces that do not have a more specific
// setting.
//
// The options in the Config that are nil will be defaulted from
// the respective Default* settings.
DefaultNamespaces map[string]Config
Expand Down Expand Up @@ -220,6 +231,11 @@ type ByObject struct {
// Settings in the map value that are unset will be defaulted.
// Use an empty value for the specific setting to prevent that.
//
// It is possible to have specific Config for just some namespaces
// but cache all namespaces by using the AllNamespaces const as the map key.
// This will then include all namespaces that do not have a more specific
// setting.
//
// A nil map allows to default this to the cache's DefaultNamespaces setting.
// An empty map prevents this and means that all namespaces will be cached.
//
Expand Down Expand Up @@ -399,6 +415,9 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {

for namespace, cfg := range opts.DefaultNamespaces {
cfg = defaultConfig(cfg, optionDefaultsToConfig(&opts))
if namespace == metav1.NamespaceAll {
cfg.FieldSelector = fields.AndSelectors(appendIfNotNil(namespaceAllSelector(maps.Keys(opts.DefaultNamespaces)), cfg.FieldSelector)...)
}
opts.DefaultNamespaces[namespace] = cfg
}

Expand All @@ -425,6 +444,15 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
// 3. Default from the global defaults
config = defaultConfig(config, optionDefaultsToConfig(&opts))

if namespace == metav1.NamespaceAll {
config.FieldSelector = fields.AndSelectors(
appendIfNotNil(
namespaceAllSelector(maps.Keys(byObject.Namespaces)),
config.FieldSelector,
)...,
)
}

byObject.Namespaces[namespace] = config
}

Expand Down Expand Up @@ -464,3 +492,21 @@ func defaultConfig(toDefault, defaultFrom Config) Config {

return toDefault
}

func namespaceAllSelector(namespaces []string) fields.Selector {
selectors := make([]fields.Selector, 0, len(namespaces)-1)
for _, namespace := range namespaces {
if namespace != metav1.NamespaceAll {
selectors = append(selectors, fields.OneTermNotEqualSelector("metadata.namespace", namespace))
}
}

return fields.AndSelectors(selectors...)
}

func appendIfNotNil[T comparable](a, b T) []T {
if b != *new(T) {
return []T{a, b}
}
return []T{a}
}
57 changes: 57 additions & 0 deletions pkg/cache/cache_test.go
Expand Up @@ -1543,6 +1543,9 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
}
return obtainedPodNames
}, ConsistOf(tc.expectedPods)))
for _, pod := range obtainedStructuredPodList.Items {
Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) //nolint:gosec // We don't retain the pointer
}

By("Checking with unstructured")
obtainedUnstructuredPodList := unstructured.UnstructuredList{}
Expand All @@ -1560,6 +1563,9 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
}
return obtainedPodNames
}, ConsistOf(tc.expectedPods)))
for _, pod := range obtainedUnstructuredPodList.Items {
Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) //nolint:gosec // We don't retain the pointer
}

By("Checking with metadata")
obtainedMetadataPodList := metav1.PartialObjectMetadataList{}
Expand All @@ -1577,6 +1583,9 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
}
return obtainedPodNames
}, ConsistOf(tc.expectedPods)))
for _, pod := range obtainedMetadataPodList.Items {
Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) //nolint:gosec // We don't retain the pointer
}
},
Entry("when selectors are empty it has to inform about all the pods", selectorsTestCase{
expectedPods: []string{"test-pod-1", "test-pod-2", "test-pod-3", "test-pod-4", "test-pod-5", "test-pod-6"},
Expand Down Expand Up @@ -1789,6 +1798,54 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
},
expectedPods: []string{},
}),
Entry("Only NamespaceAll in DefaultNamespaces returns all pods", selectorsTestCase{
options: cache.Options{
DefaultNamespaces: map[string]cache.Config{
metav1.NamespaceAll: {},
},
},
expectedPods: []string{"test-pod-1", "test-pod-2", "test-pod-3", "test-pod-4", "test-pod-5", "test-pod-6"},
}),
Entry("Only NamespaceAll in ByObject.Namespaces returns all pods", selectorsTestCase{
options: cache.Options{
ByObject: map[client.Object]cache.ByObject{
&corev1.Pod{}: {
Namespaces: map[string]cache.Config{
metav1.NamespaceAll: {},
},
},
},
},
expectedPods: []string{"test-pod-1", "test-pod-2", "test-pod-3", "test-pod-4", "test-pod-5", "test-pod-6"},
}),
Entry("NamespaceAll in DefaultNamespaces creates a cache for all Namespaces that are not in DefaultNamespaces", selectorsTestCase{
options: cache.Options{
DefaultNamespaces: map[string]cache.Config{
metav1.NamespaceAll: {},
testNamespaceOne: {
// labels.Nothing when serialized matches everything, so we have to construct our own "match nothing" selector
LabelSelector: labels.SelectorFromSet(labels.Set{"no-present": "not-present"})},
},
},
// All pods that are not in NamespaceOne
expectedPods: []string{"test-pod-2", "test-pod-3", "test-pod-4", "test-pod-6"},
}),
Entry("NamespaceAll in ByObject.Namespaces creates a cache for all Namespaces that are not in ByObject.Namespaces", selectorsTestCase{
options: cache.Options{
ByObject: map[client.Object]cache.ByObject{
&corev1.Pod{}: {
Namespaces: map[string]cache.Config{
metav1.NamespaceAll: {},
testNamespaceOne: {
// labels.Nothing when serialized matches everything, so we have to construct our own "match nothing" selector
LabelSelector: labels.SelectorFromSet(labels.Set{"no-present": "not-present"})},
},
},
},
},
// All pods that are not in NamespaceOne
expectedPods: []string{"test-pod-2", "test-pod-3", "test-pod-4", "test-pod-6"},
}),
)
})
Describe("as an Informer", func() {
Expand Down
4 changes: 4 additions & 0 deletions pkg/cache/multi_namespace_cache.go
Expand Up @@ -23,6 +23,7 @@ import (

corev1 "k8s.io/api/core/v1"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
toolscache "k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -210,6 +211,9 @@ func (c *multiNamespaceCache) Get(ctx context.Context, key client.ObjectKey, obj

cache, ok := c.namespaceToCache[key.Namespace]
if !ok {
if global, hasGlobal := c.namespaceToCache[metav1.NamespaceAll]; hasGlobal {
return global.Get(ctx, key, obj, opts...)
}
return fmt.Errorf("unable to get: %v because of unknown namespace for the cache", key)
}
return cache.Get(ctx, key, obj, opts...)
Expand Down