Skip to content

Commit

Permalink
sparkles: Cache: Allow defining options that apply to all namespaces …
Browse files Browse the repository at this point in the history 
…that themselves have no explicit config

This change allows to define a cache selector config that applies to all
namespaces that themselves do not have an explicit config. An example
would be "Cache all namespaces without selector, except for namespace
foo, there use labelSelector bar=baz".

More as a side effect than intentionally, this also makes it valid to use
the empty string as a key in the `Namespaces` and `byObject.Namespaces`
config of the cache. This is very useful to for example have a
`namespace` CLI flag that might be empty.

This change is the last missing bit to finish the implementation of the
[cache options design doc](./designs/cache_options.md).
  • Loading branch information
@alvaroaleman
alvaroaleman authored and k8s-infra-cherrypick-robot committed Oct 10, 2023
1 parent 8361246 commit b9c691d
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
Original file line number Diff line number Diff line change
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 @@ -214,6 +225,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 @@ -392,6 +408,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 @@ -418,6 +437,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 @@ -457,3 +485,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 b9c691d

Please sign in to comment.