Skip to content

Commit

Permalink
Merge pull request #2528 from kubernetes-sigs/allow-all-but
Browse files Browse the repository at this point in the history 
✨ Cache: Allow defining options that apply to all namespaces that themselves have no explicit config
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Oct 8, 2023
2 parents d94cf60 + 414b86e commit 968daa8
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 0 deletions.
46 changes: 46 additions & 0 deletions pkg/cache/cache.go
View file
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
View file
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
View file
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

0 comments on commit 968daa8

Please sign in to comment.